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>,
	Stephen Smalley <stephen.smalley@gmail.com>,
	casey@schaufler-ca.com
Cc: "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"selinux@vger.kernel.org" <selinux@vger.kernel.org>,
	"Schaufler, Casey" <casey.schaufler@intel.com>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"jethro@fortanix.com" <jethro@fortanix.com>,
	"greg@enjellic.com" <greg@enjellic.com>,
	"sds@tycho.nsa.gov" <sds@tycho.nsa.gov>,
	"jarkko.sakkinen@linux.intel.com"
	<jarkko.sakkinen@linux.intel.com>,
	"Christopherson, Sean J" <sean.j.christopherson@intel.com>
Subject: Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
Date: Mon, 1 Jul 2019 12:53:58 -0700
Message-ID: <f59529e4-6cc8-2405-d7db-2519727f9a80@schaufler-ca.com> (raw)
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551D585@ORSMSX116.amr.corp.intel.com>

On 7/1/2019 10:57 AM, Xing, Cedric wrote:
>> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
>> owner@vger.kernel.org] On Behalf Of Casey Schaufler
>>
>> On 6/28/2019 6:37 PM, Stephen Smalley wrote:
>>> On Fri, Jun 28, 2019 at 1:22 PM Casey Schaufler <casey@schaufler-
>> ca.com> wrote:
>>>> On 6/27/2019 5:47 PM, Xing, Cedric wrote:
>>>>>> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
>>>>>> Sent: Thursday, June 27, 2019 4:37 PM
>>>>>>>> This code should not be mixed in with the LSM infrastructure.
>>>>>>>> It should all be contained in its own module, under
>> security/enclave.
>>>>>>> lsm_ema is *intended* to be part of the LSM infrastructure.
>>>>>> That's not going to fly, not for a minute.
>>>>> Why not, if there's a need for it?
>>>>>
>>>>> And what's the concern here if it becomes part of the LSM
>> infrastructure.
>>>> The LSM infrastructure provides a framework for hooks and allocation
>>>> of blobs. That's it. It's a layer for connecting system features like
>>>> VFS, IPC and the IP stack to the security modules. It does not
>>>> implement any policy of it's own. We are not going to implement SGX
>>>> or any other mechanism within the LSM infrastructure.
>>> I don't think you understand the purpose of this code.  It isn't
>>> implementing SGX, nor is it needed by SGX.
>>> It is providing shared infrastructure for security modules, similar to
>>> lsm_audit.c, so that security modules can enforce W^X or similar
>>> memory protection guarantees for SGX enclave memory, which has unique
>>> properties that render the existing mmap and mprotect hooks
>>> insufficient. They can certainly implement it only for SELinux, but
>>> then any other security module that wants to provide such guarantees
>>> will have to replicate that code.
>> I am not objecting to the purpose of the code.
>> I *am* objecting to calling it part of the LSM infrastructure.
>> It needs to be it's own thing, off somewhere else.
>> It must not use the lsm_ prefix. That's namespace pollution.
>> The code must not be embedded in the LSM infrastructure code, that
>> breaks with how everything else works.
> If you understand the purpose,

The purpose is to support the SGX hardware, is it not?
If you don't have SGX hardware (e.g. MIPS, ARM, s390) you
don't need this code.

> then why are you objecting the lsm_ prefix as they are APIs to be used by all LSM modules?

We name interfaces based on what they provide, not who consumes them.
As your code provides enclave services, that is how they should be named.

>  Or what should be the prefix in your mind?

I'm pretty sure that I've consistently suggested "enclave". 

> Or what kind of APIs do you think can qualify the lsm_ prefix?

Code that implements the LSM infrastructure. There is one LSM
blob allocation interface, lsm_inode_alloc(), that is used in
early set-up that is exported. As I've mentioned more than once,
enclave/page management is not an LSM infrastructure function,
it's a memory management function.

> And I'd like to clarify that it doesn't break anything, but is just a bit different, in that security_enclave_load() and security_file_free() call into those APIs.

There should be nothing in security_enclave_load() except calls to the enclave_load()
hooks (e.g. selinux_enclave_load()). There should be nothing in security_file_free()
except file blob management calls to the file_free() hooks (e.g. apparmor_file_free()). 

> But there's a need for them because otherwise code/data would have to be duplicated among LSMs

There's plenty of code duplication among the LSMs, because a lot
of what they do is the same thing. Someday there may be an effort
to address some of that, but I don't think it's on anybody's radar.
As for data duplication, there's a reason we use lots of pointers.

> and the logic would be harder to comprehend.

Keeping the layering clean is critical to comprehension.
There's a lot of VFS code that could have been implemented
within the LSM infrastructure, but I don't think that anyone
would argue that it should have been.

> So that's a trade-off.

I remain completely unconvinced that your proposal
represents a good way to implement you scheme.

> Then what's the practical drawback of doing that?

Name space pollution.
Layering violation.
Architecture specific implementation detail in a
general infrastructure.

> If no, why would we want to pay for the cost for not doing that?

Modularity and maintainability come directly to mind.

>> ... and the notion that you allocate data for one blob that gets freed
>> relative to another breaks the data management model.
> What do you mean here?

You're freeing the EMA data from security_file_free().
If selinux wants to free EMA data it has allocated in
selinux_enclave_load() in selinux_file_free() that's fine,
but the LSM infrastructure has no need to know about it.
EMA needs to manage its own data, just like VFS does.
The LSM infrastructure provides blob management so that
the security modules can extend data if they want to.

> EMA blobs are allocated/freed *not* relative to any other blobs.

In the code you proposed they are freed in security_file_free().
That is for file blob management. 



  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
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 [this message]
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=f59529e4-6cc8-2405-d7db-2519727f9a80@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=casey.schaufler@intel.com \
    --cc=cedric.xing@intel.com \
    --cc=greg@enjellic.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=jmorris@namei.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=sds@tycho.nsa.gov \
    --cc=sean.j.christopherson@intel.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley@gmail.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 \
		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