RE: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks

From: "Xing, Cedric" <cedric.xing@intel.com>
To: Casey Schaufler <casey@schaufler-ca.com>,
	Stephen Smalley <stephen.smalley@gmail.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 17:57:40 +0000	[thread overview]
Message-ID: <960B34DE67B9E140824F1DCDEC400C0F6551D585@ORSMSX116.amr.corp.intel.com> (raw)
In-Reply-To: <18833f2e-9d18-1f39-6bc5-9242910ab25c@schaufler-ca.com>

> 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, then why are you objecting the lsm_ prefix as they are APIs to be used by all LSM modules? Or what should be the prefix in your mind? Or what kind of APIs do you think can qualify the lsm_ prefix?

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. But there's a need for them because otherwise code/data would have to be duplicated among LSMs and the logic would be harder to comprehend. So that's a trade-off. Then what's the practical drawback of doing that? If no, why would we want to pay for the cost for not doing that?

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

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