On Fri, May 24, 2019 at 10:42:43AM -0700, Sean Christopherson wrote:

Hmm, I've been thinking more about pulling permissions from the source
page.  Conceptually I'm not sure we need to meet the same requirements as
non-enclave DSOs while the enclave is being built, i.e. do we really need
to force userspace to fully map the enclave in normal memory?
 
Consider the Graphene scenario where it's building an enclave on the fly.
Pulling permissions from the source VMAs means Graphene has to map the
code pages of the enclave with X.  This means Graphene will need EXEDMOD
(or EXECMEM if Graphene isn't careful).  In a non-SGX scenario this makes
perfect sense since there is no way to verify the end result of RW->RX.

But for SGX, assuming enclaves are whitelisted by their sigstruct (checked
at EINIT) and because page permissions affect sigstruct.MRENCLAVE, it *is*
possible to verify the resulting RX contents.  E.g. for the purposes of
LSMs, can't we use the .sigstruct file as a proxy for the enclave and
require FILE__EXECUTE on the .sigstruct inode to map/run the enclave?

Stephen, is my logic sound?


If so...

  - Require FILE__READ+FILE__EXECUTE on .sigstruct to mmap() the enclave.

  - Prevent userspace from mapping the enclave with permissions beyond the
    original permissions of the enclave.  This can be done by populating
    VM_MAY{READ,WRITE,EXEC} from the SECINFO (same basic concept as Andy's
    proposals).  E.g. pre-EINIT, mmap() and mprotect() can only succeed
    with PROT_NONE.

  - Require FILE__{READ,WRITE,EXECUTE} on /dev/sgx/enclave for simplicity,
    or provide an alternate SGX_IOC_MPROTECT if we want to sidestep the
    FILE__WRITE requirement.

One more thought.  EADD (and the equivalent SGX2 flow) could do
security_mmap_file() with a NULL file on the SECINFO permissions, which
would trigger PROCESS_EXECMEM if an enclave attempts to map a page RWX.

No changes are required to LSMs, SGX1 has a single LSM touchpoint in its
mmap(), and I *think* the only required userspace change is to mmap()
PROT_NONE when allocating the enclave's virtual address range.

As for Graphene, it doesn't need extra permissions to run its enclaves,
it just needs a way to install .sigstruct, which is a generic permissions
problem and not SGX specific.


For SGX2 maybe:

  - No additional requirements to map an EAUG'd page as RW page.  Not
    aligned with standard MAP_SHARED behavior, but we really don't want
    to require FILE__WRITE, and thus allow writes to .sigstruct. 
 
  - Require FILE__EXECMOD on the .sigstruct to map previously writable
    page as executable (which indirectly includes all EAUG'd pages).
    Wiring this up will be a little funky, but we again we don't want
    to require FILE__WRITE on .sigstruct.