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

[Xing, Cedric]

Hi Andy,

I'm just curious how Sean convinced you that "MAXPERM doesn't work". More specifically, I'm objecting to the statement of "the enclave loader won't know at load time whether a given EAUG-ed page will ever be executed".

I'm still new to LSM so my understanding may not be correct. But I think LSM policy defines a boundary that "loosely" restricts what a process can do. By "loosely", I mean it is usually more permissive than a process needs. For example, FILE__EXECMOD basically says there are self-modifying code in a file, but it isn't specific on which pages contain self-modifying code, hence *all* pages are allowed mprotect(RW->RX) even though only a (small) subset actually need it. LSM policies are static too, as FILE__EXECMOD is given by a system admin to be associated with the disk file, instead of being requested/programmed by any processes loading/mapping that file.

So I think the same rationale applies to enclaves. Your original idea of MAXPERM is the policy set forth by system admin and shall *never* change at runtime. If an enclave is dynamically linked and needs to bring in code pages at runtime, the admin needs to enable it by setting, say ENCLAVE__EXECMOD, in the sigstruct file. Then all EAUG'ed pages will receive RWX as MAXPERM. The process would then mprotect() selective pages to be RX but which exact set of pages doesn't concern LSM usually.

> From: Andy Lutomirski [mailto:luto@xxxxxxxxxx]
> Sent: Thursday, May 30, 2019 12:21 PM
> 
> On Thu, May 30, 2019 at 11:01 AM Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> wrote:
> >
> > On Thu, May 30, 2019 at 09:14:10AM -0700, Andy Lutomirski wrote:
> > > On Thu, May 30, 2019 at 8:04 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> > > >
> > > > On 5/30/19 10:31 AM, Andy Lutomirski wrote:
> > > > > Hi all-
> > > > >
> > > > > After an offline discussion with Sean yesterday, here are some
> > > > > updates to the user API parts of my proposal.
> > > > >
> > > > > Unfortunately, Sean convinced me that MAXPERM doesn't work the
> > > > > way I described it because, for SGX2, the enclave loader won't
> > > > > know at load time whether a given EAUG-ed page will ever be
> > > > > executed.  So here's an update.
> > > > >
> > > > > First, here are the requrements as I see them, where EXECUTE,
> > > > > EXECMOD, and EXECMEM could be substituted with other rules at
> > > > > the LSM's
> > > > > discretion:
> > > > >
> > > > >   - You can create a WX or RWX mapping if and only if you have EXECMEM.
> > > > >
> > > > >   - To create an X mapping of an enclave page that has ever been
> > > > > W, you need EXECMOD.
> > > >
> > > > EXECMOD to what file? The enclave file from which the page's
> > > > content originated, the sigstruct file, or /dev/sgx/enclave?
> > >
> > > I leave that decision to you :)  The user should need permission to
> > > do an execmod thing on an enclave, however that wants to be encoded.
> >
> > But that decision dictates how the SGX API handles sigstruct.  If LSMs
> > want to associate EXECMOD with sigstruct, then SGX needs to take
> > sigstruct early and hold a reference to the file for the lifetime of the enclave.
> > And if we're going to do that, the whole approach of inheriting
> > permissions from source VMAs becomes unnecessary complexity.
> >
> > > >
> > > > >   - To create an X mapping of an enclave page that came from
> > > > > EADD, you need EXECUTE on the source file.  Optionally, we could
> > > > > also permit this if you have EXECMOD.
> > > >
> > > > What is the "source file" i.e. the target of the check?  Enclave
> > > > file, sigstruct file, or /dev/sgx/enclave?
> > >
> > > Enclave file -- that is, the file backing the vma from which the data is loaded.
> >
> > It wasn't explicitly called out in Andy's proposal(s), but the idea is
> > that the SGX driver would effectively inherit permissions from the
> > source VMA (EADD needs a source for the initial value of the encave page).
> 
> I actually meant for it to *not* work like this.  I don't want the source VMA to have to
> be VM_EXEC.  I think the LSM should just check permissions on ->vm_file.

-Cedric