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

[Xing, Cedric]

Hi Andy,

> So I think we need a better EADD ioctl that explicitly does work on 
> PROT_READ|PROT_EXEC enclave memory but makes up for by validating the
> *source* of the data.  The effect will be similar to mapping a 
> labeled, appraised, etc file as PROT_EXEC.  Maybe, in extreme
> pseudocode:
> 
> fd = open(â/dev/sgx/enclaveâ);
> ioctl(fd, SGX_CREATE_FROM_FILE, file_fd); // fd now inherits the LSM 
> label from the file, or is otherwise marked.
> mmap(fd, PROT_READ|PROT_EXEC, ...);
> 
> I suppose that an alternative would be to delegate all the EADD calls 
> to a privileged daemon, but thatâs nasty.

I agree with you that *source* of EPC pages shall be validated. But instead of doing it in the driver, I think an alternative could be to treat an enclave file as a regular shared object so all IMA/LSM checks could be triggered/performed as part of the loading process, then let the driver "copy" those pages to EPC. By "copy", I mean both the page content and _permissions_ are copied, which differs from the current driver's behavior of copying page content only (while permissions are taken from ioctl parameters). That said, if an adversary cannot inject any code into a process in regular pages, then it cannot inject any code to any EPC pages in that process either, simply because the latter depends on the former. 

Security wise, it is also assumed that duplicating (both content and permissions of) an existing page within a process will not threaten the security of that process. That assumption may not always be true but in practice, the current LSM architecture doesn't seem to have that in scope, so this proposal I think still aligns with LSM in that sense. 

If compared to the idea of "enclave loader inside kernel", I think this alternative is much simpler and more flexible. In particular,
* It requires minimal change to the driver - just take EPCM permissions from source pages' VMA instead of from ioctl parameter.
* It requires little change to user mode enclave loader - just mmap() enclave file in the same way as dlopen() would do, then all IMA/LSM checks applicable to shared objects will be triggered/performed  transparently.
* It doesn't assume/tie to specific enclave formats.
* It is extensible - Today every regular page within a process is allowed implicitly to be the source for an EPC page. In future, if at all desirable/necessary, IMA/LSM could be extended to leave a flag somewhere (e.g. in VMA) to indicate explicitly whether a regular page (or range) could be a source for EPC. Then SGX specific hooks/policies could be supported easily.

How do you think?

-Cedric