Re: [PATCH 5/6] intel_sgx: driver documentation

From: Andy Lutomirski <luto@kernel.org>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	gregkh@linuxfoundation.org
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, Jonathan Corbet <corbet@lwn.net>,
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
	<linux-kernel@vger.kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Borislav Petkov <bpetkov@suse.de>
Subject: Re: [PATCH 5/6] intel_sgx: driver documentation
Date: Mon, 25 Apr 2016 13:01:06 -0700	[thread overview]
Message-ID: <571E7782.80107@kernel.org> (raw)
In-Reply-To: <1461605698-12385-6-git-send-email-jarkko.sakkinen@linux.intel.com>

On 04/25/2016 10:34 AM, Jarkko Sakkinen wrote:
> +SGX_IOCTL_ENCLAVE_INIT
> +
> +Initializes an enclave given by SIGSTRUCT and EINITTOKEN. Executes EINIT leaf
> +instruction that will check that the measurement matches the one SIGSTRUCT and
> +EINITTOKEN. EINITTOKEN is a data blob given by a special enclave called Launch
> +Enclave and it is signed with a CPU's Launch Key.
>

Having thought about this for ten minutes, I have the following thought:

I think that we should seriously consider not allowing user code to 
supply EINITTOKEN at all.  Here's why:

1. The nominal purpose of this thing is "launch control."  I think that 
the decision of whether to launch an enclave belongs in the kernel to 
the extent that the kernel has the ability to control this.

2. I think that launch control is actively insecure (assuming that the 
use case is what I think it is).  Since the kernel might have some 
interest in controlling whether an enclave can launch (I think this is 
entirely reasonable) and since that policy might reasonably be expressed 
in the form of a launch enclave, I think that the *kernel* should 
generate the actual EINITTOKEN object.  (I also reported, off-list, what 
I think is a significant security issue under some usage models that is 
mitigated if the user isn't allowed to supply their own EINITTOKEN of 
unknown provenance.)

3. On a CPU with unlocked IA32_SGXLEPUBKEYHASH, I think that the kernel 
should ship, *in the kernel image*, a binary corresponding to an 
open-source "launch anything" enclave.  The kernel should, when 
appropriate, use this thing to generate EINITTOKEN objects.  User code 
should *not* have to think about where this "launch anything" enclave 
comes from or whether it's the same on all kernels.  (I think that the 
best way to do this would be to try to build it deterministically using 
a well-known key pair.  This should be very easy to do.)  If someone 
wants to turn this feature off, let them do so via sysctl.

If someone wants to supply their own launch enclave, then let them 
either feed it to the kernel or enable some non-default privileged 
option to allow them to supply EINITTOKEN directly.

Actually implementing this is going to be interesting, because the 
kernel will have to call out to CPL 3 to do it.  It's not *that* hard, 
though, as the exiting kernel thread API should be more or less adequate.

--Andy