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

From: Stephen Smalley
Date: Fri May 17 2019 - 16:50:35 EST

On 5/17/19 4:14 PM, Andy Lutomirski wrote:

On May 17, 2019, at 1:09 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:

On 5/17/19 3:28 PM, Sean Christopherson wrote:
On Fri, May 17, 2019 at 02:05:39PM -0400, Stephen Smalley wrote:
On 5/17/19 1:12 PM, Andy Lutomirski wrote:

How can that work? Unless the API changes fairly radically, users
fundamentally need to both write and execute the enclave. Some of it will
be written only from already executable pages, and some privilege should be
needed to execute any enclave page that was not loaded like this.

I'm not sure what the API is. Let's say they do something like this:

fd = open("/dev/sgx/enclave", O_RDONLY);
addr = mmap(NULL, size, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
stuff addr into ioctl args
ioctl(fd, ENCLAVE_CREATE, &ioctlargs);
ioctl(fd, ENCLAVE_ADD_PAGE, &ioctlargs);
ioctl(fd, ENCLAVE_INIT, &ioctlargs);
That's rougly the flow, except that that all enclaves need to have RW and
X EPC pages.
The important points are that they do not open /dev/sgx/enclave with write
access (otherwise they will trigger FILE__WRITE at open time, and later
encounter FILE__EXECUTE as well during mmap, thereby requiring both to be
allowed to /dev/sgx/enclave), and that they do not request PROT_WRITE to the
resulting mapping (otherwise they will trigger FILE__WRITE at mmap time).
Then only FILE__READ and FILE__EXECUTE are required to /dev/sgx/enclave in

If they switch to an anon inode, then any mmap PROT_EXEC of the opened file
will trigger an EXECMEM check, at least as currently implemented, as we have
no useful backing inode information.
Yep, and that's by design in the overall proposal. The trick is that
ENCLAVE_ADD takes a source VMA and copies the contents *and* the
permissions from the source VMA. The source VMA points at regular memory
that was mapped and populated using existing mechanisms for loading DSOs.
E.g. at a high level:
source_fd = open("/home/sean/path/to/my/enclave", O_RDONLY);
for_each_chunk {
<hand waving - mmap()/mprotect() the enclave file into regular memory>
enclave_fd = open("/dev/sgx/enclave", O_RDWR); /* allocs anon inode */
enclave_addr = mmap(NULL, size, PROT_READ, MAP_SHARED, enclave_fd, 0);
ioctl(enclave_fd, ENCLAVE_CREATE, {enclave_addr});
for_each_chunk {
struct sgx_enclave_add ioctlargs = {
.offset = chunk.offset,
.source = chunk.addr,
.size = chunk.size,
.type = chunk.type, /* SGX specific metadata */
ioctl(fd, ENCLAVE_ADD, &ioctlargs); /* modifies enclave's VMAs */
ioctl(fd, ENCLAVE_INIT, ...);
Userspace never explicitly requests PROT_EXEC on enclave_fd, but SGX also
ensures userspace isn't bypassing LSM policies by virtue of copying the
permissions for EPC VMAs from regular VMAs that have already gone through
LSM checks.

Is O_RDWR required for /dev/sgx/enclave or would O_RDONLY suffice? Do you do anything other than ioctl() calls on it?

What's the advantage of allocating an anon inode in the above? At present anon inodes are exempted from inode-based checking, thereby losing the ability to perform SELinux ioctl whitelisting, unlike the file-backed /dev/sgx/enclave inode.

How would SELinux (or other security modules) restrict the authorized enclaves that can be loaded via this interface? Would the sgx driver invoke a new LSM hook with the regular/source VMAs as parameters and allow the security module to reject the ENCLAVE_ADD operation? That could be just based on the vm_file (e.g. whitelist what enclave files are permitted in general) or it could be based on both the process and the vm_file (e.g. only allow specific enclaves to be loaded into specific processes).

This is the idea behind the .sigstruct file. The driver could call a new hook to approve or reject the .sigstruct. The sigstruct contains a hash of the whole enclave and a signature by the author.

Ok, so same idea but moved to ENCLAVE_INIT and passing the vma or file for the sigstruct instead of the enclave.