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

Hi Stephen,

On 5/24/19 3:24 AM, Xing, Cedric wrote:

quoted

When we talk about EPC page permissions with SGX2 in mind, I think we

should distinguish between initial permissions and runtime permissions.
Initial permissions refer to the page permissions set at EADD. They are
technically set by "untrusted" code so should go by policies similar to
those applicable to regular shared objects. Runtime permissions refer to
the permissions granted by EMODPE, EAUG and EACCEPTCOPY. They are
resulted from inherent behavior of the enclave, which in theory is
determined by the enclave's measurements (MRENCLAVE and/or MRSIGNER).

quoted

And we have 2 distinct files to work with - the enclave file and

/dev/sgx/enclave. And I consider the enclave file a logical source for
initial permissions while /dev/sgx/enclave is a means to control runtime
permissions. Then we can have a simpler approach like the pseudo code
below.

quoted

/**
  * Summary:
  * - The enclave file resembles a shared object that contains

RO/RX/RW segments

quoted

  * - FILE__* are assigned to /dev/sgx/enclave, to determine

acceptable permissions to mmap()/mprotect(), valid combinations are

quoted

  *   + FILE__READ - Allow SGX1 enclaves only
  *   + FILE__READ|FILE__WRITE - Allow SGX2 enclaves to expand data

segments (e.g. heaps, stacks, etc.)

quoted

  *   + FILE__READ|FILE__WRITE|FILE__EXECUTE - Allow SGX2 enclaves to

expend both data and code segments. This is necessary to support
dynamically linked enclaves (e.g. Graphene)

quoted

  *   + FILE__READ|FILE__EXECUTE - Allow RW->RX changes for SGX1

enclaves - necessary to support dynamically linked enclaves (e.g.
Graphene) on SGX1. EXECMEM is also required for this to work

I think EXECMOD would fit better than EXECMEM for this case; the former
is applied for RW->RX changes for private file mappings while the latter
is applied for WX private file mappings.

quoted

  *   + <None> - Disallow the calling process to launch any enclaves
  */

/* Step 1: mmap() the enclave file according to the segment attributes
(similar to what dlopen() would do for regular shared objects) */ int
image_fd = open("/path/to/enclave/file", O_RDONLY);

FILE__READ checked to enclave file upon open().

Yes. We'd like to have the enclave file pass LSM/IMA checks and let EPC pages "inherit" the permissions from it as "initial" permissions.

quoted

foreach phdr in loadable segments /* phdr->p_type == PT_LOAD */ {
     /* <segment permission> below is subject to LSM checks */
     loadable_segments[i] = mmap(NULL, phdr->p_memsz, MAP_PRIATE,
<segment permission>, image_fd, phdr->p_offset);

FILE__READ revalidated and FILE__EXECUTE checked to enclave file upon
mmap() for PROT_READ and PROT_EXEC respectively.  FILE__WRITE not
checked even for PROT_WRITE mappings since it is a private file mapping
and writes do not reach the file.  EXECMEM checked if any segment
permission has both W and X simultaneously.  EXECMOD checked on any
subsequent mprotect() RW->RX changes (if modified).

Yes. The intention here is to make sure all X pages come directly from file (unless EXECMEM or EXECMOD is granted). And because the driver will grant X only if the source page also has X, we can assert that all executable EPC pages are loaded from a file that has passed LSM/IMA checks.

quoted

}

/* Step 2: Create enclave */
int enclave_fd = open("/dev/sgx/enclave", O_RDONLY /* or O_RDWR for
SGX2 enclaves */);

FILE__READ checked (SGX1) or both FILE__READ and FILE__WRITE checked
(SGX2) to /dev/sgx/enclave upon open().  Assuming that we are returning
an open file referencing the /dev/sgx/enclave inode and not an anon
inode, else we lose all subsequent FILE__* checking on mmap/mprotect and
trigger EXECMEM on any mmap/mprotect PROT_EXEC.

Yes, the returned fd will be referencing /dev/sgx/enclave. The intention here is to limit EPC "runtime" permissions by the permissions granted to /dev/sgx/enclave, in order to allow user/administrator to specify what kinds of enclaves a given process can launch. Per your earlier comments, FILE__EXECMOD is probably also needed to support dynamically linked enclaves (that require RW->RX changes).

quoted

void *enclave_base = mmap(NULL, <enclave size>, MAP_SHARED, PROT_READ,
enclave_fd, 0); /* Only FILE__READ is required here */

FILE__READ revalidated to /dev/sgx/enclave upon mmap().

Yes. This mmap() is to set "default" permissions for regions that do *not* have EPC pages populated. It is significant only for SGX2, to specify what action to take by the SGX driver upon #PF with those regions. For example, a R attempt (usually triggered by EACCEPT) within a RW region will cause SGX driver to EAUG a page at the fault address.

quoted

ioctl(enclave_fd, IOC_ECREATE, ...);

/* Step 3: EADD and map initial EPC pages */ foreach s in
loadable_segments {
     /* IOC_EADD_AND_MAP_SEGMENT will make sure s->perm is a subset of

VMA permissions of the source pages, and use that as *both* EPCM and VMA
permissions).

quoted

      * Given enclave_fd may have FILE__READ only, LSM has to be

bypassed so the "mmap" part has to be done inside the driver.

quoted

      * Initial EPC pages will be mapped only once, so no inode is

needed to remember the initial permissions. mmap/mprotect afterwards are
subject to FILE__* on /dev/sgx/enclave

quoted

      * The key point here is: permissions of source pages govern

initial permissions of EADD'ed pages, regardless FILE__* on
/dev/sgx/enclave

quoted

      */
     ioctl(enclave_fd, IOC_EADD_AND_MAP_SEGMENT, s->base, s->size,
s->perm...); }
/* EADD other enclave components, e.g. TCS, stacks, heaps, etc. */
ioctl(enclave_fd, IOC_EADD_AND_MAP_SEGMENT, tcs, 0x1000, RW |
PT_TCS...); ioctl(enclave_fd, IOC_EADD_AND_MAP_SEGMENT, <zero page>,
<stack size>, RW...); ...

/* Step 4 (SGX2 only): Reserve ranges for additional heaps, stacks,
etc. */
/* FILE__WRITE required to allow expansion of data segments at runtime
*/
/* Key point here is: permissions, if needed to change at runtime, are
subject to FILL__* on /dev/sgx/enclave */ mprotect(<heap address>,
<heap size>, PROT_READ | PROT_WRITE);

FILE__READ and FILE__WRITE revalidated to /dev/sgx/enclave upon
mprotect().

Yes. The intention here is to limit "runtime" permissions by accesses granted to the calling process to /dev/sgx/enclave. The "initial" permissions are set by ioctl to bypass LSM, because they are derived/determined by the enclave file.

Alternatively, the driver can remember "initial" permissions for each EPC page at IOC_EADD, to be committed at IOC_EINIT. Then this new IOC_EADD_AND_MAP will not be needed. 

quoted

/* Step 5: EINIT */
ioctl(IOC_EINIT, <sigstruct>...);

/* Step 6 (SGX2 only): Set RX for dynamically loaded code pages (e.g.
Graphene, encrypted enclaves, etc.) as needed, at runtime */
/* FILE__EXECUTE required */
mprotect(<RX address>, <RX size>, PROT_READ | PROT_EXEC);

FILE__READ revalidated and FILE__EXECUTE checked to /dev/sgx/enclave
upon mprotect().  Cumulative set of checks at this point is
FILE__READ|FILE__WRITE|FILE__EXECUTE.

What would the step be for a SGX1 RW->RX change?  How would that trigger
EXECMOD?  Do we really need to distinguish it from the SGX2 dynamically
loaded code case?

Per your earlier comments, FILE__EXECMOD is also needed I think to allow RW->RX changes.

FILE__WRITE controls EAUG. I'm not judging its necessity, but just saying they are both valid combinations. To minimize impact to LSM, I don't want to special-case /dev/sgx/enclave. And the current semantics of FILE__* distinguish those two naturally.

BTW, there are usages, such as encrypted enclaves (https://github.com/intel/linux-sgx-pcl), requiring RW->RX but not EAUG. Graphene could also run on SGX1, provided that pages needed by shared objects are all pre-allocated before EINIT. All those could run without FILE__WRITE.

quoted

-Cedric