Re: [PATCH v38 10/24] mm: Add vm_ops->mprotect()

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Thu, Sep 24, 2020 at 07:50:15AM -0700, Dave Hansen wrote:
> On 9/23/20 7:33 AM, Jarkko Sakkinen wrote:
> > The consequence is that enclaves are best created with an ioctl API and the
> > access control can be based only to the origin of the source file for the
> > enclave data, i.e. on VMA file pointer and page permissions. For example,
> > this could be done with LSM hooks that are triggered in the appropriate
> > ioctl's and they could make the access control decision based on this
> > information.
> > 
> > Unfortunately, there is ENCLS[EMODPE] that a running enclave can use to
> > upgrade its permissions. If we do not limit mmap() and mprotect(), enclave
> > could upgrade its permissions by using EMODPE followed by an appropriate
> > mprotect() call. This would be completely hidden from the kernel.
> > 
> > Add 'mprotect' hook to vm_ops, so that a callback can be implemeted for SGX
> > that will ensure that {mmap, mprotect}() permissions do not surpass any of
> > the original page permissions. This feature allows to maintain and refine
> > sane access control for enclaves.
> 
> Maybe I'm just being dense, but I still don't have a clear idea what
> function this hook serves.
> 
> I understand that SGX has an orthogonal set of page permissions to the
> normal x86 page tables.  It needs these so that the OS can't play nasty
> tricks on the enclave, like removing read-only protections that provide
> hardening.
> 
> But, I still don't get the connection to mprotect() and the x86 paging
> permissions.  If the enclave's permissions are orthogonal, then why
> bother with this hook?  Why does the OS view of the enclave's memory matter?

Consider SGX_IOC_ENCLAVE_ADD_PAGES. It essentially takes from MAC
perpective two items:

1. Source represented as a VMA, and in the end as vm_file, i.e. the
   origin. This good place to anchor a security policy.
2. Permissions.

Since enclaves are dynamically constructed and do not go through
execve() path but are dynamically loaded this is a good place hook make
an LSM hook. Lets imagine that we have an LSM hook in place.

Now when enclave the enclave is loaded the LSM makes decisions based
on its policy, permissions and the origin of the enclave file. After
this decisions has been made it is sane to assume that the enclave
could not be run with higher permissions, right?

Now what an adversary could do after the enclave has been loaded and
initialized is to call EMODPE inside the enclave to upgrade permissions
to some pages. After that mprotect() can be applied to use the new
permissions.

The mprotect() callback keeps any higher permissions than originally
accepted by the MAC policy unusable.

Also without any LSM hooks this allows to have a more privileged launcher
process to build enclaves (e.g. have someone to own /dev/sgx/enclave)
and send fd's to other processes to run them. And with that callback in
place the launcher process can cap the permissions at build time.

That's where the indirect connection originates from. Should I write
this user story to the commit message as an addition. Would that help?

For v39 I did make a change that noexec partitions are categorically
discluded as an origin for enclaves. Before that it was that NX
pages from such partitions are allowed but I thought that it is better
simplify things since they are anyway complex. I'm 100% we can make
that kind of support for noexec partitions later on, if anyone ever
wants that anyway.

/Jarkko