Re: x86/sgx: v23-rc2

From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: "Dr. Greg" <greg@enjellic.com>,
	Jethro Beekman <jethro@fortanix.com>,
	Andy Lutomirski <luto@kernel.org>,
	"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"serge.ayoun@intel.com" <serge.ayoun@intel.com>,
	"shay.katz-zamir@intel.com" <shay.katz-zamir@intel.com>
Subject: Re: x86/sgx: v23-rc2
Date: Thu, 5 Mar 2020 11:51:57 -0800	[thread overview]
Message-ID: <20200305195157.GO11500@linux.intel.com> (raw)
In-Reply-To: <20200221130009.GB3112@linux.intel.com>

On Fri, Feb 21, 2020 at 03:00:31PM +0200, Jarkko Sakkinen wrote:
> On Thu, Feb 20, 2020 at 07:19:13PM -0600, Dr. Greg wrote:
> > > > This would seem to imply that the driver is rather firmly architected
> > > > on the notion of one open() per enclave, a concept that Jethro seems
> > > > to have issues with.
> > 
> > > I don't understand what concept you are talking about.
> > 
> > If memory serves me correctly, Jethro envisioned a model where a
> > single open of the SGX driver node would return a file descriptor that
> > could then be used to create/load/initialize multiple enclaves.  Your
> > clarifications indicate that a separate open will be needed for each
> > and every enclave instance that will be orchestrated.
> > 
> > Jethro, if I'm mistating your position on this, please jump in and
> > clarify.
> 
> Ah.
> 
> You are speaking about having a factory to create enclaves and a
> management interface. I.e. you'd have ioctl to create enclave that gives
> you a file descriptor to access its management interface.
> 
> Out of top of my head I cannot recall why this was not favored in the
> end but generally speaking added complexity should be justified by some
> considerably strong measures.

The primary issue is that having an ioctl() to create enclaves means the
enclave fd would be an anon inode.  From a security (well, SELinux)
perspective, anon inodes are problematic for two reasons:

  - Anon inodes are exempt from SELinux ioctl() whitelisting.  This
    probably isn't a huge deal?

  - Mapping an anon inode with PROT_EXEC requires SELinux's EXECMEM, i.e.
    all enclave runtimes would require EXECMEM, which is a big step
    backwards in terms of security and drew the ire of Linus[1].

That being said, there is a proposed patch set to add support for "secure"
anon inodes[2], which I believe would eliminate all of the above problems.
Of course we'd be stuck with the current anon inode behavior until that
patch set lands (I'm feeling optomistic today ;-) ).

[1] https://lkml.kernel.org/r/CAHk-=wjmT=uC1=18ZYV1CMfP_FBUEjh9_rabH0g+a0z-L0cgHg@mail.gmail.com
[2] https://lkml.kernel.org/r/20200214032635.75434-1-dancol@google.com