From: Stephen Smalley <email@example.com> To: "Xing, Cedric" <firstname.lastname@example.org>, Casey Schaufler <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, James Morris <email@example.com> Subject: Re: [RFC PATCH v3 3/4] X86/sgx: Introduce EMA as a new LSM module Date: Thu, 11 Jul 2019 09:51:19 -0400 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On 7/9/19 8:55 PM, Xing, Cedric wrote: > On 7/9/2019 5:10 PM, Casey Schaufler wrote: >> On 7/9/2019 3:13 PM, Xing, Cedric wrote: >>> On 7/8/2019 4:53 PM, Casey Schaufler wrote: >>>> On 7/8/2019 10:16 AM, Xing, Cedric wrote: >>>>> On 7/8/2019 9:26 AM, Casey Schaufler wrote: >>>>>> In this scheme you use an ema LSM to manage your ema data. >>>>>> A quick sketch looks like: >>>>>> >>>>>> sgx_something_in() calls >>>>>> security_enclave_load() calls >>>>>> ema_enclave_load() >>>>>> selinux_enclave_load() >>>>>> otherlsm_enclave_load() >>>>>> >>>>>> Why is this better than: >>>>>> >>>>>> sgx_something_in() calls >>>>>> ema_enclave_load() >>>>>> security_enclave_load() calls >>>>>> selinux_enclave_load() >>>>>> otherlsm_enclave_load() >>>>> >>>>> Are you talking about moving EMA somewhere outside LSM? >>>> >>>> Yes. That's what I've been saying all along. >>>> >>>>> If so, where? >>>> >>>> I tried to make it obvious. Put the call to your EMA code >>>> on the line before you call security_enclave_load(). >>> >>> Sorry but I'm still confused. >>> >>> EMA code is used by LSMs only. Making it callable from other parts of >>> the kernel IMHO is probably not a good idea. And more importantly I >>> don't understand the motivation behind it. Would you please elaborate? >> >> LSM modules implement additional access control restrictions. >> The EMA code does not do that, it provides management of data >> that is used by security modules. It is not one itself. VFS >> also performs this role, but no one would consider making VFS >> a security module. > > You are right. EMA is more like a helper library than a real LSM. But > the practical problem is, it has a piece of initialization code, to > basically request some space in the file blob from the LSM > infrastructure. That cannot be done by any LSMs at runtime. So it has to > either be done in LSM infrastructure directly, or make itself an LSM to > make its initialization function invoked by LSM infrastructure > automatically. You have objected to the former, so I switched to the > latter. Are you now objecting to the latter as well? Then what are you > suggesting, really? > > VFS is a completely different story. It's the file system abstraction so > it has a natural place to live in the kernel, and its initialization > doesn't depend on the LSM infrastructure. EMA on the other hand, shall > belong to LSM because it is both produced and consumed within LSM. > > And, Stephen, do you have an opinion on this? I don't really understand Casey's position. I also wouldn't necessarily view his opinion on the matter as necessarily authoritative since he is not the LSM maintainer as far as I know although he has contributed a lot of changes in recent years. I understood the architecture of the original EMA implementation (i.e. a support library to be used by the security modules to help them in implementing their own access control policies), although I do have some concerns about the complexity and lifecycle issues, and wonder if a simpler model as suggested by Dr. Greg isn't feasible. I'd also feel better if there was clear consensus among all of the @intel.com participants that this is the right approach. To date that has seemed elusive. If there were consensus that the EMA approach was the right one and that a simpler model is not feasible, and if the only obstacle to adopting EMA was Casey's objections, then I'd say just put it directly into SELinux and be done with it. I originally thought that was a mistake but if other security modules don't want the support, so be it. > >>>>>>> +/** >>>>>>> + * ema - Enclave Memory Area structure for LSM modules >>>>>> >>>>>> LSM modules is redundant. "LSM" or "LSMs" would be better. >>>>> >>>>> Noted >>>>> >>>>>>> diff --git a/security/Makefile b/security/Makefile >>>>>>> index c598b904938f..b66d03a94853 100644 >>>>>>> --- a/security/Makefile >>>>>>> +++ b/security/Makefile >>>>>>> @@ -28,6 +28,7 @@ obj-$(CONFIG_SECURITY_YAMA) += yama/ >>>>>>> obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/ >>>>>>> obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/ >>>>>>> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o >>>>>>> +obj-$(CONFIG_INTEL_SGX) += commonema.o >>>>>> >>>>>> The config option and the file name ought to match, >>>>>> or at least be closer. >>>>> >>>>> Just trying to match file names as "capability" uses commoncap.c. >>>> >>>> Fine, then you should be using CONFIG_SECURITY_EMA. >>>> >>>>> >>>>> Like I said, this feature could potentially be used by TEEs other >>>>> than SGX. For now, SGX is the only user so it is tied to >>>>> CONFIG_INTEL_SGX. I can rename it to ema.c or enclave.c. Do you >>>>> have a preference? >>>> >>>> Make >>>> CONFIG_SECURITY_EMA >>>> depends on CONFIG_INTEL_SGX >>>> >>>> When another TEE (maybe MIPS_SSRPQ) comes along you can have >>>> >>>> CONFIG_SECURITY_EMA >>>> depends on CONFIG_INTEL_SGX || CONFIG_MIPS_SSRPQ >>> >>> Your suggestions are reasonable. Given such config change wouldn't >>> affect any code, can we do it later, >> >> That doesn't make the current options any less confusing, >> and it will be easier to make the change now than at some >> point in the future. >> >>> e.g., when additional TEEs come online and make use of these new >>> hooks? After all, security_enclave_init() will need amendment anyway >>> as one of its current parameters is of type 'struct sgx_sigstruct', >>> which will need to be replaced with something more generic. At the >>> time being, I'd like to keep things intuitive so as not to confuse >>> reviewers. >> >> Reviewers (including me) are already confused by the inconsistency. > > OK. Let me make this change. > >>>>> >>>>>>> diff --git a/security/commonema.c b/security/commonema.c >>>>>> >>>>>> Put this in a subdirectory. Please. >>>>> >>>>> Then why is commoncap.c located in this directory? I'm just trying >>>>> to match the existing convention. >>>> >>>> commoncap is not optional. It is a base part of the >>>> security subsystem. ema is optional. >>> >>> Alright. I'd move it into a sub-folder and rename it to ema.c. Would >>> you be ok with that? >> >> Sounds fine. > > This is another part that confuses me. Per you comment here, I think you > are OK with EMA being part of LSM (I mean, living somewhere under > security/). But your other comment of calling ema_enclave_load() > alongside security_enclave_load() made me think EMA and LSM were > separate. What do you want really?
next prev parent reply index Thread overview: 156+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-19 22:23 [RFC PATCH v4 00/12] security: x86/sgx: SGX vs. LSM Sean Christopherson 2019-06-19 22:23 ` [RFC PATCH v4 01/12] x86/sgx: Use mmu_notifier.release() instead of per-vma refcounting Sean Christopherson 2019-06-20 21:03 ` Jarkko Sakkinen 2019-07-08 14:57 ` Sean Christopherson 2019-07-09 16:18 ` Jarkko Sakkinen 2019-06-19 22:23 ` [RFC PATCH v4 02/12] x86/sgx: Do not naturally align MAP_FIXED address Sean Christopherson 2019-06-20 21:09 ` Jarkko Sakkinen 2019-06-20 22:09 ` Jarkko Sakkinen 2019-06-19 22:23 ` [RFC PATCH v4 03/12] selftests: x86/sgx: Mark the enclave loader as not needing an exec stack Sean Christopherson 2019-06-20 21:17 ` Jarkko Sakkinen 2019-06-19 22:23 ` [RFC PATCH v4 04/12] x86/sgx: Require userspace to define enclave pages' protection bits Sean Christopherson 2019-06-21 1:07 ` Jarkko Sakkinen 2019-06-21 1:16 ` Jarkko Sakkinen 2019-06-21 16:42 ` Xing, Cedric 2019-07-08 16:34 ` Sean Christopherson 2019-07-08 17:29 ` Xing, Cedric 2019-07-01 18:00 ` Andy Lutomirski 2019-07-01 19:22 ` Xing, Cedric 2019-06-19 22:23 ` [RFC PATCH v4 05/12] x86/sgx: Enforce noexec filesystem restriction for enclaves Sean Christopherson 2019-06-21 1:26 ` Jarkko Sakkinen 2019-07-07 19:03 ` Sean Christopherson 2019-06-19 22:23 ` [RFC PATCH v4 06/12] mm: Introduce vm_ops->may_mprotect() Sean Christopherson 2019-06-21 1:35 ` Jarkko Sakkinen 2019-06-19 22:23 ` [RFC PATCH v4 07/12] LSM: x86/sgx: Introduce ->enclave_map() hook for Intel SGX Sean Christopherson 2019-06-21 2:28 ` Jarkko Sakkinen 2019-06-21 16:54 ` Xing, Cedric 2019-06-25 20:48 ` Stephen Smalley 2019-06-27 20:29 ` Xing, Cedric 2019-07-07 18:01 ` Sean Christopherson 2019-06-19 22:23 ` [RFC PATCH v4 08/12] security/selinux: Require SGX_MAPWX to map enclave page WX Sean Christopherson 2019-06-21 17:09 ` Xing, Cedric 2019-06-25 21:05 ` Stephen Smalley 2019-06-27 20:26 ` Xing, Cedric 2019-06-25 20:19 ` Stephen Smalley 2019-06-26 12:49 ` Dr. Greg 2019-06-19 22:23 ` [RFC PATCH v4 09/12] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX Sean Christopherson 2019-06-21 17:05 ` Xing, Cedric 2019-06-25 21:01 ` Stephen Smalley 2019-06-25 21:49 ` Stephen Smalley 2019-06-27 19:38 ` Xing, Cedric 2019-06-19 22:23 ` [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation Sean Christopherson 2019-06-21 21:22 ` Xing, Cedric 2019-06-25 21:09 ` Stephen Smalley 2019-06-27 20:19 ` Xing, Cedric 2019-06-28 16:16 ` Stephen Smalley 2019-06-28 21:20 ` Xing, Cedric 2019-06-29 1:15 ` Stephen Smalley 2019-07-01 18:14 ` Xing, Cedric 2019-06-29 23:41 ` Andy Lutomirski 2019-07-01 17:46 ` Xing, Cedric 2019-07-01 17:53 ` Andy Lutomirski 2019-07-01 18:54 ` Xing, Cedric 2019-07-01 19:03 ` Xing, Cedric 2019-07-01 19:32 ` Andy Lutomirski 2019-07-01 20:03 ` Xing, Cedric 2019-07-07 18:46 ` Sean Christopherson 2019-06-25 20:34 ` Stephen Smalley 2019-06-19 22:24 ` [RFC PATCH v4 11/12] security/apparmor: " Sean Christopherson 2019-06-19 22:24 ` [RFC PATCH v4 12/12] LSM: x86/sgx: Show line of sight to LSM support SGX2's EAUG Sean Christopherson 2019-06-21 17:18 ` Xing, Cedric 2019-07-08 14:34 ` Sean Christopherson 2019-06-21 1:32 ` [RFC PATCH v4 00/12] security: x86/sgx: SGX vs. LSM Jarkko Sakkinen 2019-06-27 18:56 ` [RFC PATCH v2 0/3] security/x86/sgx: SGX specific LSM hooks Cedric Xing 2019-07-03 23:16 ` Jarkko Sakkinen 2019-07-03 23:22 ` Jarkko Sakkinen 2019-07-03 23:23 ` Jarkko Sakkinen 2019-07-06 5:04 ` Xing, Cedric 2019-07-08 14:46 ` Jarkko Sakkinen 2019-07-07 23:41 ` [RFC PATCH v3 0/4] " Cedric Xing 2019-07-08 15:55 ` Sean Christopherson 2019-07-08 17:49 ` Xing, Cedric 2019-07-08 18:49 ` Sean Christopherson 2019-07-08 22:26 ` Xing, Cedric 2019-07-07 23:41 ` [RFC PATCH v3 1/4] x86/sgx: Add " Cedric Xing 2019-07-07 23:41 ` [RFC PATCH v3 2/4] x86/64: Call LSM hooks from SGX subsystem/module Cedric Xing 2019-07-09 1:03 ` Sean Christopherson 2019-07-07 23:41 ` [RFC PATCH v3 3/4] X86/sgx: Introduce EMA as a new LSM module Cedric Xing 2019-07-08 16:26 ` Casey Schaufler 2019-07-08 17:16 ` Xing, Cedric 2019-07-08 23:53 ` Casey Schaufler 2019-07-09 22:13 ` Xing, Cedric 2019-07-10 0:10 ` Casey Schaufler 2019-07-10 0:55 ` Xing, Cedric 2019-07-10 21:14 ` Casey Schaufler 2019-07-11 13:51 ` Stephen Smalley [this message] 2019-07-11 15:12 ` Sean Christopherson 2019-07-11 16:11 ` Stephen Smalley 2019-07-11 16:25 ` Sean Christopherson 2019-07-11 16:32 ` Stephen Smalley 2019-07-11 23:41 ` Xing, Cedric 2019-07-07 23:41 ` [RFC PATCH v3 4/4] x86/sgx: Implement SGX specific hooks in SELinux Cedric Xing 2019-07-09 1:33 ` Sean Christopherson 2019-07-09 21:26 ` Xing, Cedric 2019-07-10 15:49 ` Sean Christopherson 2019-07-10 16:08 ` Jethro Beekman 2019-07-10 18:16 ` Xing, Cedric 2019-07-10 17:54 ` Xing, Cedric 2019-06-27 18:56 ` [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks Cedric Xing 2019-06-27 22:06 ` Casey Schaufler 2019-06-27 22:52 ` Xing, Cedric 2019-06-27 23:37 ` Casey Schaufler 2019-06-28 0:47 ` Xing, Cedric 2019-06-28 17:22 ` Casey Schaufler 2019-06-28 22:29 ` Xing, Cedric 2019-06-29 1:37 ` Stephen Smalley 2019-06-29 21:35 ` Casey Schaufler 2019-07-01 17:57 ` Xing, Cedric 2019-07-01 19:53 ` Casey Schaufler 2019-07-01 21:45 ` Xing, Cedric 2019-07-01 23:11 ` Casey Schaufler 2019-07-02 7:42 ` Xing, Cedric 2019-07-02 15:44 ` Casey Schaufler 2019-07-03 9:46 ` Dr. Greg 2019-07-03 15:32 ` Casey Schaufler 2019-07-07 13:30 ` Dr. Greg 2019-07-09 0:02 ` Casey Schaufler 2019-07-09 1:52 ` Sean Christopherson 2019-07-09 21:16 ` Xing, Cedric 2019-07-11 10:22 ` Dr. Greg 2019-07-15 22:23 ` Andy Lutomirski 2019-06-28 16:37 ` Stephen Smalley 2019-06-28 21:53 ` Xing, Cedric 2019-06-29 1:22 ` Stephen Smalley 2019-07-01 18:02 ` Xing, Cedric 2019-06-29 23:46 ` Andy Lutomirski 2019-07-01 17:11 ` Xing, Cedric 2019-07-01 17:58 ` Andy Lutomirski 2019-07-01 18:31 ` Xing, Cedric 2019-07-01 19:36 ` Andy Lutomirski 2019-07-01 19:56 ` Xing, Cedric 2019-07-02 2:29 ` Andy Lutomirski 2019-07-02 6:35 ` Xing, Cedric 2019-06-27 18:56 ` [RFC PATCH v2 2/3] x86/sgx: Call LSM hooks from SGX subsystem/module Cedric Xing 2019-06-27 18:56 ` [RFC PATCH v2 3/3] x86/sgx: Implement SGX specific hooks in SELinux Cedric Xing 2019-07-05 16:05 ` [RFC PATCH v4 00/12] security: x86/sgx: SGX vs. LSM Jarkko Sakkinen 2019-07-08 17:29 ` Sean Christopherson 2019-07-08 17:33 ` Xing, Cedric 2019-07-09 16:22 ` Jarkko Sakkinen 2019-07-09 17:09 ` Sean Christopherson 2019-07-09 20:41 ` Xing, Cedric 2019-07-09 22:25 ` Sean Christopherson 2019-07-09 23:11 ` Xing, Cedric 2019-07-10 16:57 ` Sean Christopherson 2019-07-10 20:19 ` Jarkko Sakkinen 2019-07-10 20:31 ` Sean Christopherson 2019-07-11 9:06 ` Jarkko Sakkinen 2019-07-10 22:00 ` Jarkko Sakkinen 2019-07-10 22:16 ` Jarkko Sakkinen 2019-07-10 23:16 ` Xing, Cedric 2019-07-11 9:26 ` Jarkko Sakkinen 2019-07-11 14:32 ` Stephen Smalley 2019-07-11 17:51 ` Jarkko Sakkinen 2019-07-12 0:08 ` Xing, Cedric 2019-07-10 1:28 ` Dr. Greg 2019-07-10 2:04 ` Xing, Cedric 2019-07-10 3:21 ` Jethro Beekman
Reply instructions: You may reply publicly 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 \ --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 \ /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
Linux-Sgx Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/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 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \ firstname.lastname@example.org public-inbox-index linux-sgx Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx AGPL code for this site: git clone https://public-inbox.org/public-inbox.git