Message-ID: <960B34DE67B9E140824F1DCDEC400C0F4E85E989@ORSMSX116.amr.corp.intel.com>
Date:   Sun, 24 Mar 2019 08:59:48 +0000
From:   "Xing, Cedric" <cedric.xing@...el.com>
To:     Andy Lutomirski <luto@...nel.org>
CC:     "Christopherson, Sean J" <sean.j.christopherson@...el.com>,
        "Jarkko Sakkinen" <jarkko.sakkinen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "npmccallum@...hat.com" <npmccallum@...hat.com>,
        "Ayoun, Serge" <serge.ayoun@...el.com>,
        "Katz-zamir, Shay" <shay.katz-zamir@...el.com>,
        "Huang, Haitao" <haitao.huang@...el.com>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Svahn, Kai" <kai.svahn@...el.com>, "bp@...en8.de" <bp@...en8.de>,
        "josh@...htriplett.org" <josh@...htriplett.org>,
        "Huang, Kai" <kai.huang@...el.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "Dave Hansen" <dave.hansen@...ux.intel.com>,
        Haitao Huang <haitao.huang@...ux.intel.com>,
        Jethro Beekman <jethro@...tanix.com>,
        "Dr . Greg Wettstein" <greg@...ellic.com>
Subject: RE: [PATCH v19,RESEND 24/27] x86/vdso: Add
 __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

Hi Andy,

Thank you for your valuable feedbacks!

Per what you have been saying, your feedbacks come from different angles - i.e. functionality vs. security, but they are mixed up somehow. As an effort to make the discussion more constructive going forward, I'd like you to acknowledge that, in terms of functionality, my proposal is a superset of the current patch, as I have proven by implementing Sean's API using mine as a subroutine. That said, if you are satisfied with his, you should be satisfied with mine as well, from functional perspective. And because of that, I'll try to interpret/address your concerns from security perspective unless otherwise noted. I'm aware that there's still subtle difference between Sean's API and mine - e.g. my proposal consumes 24 bytes more stack space (for the same functionality, i.e. exit callback is null) than his, due to the additional parameters. But I don't believe that would become a "make it or break it" situation in practice.

> I’m going to put my vDSO maintainer hat on for a minute.  Cedric, your
> proposal has the following issues related specifically to the vDSO:
> 
> It inherently contains indirect branches.  This means that, on retpoline
> configurations, it probably needs to use retpolines.  This is doable,
> but it’s nasty, and you need to worry about register clobbers.

Only the weakest link matters in security. With dynamic linking in use, this additional indirect CALL can't make things worse. But I'm open to, and in fact also willing to, apply whatever mitigation that you think is satisfactory (or that has been applied to other indirect branches, such as in PLT), such as retpoline. Btw, don't worry about register clobbers because we have at least %rax at our disposal.

> 
> It uses effectively unbounded stack space. The vDSO timing functions are
> already a problem for Go, and this is worse.

If targeting the same functionality (i.e. no exit callback), my API uses exactly 24 bytes more than Sean's. Is it really the case that those 24 bytes will break Go?

> 
> And with my vDSO hat back off, I find it disappointing that SGX SDKs
> seem willing to couple the SGX enclaves so tightly to their host ABIs.
> An *unmodified* SGX enclave should be able to run, without excessive
> annoyance, in a Windows process, a Linux process, a C process, a Java
> process, a Go process, and pretty much any other process.  Saying “I’ll
> just recompile it” is a bad solution — for enclaves that use MRENCLAVE,
> you can’t, and for enclaves that use MRSIGNER, you need to deal with the
> fact the protecting the signing key is a big deal.
> Someone should be able to port the entire host program to a different
> language without losing secrets and without access to a signing key.

I'm not sure which SGX SDKs you are referring to. But for Intel SGX SDK, we defined our own ABI that is consistent between Windows and Linux - i.e. there's no technical problem to load on Windows an enclave built on Linux or vice versa. In terms of what programming languages they can work with, I have to say it was designed exclusively for C/C++. Fortunately, there's usually a "native" interface (e.g. JNI, cgo, etc.) supported by a language runtime so it hasn't been a roadblock so far. Alternatively, the enclave vendor could ship an enclave along with an "interface" shared object that encapsulates all of the marshaling specifics, then the combination of that enclave and its "interface" shared object may be able to work "universally", which should be close to what you want.

The idea we had, when Intel SGX SDK was designed, was that different SDKs would be developed for different languages to take advantage of specific language features. That is similar to different programming languages were invented to target different usages. As we all know, every programming language has both advantages and disadvantages, hence no single language dominates. And that same idea applies to SGX SDKs. If there existed an SDK that worked with everything, probably it wouldn't work well with anything.

> 
> Cedric, your proposal allows an enclave to muck with RSP, but not in a
> way that’s particularly pleasant.

From security perspective, it is SGX ISA, but NOT any particular ABI, that allows enclaves "to muck with RSP". 

> Since the ISA is set in stone, we
> can’t do anything about the enclave’s access to its caller’s registers.
> I would love to see a straightforward way to run an enclave such that it
> does not access the main untrusted stack at all — uRSP and uRBP should
> be arbitrary values passed in the untrusted code, and the values the
> enclave sets should be relayed back to the caller but otherwise not have
> any effect.  Sadly I see no way to do this short of using GSBASE to
> store the real untrusted stack pointer.

I understand your sadness. You are "hoping" SGX to be a sandbox technology (i.e. to prevent enclave from reaching out into the host) but that wasn't the security objective when SGX was defined.

Anyway, SGX is what it is. A restrictive ABI only takes away flexibilities from "good" enclaves but can NEVER restrict malicious ones, so Sean's ABI cannot offer what you want.

> Other than the segment bases, there appear to be literally zero
> untrusted registers that are reliably preserved across an enclave entry
> and exit.  I suppose we should use a syscall to help.

The good news is with CET, there are viable solutions to implement bi-directional protection as you would hope. You are more than welcome to ask me offline for more details.

> 
> Since the above tricks seem unlikely to make it into the kernel, I think
> we’re doing everyone a favor if the Linux APIs strongly encourage SDK
> authors to build enclaves in a way that they don’t make problematic
> assumptions about the untrusted world. I would really like to see
> enclaves generated by the Linux SDK work on Windows and vice versa.

As said in my previous email, this vDSO API isn't even compliant to x86_64 ABI and is absolutely NOT for average developers. Instead, host/enclave communications are expected to be handled by SDKs and those developers will be very aware of the limitations of their targeted environments, and will need the freedom to deploy optimal solutions. 

I understand your intention to advocate the programming model that you believe is "right". But there are 7 billion people on this planet and the "right" thing for you could be "wrong" for others, especially in future usages/situations that can't be foreseen today. Software is stacked, with the lower layers being more generic and higher layers being more specific. This vDSO API is sitting at the bottom of the stack, therefore shall be as generic as possible. A better approach to advocate your idea is to wrap it (i.e. to implement it using the more generic vDSO API as a subroutine) in a library for the public to choose (and you can imagine others bearing different ideas will do the same). Then good ideas will stand out!

-Cedric

Powered by blists - more mailing lists