[RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation

[Xing, Cedric]

> From: Andy Lutomirski [mailto:luto at kernel.org]
> Sent: Monday, July 01, 2019 10:54 AM
> 
> On Mon, Jul 1, 2019 at 10:46 AM Xing, Cedric <cedric.xing at intel.com>
> wrote:
> >
> > > From: Andy Lutomirski [mailto:luto at kernel.org]
> > > Sent: Saturday, June 29, 2019 4:42 PM
> > >
> > > On Tue, Jun 25, 2019 at 2:09 PM Stephen Smalley <sds at tycho.nsa.gov>
> > > wrote:
> > > >
> > > > On 6/21/19 5:22 PM, Xing, Cedric wrote:
> > > > >> From: Christopherson, Sean J
> > > > >> Sent: Wednesday, June 19, 2019 3:24 PM
> > > > >>
> > > > >> Intended use of each permission:
> > > > >>
> > > > >>    - SGX_EXECDIRTY: dynamically load code within the enclave
> itself
> > > > >>    - SGX_EXECUNMR: load unmeasured code into the enclave, e.g.
> > > > >> Graphene
> > > > >
> > > > > Why does it matter whether a code page is measured or not?
> > > >
> > > > It won't be incorporated into an attestation?
> > > >
> > >
> > > Also, if there is, in parallel, a policy that limits the set of
> > > enclave SIGSTRUCTs that are accepted, requiring all code be measured
> > > makes it harder to subvert by writing incompetent or maliciously
> > > incompetent enclaves.
> >
> > As analyzed in my reply to one of Stephen's comments, no executable
> page shall be "enclave only" as enclaves have access to host's memory,
> so if an executable page in regular memory is considered posting a
> threat to the process, it should be considered posting the same threat
> inside an enclave as well.

What I was trying to say was, an executable page, if considered a threat to the enclosing process, should always be considered a threat no matter it is in that process's memory or inside an enclave enclosed in that same process's address space.

Therefore, for a page in regular memory, if it is denied executable, it is because it is considered a potential security threat to the enclosing process, so it shall not be used as the source for an executable enclave page, as the same threat exists regardless it is in regular memory or EPC. Does that make more sense?

> 
> Huh?  The SDM (37.3 in whateve version I'm reading) says "Code fetches
> from inside an enclave to a linear address outside that enclave result
> in a #GP(0) exception."  Enclaves execute enclave code only.
> 
> In any event, I believe we're discussing taking readable memory from
> outside the enclave and copying it to an executable code inside the
> enclave.

You are correct. SGX ISA doesn't care the source page as it only takes care of the security the enclave itself. 

But LSM on the other hand also takes care of the enclosing process. That said, a page, if denied executable because it is considered a potential threat to the process by LSM, should also be denied (by LSM) as the source for an executable enclave page because the same threat would exist even if it resides inside an enclave, for enclaves have access to all of the enclosing process's memory.

> 
> >
> > That said, every executable enclave page should have an executable
> > source page (doesn’t have to executable, as long as mprotect(X) would
> > succeed on it, as shown in my patch)
> 
> Does Sean's series require this?  I think that, if we can get away with
> it, it's a lot nicer to *not* require user code to map the source pages
> PROT_EXEC.  Some policy may check that it's VM_MAYEXEC or check some
> other attribute of the VMA, but actually requiring PROT_EXEC seems like
> we're weakening existing hardening measures to enforce a policy, which
> is a mistake.

My patch doesn't require X on source pages either. I said "would", meaning X *would* be granted but doesn't have to be granted. You can see this in selinux_enclave_load() calling selinux_file_mprotect() in my code. The purpose is to determine if X *would* be granted to the source pages without actually granting X.