x86/sgx: v23-rc2

x86/sgx: v23-rc2

[Jarkko Sakkinen]

tag v23-rc2
Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date:   Thu Oct 10 14:33:07 2019 +0300

x86/sgx: v23-rc1 patch set

* Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
* In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
  (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
  flow can be only reverted by killing the whole enclave.
* Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
  it should have been bit 6 (Table 37-3 in the SDM).
* Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
  error code.
* In v22 __uaccess_begin() was used to pin the source page in
  __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
  deadlock (mmap_sem might get locked twice in the same thread).
-----BEGIN PGP SIGNATURE-----

iJYEABYIAD4WIQRE6pSOnaBC00OEHEIaerohdGur0gUCXZ8XTyAcamFya2tvLnNh
a2tpbmVuQGxpbnV4LmludGVsLmNvbQAKCRAaerohdGur0phXAP9QPYcpyUTSO9hk
sG/pV7vsIjS4lO6pxBCgWCtg3/ZkvAEApLCu7mFvyZs3rDcbOlQA+nQiVv+rUwzu
tsYmW2YsgQ4=
=VeL3
-----END PGP SIGNATURE-----

/Jarkko

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Thu, Oct 10, 2019 at 02:37:45PM +0300, Jarkko Sakkinen wrote:
> tag v23-rc2
> Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Date:   Thu Oct 10 14:33:07 2019 +0300
> 
> x86/sgx: v23-rc1 patch set
> 
> * Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
> * In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
>   (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
>   flow can be only reverted by killing the whole enclave.
> * Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
>   it should have been bit 6 (Table 37-3 in the SDM).
> * Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
>   error code.
> * In v22 __uaccess_begin() was used to pin the source page in
>   __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
>   deadlock (mmap_sem might get locked twice in the same thread).

__uaccess_begin() is also needed to performan access checks the legit
user space address. What we can do is to use get_user_pages() just to
make sure that the page is faulted while we perform ENCLS[EADD].

I updated the master branch with the fix for this. Now the access
pattern is:

	ret = get_user_pages(src, 1, 0, &src_page, NULL);
	if (ret < 1)
		return ret;

	__uaccess_begin();

	pginfo.secs = (unsigned long)sgx_epc_addr(encl->secs.epc_page);
	pginfo.addr = SGX_ENCL_PAGE_ADDR(encl_page);
	pginfo.metadata = (unsigned long)secinfo;
	pginfo.contents = (unsigned long)src;

	ret = __eadd(&pginfo, sgx_epc_addr(epc_page));

	__uaccess_end();
	put_page(src_page);

/Jarkko

Re: x86/sgx: v23-rc2

[Sean Christopherson]

On Thu, Oct 10, 2019 at 04:37:53PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 10, 2019 at 02:37:45PM +0300, Jarkko Sakkinen wrote:
> > tag v23-rc2
> > Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Date:   Thu Oct 10 14:33:07 2019 +0300
> > 
> > x86/sgx: v23-rc1 patch set
> > 
> > * Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
> > * In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
> >   (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
> >   flow can be only reverted by killing the whole enclave.
> > * Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
> >   it should have been bit 6 (Table 37-3 in the SDM).
> > * Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
> >   error code.
> > * In v22 __uaccess_begin() was used to pin the source page in
> >   __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
> >   deadlock (mmap_sem might get locked twice in the same thread).
> 
> __uaccess_begin() is also needed to performan access checks the legit

__uaccess_begin() doesn't check the address space, it temporarily disables
SMAP/SMEP so that the kernel can access a user mapping.  An explicit
access_ok() call should be added as well.

> user space address. What we can do is to use get_user_pages() just to
> make sure that the page is faulted while we perform ENCLS[EADD].


> I updated the master branch with the fix for this. Now the access
> pattern is:
> 
> 	ret = get_user_pages(src, 1, 0, &src_page, NULL);
> 	if (ret < 1)
> 		return ret;
> 
> 	__uaccess_begin();

This should be immediately before __eadd().  I also think it'd be a good
idea to disable page faults around __eadd() so that an unexpected #PF
manifests as an __eadd() failure and not a kernel hang.

> 	pginfo.secs = (unsigned long)sgx_epc_addr(encl->secs.epc_page);
> 	pginfo.addr = SGX_ENCL_PAGE_ADDR(encl_page);
> 	pginfo.metadata = (unsigned long)secinfo;
> 	pginfo.contents = (unsigned long)src;

> 	ret = __eadd(&pginfo, sgx_epc_addr(epc_page));
> 
> 	__uaccess_end();
> 	put_page(src_page);

Not shown here, but mmap_sem doesn't need to be held through EEXTEND. The
lock issue is that down_read() will block if there is a pending
down_write(), e.g. if userspace is doing mprotect() at the same time as
EADD, then deadlock will occur if EADD faults.  Holding encl->lock without
mmap_sem is perfectly ok.

I'll send a small series with the above changes.

Re: x86/sgx: v23-rc2

[Sean Christopherson]

On Thu, Oct 10, 2019 at 10:09:21AM -0700, Sean Christopherson wrote:
> On Thu, Oct 10, 2019 at 04:37:53PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Oct 10, 2019 at 02:37:45PM +0300, Jarkko Sakkinen wrote:
> > > tag v23-rc2
> > > Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > Date:   Thu Oct 10 14:33:07 2019 +0300
> > > 
> > > x86/sgx: v23-rc1 patch set
> > > 
> > > * Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
> > > * In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
> > >   (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
> > >   flow can be only reverted by killing the whole enclave.
> > > * Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
> > >   it should have been bit 6 (Table 37-3 in the SDM).
> > > * Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
> > >   error code.
> > > * In v22 __uaccess_begin() was used to pin the source page in
> > >   __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
> > >   deadlock (mmap_sem might get locked twice in the same thread).
> > 
> > __uaccess_begin() is also needed to performan access checks the legit
> 
> __uaccess_begin() doesn't check the address space, it temporarily disables
> SMAP/SMEP so that the kernel can access a user mapping.  An explicit
> access_ok() call should be added as well.
> 
> > user space address. What we can do is to use get_user_pages() just to
> > make sure that the page is faulted while we perform ENCLS[EADD].
> 
> 
> > I updated the master branch with the fix for this. Now the access
> > pattern is:
> > 
> > 	ret = get_user_pages(src, 1, 0, &src_page, NULL);
> > 	if (ret < 1)
> > 		return ret;
> > 
> > 	__uaccess_begin();
> 
> This should be immediately before __eadd().  I also think it'd be a good
> idea to disable page faults around __eadd() so that an unexpected #PF
> manifests as an __eadd() failure and not a kernel hang.

Scratch that, using the userspace is address is flat out wrong.  gup()
doesn't guarantee the userspace mapping is valid, i.e. EADD can still get
a #PF and deadlock.  AIUI, the correct/bulletproof approach is to kmap()
after gup() and use the kernel translation.

Re: x86/sgx: v23-rc2

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

Hi all,

I wrote a preliminary patch integrating the latest kernel patches with the Rust EDP, see https://github.com/fortanix/rust-sgx/pull/181 . I tested against (I think) v23-rc2 with the 3 patch sets from Sean applied. In particular, I started from https://github.com/jsakkine-intel/linux-sgx 8915aef. It would be nice if you'd use git tags so everyone can be on the same page.

I haven't done a whole lot of testing, so I can't speak to the stability of the driver. But I do have some comments regarding the API.



UAPI:

This got a whole lot more complex for userspace compared to the out-of-tree driver.

1. Manually needing to mmap a naturally-aligned memory region by allocating too much memory and then unmapping parts is quite annoying. Why was the auto-aligning removed? I think this will need to be handled the same for every consumer of SGX, so I don't see why this is not handled in the kernel. It never seems wrong to align if NULL is passed as the requested address. Alternatively, is there room in the flags for a MAP_ALIGNED bit?

2. Having to re-open the device for every enclave is also annoying. This means you need a filesystem available throughout the process lifetime. I tried dup, but that doesn't work. Can we make dup work?

3. Needing to mprotect every page with the precise permissions needed after EINIT is really bad. This means I have to remember this data for every page between EADD and EINIT. I don't care about SELinux, I trust the ECPM will do its job for me. Can we make it so that I can protect the whole range at once, or protect the individual pages at EADD time?



VDSO:

It is *difficult* to link to weakly link to a symbol in the VDSO. Anyway, I figured it out.

1. What if I don't want to automatically ERESUME after kernel interrupt?

2. I normally do a sanity check after ENCLU[EENTER] that EAX = EEXIT. The current implementation just clears EAX instead without looking at it.

--
Jethro Beekman | Fortanix

On 2019-10-10 13:37, Jarkko Sakkinen wrote:
> tag v23-rc2
> Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Date:   Thu Oct 10 14:33:07 2019 +0300
> 
> x86/sgx: v23-rc1 patch set
> 
> * Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
> * In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
>   (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
>   flow can be only reverted by killing the whole enclave.
> * Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
>   it should have been bit 6 (Table 37-3 in the SDM).
> * Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
>   error code.
> * In v22 __uaccess_begin() was used to pin the source page in
>   __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
>   deadlock (mmap_sem might get locked twice in the same thread).
> -----BEGIN PGP SIGNATURE-----
> 
> iJYEABYIAD4WIQRE6pSOnaBC00OEHEIaerohdGur0gUCXZ8XTyAcamFya2tvLnNh
> a2tpbmVuQGxpbnV4LmludGVsLmNvbQAKCRAaerohdGur0phXAP9QPYcpyUTSO9hk
> sG/pV7vsIjS4lO6pxBCgWCtg3/ZkvAEApLCu7mFvyZs3rDcbOlQA+nQiVv+rUwzu
> tsYmW2YsgQ4=
> =VeL3
> -----END PGP SIGNATURE-----
> 
> /Jarkko
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]

Re: x86/sgx: v23-rc2

[Sean Christopherson]

On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote:
> UAPI:
> 
> This got a whole lot more complex for userspace compared to the out-of-tree
> driver.
> 
> 1. Manually needing to mmap a naturally-aligned memory region by allocating
> too much memory and then unmapping parts is quite annoying. Why was the
> auto-aligning removed? I think this will need to be handled the same for
> every consumer of SGX, so I don't see why this is not handled in the kernel.
> It never seems wrong to align if NULL is passed as the requested address.
> Alternatively, is there room in the flags for a MAP_ALIGNED bit?

I'm pretty sure everyone agrees it's annoying.  The short of it is that
the SGX driver is the wrong place to do the alignment.  The driver could
key off addr=0, but we don't want to take on that implicit behavior.

A MAP_ALIGNED flag to have the allocation be naturally aligned is the
ideal solution.  It's definitely something we should pursue, but that can
and probably should be done in parallel to the SGX series.

> 2. Having to re-open the device for every enclave is also annoying. This
> means you need a filesystem available throughout the process lifetime. I
> tried dup, but that doesn't work. Can we make dup work?

The semantics of dup() won't get you what want, as dup() just creates a
new descriptor pointing at the same file.

An alternative solution that was proposed was to have an ioctl() for
creating an enclave.  But that means using an anonymous inode, which runs
afoul of SELinux permissions, e.g. every _process_ that runs enclaves
would require EXECMEM.  Linus was quite clear that SGX wouldn't be merged
if using it required users to degrade existing security.

I'm open to other ideas.  I wasn't aware this was a pain point and file
stuff isn't exactly my area of expertise, so I haven't put much/any
thought into alternatives.

> 3. Needing to mprotect every page with the precise permissions needed after
> EINIT is really bad. This means I have to remember this data for every page
> between EADD and EINIT. I don't care about SELinux, I trust the ECPM will do
> its job for me. Can we make it so that I can protect the whole range at once,
> or protect the individual pages at EADD time?

You can mprotect() or mmap(..., MAP_FIXED) an enclave range once all
pages covered by the specified range have been added to the enclave, i.e.
at EADD.  I double checked this with the selftest.  Holler if you're
seeing different behavior.


> VDSO:
> 
> It is *difficult* to link to weakly link to a symbol in the VDSO. Anyway, I
> figured it out.
> 
> 1. What if I don't want to automatically ERESUME after kernel interrupt?

Do EENTER/ERESUME directly instead of going through the vDSO.

> 2. I normally do a sanity check after ENCLU[EENTER] that EAX = EEXIT. The
> current implementation just clears EAX instead without looking at it.

Hmm, the only reason I can think of for checking EAX would be to support
userspace mucking with EAX in a #DB/#BP signal handler.  At that point, I
would expect the signal handler to modify RIP as well.  Reaching the XOR
via any other non-EEXIT path would require a kernel bug.

Was there a specific scenario or use case you had in mind?  I'm not
against adding a check, I just don't see what value it would provide.

Re: x86/sgx: v23-rc2

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 4949 bytes --]

On 2019-10-11 20:15, Sean Christopherson wrote:
> On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote:
>> UAPI:
>>
>> This got a whole lot more complex for userspace compared to the out-of-tree
>> driver.
>>
>> 1. Manually needing to mmap a naturally-aligned memory region by allocating
>> too much memory and then unmapping parts is quite annoying. Why was the
>> auto-aligning removed? I think this will need to be handled the same for
>> every consumer of SGX, so I don't see why this is not handled in the kernel.
>> It never seems wrong to align if NULL is passed as the requested address.
>> Alternatively, is there room in the flags for a MAP_ALIGNED bit?
> 
> I'm pretty sure everyone agrees it's annoying.  The short of it is that
> the SGX driver is the wrong place to do the alignment.  The driver could
> key off addr=0, but we don't want to take on that implicit behavior.

Why not?

> A MAP_ALIGNED flag to have the allocation be naturally aligned is the
> ideal solution.  It's definitely something we should pursue, but that can
> and probably should be done in parallel to the SGX series.
> 
>> 2. Having to re-open the device for every enclave is also annoying. This
>> means you need a filesystem available throughout the process lifetime. I
>> tried dup, but that doesn't work. Can we make dup work?
> 
> The semantics of dup() won't get you what want, as dup() just creates a
> new descriptor pointing at the same file.
> 
> An alternative solution that was proposed was to have an ioctl() for
> creating an enclave.  But that means using an anonymous inode, which runs
> afoul of SELinux permissions, e.g. every _process_ that runs enclaves
> would require EXECMEM.  Linus was quite clear that SGX wouldn't be merged
> if using it required users to degrade existing security.

It's ok if it's the same inode, it just needs to be a different struct file.

> I'm open to other ideas.  I wasn't aware this was a pain point and file
> stuff isn't exactly my area of expertise, so I haven't put much/any
> thought into alternatives.

The default permissions for /dev/sgx/enclave are root-only. This means you want to be able to do the same thing as network servers: initialize some resources as root, then drop privileges. This used to mean opening /dev/sgx and keeping the fd around which meant you could launch enclaves at will. With the new API, this is no longer possible, you can only launch one enclave per fd. Is there a different type of operation that doesn't just duplicate the fd but also the struct file? If not, can we add an ioctl for that?

There are other scenarios where it's not just the permissions on /dev/sgx/enclave that are the problem but using the filesystem in general that is. Maybe you've used seccomp to disable file operations, etc.

>> 3. Needing to mprotect every page with the precise permissions needed after
>> EINIT is really bad. This means I have to remember this data for every page
>> between EADD and EINIT. I don't care about SELinux, I trust the ECPM will do
>> its job for me. Can we make it so that I can protect the whole range at once,
>> or protect the individual pages at EADD time?
> 
> You can mprotect() or mmap(..., MAP_FIXED) an enclave range once all
> pages covered by the specified range have been added to the enclave, i.e.
> at EADD.  I double checked this with the selftest.  Holler if you're
> seeing different behavior.

I'd swear I tried this flow and I was getting EACCES. But I implemented it again now, and it works fine. So this is not an issue.

I also saw mmap(..., MAP_FIXED) being used in the selftest. Is there a reason to use this over mprotect?

>> VDSO:
>>
>> It is *difficult* to link to weakly link to a symbol in the VDSO. Anyway, I
>> figured it out.
>>
>> 1. What if I don't want to automatically ERESUME after kernel interrupt?
> 
> Do EENTER/ERESUME directly instead of going through the vDSO.

That kind of defeats the point.

>> 2. I normally do a sanity check after ENCLU[EENTER] that EAX = EEXIT. The
>> current implementation just clears EAX instead without looking at it.
> 
> Hmm, the only reason I can think of for checking EAX would be to support
> userspace mucking with EAX in a #DB/#BP signal handler.  At that point, I
> would expect the signal handler to modify RIP as well.  Reaching the XOR
> via any other non-EEXIT path would require a kernel bug.

Or a CPU bug.

> Was there a specific scenario or use case you had in mind?  I'm not
> against adding a check, I just don't see what value it would provide.

Nothing specific. It just seems like a prudent thing to do when messing with control flow in unexpected ways. Alternatively, just remove the xor, and change the API of the function so that 3 is the normal return value? Then the user can decide themselves if they think the check is worth it.

--
Jethro Beekman | Fortanix


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]

Re: x86/sgx: v23-rc2

[Sean Christopherson]

+Cc Andy

On Mon, Oct 14, 2019 at 08:43:09AM +0000, Jethro Beekman wrote:
> On 2019-10-11 20:15, Sean Christopherson wrote:
> > On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote:
> >> UAPI:
> >>
> >> This got a whole lot more complex for userspace compared to the out-of-tree
> >> driver.
> >>
> >> 1. Manually needing to mmap a naturally-aligned memory region by allocating
> >> too much memory and then unmapping parts is quite annoying. Why was the
> >> auto-aligning removed? I think this will need to be handled the same for
> >> every consumer of SGX, so I don't see why this is not handled in the kernel.
> >> It never seems wrong to align if NULL is passed as the requested address.
> >> Alternatively, is there room in the flags for a MAP_ALIGNED bit?
> > 
> > I'm pretty sure everyone agrees it's annoying.  The short of it is that
> > the SGX driver is the wrong place to do the alignment.  The driver could
> > key off addr=0, but we don't want to take on that implicit behavior.
> 
> Why not?

Because it's a hack.  If a MAP_ALIGNED flag is added then SGX is stuck
with kludgy code that serves no purpose.  And userspace needs to manually
align the result if it provides an actual hint.  Regardless of whether
there are use cases for providing a hint for ELRANGE, having divergent
logic is ugly.

> > A MAP_ALIGNED flag to have the allocation be naturally aligned is the
> > ideal solution.  It's definitely something we should pursue, but that can
> > and probably should be done in parallel to the SGX series.
> > 
> >> 2. Having to re-open the device for every enclave is also annoying. This
> >> means you need a filesystem available throughout the process lifetime. I
> >> tried dup, but that doesn't work. Can we make dup work?
> > 
> > The semantics of dup() won't get you what want, as dup() just creates a
> > new descriptor pointing at the same file.
> > 
> > An alternative solution that was proposed was to have an ioctl() for
> > creating an enclave.  But that means using an anonymous inode, which runs
> > afoul of SELinux permissions, e.g. every _process_ that runs enclaves
> > would require EXECMEM.  Linus was quite clear that SGX wouldn't be merged
> > if using it required users to degrade existing security.
> 
> It's ok if it's the same inode, it just needs to be a different struct file.
> 
> > I'm open to other ideas.  I wasn't aware this was a pain point and file
> > stuff isn't exactly my area of expertise, so I haven't put much/any
> > thought into alternatives.
> 
> The default permissions for /dev/sgx/enclave are root-only. This means you
> want to be able to do the same thing as network servers: initialize some
> resources as root, then drop privileges. This used to mean opening /dev/sgx
> and keeping the fd around which meant you could launch enclaves at will. With
> the new API, this is no longer possible, you can only launch one enclave per
> fd. Is there a different type of operation that doesn't just duplicate the fd
> but also the struct file? If not, can we add an ioctl for that?

My approach to this would be to chown /dev/sgx/enclave so that it's owned
by root but accessible to users belonging to an sgx-specific group, e.g.
via a udev rule.

> There are other scenarios where it's not just the permissions on
> /dev/sgx/enclave that are the problem but using the filesystem in general
> that is. Maybe you've used seccomp to disable file operations, etc.

Andy and Jarkko, thoughts?

> >> 3. Needing to mprotect every page with the precise permissions needed after
> >> EINIT is really bad. This means I have to remember this data for every page
> >> between EADD and EINIT. I don't care about SELinux, I trust the ECPM will do
> >> its job for me. Can we make it so that I can protect the whole range at once,
> >> or protect the individual pages at EADD time?
> > 
> > You can mprotect() or mmap(..., MAP_FIXED) an enclave range once all
> > pages covered by the specified range have been added to the enclave, i.e.
> > at EADD.  I double checked this with the selftest.  Holler if you're
> > seeing different behavior.
> 
> I'd swear I tried this flow and I was getting EACCES. But I implemented it
> again now, and it works fine. So this is not an issue.

I wouldn't be surprised if it was broken at some point.

> I also saw mmap(..., MAP_FIXED) being used in the selftest. Is there a reason
> to use this over mprotect?

To test that SGX handles MAP_FIXED correctly.  At one point the driver was
ignoring it.

> >> VDSO:
> >>
> >> It is *difficult* to link to weakly link to a symbol in the VDSO. Anyway, I
> >> figured it out.
> >>
> >> 1. What if I don't want to automatically ERESUME after kernel interrupt?
> > 
> > Do EENTER/ERESUME directly instead of going through the vDSO.
> 
> That kind of defeats the point.

Of providing the vDSO?  My stance on not allowing userspace fine-grained
control over asynchronous enclave exists hasn't changed since we last
discussed the issue[1].  Preventing userspace from hooking interrupts can
help mitigate certain attacks, and it also aligns with normal userspace,
which doesn't get to hook interrupts either.

[1] https://lkml.kernel.org/r/20181102171251.GE7393@linux.intel.com

> >> 2. I normally do a sanity check after ENCLU[EENTER] that EAX = EEXIT. The
> >> current implementation just clears EAX instead without looking at it.
> > 
> > Hmm, the only reason I can think of for checking EAX would be to support
> > userspace mucking with EAX in a #DB/#BP signal handler.  At that point, I
> > would expect the signal handler to modify RIP as well.  Reaching the XOR
> > via any other non-EEXIT path would require a kernel bug.
> 
> Or a CPU bug.

Eh, it's possible, but odds are very high that a CPU bug in this area
would have other side effects, i.e. would be noticed quite quickly even
without an explicit check.

> > Was there a specific scenario or use case you had in mind?  I'm not
> > against adding a check, I just don't see what value it would provide.
> 
> Nothing specific. It just seems like a prudent thing to do when messing with
> control flow in unexpected ways. Alternatively, just remove the xor, and
> change the API of the function so that 3 is the normal return value? Then the
> user can decide themselves if they think the check is worth it.

Andy, thoughts?  We also never bottomed out what should be returned when
an exception is reported to the caller[2], i.e. if there's a better option
than -EFAULT.

[2] https://lkml.kernel.org/r/90D05734-1583-4306-A9A4-18E4A1390F3B@amacapital.net

Re: x86/sgx: v23-rc2

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 3765 bytes --]

On 2019-10-17 19:57, Sean Christopherson wrote:
> +Cc Andy
> 
> On Mon, Oct 14, 2019 at 08:43:09AM +0000, Jethro Beekman wrote:
>> On 2019-10-11 20:15, Sean Christopherson wrote:
>>> On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote:
>>>> UAPI:
>>>>
>>>> This got a whole lot more complex for userspace compared to the out-of-tree
>>>> driver.
>>>>
>>>> 1. Manually needing to mmap a naturally-aligned memory region by allocating
>>>> too much memory and then unmapping parts is quite annoying. Why was the
>>>> auto-aligning removed? I think this will need to be handled the same for
>>>> every consumer of SGX, so I don't see why this is not handled in the kernel.
>>>> It never seems wrong to align if NULL is passed as the requested address.
>>>> Alternatively, is there room in the flags for a MAP_ALIGNED bit?
>>>
>>> I'm pretty sure everyone agrees it's annoying.  The short of it is that
>>> the SGX driver is the wrong place to do the alignment.  The driver could
>>> key off addr=0, but we don't want to take on that implicit behavior.
>>
>> Why not?
> 
> Because it's a hack.  If a MAP_ALIGNED flag is added then SGX is stuck
> with kludgy code that serves no purpose.  And userspace needs to manually
> align the result if it provides an actual hint.  Regardless of whether
> there are use cases for providing a hint for ELRANGE, having divergent
> logic is ugly.
> 
>>> A MAP_ALIGNED flag to have the allocation be naturally aligned is the
>>> ideal solution.  It's definitely something we should pursue, but that can
>>> and probably should be done in parallel to the SGX series.
>>>
>>>> 2. Having to re-open the device for every enclave is also annoying. This
>>>> means you need a filesystem available throughout the process lifetime. I
>>>> tried dup, but that doesn't work. Can we make dup work?
>>>
>>> The semantics of dup() won't get you what want, as dup() just creates a
>>> new descriptor pointing at the same file.
>>>
>>> An alternative solution that was proposed was to have an ioctl() for
>>> creating an enclave.  But that means using an anonymous inode, which runs
>>> afoul of SELinux permissions, e.g. every _process_ that runs enclaves
>>> would require EXECMEM.  Linus was quite clear that SGX wouldn't be merged
>>> if using it required users to degrade existing security.
>>
>> It's ok if it's the same inode, it just needs to be a different struct file.
>>
>>> I'm open to other ideas.  I wasn't aware this was a pain point and file
>>> stuff isn't exactly my area of expertise, so I haven't put much/any
>>> thought into alternatives.
>>
>> The default permissions for /dev/sgx/enclave are root-only. This means you
>> want to be able to do the same thing as network servers: initialize some
>> resources as root, then drop privileges. This used to mean opening /dev/sgx
>> and keeping the fd around which meant you could launch enclaves at will. With
>> the new API, this is no longer possible, you can only launch one enclave per
>> fd. Is there a different type of operation that doesn't just duplicate the fd
>> but also the struct file? If not, can we add an ioctl for that?
> 
> My approach to this would be to chown /dev/sgx/enclave so that it's owned
> by root but accessible to users belonging to an sgx-specific group, e.g.
> via a udev rule.
> 
>> There are other scenarios where it's not just the permissions on
>> /dev/sgx/enclave that are the problem but using the filesystem in general
>> that is. Maybe you've used seccomp to disable file operations, etc.
> 
> Andy and Jarkko, thoughts?

Folks, any more thoughts on how to resolve the issue that you need to call open() for every enclave?

--
Jethro Beekman | Fortanix


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
> >> There are other scenarios where it's not just the permissions on
> >> /dev/sgx/enclave that are the problem but using the filesystem in general
> >> that is. Maybe you've used seccomp to disable file operations, etc.
> > 
> > Andy and Jarkko, thoughts?
> 
> Folks, any more thoughts on how to resolve the issue that you need to
> call open() for every enclave?

Why is it an issue?

/Jarkko

Re: x86/sgx: v23-rc2

[Jethro Beekman]

On 2020-02-15 08:24, Jarkko Sakkinen wrote:
> On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
>>>> There are other scenarios where it's not just the permissions on
>>>> /dev/sgx/enclave that are the problem but using the filesystem in general
>>>> that is. Maybe you've used seccomp to disable file operations, etc.
>>>
>>> Andy and Jarkko, thoughts?
>>
>> Folks, any more thoughts on how to resolve the issue that you need to
>> call open() for every enclave?
> 
> Why is it an issue?

Already discussed in https://www.spinics.net/lists/linux-sgx/msg02075.html

--
Jethro Beekman | Fortanix

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Mon, Feb 17, 2020 at 09:52:25AM +0100, Jethro Beekman wrote:
> On 2020-02-15 08:24, Jarkko Sakkinen wrote:
> > On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
> >>>> There are other scenarios where it's not just the permissions on
> >>>> /dev/sgx/enclave that are the problem but using the filesystem in general
> >>>> that is. Maybe you've used seccomp to disable file operations, etc.
> >>>
> >>> Andy and Jarkko, thoughts?
> >>
> >> Folks, any more thoughts on how to resolve the issue that you need to
> >> call open() for every enclave?
> > 
> > Why is it an issue?
> 
> Already discussed in https://www.spinics.net/lists/linux-sgx/msg02075.html

Not anyone has to have access to open /dev/sgx/enclave in order to use
enclaves.

It is as much a problem as for practically any driver that provides
devices for some use.

/Jarkko

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Mon, Feb 17, 2020 at 08:55:26PM +0200, Jarkko Sakkinen wrote:
> On Mon, Feb 17, 2020 at 09:52:25AM +0100, Jethro Beekman wrote:
> > On 2020-02-15 08:24, Jarkko Sakkinen wrote:
> > > On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
> > >>>> There are other scenarios where it's not just the permissions on
> > >>>> /dev/sgx/enclave that are the problem but using the filesystem in general
> > >>>> that is. Maybe you've used seccomp to disable file operations, etc.
> > >>>
> > >>> Andy and Jarkko, thoughts?
> > >>
> > >> Folks, any more thoughts on how to resolve the issue that you need to
> > >> call open() for every enclave?
> > > 
> > > Why is it an issue?
> > 
> > Already discussed in https://www.spinics.net/lists/linux-sgx/msg02075.html
> 
> Not anyone has to have access to open /dev/sgx/enclave in order to use
      ~~~~~~
      everyone

/Jarkko

Re: x86/sgx: v23-rc2

[Dr. Greg Wettstein]

On Mon, Feb 17, 2020 at 08:55:26PM +0200, Jarkko Sakkinen wrote:

Good morning, I hope the week is starting well for everyone.

> On Mon, Feb 17, 2020 at 09:52:25AM +0100, Jethro Beekman wrote:
> > On 2020-02-15 08:24, Jarkko Sakkinen wrote:
> > > On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
> > >>>> There are other scenarios where it's not just the permissions on
> > >>>> /dev/sgx/enclave that are the problem but using the filesystem in general
> > >>>> that is. Maybe you've used seccomp to disable file operations, etc.
> > >>>
> > >>> Andy and Jarkko, thoughts?
> > >>
> > >> Folks, any more thoughts on how to resolve the issue that you need to
> > >> call open() for every enclave?
> > > 
> > > Why is it an issue?
> > 
> > Already discussed in https://www.spinics.net/lists/linux-sgx/msg02075.html

I believe an accurate summary of Dr. Beekman's concerns are as
follows:

1.) He envisions a need for an enclave orchestrator that uses root
privileges to open the SGX driver device and then drop privileges,
presumably in a permanent fashion.  The orchestrator would then use
the filehandle to load and initialize multiple enclaves on request.

2.) The enclave orchestrator may be run in an environment that has
SECCOMP limitations on the ability to conduct filesystem operations.

> Not everyone has to have access to open /dev/sgx/enclave in order to
> use enclaves.

That depends on the architecture of the runtime.

The Intel PSW has a model where the AESM daemon orchestrates
activities for applications that wish to use enclaves.

Our Secure Runtime Development Environment (SRDE), on the other hand,
uses a model where each application independently manages its use of
enclaves.  In our model, the SGX driver needs to be accessible by
whatever privilege level an application chooses to run at.

As a result of this model, our SRDE has around an order of magnitude
less complexity then the AESM model and virtually no external
dependencies.  At last count the AESM depends on about 35 separate
shared library systems.

Our model allows us to build applications that are statically linked
and completely self-contained.  A concept of reasonable utility for
containerized environments and embedded systems.  We also avoid C++
that allows us to build MUSL based security stacks.

> It is as much a problem as for practically any driver that provides
> devices for some use.

The only customers of this driver are the runtime developers and that
is currently an extremely limited spectrum of users.

I will concede that I don't understand the SECCOMP issue.  Jethro may
or may not be able to discuss an architecture that would not have
basic filesystem operations available to it.

The fundamental issue, that there appears to be a reluctance to
discuss, is that DAC/MAC controls are not relevant to SGX,
particularly SGX2 based systems, which is what most of this stuff
will be running on.

> /Jarkko

Have a good day.

Dr. Greg

As always,
Dr. Greg Wettstein, Ph.D    Worker / Principal Engineer
IDfusion, LLC
4206 19th Ave N.            Specialists in SGX secured infrastructure.
Fargo, ND  58102
PH: 701-281-1686            CELL: 701-361-2319
EMAIL: gw@idfusion.org
------------------------------------------------------------------------------
"... remember that innovation is saying 'no' to 1000 things."
                                -- Moxie Marlinspike

Re: x86/sgx: v23-rc2

[Andy Lutomirski]

> On Feb 18, 2020, at 2:43 AM, Dr. Greg Wettstein <gw@idfusion.org> wrote:
> 
> On Mon, Feb 17, 2020 at 08:55:26PM +0200, Jarkko Sakkinen wrote:
> 
> Good morning, I hope the week is starting well for everyone.
> 
>>> On Mon, Feb 17, 2020 at 09:52:25AM +0100, Jethro Beekman wrote:
>>> On 2020-02-15 08:24, Jarkko Sakkinen wrote:
>>>> On Thu, Feb 13, 2020 at 03:10:24PM +0100, Jethro Beekman wrote:
>>>>>>> There are other scenarios where it's not just the permissions on
>>>>>>> /dev/sgx/enclave that are the problem but using the filesystem in general
>>>>>>> that is. Maybe you've used seccomp to disable file operations, etc.
>>>>>> 
>>>>>> Andy and Jarkko, thoughts?
>>>>> 
>>>>> Folks, any more thoughts on how to resolve the issue that you need to
>>>>> call open() for every enclave?
>>>> 
>>>> Why is it an issue?
>>> 
>>> Already discussed in https://www.spinics.net/lists/linux-sgx/msg02075.html
> 
> I believe an accurate summary of Dr. Beekman's concerns are as
> follows:
> 
> 1.) He envisions a need for an enclave orchestrator that uses root
> privileges to open the SGX driver device and then drop privileges,
> presumably in a permanent fashion.  The orchestrator would then use
> the filehandle to load and initialize multiple enclaves on request.
> 
> 2.) The enclave orchestrator may be run in an environment that has
> SECCOMP limitations on the ability to conduct filesystem operations.
> 
>> Not everyone has to have access to open /dev/sgx/enclave in order to
>> use enclaves.
> 
> That depends on the architecture of the runtime.
> 
> The Intel PSW has a model where the AESM daemon orchestrates
> activities for applications that wish to use enclaves.
> 
> Our Secure Runtime Development Environment (SRDE), on the other hand,
> uses a model where each application independently manages its use of
> enclaves.  In our model, the SGX driver needs to be accessible by
> whatever privilege level an application chooses to run at.
> 
> As a result of this model, our SRDE has around an order of magnitude
> less complexity then the AESM model and virtually no external
> dependencies.  At last count the AESM depends on about 35 separate
> shared library systems.
> 
> Our model allows us to build applications that are statically linked
> and completely self-contained.  A concept of reasonable utility for
> containerized environments and embedded systems.  We also avoid C++
> that allows us to build MUSL based security stacks.
> 
>> It is as much a problem as for practically any driver that provides
>> devices for some use.
> 
> The only customers of this driver are the runtime developers and that
> is currently an extremely limited spectrum of users.
> 
> I will concede that I don't understand the SECCOMP issue.  Jethro may
> or may not be able to discuss an architecture that would not have
> basic filesystem operations available to it.

Jethro, is there a way you could use SCM_RIGHTS to solve your problem?  In principle, the driver could allow an enclave fd that isn’t ECREATEd yet to be forked with an ioctl, but ISTM this is extra complexity and there should be a well understood use case before something like this is added.

> 
> The fundamental issue, that there appears to be a reluctance to
> discuss, is that DAC/MAC controls are not relevant to SGX,
> particularly SGX2 based systems, which is what most of this stuff
> will be running on.
> 

No, this isn’t the fundamental issue, because DAC/MAC is just as relevant to systems with SGX and SGX2.  I’m sure that there will be systems that work with SGX and with everything running as root (or even SGX on a kernel without access control at all), but these will be a minority. Even if you have absolutely perfect protection of sensitive data, you sacrifice any sort of defense in depth against an attacker DoSing you in any number of amazing ways.  Or, for that matter, leaking keys derived from the provisioning key.

(Don’t forget that, no matter how carefully your system might lock down the SGXPUBKEYHASH MSRs and control the associated keys, that lockdown is being done by non-enclave *software*, and a bad guy who gets root is quite likely to be able to unlock the registers by upgrading their root access to control the firmware.  Or get a launch token that can’t be revoked because SGX doesn’t have a token expiration mechanism.)

No one wants to discuss your DAC-less model because, as far as I can tell, no one agrees with you.

>> /Jarkko
> 
> Have a good day.
> 
> Dr. Greg
> 
> As always,
> Dr. Greg Wettstein, Ph.D    Worker / Principal Engineer
> IDfusion, LLC
> 4206 19th Ave N.            Specialists in SGX secured infrastructure.
> Fargo, ND  58102
> PH: 701-281-1686            CELL: 701-361-2319
> EMAIL: gw@idfusion.org
> ------------------------------------------------------------------------------
> "... remember that innovation is saying 'no' to 1000 things."
>                                -- Moxie Marlinspike

Re: x86/sgx: v23-rc2

[Dr. Greg]

On Tue, Feb 18, 2020 at 07:00:23AM -0800, Andy Lutomirski wrote:

Good evening Andy, always good to hear from you.

> > On Feb 18, 2020, at 2:43 AM, Dr. Greg Wettstein <gw@idfusion.org> wrote:
> > The fundamental issue, that there appears to be a reluctance to
> > discuss, is that DAC/MAC controls are not relevant to SGX,
> > particularly SGX2 based systems, which is what most of this stuff
> > will be running on.

> No, this isn't the fundamental issue, because DAC/MAC is just as
> relevant to systems with SGX and SGX2.  I'm sure that there will
> be systems that work with SGX and with everything running as root
> (or even SGX on a kernel without access control at all), but these
> will be a minority. Even if you have absolutely perfect protection
> of sensitive data, you sacrifice any sort of defense in depth
> against an attacker DoSing you in any number of amazing ways.  Or,
> for that matter, leaking keys derived from the provisioning key.

I should have been more precise, my apologies.

When I was suggesting that DAC/MAC controls were not relevant, I was
referring to their relevance with respect to protecting the platform
from running code that does not have defined provenance or origin.

The last time we had this conversation, it spurred you to make issue
of the fact that the current driver allows writable memory to be
executable, without the contents of the memory being properly vetted
by the LSM, without doubt an important issue.  LWN even ran an article
on the issue.

I'm certainly not argueing against defense in depth or the merits of
DAC/MAC controls.  I'm simply stating that they are irrelevant, with
respect to the concerns that you raised about code provenance and
origin, on flexible launch control platforms that support SGX2 and
EDMM.  Platforms that by and large will be the primary target for this
driver.

I have no illusions of convincing you or anyone else involved in the
development of this driver, of the merits of our concern.  However,
anyone who has worked on this technology at length, and is
intellectually honest, knows it is a legitimate security issue.

Bruce Schneier even weighed in on the issue, although from a somewhat
different perspective:

https://www.schneier.com/blog/archives/2017/03/using_intels_sg.html

I only bring up the issue in the interests of intellectual honesty and
the fact that I'm sure our exchanges will go down in the annals of
security history as being as important as the Lincoln/Douglas debates.

It is one of my jobs to follow the security literature closely.
Everyone who does so can easily envision a future abstract reading
something like the following:

"Using an unmodified and stock DIST_OS kernel we were able to
demonstrate the download and execution of malicious code into an
enclave that bypasses all security controls in the Linux operating
system allowing us to recover XXXX information from the platform with
subsequent exfiltration of the information from the platform without
detection."

:-)

FWIW, from your statement above, it is unclear how DAC/MAC controls of
any type or depth would prevent leaking keys derived from the
provisioning key.  One prevents leakage of provisioning derived keys
by implementing cryptographic provenance of code execution so that you
have some modicum of trust that enclave code that has access to the
provisioning key can be trusted to 'do the right thing'.

> (Don't forget that, no matter how carefully your system might lock
> down the SGXPUBKEYHASH MSRs and control the associated keys, that
> lockdown is being done by non-enclave *software*, and a bad guy who
> gets root is quite likely to be able to unlock the registers by
> upgrading their root access to control the firmware.  Or get a launch
> token that can't be revoked because SGX doesn't have a token
> expiration mechanism.)

Dispassionate observers would note that you make the case for locked
launch control registers.... :-)

At an SGX engineering meeting in Israel last summer, we made the case
for the fact that locked vs. unlocked platforms should be a BIOS
configurable option.  With an additional option that specifying locked
also allows specification of what the identity modulus signature
should be.  That would seem to be the best of all worlds, we will see
what happens.

One of the pushbacks we received, is that SGX is supposed to be immune
from firmware manipulation, which our suggested approach would open
the door for, which we noted was irrelevant given the trajectory that
the Linux kernel driver is on, ie. no cryptographic controls over code
origin and provenance.

Just to provide a frame of reference, our interest in SGX is with
respect to its guarantee of integrity of execution, for the purposes
of verifying that the kernel could not have executed code that was
outside a desired behavioral definition for the platform.

We namespaced a modified implementation of Linux IMA and our container
launch system pairs each behavioral canister with an SGX enclave that
introspects the namespace's behavior.  The enclave labels a process
attempting an extra-dimensional Turing event as a 'bad actor', subject
to appropriate discipline by a modified LSM.

So known cryptographic integrity and provenance of executed code
positions us to make a strong attestation statement regarding the
integrity of the platform as a whole.  There is a significant body of
interest in the ability to support such statements.

We also addressed the problem of perpetual launch tokens but that is
an issue for another time.

> No one wants to discuss your DAC-less model because, as far as I can
> tell, no one agrees with you.

Stating that no one, in a world of 7.7 billion people, agrees with us
on these issues would seem to make that contention vulnerable to any
finding of a defect in absolutism.

The number of downloads of our patch set and GIT pulls of our modified
version of Jarkko's driver speaks otherwise, and has convinced us to
expend the cycles needed to continue maintaining and making available
an approach that provides some notion of cryptographic based
provenance and origin of SGX based execution on FLC platforms.

We tend to take solace and energy from the fact that Henry Ford's
contemporaries criticized him for not spending his time breeding
faster horses that ate and drank less.

Best wishes for an enjoyable and productive weekend to everyone.

Dr. Greg

As always,
Dr. Greg Wettstein, Ph.D, Worker
IDfusion, LLC               SGX secured infrastructure and
4206 N. 19th Ave.           autonomously self-defensive platforms.
Fargo, ND  58102
PH: 701-281-1686            EMAIL: greg@idfusion.net
------------------------------------------------------------------------------
"Now Ed, is there a need to be concerned about edging?"
                                -- Alan Biddison - Snowmass 2019
                                   Resurrection.

Re: x86/sgx: v23-rc2

[Andy Lutomirski]

On Fri, Feb 21, 2020 at 7:16 PM Dr. Greg <greg@enjellic.com> wrote:
>
> On Tue, Feb 18, 2020 at 07:00:23AM -0800, Andy Lutomirski wrote:
>
> > > On Feb 18, 2020, at 2:43 AM, Dr. Greg Wettstein <gw@idfusion.org> wrote:

> > No, this isn't the fundamental issue, because DAC/MAC is just as
> > relevant to systems with SGX and SGX2.  I'm sure that there will
> > be systems that work with SGX and with everything running as root
> > (or even SGX on a kernel without access control at all), but these
> > will be a minority. Even if you have absolutely perfect protection
> > of sensitive data, you sacrifice any sort of defense in depth
> > against an attacker DoSing you in any number of amazing ways.  Or,
> > for that matter, leaking keys derived from the provisioning key.
>
> I should have been more precise, my apologies.
>
> When I was suggesting that DAC/MAC controls were not relevant, I was
> referring to their relevance with respect to protecting the platform
> from running code that does not have defined provenance or origin.

Windows systems quite regularly get owned by code with "defined
provenance or origin" in the sense that it was signed by a valid
Authenticode certificate. :)

> I'm certainly not argueing against defense in depth or the merits of
> DAC/MAC controls.  I'm simply stating that they are irrelevant, with
> respect to the concerns that you raised about code provenance and
> origin, on flexible launch control platforms that support SGX2 and
> EDMM.  Platforms that by and large will be the primary target for this
> driver.

I have a little FLC platform on my desk.  What I don't have is an FLC
platform with anything resembling a policy that helps here at all.

> I only bring up the issue in the interests of intellectual honesty and
> the fact that I'm sure our exchanges will go down in the annals of
> security history as being as important as the Lincoln/Douglas debates.

That would be amazing!

>
> It is one of my jobs to follow the security literature closely.
> Everyone who does so can easily envision a future abstract reading
> something like the following:
>
> "Using an unmodified and stock DIST_OS kernel we were able to
> demonstrate the download and execution of malicious code into an
> enclave that bypasses all security controls in the Linux operating
> system allowing us to recover XXXX information from the platform with
> subsequent exfiltration of the information from the platform without
> detection."
>

Alas, the significance of this seems to depend on what XXXX is.  If
XXXX is a detail of the specific machine (a derived provisioning key,
a launch token, etc.), then the paper won't be terribly interesting.
As root (or someone who can escalate to root or better) on a given
system, I can extract *tons* of identifying details of the platform.
If XXXX is a secret created by the user (a sealing key, something
sealed by SGX, a private key or secret that allows one to pretend to
be an enclave with MRSIGNER=YYY, etc), then launch control and DAC are
essentially irrelevant to the discussion.  What actually happened is
that some code signed by YYY screwed up.  And this is because launch
control is not part of the SGX root of trust, nor is launch control
even a necessary part of the SGX security model.  SGX with launch
control deleted entirely would be just as useful, if potentially a bit
more dangerous in terms of being able to make malware hard to analyze.

> > (Don't forget that, no matter how carefully your system might lock
> > down the SGXPUBKEYHASH MSRs and control the associated keys, that
> > lockdown is being done by non-enclave *software*, and a bad guy who
> > gets root is quite likely to be able to unlock the registers by
> > upgrading their root access to control the firmware.  Or get a launch
> > token that can't be revoked because SGX doesn't have a token
> > expiration mechanism.)
>
> Dispassionate observers would note that you make the case for locked
> launch control registers.... :-)
>
> At an SGX engineering meeting in Israel last summer, we made the case
> for the fact that locked vs. unlocked platforms should be a BIOS
> configurable option.  With an additional option that specifying locked
> also allows specification of what the identity modulus signature
> should be.  That would seem to be the best of all worlds, we will see
> what happens.
>
> One of the pushbacks we received, is that SGX is supposed to be immune
> from firmware manipulation, which our suggested approach would open
> the door for, which we noted was irrelevant given the trajectory that
> the Linux kernel driver is on, ie. no cryptographic controls over code
> origin and provenance.

I don't believe I have ever said that Linux shouldn't support
cryptographic verification of SGX code provenance or that Linux
shouldn't eventually support locked launch control MSRs.  What I've
said is that the initial upstream version of the driver doesn't need
this, and that adding support for locked MSRs is tricky and needs some
design work.

FWIW, I didn't expect upstreaming to take anywhere near this long.

>
> Just to provide a frame of reference, our interest in SGX is with
> respect to its guarantee of integrity of execution, for the purposes
> of verifying that the kernel could not have executed code that was
> outside a desired behavioral definition for the platform.

SGX cannot and will never prevent the kernel from executing normal
CPL0 code of any sort.  I'm not really sure what you're getting at
here.  If you have magically trustworthy firmware and locked launch
control registers, you can prevent the kernel from executing
unapproved *enclave* code.  It's not entirely clear to me why you
consider unapproved enclave code to be worse than any other sort of
unapproved code.

Re: x86/sgx: v23-rc2

[Dr. Greg]

On Fri, Feb 21, 2020 at 09:41:24PM -0800, Andy Lutomirski wrote:

Good afternoon, I hope the weekend is going well for everyone.

> On Fri, Feb 21, 2020 at 7:16 PM Dr. Greg <greg@enjellic.com> wrote:
> > I should have been more precise, my apologies.
> >
> > When I was suggesting that DAC/MAC controls were not relevant, I was
> > referring to their relevance with respect to protecting the platform
> > from running code that does not have defined provenance or origin.

> Windows systems quite regularly get owned by code with "defined
> provenance or origin" in the sense that it was signed by a valid
> Authenticode certificate. :)

Excellent observation, signing is a metric of identity not
correctness.

Since the first HASP papers were published, Intel has been consistent
with respect to their guidance that buggy code inside of an enclave is
still buggy code.

Windows code signing is designed to provide a guarantee that code is
of known origin.  SGX signing serves the same purpose, the objective
of cryptographic launch control is to protect the platform from the
technology itself, through the use of cryptographic identity controls.

> > I'm certainly not argueing against defense in depth or the merits
> > of DAC/MAC controls.  I'm simply stating that they are irrelevant,
> > with respect to the concerns that you raised about code provenance
> > and origin, on flexible launch control platforms that support SGX2
> > and EDMM.  Platforms that by and large will be the primary target
> > for this driver.

> I have a little FLC platform on my desk.  What I don't have is an
> FLC platform with anything resembling a policy that helps here at
> all.

I believe you have a Gemini Lake SOC based NUC, presumably a NUC7CJYH
or NUC7PJYH.  In addition to FLC, that system has support for SGX2
instructions and thus EDMM.

So you have everything you need to demonstrate the ability to download
and execute code without the OS being able to pass judgement on the
code, other then the OS identity of the user accessing the device node
that loaded a bootstrap enclave.

> > I only bring up the issue in the interests of intellectual honesty
> > and the fact that I'm sure our exchanges will go down in the
> > annals of security history as being as important as the
> > Lincoln/Douglas debates.

> That would be amazing!

The driver initiative has hopefully benefited from a technically
honest discussion of the issues.

> > It is one of my jobs to follow the security literature closely.
> > Everyone who does so can easily envision a future abstract reading
> > something like the following:
> >
> > "Using an unmodified and stock DIST_OS kernel we were able to
> > demonstrate the download and execution of malicious code into an
> > enclave that bypasses all security controls in the Linux operating
> > system allowing us to recover XXXX information from the platform with
> > subsequent exfiltration of the information from the platform without
> > detection."

> Alas, the significance of this seems to depend on what XXXX is.  If
> XXXX is a detail of the specific machine (a derived provisioning key,
> a launch token, etc.), then the paper won't be terribly interesting.
> As root (or someone who can escalate to root or better) on a given
> system, I can extract *tons* of identifying details of the platform.
> If XXXX is a secret created by the user (a sealing key, something
> sealed by SGX, a private key or secret that allows one to pretend to
> be an enclave with MRSIGNER=YYY, etc), then launch control and DAC are
> essentially irrelevant to the discussion.  What actually happened is
> that some code signed by YYY screwed up.  And this is because launch
> control is not part of the SGX root of trust, nor is launch control
> even a necessary part of the SGX security model.  SGX with launch
> control deleted entirely would be just as useful, if potentially a bit
> more dangerous in terms of being able to make malware hard to
> analyze.

You are absolutely correct, cryptographic access control doesn't
influence the enclave security guarantees that SGX implements.  Launch
control is about imposing a cryptographic identity guarantee on the
origin of code executed in enclave context.

Windows Authenticode is about protecting the platform from an entity
attempting to execute malicious code on a platform.  SGX launch
control is about protecting a platform from malicious code running in
an enclave.

The challenge with an EDMM capable SGX platform is that anyone with
the ability to load, initialize and execute an enclave is capable of
bringing code that has potentially adversarial intent onto the
platform and to execute that code.  The executing code has full
visibility into the address space of the process that loaded the
enclave without the need for OCALL's.

I'm confident the security research community will come up with
interesting things that can be accomplished within the constraints of
this model.

The code being loaded onto the platform will be encrypted and
integrity protected so there will be no visibility into the code being
loaded.  By design, once loaded, the execution and functioning of the
code will also have no visibility by the platform, except possibly
through the use and analysis of various side channels.

These are all well understood issues.  Joanna Rutkowska was discussing
the implications of all this seven years ago:

https://blog.invisiblethings.org/2013/09/23/thoughts-on-intels-upcoming-software.html

Everyone seems to be in agreement that the LSM should be able to have
a 'look' at enclave code, although the initial driver implementation
won't have support for that.  If we pride ourselves on being
intellectually honest, we have to ask ourselves why we would even
spend time on adding LSM support given that it will largely amount to
security theater.

There is a high probability that taking advantage of this
functionality will be popular, given the fact that due to user demand,
Intel has implemented support for Protected Code Loading (PCL) in
their SDK.  Anyone that is interested, can look at the implementation
in the following directory of the Intel SGX SDK:

SampleCode/SampleEnclavePCL

The current model involves encrypting the enclave .so file with
subsequent decryption by a loader enclave.  An EDMM based solution
will be both more straight forward and secure from tampering.

There is a great deal of interest in a model where the system
administrators and/or platform deployers have no visibility into the
code or data being executed on the platform.  The Confidential
Computing Consortium (CCC), composed of all the 'bigs - The Linux
Foundation, IBM, Microsoft, GOOGLE, Tencent, Alibaba et.al', has been
put together to support this model.

I suspect a lot of the push to ship code and worry about some of these
pesky security details later arises from a desire to support this
initiative.

Russinovich, the CTO of Microsoft, has a discussion about all of this
on his blob, including a link to the CCC site:

https://cloudblogs.microsoft.com/opensource/2019/08/21/microsoft-partners-linux-foundation-announce-confidential-computing-consortium/

If you check the CCC site itself you will see a reference to the
possibility of this technology being used for nefarious purposes and
that there are research initiatives and best practices that will be
promoted to address this.

All of this dovetails, somewhat, with the 'Zero-Trust' initiative that
is all the rage these days.  All of this is ultimately converging on
the notion that trust needs to be based on a cryptographically secured
expression of identity.

> > > (Don't forget that, no matter how carefully your system might lock
> > > down the SGXPUBKEYHASH MSRs and control the associated keys, that
> > > lockdown is being done by non-enclave *software*, and a bad guy who
> > > gets root is quite likely to be able to unlock the registers by
> > > upgrading their root access to control the firmware.  Or get a launch
> > > token that can't be revoked because SGX doesn't have a token
> > > expiration mechanism.)
> >
> > Dispassionate observers would note that you make the case for locked
> > launch control registers.... :-)
> >
> > At an SGX engineering meeting in Israel last summer, we made the case
> > for the fact that locked vs. unlocked platforms should be a BIOS
> > configurable option.  With an additional option that specifying locked
> > also allows specification of what the identity modulus signature
> > should be.  That would seem to be the best of all worlds, we will see
> > what happens.
> >
> > One of the pushbacks we received, is that SGX is supposed to be immune
> > from firmware manipulation, which our suggested approach would open
> > the door for, which we noted was irrelevant given the trajectory that
> > the Linux kernel driver is on, ie. no cryptographic controls over code
> > origin and provenance.

> I don't believe I have ever said that Linux shouldn't support
> cryptographic verification of SGX code provenance or that Linux
> shouldn't eventually support locked launch control MSRs.  What I've
> said is that the initial upstream version of the driver doesn't need
> this, and that adding support for locked MSRs is tricky and needs
> some design work.

It would seem the consensus of opinion is to ship the driver and then
worry about the security details later.

By now everyone has seen a full discussion on the issues involved,
hopefully it will help to provide a framework for future refinements
to the driver.

In the meantime we will be announcing patches to add cryptographic
trust controls for the driver, so that will be an option for those who
have concerns about the issue until when and if there is consensus on
how to address these issues in the mainline driver.

> FWIW, I didn't expect upstreaming to take anywhere near this long.

I don't think anyone did.

> > Just to provide a frame of reference, our interest in SGX is with
> > respect to its guarantee of integrity of execution, for the
> > purposes of verifying that the kernel could not have executed code
> > that was outside a desired behavioral definition for the platform.

> SGX cannot and will never prevent the kernel from executing normal
> CPL0 code of any sort.  I'm not really sure what you're getting at
> here.  If you have magically trustworthy firmware and locked launch
> control registers, you can prevent the kernel from executing
> unapproved *enclave* code.  It's not entirely clear to me why you
> consider unapproved enclave code to be worse than any other sort of
> unapproved code.

I believe we have a full implementation and demonstration of the
ability of SGX to control what the kernel chooses to execute or
actions it conducts, at least within the framework of the controls
that the LSM provides.  I replied to the KRSI post with a brief
description of our work.

The ultimate future of an initiative such as the CCC is going to
depend on cloud clients being able to have some indication of what
trust to afford to the entire platform.  That will ultimately come
down to the ability to have a cryptographic statement, within modeling
limits, of what behaviors the platform has been allowed to express.

Given the implications of SGX2/EDMM, that will also include a
statement of what cryptographic identities have been able to execute
code in enclave context.

But we won't go down that rathole now.

Best wishes for a productive week.

Dr. Greg

As always,
Dr. Greg Wettstein, Ph.D    Worker / Principal Engineer
IDfusion, LLC
4206 19th Ave N.            Specialists in SGX secured infrastructure.
Fargo, ND  58102
PH: 701-281-1686            CELL: 701-361-2319
EMAIL: gw@idfusion.org
------------------------------------------------------------------------------
"It would appear that we have reached the limits of what it is
 possible to achieve with computer technology, although one should be
 careful with such statements, as they tend to sound pretty silly in 5
 years."
                                -- John Von Neumann (ca. 1949)

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Fri, Feb 21, 2020 at 09:16:08PM -0600, Dr. Greg wrote:
> Dispassionate observers would note that you make the case for locked
> launch control registers.... :-)
> 
> At an SGX engineering meeting in Israel last summer, we made the case
> for the fact that locked vs. unlocked platforms should be a BIOS
> configurable option.  With an additional option that specifying locked
> also allows specification of what the identity modulus signature
> should be.  That would seem to be the best of all worlds, we will see
> what happens.
> 
> One of the pushbacks we received, is that SGX is supposed to be immune
> from firmware manipulation, which our suggested approach would open
> the door for, which we noted was irrelevant given the trajectory that
> the Linux kernel driver is on, ie. no cryptographic controls over code
> origin and provenance.
> 
> Just to provide a frame of reference, our interest in SGX is with
> respect to its guarantee of integrity of execution, for the purposes
> of verifying that the kernel could not have executed code that was
> outside a desired behavioral definition for the platform.

I'm not too opionated with this. The way things are is the consensus
what is the least common denominator of things that can be accepted by
the majority involved with the kernel.

What I'm deeply opionated is that locked configuration cannot be a part
of the current patch set. I'm neither going to proactively push such
support when the code is in the upstream. However, I'm always ready to
review any code changes and look into arguments (usually contained in
the cover letter) in a neutral fashion, no matter what the code change
is.

/Jarkko

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Tue, Feb 18, 2020 at 04:42:43AM -0600, Dr. Greg Wettstein wrote:
> I believe an accurate summary of Dr. Beekman's concerns are as
> follows:
> 
> 1.) He envisions a need for an enclave orchestrator that uses root
> privileges to open the SGX driver device and then drop privileges,
> presumably in a permanent fashion.  The orchestrator would then use
> the filehandle to load and initialize multiple enclaves on request.
> 
> 2.) The enclave orchestrator may be run in an environment that has
> SECCOMP limitations on the ability to conduct filesystem operations.

Also UDS sockets with SCM_RIGHTS should work.

/Jarkko

Re: x86/sgx: v23-rc2

[Dr. Greg]

On Tue, Feb 18, 2020 at 05:52:47PM +0200, Jarkko Sakkinen wrote:

Good morning, I hope the day is going well for everyone.

> On Tue, Feb 18, 2020 at 04:42:43AM -0600, Dr. Greg Wettstein wrote:
> > I believe an accurate summary of Dr. Beekman's concerns are as
> > follows:
> > 
> > 1.) He envisions a need for an enclave orchestrator that uses root
> > privileges to open the SGX driver device and then drop privileges,
> > presumably in a permanent fashion.  The orchestrator would then use
> > the filehandle to load and initialize multiple enclaves on request.
> > 
> > 2.) The enclave orchestrator may be run in an environment that has
> > SECCOMP limitations on the ability to conduct filesystem operations.

> Also UDS sockets with SCM_RIGHTS should work.

The first clarification, I'm assuming you mean passing SCM_RIGHTS over
AF_UNIX/UNIX-domain sockets, not UDP sockets?

Secondly, it was my understanding that the reason major renovations to
the driver were initiated, almost a year ago now, was to create a
situation where there was a filehandle per enclave in order for the
LSM to get a 'look' at the executable TEXT that was being deployed to
the enclave.

It appears that the sgx_open() function, that is installed as the
->open method for the /dev/sgx/enclave device node, allocates the
master enclave definition structure, that is installed, and
subsequently released, as private data for the file instance object.
This master enclave structure has the reference to the virtual memory
definition for the enclave that is opened.

This would seem to imply that the driver is rather firmly architected
on the notion of one open() per enclave, a concept that Jethro seems
to have issues with.

> /Jarkko

Have a good day.

Dr. Greg

As always,
Dr. Greg Wettstein, Ph.D, Worker
IDfusion, LLC               SGX secured infrastructure and
4206 N. 19th Ave.           autonomously self-defensive platforms.
Fargo, ND  58102
PH: 701-281-1686            EMAIL: greg@idfusion.net
------------------------------------------------------------------------------
"Artifical Intelligence stands no chance against Natural Stupidity."
                                -- John Henders

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Wed, Feb 19, 2020 at 10:26:40AM -0600, Dr. Greg wrote:
> On Tue, Feb 18, 2020 at 05:52:47PM +0200, Jarkko Sakkinen wrote:
> 
> Good morning, I hope the day is going well for everyone.
> 
> > On Tue, Feb 18, 2020 at 04:42:43AM -0600, Dr. Greg Wettstein wrote:
> > > I believe an accurate summary of Dr. Beekman's concerns are as
> > > follows:
> > > 
> > > 1.) He envisions a need for an enclave orchestrator that uses root
> > > privileges to open the SGX driver device and then drop privileges,
> > > presumably in a permanent fashion.  The orchestrator would then use
> > > the filehandle to load and initialize multiple enclaves on request.
> > > 
> > > 2.) The enclave orchestrator may be run in an environment that has
> > > SECCOMP limitations on the ability to conduct filesystem operations.
> 
> > Also UDS sockets with SCM_RIGHTS should work.
> 
> The first clarification, I'm assuming you mean passing SCM_RIGHTS over
> AF_UNIX/UNIX-domain sockets, not UDP sockets?

UNIX domain sockets.

> Secondly, it was my understanding that the reason major renovations to
> the driver were initiated, almost a year ago now, was to create a
> situation where there was a filehandle per enclave in order for the
> LSM to get a 'look' at the executable TEXT that was being deployed to
> the enclave.

File handle per enclave was driven by many things, e.g. ability to
access enclave from multiple processes either through forking or UDS
sockets.

LSM specifc changes are not part of the patch set but file handle based
model is adaptable to LSM's later on.

> It appears that the sgx_open() function, that is installed as the
> ->open method for the /dev/sgx/enclave device node, allocates the
> master enclave definition structure, that is installed, and
> subsequently released, as private data for the file instance object.
> This master enclave structure has the reference to the virtual memory
> definition for the enclave that is opened.

You can have multiple references to an enclave by using formentioned
tools.

> This would seem to imply that the driver is rather firmly architected
> on the notion of one open() per enclave, a concept that Jethro seems
> to have issues with.

I don't understand what concept you are talking about.

/Jarkko

Re: x86/sgx: v23-rc2

[Dr. Greg]

On Thu, Feb 20, 2020 at 09:57:11PM +0200, Jarkko Sakkinen wrote:

Good evening to everyone.

> On Wed, Feb 19, 2020 at 10:26:40AM -0600, Dr. Greg wrote:
> > On Tue, Feb 18, 2020 at 05:52:47PM +0200, Jarkko Sakkinen wrote:
> > 
> > Good morning, I hope the day is going well for everyone.
> > 
> > > On Tue, Feb 18, 2020 at 04:42:43AM -0600, Dr. Greg Wettstein wrote:
> > > > I believe an accurate summary of Dr. Beekman's concerns are as
> > > > follows:
> > > > 
> > > > 1.) He envisions a need for an enclave orchestrator that uses root
> > > > privileges to open the SGX driver device and then drop privileges,
> > > > presumably in a permanent fashion.  The orchestrator would then use
> > > > the filehandle to load and initialize multiple enclaves on request.
> > > > 
> > > > 2.) The enclave orchestrator may be run in an environment that has
> > > > SECCOMP limitations on the ability to conduct filesystem operations.
> > 
> > > Also UDS sockets with SCM_RIGHTS should work.
> > 
> > The first clarification, I'm assuming you mean passing SCM_RIGHTS over
> > AF_UNIX/UNIX-domain sockets, not UDP sockets?

> UNIX domain sockets.

That is what I thought, I wanted to make sure that UDS wasn't a typo.

> > It appears that the sgx_open() function, that is installed as the
> > ->open method for the /dev/sgx/enclave device node, allocates the
> > master enclave definition structure, that is installed, and
> > subsequently released, as private data for the file instance object.
> > This master enclave structure has the reference to the virtual memory
> > definition for the enclave that is opened.

> You can have multiple references to an enclave by using formentioned
> tools.

Yes, I understand multiple references to a file descriptor/enclave,
but see below.

> > This would seem to imply that the driver is rather firmly architected
> > on the notion of one open() per enclave, a concept that Jethro seems
> > to have issues with.

> I don't understand what concept you are talking about.

If memory serves me correctly, Jethro envisioned a model where a
single open of the SGX driver node would return a file descriptor that
could then be used to create/load/initialize multiple enclaves.  Your
clarifications indicate that a separate open will be needed for each
and every enclave instance that will be orchestrated.

Jethro, if I'm mistating your position on this, please jump in and
clarify.

> /Jarkko

Have a good end of the week.

Dr. Greg

As always,
Dr. Greg Wettstein, Ph.D, Worker
IDfusion, LLC               SGX secured infrastructure and
4206 N. 19th Ave.           autonomously self-defensive platforms.
Fargo, ND  58102
PH: 701-281-1686            EMAIL: greg@idfusion.net
------------------------------------------------------------------------------
"Real Programmers consider "what you see is what you get" to be just as
 bad a concept in Text Editors as it is in women.  No, the Real
 Programmer wants a "you asked for it, you got it" text editor --
 complicated, cryptic, powerful, unforgiving, dangerous."
                                -- Matthias Schniedermeyer

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
> > > This would seem to imply that the driver is rather firmly architected
> > > on the notion of one open() per enclave, a concept that Jethro seems
> > > to have issues with.
> 
> > I don't understand what concept you are talking about.
> 
> If memory serves me correctly, Jethro envisioned a model where a
> single open of the SGX driver node would return a file descriptor that
> could then be used to create/load/initialize multiple enclaves.  Your
> clarifications indicate that a separate open will be needed for each
> and every enclave instance that will be orchestrated.
> 
> Jethro, if I'm mistating your position on this, please jump in and
> clarify.

Ah.

You are speaking about having a factory to create enclaves and a
management interface. I.e. you'd have ioctl to create enclave that gives
you a file descriptor to access its management interface.

Out of top of my head I cannot recall why this was not favored in the
end but generally speaking added complexity should be justified by some
considerably strong measures.

/Jarkko

Re: x86/sgx: v23-rc2

[Sean Christopherson]

On Fri, Feb 21, 2020 at 03:00:31PM +0200, Jarkko Sakkinen wrote:
> On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
> > > > This would seem to imply that the driver is rather firmly architected
> > > > on the notion of one open() per enclave, a concept that Jethro seems
> > > > to have issues with.
> > 
> > > I don't understand what concept you are talking about.
> > 
> > If memory serves me correctly, Jethro envisioned a model where a
> > single open of the SGX driver node would return a file descriptor that
> > could then be used to create/load/initialize multiple enclaves.  Your
> > clarifications indicate that a separate open will be needed for each
> > and every enclave instance that will be orchestrated.
> > 
> > Jethro, if I'm mistating your position on this, please jump in and
> > clarify.
> 
> Ah.
> 
> You are speaking about having a factory to create enclaves and a
> management interface. I.e. you'd have ioctl to create enclave that gives
> you a file descriptor to access its management interface.
> 
> Out of top of my head I cannot recall why this was not favored in the
> end but generally speaking added complexity should be justified by some
> considerably strong measures.

The primary issue is that having an ioctl() to create enclaves means the
enclave fd would be an anon inode.  From a security (well, SELinux)
perspective, anon inodes are problematic for two reasons:

  - Anon inodes are exempt from SELinux ioctl() whitelisting.  This
    probably isn't a huge deal?

  - Mapping an anon inode with PROT_EXEC requires SELinux's EXECMEM, i.e.
    all enclave runtimes would require EXECMEM, which is a big step
    backwards in terms of security and drew the ire of Linus[1].

That being said, there is a proposed patch set to add support for "secure"
anon inodes[2], which I believe would eliminate all of the above problems.
Of course we'd be stuck with the current anon inode behavior until that
patch set lands (I'm feeling optomistic today ;-) ).

[1] https://lkml.kernel.org/r/CAHk-=wjmT=uC1=18ZYV1CMfP_FBUEjh9_rabH0g+a0z-L0cgHg@mail.gmail.com
[2] https://lkml.kernel.org/r/20200214032635.75434-1-dancol@google.com

Re: x86/sgx: v23-rc2

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

On 2020-03-05 20:51, Sean Christopherson wrote:
> On Fri, Feb 21, 2020 at 03:00:31PM +0200, Jarkko Sakkinen wrote:
>> On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
>>>>> This would seem to imply that the driver is rather firmly architected
>>>>> on the notion of one open() per enclave, a concept that Jethro seems
>>>>> to have issues with.
>>>
>>>> I don't understand what concept you are talking about.
>>>
>>> If memory serves me correctly, Jethro envisioned a model where a
>>> single open of the SGX driver node would return a file descriptor that
>>> could then be used to create/load/initialize multiple enclaves.  Your
>>> clarifications indicate that a separate open will be needed for each
>>> and every enclave instance that will be orchestrated.
>>>
>>> Jethro, if I'm mistating your position on this, please jump in and
>>> clarify.
>>
>> Ah.
>>
>> You are speaking about having a factory to create enclaves and a
>> management interface. I.e. you'd have ioctl to create enclave that gives
>> you a file descriptor to access its management interface.
>>
>> Out of top of my head I cannot recall why this was not favored in the
>> end but generally speaking added complexity should be justified by some
>> considerably strong measures.
> 
> The primary issue is that having an ioctl() to create enclaves means the
> enclave fd would be an anon inode.

Why would it have to be an anon inode? Can't it just be the same inode as the device file? As I mentioned it just needs to be a different struct file (since that's what actually owns the enclave). I'm envisioning you still need to open the device first (once).

Yes, a UDS with SCM_RIGHTS could work, this is similar from a capability model perspective. It does make things a lot more complicated if you want to employ userspace security techniques. For example, when using seccomp a common use is to avoid the creation of arbitrary fds. It's rather hard to write a good set of rules for recvmsg with SCM_RIGHTS since the fds are passed from outside. Whitelisting a single ioctl is much easier. Also, you'd need to create a whole separate process whose job it is to hand out these enclave fds. That process still needs to be privileged and would probably need its own set of seccomp rules.

--
Jethro Beekman | Fortanix


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]

Re: x86/sgx: v23-rc2

[Sean Christopherson]

On Thu, Mar 05, 2020 at 09:34:01PM +0100, Jethro Beekman wrote:
> On 2020-03-05 20:51, Sean Christopherson wrote:
> > On Fri, Feb 21, 2020 at 03:00:31PM +0200, Jarkko Sakkinen wrote:
> >> On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
> >>>>> This would seem to imply that the driver is rather firmly architected
> >>>>> on the notion of one open() per enclave, a concept that Jethro seems
> >>>>> to have issues with.
> >>>
> >>>> I don't understand what concept you are talking about.
> >>>
> >>> If memory serves me correctly, Jethro envisioned a model where a
> >>> single open of the SGX driver node would return a file descriptor that
> >>> could then be used to create/load/initialize multiple enclaves.  Your
> >>> clarifications indicate that a separate open will be needed for each
> >>> and every enclave instance that will be orchestrated.
> >>>
> >>> Jethro, if I'm mistating your position on this, please jump in and
> >>> clarify.
> >>
> >> Ah.
> >>
> >> You are speaking about having a factory to create enclaves and a
> >> management interface. I.e. you'd have ioctl to create enclave that gives
> >> you a file descriptor to access its management interface.
> >>
> >> Out of top of my head I cannot recall why this was not favored in the
> >> end but generally speaking added complexity should be justified by some
> >> considerably strong measures.
> > 
> > The primary issue is that having an ioctl() to create enclaves means the
> > enclave fd would be an anon inode.
> 
> Why would it have to be an anon inode? Can't it just be the same inode as the
> device file? As I mentioned it just needs to be a different struct file
> (since that's what actually owns the enclave). I'm envisioning you still need
> to open the device first (once).

Hmm, poking around, alloc_file_clone() looks like it would do the trick.
But we are waaaaay outside my area of expertise, I'll let Andy or someone
else weigh in on whether or not this is sane.

> Yes, a UDS with SCM_RIGHTS could work, this is similar from a capability
> model perspective. It does make things a lot more complicated if you want to
> employ userspace security techniques. For example, when using seccomp a
> common use is to avoid the creation of arbitrary fds. It's rather hard to
> write a good set of rules for recvmsg with SCM_RIGHTS since the fds are
> passed from outside. Whitelisting a single ioctl is much easier. Also, you'd
> need to create a whole separate process whose job it is to hand out these
> enclave fds. That process still needs to be privileged and would probably
> need its own set of seccomp rules.
> 
> -- Jethro Beekman | Fortanix
> 

Re: x86/sgx: v23-rc2

[Jarkko Sakkinen]

On Thu, Mar 05, 2020 at 11:51:57AM -0800, Sean Christopherson wrote:
> On Fri, Feb 21, 2020 at 03:00:31PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
> > > > > This would seem to imply that the driver is rather firmly architected
> > > > > on the notion of one open() per enclave, a concept that Jethro seems
> > > > > to have issues with.
> > > 
> > > > I don't understand what concept you are talking about.
> > > 
> > > If memory serves me correctly, Jethro envisioned a model where a
> > > single open of the SGX driver node would return a file descriptor that
> > > could then be used to create/load/initialize multiple enclaves.  Your
> > > clarifications indicate that a separate open will be needed for each
> > > and every enclave instance that will be orchestrated.
> > > 
> > > Jethro, if I'm mistating your position on this, please jump in and
> > > clarify.
> > 
> > Ah.
> > 
> > You are speaking about having a factory to create enclaves and a
> > management interface. I.e. you'd have ioctl to create enclave that gives
> > you a file descriptor to access its management interface.
> > 
> > Out of top of my head I cannot recall why this was not favored in the
> > end but generally speaking added complexity should be justified by some
> > considerably strong measures.
> 
> The primary issue is that having an ioctl() to create enclaves means the
> enclave fd would be an anon inode.  From a security (well, SELinux)
> perspective, anon inodes are problematic for two reasons:
> 
>   - Anon inodes are exempt from SELinux ioctl() whitelisting.  This
>     probably isn't a huge deal?
> 
>   - Mapping an anon inode with PROT_EXEC requires SELinux's EXECMEM, i.e.
>     all enclave runtimes would require EXECMEM, which is a big step
>     backwards in terms of security and drew the ire of Linus[1].
> 
> That being said, there is a proposed patch set to add support for "secure"
> anon inodes[2], which I believe would eliminate all of the above problems.
> Of course we'd be stuck with the current anon inode behavior until that
> patch set lands (I'm feeling optomistic today ;-) ).
> 
> [1] https://lkml.kernel.org/r/CAHk-=wjmT=uC1=18ZYV1CMfP_FBUEjh9_rabH0g+a0z-L0cgHg@mail.gmail.com
> [2] https://lkml.kernel.org/r/20200214032635.75434-1-dancol@google.com

Still does not address what I said in my response: what are the
strong enough measures to use anon inode in the first place even
if there is a "secure" version.

/Jarkko