RE: [PATCH v20 00/28] Intel SGX1 support

[Xing, Cedric]

Hi Andy,

> Let me make sure I'm understanding this correctly: when an enclave tries
> to execute code, it only works if *both* the EPCM and the page tables
> grant the access, right?  This seems to be that 37.3 is trying to say.
> So we should probably just ignore SECINFO for these purposes.
> 
> But thinking this all through, it's a bit more complicated than any of
> this.  Looking at the SELinux code for inspiration, there are quite a
> few paths, but they boil down to two cases: EXECUTE is the right to map
> an unmodified file executably, and EXECMOD/EXECMEM (the distinction
> seems mostly irrelevant) is the right to create (via mmap or mprotect) a
> modified anonymous file mapping or a non-file-backed mapping that is
> executable.  So, if we do nothing, then mapping an enclave with execute
> permission will require either EXECUTE on the enclave inode or
> EXECMOD/EXECMEM, depending on exactly how this gets set up.
> 
> So all is well, sort of.  The problem is that I expect there will be
> people who want enclaves to work in a process that does not have these
> rights.  To make this work, we probably need do some surgery on SELinux.
> ISTM the act of copying (via the EADD ioctl) data from a PROT_EXEC
> mapping to an enclave should not be construed as "modifying"
> the enclave for SELinux purposes.  Actually doing this could be awkward,
> since the same inode will have executable parts and non-executable parts,
> and SELinux can't really tell the difference.
> 

Enclave files are pretty much like shared objects in that they both contain executable code mapped into the host process's address space. Do shared objects need EXECUTE to be mapped executable? If so, why would people not want EXECUTE in enclave files?

Wrt to which part of an executable file is executable, the limitation resides in security_mmap_file(), which doesn't take the range of bytes as input. It isn't SGX specific. I'd say just let enclaves inherit applicable checks for shared objects. Then it will also inherit all future enhancements transparently.

> Maybe the enclave should track a bitmap of which pages have ever been
> either mapped for write or EADDed with a *source* that wasn't PROT_EXEC.
> And then SELinux could learn to allow those pages (and only those pages)
> to be mapped executably without EXECUTE or EXECMOD or whatever
> permission.

What do you mean by "enclave" here? The enclave range (ELRANGE) created by mmap()'ing /dev/sgx/enclave device? My argument is, an enclave's sanity/insanity is determined at load time (EADD/EEXTEND and EINIT) and all page accesses are enforced by EPCM, so PTE permissions really don't matter. As I discussed in an earlier email, I'd allow RWX for any range backed by /dev/sgx/enclave device file by default, unless an SGX-aware LSM module/policy objects to that (e.g. via a new security_sgx_mprotect() LSM hook).

> 
> Does this seem at all reasonable?
> 
> I suppose it's not the end of the world if the initially merged version
> doesn't do this, as long as there's some reasonable path to adding a
> mechanism like this when there's demand for it.

Agreed!

-Cedric