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
Because /dev/sgx/enclave is NOT private, SELinux's file_map_prot_check()
will only require FILE__WRITE and FILE__EXECUTE to mprotect() enclave
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
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.