Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: "Xing, Cedric" <cedric.xing@intel.com>,
	linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, casey@schaufler-ca.com
Subject: Re: [RFC PATCH v3 3/4] X86/sgx: Introduce EMA as a new LSM module
Date: Wed, 10 Jul 2019 14:14:14 -0700
Message-ID: <594ebd19-652d-ab6f-e098-515d50e80ce6@schaufler-ca.com> (raw)
In-Reply-To: <e2a0d952-b4d4-f8f3-ee58-eba63f30dc66@intel.com>

On 7/9/2019 5: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.

So far, so good ...

> EMA is more like a helper library than a real LSM.

Then you should use it as such.

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

The security modules that want to use EMA should
request that space.

> That cannot be done by any LSMs at runtime.

Sure it can. And it has to. What if you don't
have any security modules that use the EMA data?
Surely you don't want to be allocating blob space
for EMA data if no one is going to use it.

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

That is not true. The security module that wants to use
the EMA data can call whatever allocation function you use.
Or, the call can be made from the code just before you call
the security hook, which would be identical to calling it
as a "first" hook.

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

Call your allocation function just before you call
security_enclave_load(). There is no way that selinux_enclave_load()
could tell the difference.

> 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 this is the enclave abstraction, or rather, should be
according to at least half the people joining in on the
thread. It does not belong in the LSM infrastructure because
it is it's own thing, with its own state and data, which it
needs to maintain in its own way and place. It needs interfaces
so that security modules can use that information appropriately.
It needs a hook or two so that the enclave abstraction can ask
the security modules to make decisions.

>
> And, Stephen, do you have an opinion on this?
>
>>>>>>> +/**
>>>>>>> + * 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.

Thank you.

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

Ah. Being in the security directory does not mean it's
a part of the LSM system. Keys and integrity are security
subsystems that are related to, but not part of, the LSM
sub-system.

> (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?

Please stop asking the same question over and over.
You're not going to get a different answer from what
you've gotten already. Look back at it what's already
been said.



  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 [this message]
2019-07-11 13:51                 ` Stephen Smalley
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=594ebd19-652d-ab6f-e098-515d50e80ce6@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=cedric.xing@intel.com \
    --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

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 \
		linux-sgx@vger.kernel.org linux-sgx@archiver.kernel.org
	public-inbox-index linux-sgx


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