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

[Andy Lutomirski <luto@kernel.org>]

On Mon, Jul 1, 2019 at 11:54 AM Xing, Cedric <cedric.xing@intel.com> wrote:
>
> > From: Andy Lutomirski [mailto:luto@kernel.org]
> > Sent: Monday, July 01, 2019 10:54 AM
> >
> > On Mon, Jul 1, 2019 at 10:46 AM Xing, Cedric <cedric.xing@intel.com>
> > wrote:
> > >
> > > > From: Andy Lutomirski [mailto:luto@kernel.org]
> > > > Sent: Saturday, June 29, 2019 4:42 PM
> > > >
> > > > On Tue, Jun 25, 2019 at 2:09 PM Stephen Smalley <sds@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?

It does make sense, but I'm not sure it's correct to assume that any
LSM policy will always allow execution on enclave source pages if it
would allow execution inside the enclave.  As an example, here is a
policy that seems reasonable:

Task A cannot execute dynamic non-enclave code (no execmod, no
execmem, etc -- only approved unmodified file pages can be executed).
But task A can execute an enclave with MRENCLAVE == such-and-such, and
that enclave may be loaded from regular anonymous memory -- the
MRENCLAVE is considered enough verification.

>
> 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.

As above, I'm not convinced this assumption is valid.