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

[Sean Christopherson]

On Fri, May 17, 2019 at 12:37:40PM -0400, Stephen Smalley wrote:
> On 5/17/19 12:20 PM, Stephen Smalley wrote:
> >On 5/17/19 11:09 AM, Sean Christopherson wrote:
> >>I think we may want to change the SGX API to alloc an anon inode for each
> >>enclave instead of hanging every enclave off of the /dev/sgx/enclave
> >>inode.
> >>Because /dev/sgx/enclave is NOT private, SELinux's file_map_prot_check()
> >>will only require FILE__WRITE and FILE__EXECUTE to mprotect() enclave
> >>VMAs
> >>to RWX.  Backing each enclave with an anon inode will make SELinux treat
> >>EPC memory like anonymous mappings, which is what we want (I think), e.g.
> >>making *any* EPC page executable will require PROCESS__EXECMEM (SGX is
> >>64-bit only at this point, so SELinux will always have default_noexec).
> >
> >I don't think we want to require EXECMEM (or equivalently both FILE__WRITE
> >and FILE__EXECUTE to /dev/sgx/enclave) for making any EPC page executable,
> >only if the page is also writable or previously modified.  The intent is
> >to prevent arbitrary code execution without EXECMEM (or
> >FILE__WRITE|FILE__EXECUTE), while still allowing enclaves to be created
> >without EXECMEM as long as the EPC page mapping is only ever mapped RX and
> >its initial contents came from an unmodified file mapping that was
> >PROT_EXEC (and hence already checked via FILE__EXECUTE).

The idea is that by providing an SGX ioctl() to propagate VMA permissions
from a source VMA, EXECMEM wouldn't be required to make an EPC page
executable.  E.g. userspace establishes an enclave in non-EPC memory from
an unmodified file (with FILE__EXECUTE perms), and the uses the SGX ioctl()
to copy the contents and permissions into EPC memory.

> Also, just to be clear, there is nothing inherently better about checking
> EXECMEM instead of checking both FILE__WRITE and FILE__EXECUTE to the
> /dev/sgx/enclave inode, so I wouldn't switch to using anon inodes for that
> reason.  Using anon inodes also unfortunately disables SELinux inode-based
> checking since we no longer have any useful inode information, so you'd lose
> out on SELinux ioctl whitelisting on those enclave inodes if that matters.

The problem is that all enclaves are associated with a single inode, i.e.
/dev/sgx/enclave.  /dev/sgx/enclave is a char device whose purpose is to
provide ioctls() and to allow mmap()'ing EPC memory.  In no way is it
associated with the content that actually gets loaded into EPC memory.

The actual file that contains the enclave's contents (assuming the enclave
came from a file) is a separate regular file that the SGX subsystem never
sees.

AIUI, having FILE__WRITE and FILE__EXECUTE on /dev/sgx/enclave would allow
*any* enclave/process to map EPC as RWX.  Moving to anon inodes and thus
PROCESS__EXECMEM achieves per-process granularity.