Re: SGX vs LSM (Re: [PATCH v20 00/28] Intel SGX1 support)

[Andy Lutomirski <luto@amacapital.net>]

> On May 24, 2019, at 10:42 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
>> On Fri, May 24, 2019 at 11:41:29AM -0400, Stephen Smalley wrote:
>>> On 5/24/19 3:24 AM, Xing, Cedric wrote:
>>> /**
>>> * Summary:
>>> * - The enclave file resembles a shared object that contains RO/RX/RW segments
>>> * - FILE__* are assigned to /dev/sgx/enclave, to determine acceptable permissions to mmap()/mprotect(), valid combinations are
>>> *   + FILE__READ - Allow SGX1 enclaves only
>>> *   + FILE__READ|FILE__WRITE - Allow SGX2 enclaves to expand data segments (e.g. heaps, stacks, etc.)
>>> *   + FILE__READ|FILE__WRITE|FILE__EXECUTE - Allow SGX2 enclaves to expend both data and code segments. This is necessary to support dynamically linked enclaves (e.g. Graphene)
>>> *   + FILE__READ|FILE__EXECUTE - Allow RW->RX changes for SGX1 enclaves - necessary to support dynamically linked enclaves (e.g. Graphene) on SGX1. EXECMEM is also required for this to work
>> 
>> I think EXECMOD would fit better than EXECMEM for this case; the former is
>> applied for RW->RX changes for private file mappings while the latter is
>> applied for WX private file mappings.
>> 
>>> *   + <None> - Disallow the calling process to launch any enclaves
>>> */
>>> 
>>> /* Step 1: mmap() the enclave file according to the segment attributes (similar to what dlopen() would do for regular shared objects) */
>>> int image_fd = open("/path/to/enclave/file", O_RDONLY);
>> 
>> FILE__READ checked to enclave file upon open().
>> 
>>> foreach phdr in loadable segments /* phdr->p_type == PT_LOAD */ {
>>>    /* <segment permission> below is subject to LSM checks */
>>>    loadable_segments[i] = mmap(NULL, phdr->p_memsz, MAP_PRIATE, <segment permission>, image_fd, phdr->p_offset);
>> 
>> FILE__READ revalidated and FILE__EXECUTE checked to enclave file upon mmap()
>> for PROT_READ and PROT_EXEC respectively.  FILE__WRITE not checked even for
>> PROT_WRITE mappings since it is a private file mapping and writes do not
>> reach the file.  EXECMEM checked if any segment permission has both W and X
>> simultaneously.  EXECMOD checked on any subsequent mprotect() RW->RX changes
>> (if modified).
> 
> Hmm, I've been thinking more about pulling permissions from the source
> page.  Conceptually I'm not sure we need to meet the same requirements as
> non-enclave DSOs while the enclave is being built, i.e. do we really need
> to force userspace to fully map the enclave in normal memory?
> 
> Consider the Graphene scenario where it's building an enclave on the fly.
> Pulling permissions from the source VMAs means Graphene has to map the
> code pages of the enclave with X.  This means Graphene will need EXEDMOD
> (or EXECMEM if Graphene isn't careful).  In a non-SGX scenario this makes
> perfect sense since there is no way to verify the end result of RW->RX.
> 
> But for SGX, assuming enclaves are whitelisted by their sigstruct (checked
> at EINIT) and because page permissions affect sigstruct.MRENCLAVE, it *is*
> possible to verify the resulting RX contents.  E.g. for the purposes of
> LSMs, can't we use the .sigstruct file as a proxy for the enclave and
> require FILE__EXECUTE on the .sigstruct inode to map/run the enclave?

I think it’s sound for some but not all use cases. I would imagine that a lot of users won’t restrict sigstruct at all — the “use this as a sigstruct” permission will be granted to everything and maybe even to memfd. But even users like that might want to force their enclaves to be hardened such that writable pages are never executable, in which case Graphene may need an exception to run.

But maybe I’m nuts.