From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
owner@vger.kernel.org] On Behalf Of Sean Christopherson
Sent: Friday, May 24, 2019 1:04 PM

On Fri, May 24, 2019 at 12:37:44PM -0700, Andy Lutomirski wrote:

quoted

On Fri, May 24, 2019 at 11:34 AM Xing, Cedric [off-list ref]

wrote:

quoted

quoted

If "initial permissions" for enclaves are less restrictive than
shared objects, then it'd become a backdoor for circumventing LSM
when enclave whitelisting is *not* in place. For example, an
adversary may load a page, which would otherwise never be executable,

as an executable page in EPC.

quoted

quoted

In the case a RWX page is needed, the calling process has to have a
RWX page serving as the source for EADD so PROCESS__EXECMEM will
have been checked. For SGX2, changing an EPC page to RWX is subject
to FILE__EXECMEM on /dev/sgx/enclave, which I see as a security
benefit because it only affects the enclave but not the whole

process hosting it.

quoted

So the permission would be like FILE__EXECMOD on the source enclave
page, because it would be mapped MAP_ANONYMOUS, PROT_WRITE?
MAP_SHARED, PROT_WRITE isn't going to work because that means you can
modify the file.

Was this in response to Cedric's comment, or to my comment?

Creating RWX source page requires PROCESS_EXECMEM. But as I responded to Sean earlier, I think his proposal of "aggregating" all "initial" permission checks into a single SIGSTRUCT check is probably a better approach.

quoted

I'm starting to think that looking at the source VMA permission bits
or source PTE permission bits is putting a bit too much policy into
the driver as opposed to the LSM.  How about delegating the whole
thing to an LSM hook?  The EADD operation would invoke a new hook,
something like:

int security_enclave_load_bytes(void *source_addr, struct
vm_area_struct *source_vma, loff_t source_offset, unsigned int
maxperm);

This is exactly what I was thinking. But with Sean's proposal this is probably no longer necessary.

quoted

Then you don't have to muck with mapping anything PROT_EXEC.  Instead
you load from a mapping of a file and the LSM applies whatever policy
it feels appropriate.  If the first pass gets something wrong, the
application or library authors can take it up with the SELinux folks
without breaking the whole ABI :)

(I'm proposing passing in the source_vma because this hook would be
called with mmap_sem held for read to avoid a TOCTOU race.)

If we go this route, the only substantial change to the existing
driver that's needed for an initial upstream merge is the maxperm
mechanism and whatever hopefully minimal API changes are needed to
allow users to conveniently set up the mappings.  And we don't need to
worry about how to hack around mprotect() calling into the LSM,
because the LSM will actually be aware of SGX and can just do the
right thing.

This doesn't address restricting which processes can run which enclaves,
it only allows restricting the build flow.  Or are you suggesting this
be done in addition to whitelisting sigstructs?

In the context of SELinux, new types could be defined to be associated with SIGSTRUCT (or more precisely, files containing SIGSTRUCTs). Then the LSM hook (I'd propose security_sgx_initialize_enclave) could enforce whatever...

What's the value prop beyond whitelisting sigstructs?  Realistically, I
doubt LSMs/users will want to take the performance hit of scanning the
source bytes every time an enclave is loaded.

We could add seomthing like security_enclave_mprotect() in lieu of
abusing security_file_mprotect(), but passing the full source bytes
seems a bit much.

I'd just use /dev/sgx/enclave to govern "runtime" permissions any EPC page can mmap()/mprotect() to. Then we won't need any code changes in LSM.

-Cedric