From: Andy Lutomirski <email@example.com> To: Sean Christopherson <firstname.lastname@example.org> Cc: Stephen Smalley <email@example.com>, Cedric Xing <firstname.lastname@example.org>, LSM List <email@example.com>, firstname.lastname@example.org, LKML <email@example.com>, firstname.lastname@example.org, Jarkko Sakkinen <email@example.com>, Andrew Lutomirski <firstname.lastname@example.org>, James Morris <email@example.com>, "Serge E. Hallyn" <firstname.lastname@example.org>, Paul Moore <email@example.com>, Eric Paris <firstname.lastname@example.org>, Jethro Beekman <email@example.com>, Dave Hansen <firstname.lastname@example.org>, Thomas Gleixner <email@example.com>, Linus Torvalds <firstname.lastname@example.org>, Andrew Morton <email@example.com>, firstname.lastname@example.org, email@example.com, "Ayoun, Serge" <firstname.lastname@example.org>, "Katz-zamir, Shay" <email@example.com>, "Huang, Haitao" <firstname.lastname@example.org>, Andy Shevchenko <email@example.com>, "Svahn, Kai" <firstname.lastname@example.org>, Borislav Petkov <email@example.com>, Josh Triplett <firstname.lastname@example.org>, "Huang, Kai" <email@example.com>, David Rientjes <firstname.lastname@example.org>, "Roberts, William C" <email@example.com>, Philip Tricca <firstname.lastname@example.org> Subject: Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux Date: Wed, 12 Jun 2019 12:30:20 -0700 Message-ID: <CALCETrWQT3AG+-OKBOzuw-a6VPApkNYsKqZiBmS56-b-72bfYQ@mail.gmail.com> (raw) In-Reply-To: <20190611220243.GB3416@linux.intel.com> On Tue, Jun 11, 2019 at 3:02 PM Sean Christopherson <email@example.com> wrote: > > On Tue, Jun 11, 2019 at 09:40:25AM -0400, Stephen Smalley wrote: > > I haven't looked at this code closely, but it feels like a lot of > > SGX-specific logic embedded into SELinux that will have to be repeated or > > reused for every security module. Does SGX not track this state itself? > > SGX does track equivalent state. > > There are three proposals on the table (I think): Sounds about right. I've been playing with #1 and #2 (as text, not code), and I'll post my latest thoughts on it below. But first, I should mention that I think we've gotten a bit too caught up on SELinux-y terminology like "EXECMOD" and "EXECMEM", which is relevant since the kernel has very little visibility into what the enclave is doing. Instead, I think we should think about the relevant permissions more like this: a) "execute code from a particular source, e.g. a file" b) "execute code supplied from arbitrary memory outside the enclave" c) "execute code generated within the enclave" d) "possess WX enclave memory" I think that any sensible policy that allows (b) should allow (a). Similarly, any policy that allows (d) should allow (c). I don't see any particular need for the kernel to go out of its way to ensure these relationships, though. We could plausibly also distinguish "execute measured code", although I think that the details of defining and implenenting this, especially with SGX2, could be nastier than we want to deal with. A minimal approach that mostly ignores SGX2 would be to have another permission "execute code supplied from outside the enclave that was not measured". This permission would be required on top of (a) or (b), depending on where that code comes from. If we want to map these to existing SELinux terms, we could use EXECUTE for (a), EXECMOD for (c), and EXECMEM for (d). (b) seems to also map to EXECMOD or EXECMEM depending on exactly how it happens, and I'm not sure this makes all that much sense. > > 1. Require userspace to explicitly specificy (maximal) enclave page > permissions at build time. The enclave page permissions are provided > to, and checked by, LSMs at enclave build time. > > Pros: Low-complexity kernel implementation, straightforward auditing > Cons: Sullies the SGX UAPI to some extent, may increase complexity of > SGX2 enclave loaders. In my notes, this works like this. This is similar, but not identical, to what Sean has been sending out. EADD takes flags: ALLOW_READ, ALLOW_WRITE, ALLOW_EXEC. It calls a new hook: int security_enclave_load(struct vm_area_struct *source, unsigned int flags); (Sean passed in the secinfo protection too, but I think we agreed that this could be omitted.) This hook will fail if ALLOW_EXEC is requested and the LSM doesn't consider the source VMA to be executable. Privileges (a) and (b) are implemented here. Optionally, we can enforce noexec here. The future EAUG ioctl takes the same flags, but it doesn't call security_enclave_load(). (As Cedric noted, the actual user API for EAUG is not settled, but I don't think it makes much difference here.) EINIT takes a sigstruct pointer. SGX calls a new hook: unsigned int security_enclave_init(struct sigstruct *sigstruct, struct vm_area_struct *source, unsigned int flags); This hook can return -EPERM. Otherwise it returns 0 or a combination of flags DENY_WX and DENY_X_IF_ALLOW_WRITE. The driver saves this value. These represent permissions (c) and (d). If we want to have a permission for "execute code supplied from outside the enclave that was not measured", we could have a flag like HAS_UNMEASURED_ALLOW_EXEC_PAGE that the LSM could consider. mmap() and mprotect() enforce the following rules: - Deny if a PROT_ flag is requested but the corresponding ALLOW_ flag is not set for all pages in question. - Deny if PROT_WRITE, PROT_EXEC, and DENY_WX are all set. - Deny if PROT_EXEC, ALLOW_WRITE, and DENY_X_IF_ALLOW_WRITE are all set. mprotect() and mmap() do *not* call SGX-specific LSM hooks to ask for permission, although they can optionally call an LSM hook if they hit one of the -EPERM cases for auditing purposes. I think this model works quite well in an SGX1 world. The main thing that makes me uneasy about this model is that, in SGX2, it requires that an SGX2-compatible enclave loader must pre-declare to the kernel whether it intends for its dynamically allocated memory to be ALLOW_EXEC. If ALLOW_EXEC is set but not actually needed, it will still fail if DENY_X_IF_ALLOW_WRITE ends up being set. The other version below does not have this limitation. > > 2. Pre-check LSM permissions and dynamically track mappings to enclave > pages, e.g. add an SGX mprotect() hook to restrict W->X and WX > based on the pre-checked permissions. > > Pros: Does not impact SGX UAPI, medium kernel complexity > Cons: Auditing is complex/weird, requires taking enclave-specific > lock during mprotect() to query/update tracking. Here's how this looks in my mind. It's quite similar, except that ALLOW_READ, ALLOW_WRITE, and ALLOW_EXEC are replaced with a little state machine. EADD does not take any special flags. It calls this LSM hook: int security_enclave_load(struct vm_area_struct *source); This hook can return -EPERM. Otherwise it 0 or ALLOC_EXEC_IF_UNMODIFIED (i.e. 1). This hook enforces permissions (a) and (b). The driver tracks a state for each page, and the possible states are: - CLEAN_MAYEXEC /* no W or X VMAs have existed, but X is okay */ - CLEAN_NOEXEC /* no W or X VMAs have existed, and X is not okay */ - CLEAN_EXEC /* no W VMA has existed, but an X VMA has existed */ - DIRTY /* a W VMA has existed */ The initial state for a page is CLEAN_MAYEXEC if the hook said ALLOW_EXEC_IF_UNMODIFIED and CLEAN_NOEXEC otherwise. The future EAUG does not call a hook at all and puts pages into the state CLEAN_NOEXEC. If SGX3 or later ever adds EAUG-but-don't-clear, it can call security_enclave_load() and add CLEAN_MAYEXEC pages if appropriate. EINIT takes a sigstruct pointer. SGX calls a new hook: unsigned int security_enclave_init(struct sigstruct *sigstruct, struct vm_area_struct *source, unsigned int flags); This hook can return -EPERM. Otherwise it returns 0 or a combination of flags DENY_WX and DENY_X_DIRTY. The driver saves this value. These represent permissions (c) and (d). If we want to have a permission for "execute code supplied from outside the enclave that was not measured", we could have a flag like HAS_UNMEASURED_CLEAN_EXEC_PAGE that the LSM could consider. mmap() and mprotect() enforce the following rules: - If VM_EXEC is requested and (either the page is DIRTY or VM_WRITE is requested) and DENY_X_DIRTY, then deny. - If VM_WRITE and VM_EXEC are both requested and DENY_WX, then deny. - If VM_WRITE is requested, we need to update the state. If it was CLEAN_EXEC, then we reject if DENY_X_DIRTY. Otherwise we change the state to DIRTY. - If VM_EXEC is requested and the page is CLEAN_NOEXEC, then deny. mprotect() and mmap() do *not* call SGX-specific LSM hooks to ask for permission, although they can optionally call an LSM hook if they hit one of the -EPERM cases for auditing purposes. Before the SIGSTRUCT is provided to the driver, the driver acts as though DENY_X_DIRTY and DENY_WX are both set.
next prev parent reply index Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-06 2:11 [RFC PATCH v2 0/5] security: x86/sgx: SGX vs. LSM Sean Christopherson 2019-06-06 2:11 ` [RFC PATCH v2 1/5] mm: Introduce vm_ops->may_mprotect() Sean Christopherson 2019-06-10 15:06 ` Jarkko Sakkinen 2019-06-10 15:55 ` Sean Christopherson 2019-06-10 17:47 ` Xing, Cedric 2019-06-10 19:49 ` Sean Christopherson 2019-06-10 22:06 ` Xing, Cedric 2019-06-06 2:11 ` [RFC PATCH v2 2/5] x86/sgx: Require userspace to define enclave pages' protection bits Sean Christopherson 2019-06-10 15:27 ` Jarkko Sakkinen 2019-06-10 16:15 ` Sean Christopherson 2019-06-10 17:45 ` Jarkko Sakkinen 2019-06-10 18:17 ` Sean Christopherson 2019-06-12 19:26 ` Jarkko Sakkinen 2019-06-10 18:29 ` Xing, Cedric 2019-06-10 19:15 ` Andy Lutomirski 2019-06-10 22:28 ` Xing, Cedric 2019-06-12 0:09 ` Andy Lutomirski 2019-06-12 14:34 ` Sean Christopherson 2019-06-12 18:20 ` Xing, Cedric 2019-06-06 2:11 ` [RFC PATCH v2 3/5] x86/sgx: Enforce noexec filesystem restriction for enclaves Sean Christopherson 2019-06-10 16:00 ` Jarkko Sakkinen 2019-06-10 16:44 ` Andy Lutomirski 2019-06-11 17:21 ` Stephen Smalley 2019-06-06 2:11 ` [RFC PATCH v2 4/5] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX Sean Christopherson 2019-06-07 19:58 ` Stephen Smalley 2019-06-10 16:21 ` Sean Christopherson 2019-06-10 16:05 ` Jarkko Sakkinen 2019-06-06 2:11 ` [RFC PATCH v2 5/5] security/selinux: Add enclave_load() implementation Sean Christopherson 2019-06-07 21:16 ` Stephen Smalley 2019-06-10 16:46 ` Sean Christopherson 2019-06-17 16:38 ` Jarkko Sakkinen 2019-06-10 7:03 ` [RFC PATCH v1 0/3] security/x86/sgx: SGX specific LSM hooks Cedric Xing 2019-06-10 7:03 ` [RFC PATCH v1 1/3] LSM/x86/sgx: Add " Cedric Xing 2019-06-10 7:03 ` [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux Cedric Xing 2019-06-11 13:40 ` Stephen Smalley 2019-06-11 22:02 ` Sean Christopherson 2019-06-12 9:32 ` Dr. Greg 2019-06-12 14:25 ` Sean Christopherson 2019-06-13 7:25 ` Dr. Greg 2019-06-12 19:30 ` Andy Lutomirski [this message] 2019-06-12 22:02 ` Sean Christopherson 2019-06-13 0:10 ` Xing, Cedric 2019-06-13 1:02 ` Xing, Cedric 2019-06-13 17:02 ` Stephen Smalley 2019-06-13 23:03 ` Xing, Cedric 2019-06-13 23:17 ` Sean Christopherson 2019-06-14 0:31 ` Xing, Cedric 2019-06-14 0:46 ` Sean Christopherson 2019-06-14 15:38 ` Sean Christopherson 2019-06-16 22:14 ` Andy Lutomirski 2019-06-17 16:49 ` Sean Christopherson 2019-06-17 17:08 ` Andy Lutomirski 2019-06-18 15:40 ` Dr. Greg 2019-06-14 17:16 ` Xing, Cedric 2019-06-14 17:45 ` Sean Christopherson 2019-06-14 17:53 ` Sean Christopherson 2019-06-14 20:01 ` Sean Christopherson 2019-06-16 22:16 ` Andy Lutomirski 2019-06-14 23:19 ` Dr. Greg 2019-06-11 22:55 ` Xing, Cedric 2019-06-13 18:00 ` Stephen Smalley 2019-06-13 19:48 ` Sean Christopherson 2019-06-13 21:09 ` Xing, Cedric 2019-06-13 21:02 ` Xing, Cedric 2019-06-14 0:37 ` Sean Christopherson 2019-06-10 7:03 ` [RFC PATCH v1 3/3] LSM/x86/sgx: Call new LSM hooks from SGX subsystem Cedric Xing 2019-06-10 17:36 ` [RFC PATCH v1 0/3] security/x86/sgx: SGX specific LSM hooks Jarkko Sakkinen
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CALCETrWQT3AG+-OKBOzuw-a6VPApkNYsKqZiBmS56-b-72bfYQ@mail.gmail.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
SELinux Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \ email@example.com firstname.lastname@example.org public-inbox-index selinux Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.selinux AGPL code for this site: git clone https://public-inbox.org/ public-inbox