Re: [PATCH 2/4] x86/sgx: Put enclaves into anonymous files

From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: Topi Miettinen <toiwoton@gmail.com>,
	Jethro Beekman <jethro@fortanix.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	"Schaufler, Casey" <casey.schaufler@intel.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	linux-sgx@vger.kernel.org, "Svahn, Kai" <kai.svahn@intel.com>,
	"Schlobohm, Bruce" <bruce.schlobohm@intel.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Haitao Huang <haitao.huang@linux.intel.com>,
	Ben Hutchings <ben@decadent.org.uk>
Subject: Re: [PATCH 2/4] x86/sgx: Put enclaves into anonymous files
Date: Tue, 7 Apr 2020 02:48:20 +0300	[thread overview]
Message-ID: <20200406234820.GA3956@linux.intel.com> (raw)
In-Reply-To: <CALCETrWkaUS2RU61_4KoNhn3RkW-J+SWiCQTZ623wu4b7snJ0w@mail.gmail.com>

On Mon, Apr 06, 2020 at 04:18:20PM -0700, Andy Lutomirski wrote:
> On Mon, Apr 6, 2020 at 2:24 PM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Apr 06, 2020 at 12:53:41PM -0700, Andy Lutomirski wrote:
> > > > sgxfs is somewhat trivial to implement and has one stakeholder less to
> > > > worry about. It is not really a huge stretch.
> > > >
> > > > Overally, I think it is something that we could live with. At least it
> > > > is something that does not step on others toes.
> > > >
> > > > Haitao: If we go with sgxfs route, then you can for the moment do what
> > > > Andy suggested: bind mount it to /dev/sgx.
> > >
> > > That also needs userspace support.
> > >
> > > I’ll start a thread on the udev list.
> >
> > Haven't written any code yet but just by drafting the idea I already
> > pinpointed something.
> >
> > You would add roughly this to the existing codebase:
> >
> > * struct file_system_type sgxfs
> > * struct fs_context_operations sgxfs_ctx_ops
> > * int sgxfs_get_tree()
> > * int sgxfs_fill_super()
> > * const struct tree_descr sgxfs_files[]
> >
> > The files would be defined as like this:
> >
> > static const struct tree_descr sgxfs_files[] {
> >         [SGXFS_ENCLAVE] = { "enclave", &sgxfs_encl_ops, /* ? */ },
> >         [SGXFS_PROVISION] = { "provision", &sgxfs_encl_ops, /* ? */ },
> > };
> >
> > The permissions would need to be completely static, which feels nasty
> > from maintainability viewpoint :-( And I see no gain anywhere.
> >
> > In my opinion udev defining the whole /dev as noexec has zero technical
> > merits. It is same as they would say that "we don't trust our own
> > database". There are no real security benefits as long as dev nodes are
> > configured correctly.
> 
> I tend to agree, but it could be seen as a sort-of-valid hardening measure.

Bets are not great anyway to revert it so lets just consider it as a
constraint then.

> I think the best bet is for you to ignore this and let userspace figure it out.

Yeah. You could put statically 0700 permissions and then let systemd to
set the legit ones but it just feels a way too big hammer to do a new fs
just to sort out a simple permission issue.

/Jarkko