RE: [PATCH v19,RESEND 24/27] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

["Xing, Cedric" <cedric.xing@intel.com>]

> I think you're misunderstanding me.  I'm not talking about security at
> all here.  SGX isn't a sandbox, full stop.  I'm talking about the degree
> to which an SGX enclave acts like a well-behaved black box.

Any meaningful communication requires an agreement in place. The host and the enclave could be either in agreement, or not in an agreement. In the former case, the enclave will behave while in the latter case it will misbehave. The thing is, if an agreement between them says - "Don't you enclave touch the stack", and if the enclave behaves, then it wouldn't touch the stack; or the enclave misbehaves, then that "agreement" CANNOT stop it from doing so, REGARDLESS what that agreement is.

The point is, an agreement must exist for host/enclave communication. The ABI limits what kinds of agreements they can bind to, but can NEVER enforce an agreement. The difference between Sean's ABI and mine is that mine is more relaxing (i.e. allows larger variety of agreements) but otherwise identical functionally. I truly hope you can understand that.
 
> 
> >
> > > 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.
> 
> There is no actual fundamental reason that dynamic linking has to work
> this way, and in principle, one could even use retpolines to the call
> the vDSO.  In any event, the vDSO is currently compiled with retpolines
> enabled, and if we decide to turn that off, it would be decision to be
> made independently of SGX.

Don't get me wrong! I'm just saying dynamic linking requires indirect branches so whatever mitigation used there can also apply here. I'm willing to implement the same mechanism as generally accepted in other occasions. If retpoline is the one then I will just do it.

Btw, retpoline won't work with CET though.

> 
> >
> > >
> > > 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?
> 
> You're counting wrong.  Your version uses 24 bytes + the stack size of
> the exit handler + the amount of stack consumed by the enclave, which is
> effectively unbounded.  So this whole scheme becomes unusable on
> anything other than a stack that is "large" for a totally undefined
> value of large and that has guard pages.

You misread. I said "targeting the same functionality", meaning no exit callback is used (because Sean's ABI doesn't support it). And because no callbacks will be made, only 24 more bytes will be needed.

And at this point I'm trying to stress the fact that my proposal is a superset of Sean's in terms of functionality - i.e. my proposal can do all that Sean's can do. For example, if the enclave is coded NOT to use the untrusted stack, e.g. in Fortanix's case, then it won't use the stack and the callback is unnecessary (and shall be set to NULL). That is, mine will work exactly the same as Sean's in the case of enclave not touching the stack. My apology for being excessively verbose here but your comment above prompts me that you may NOT have realized that my proposal will work exactly the SAME as Sean's when exit callback is absent (NULL).

> 
> >
> > >
> > > 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".
> 
> Again, this has nothing to do with security.  With your proposal, it's
> not possible for the caller of an enclave to decide, in an ocall
> handler, to pause and do something else.  This isn't just theoretical.
> Suppose someone wants to send a network request in an ocall handler.
> With the current RSP approach, it's difficult to do this in a program
> that uses poll / select / epoll -- you can't return out from the ocall
> until you have an answer.

Andy, your comment here further confirms that you have NOT understood my proposal.

In the case the o-call parameters are passed in registers (or separate buffers), exit handler is NOT needed (and should be set to NULL), and the API will just return at EEXIT, and then the caller can dispatch the o-call solely based on register values, which is exactly the SAME as Sean's proposal.

An exit handler is NOT necessary to support o-calls. It is needed ONLY if the host/enclave exchanges data on the stack.

My apology again for repeating myself, but my proposal is a SUPERSET of Sean's from functional perspective, and it will work in EXACTLY the SAME way as Sean's in ALL aspects when exit callback is absent. So you really have NOTHING to worry about from functional stand point! And as you said you don't worry about security either, then you really have NOTHING to worry about!

-Cedric