SELinux Archive on lore.kernel.org
 help / color / Atom feed
From: Stephen Smalley <sds@tycho.nsa.gov>
To: "Xing, Cedric" <cedric.xing@intel.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, James Morris <jmorris@namei.org>
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: <4c3e21dd-8890-e3cb-0db7-c154ac7201e1@tycho.nsa.gov> (raw)
In-Reply-To: <e2a0d952-b4d4-f8f3-ee58-eba63f30dc66@intel.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?


  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 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=4c3e21dd-8890-e3cb-0db7-c154ac7201e1@tycho.nsa.gov \
    --to=sds@tycho.nsa.gov \
    --cc=casey@schaufler-ca.com \
    --cc=cedric.xing@intel.com \
    --cc=jmorris@namei.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=selinux@vger.kernel.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 \
		selinux@vger.kernel.org selinux@archiver.kernel.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