linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	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: Fri, 6 May 2016 09:24:47 -0700	[thread overview]
Message-ID: <CALCETrXZEtitPxeDOR9oMN-Ry=f5HZJVxy_7yXm3oEpryt+c2g@mail.gmail.com> (raw)
In-Reply-To: <20160506113507.GD24074@intel.com>

On Fri, May 6, 2016 at 4:35 AM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Thu, May 05, 2016 at 05:52:31PM -0700, Andy Lutomirski wrote:
>> On Thu, May 5, 2016 at 3:45 PM, Jarkko Sakkinen
>> <jarkko.sakkinen@linux.intel.com> wrote:
>> > On Mon, Apr 25, 2016 at 01:01:06PM -0700, Andy Lutomirski wrote:
>> >> 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.
>> >
>> > I'm not sure why you would want to do this especially when you can use
>> > your own key pair for launch control in future.
>>
>> Here are my thoughts.  There are a few ways the system (firmware, MSR,
>> and Intel policy) could be configured:
>>
>> +++ status quo +++
>>
>> Applications that ship enclaves come bundled with signatures and
>> possibly certificate chains.  They call into some SDK API to request
>> launch permission.  The SDK says "ok".  Then the application asks the
>> SDK to launch it.  It's entirely unclear to me how Intel distributes
>> the LE versions that make this work on Windows.  I'm guessing they're
>> bundled in the SDK, but for all I know, they're in the kernel.
>>
>> I think that Linux should possibly refuse to support this mode
>> upstream.  If Linux were to support it, then it might actually match
>> ISV-expected behavior better if the LE was bundled with the kernel --
>> after all, the ISV already has very little visibility into where the
>> LE comes from, and it seems more like a system policy thing.
>>
>> +++ current Skylakes with hypothetical Intel "launch anything" policy +++
>>
>> If the combination of Intel policy and firmware configuration were to
>> allow anything to launch, then the OS should either impose its own
>> policy or allow anything to launch.  I can easily imagine the
>> implementation of this involving a channel by which the kernel proves
>> to the LE that firmware/platform config is intended to allow "launch
>> anything".  In this case, I think this belongs either in the kernel or
>> in a privileged daemon, and putting it in the kernel is probably much
>> more straightforward.  In any event, applications shouldn't need to
>> worry about the nitty gritty details of where exactly their EINITTOKEN
>> comes from.
>>
>> +++ future chips with unlocked IA32_SGXLEPUBKEYHASH +++
>>
>> The firmware policy really is "launch anything" in this case, and it's
>> the kernel's job to restrict that further if it likes (by programming
>> the MSRs and/or filtering EINIT).  I think that the most natural way
>> to implement this would be for the kernel to implement whatever policy
>> it likes and to either ship an LE that accepts anything as part of the
>> kernel image that's signed by a key where everyone knows the *private*
>> key or to generate a random key pair at build time, build the enclave
>> along with the kernel, and include the public key and the LE in the
>> kernel image.  In either case, I don't think that apps should need to
>> ship this special LE -- the kernel should handle it.
>>
>> If admins want to impose additional launch policy, then by all means
>> Linux should offer that.  I'm just not at all convinced that fiddling
>> with the MSRs in new and complicated ways is the right way to do that
>> -- I think the kernel should implement more advanced policy by
>> filtering EINIT.
>>
>> Keep in mind that, if the kernel is compromised, then the attacker can
>> launch anything no matter what the kernel tries to do because the
>> attacker controls the MSRs.
>>
>> +++ future chips with IA32_SGXLEPUBKEYHASH set to a non-default value +++
>>
>> I don't see a valid use case for this other than snake oil.  One may
>> well exist, but, if so, it hasn't been explained to me.  If someone
>> comes up with one, then, by all means, we could support it in the
>> kernel.  Nonetheless, an SGX-aware app that is designed to work under
>> the status quo policy or in a "launch anything" regime should still
>> work if the special local policy intends for it to work, and I don't
>> think that app should need to possess a copy of the bespoke signed LE
>> in order to function.  IOW, to allow the same app to run here if
>> policy is okay with it, I think we *have* to delegate EINITTOKEN
>> generation to something other than the SGX-using app.
>>
>>
>>
>> My intuition is that either the Windows approach to this is going to
>> end up being a mess and very unpleasant for ISVs to work with or that
>> Windows will move to a configuration in which EINITTOKEN generation is
>> delegated to the kernel or to a privileged daemon that ships with the
>> OS.  I think that Linux should do that as well to avoid
>> unpleasantness.  If Linux ends up being easier to develop SGX
>> applications on than Windows, then that's means that Linux did
>> something right.
>>
>> Also, in the unlocked-MSR case, I think that the kernel *must*
>> implement EINIT filtering.  The kernel should not allow an untrusted
>> LE to run, because that LE might leak the raw EGETKEY output to a
>> malicious party, and the implications of that might not be so nice.
>> This is yet another argument in favor of having the kernel supply the
>> LE so that the kernel can trust the LE.  (Obviously, extracting raw
>> *debug* EGETKEY output is possible even on Windows on current systems,
>> but that raw output isn't particularly useful.)
>
> MSR does not work when it is unlocked.
>

Are you *sure* about that?

>> >
>> > This would degrade the security of the SGX to the level of the running
>> > kernel binary.
>>
>> How so?  SGX is supposed to be secure even under the assumption that
>> anyone can launch anything they want and even under the assumption
>> that the kernel is malicious.  If SGX fails at that, then it would be
>> odd to blame kernel policy for that.
>
> Maybe I understood this incorrectly somhow but IMHO launch control
> policy really should reside in an enclave because there it cannot be
> tampered by malicious code.

I have heard one one explanation of why we would care if a compromised
system were to allow an otherwise unauthorized enclave to launch: if
that enclave contained encrypted malware.  But there isn't any thing
at all the kernel can do to prevent this.  It's also very much unclear
how this would be interesting from a malware standpoint, since the
more "secure" (i.e. hard-to-reverse) malware scenarios all involve
attestation as well, and the attestation system has its own set of
security features.

--Andy

  reply	other threads:[~2016-05-06 16:25 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-25 17:34 [PATCH 0/6] Intel Secure Guard Extensions Jarkko Sakkinen
2016-04-25 17:34 ` [PATCH 1/6] x86: add SGX definition to cpufeature Jarkko Sakkinen
2016-04-25 19:31   ` Andy Lutomirski
2016-04-25 19:48     ` Andi Kleen
2016-04-25 17:34 ` [PATCH 2/6] x86, sgx: common macros and definitions Jarkko Sakkinen
2016-04-25 17:34 ` [PATCH 3/6] intel_sgx: driver for Intel Secure Guard eXtensions Jarkko Sakkinen
2016-04-25 17:55   ` Greg KH
2016-04-25 19:04     ` Jarkko Sakkinen
2016-04-27  6:49   ` Jethro Beekman
2016-04-27 12:40     ` Jarkko Sakkinen
2016-04-27 23:32       ` Jethro Beekman
2016-04-29 20:04         ` Jarkko Sakkinen
2016-04-29 22:22           ` Jethro Beekman
2016-05-09  5:29             ` Jarkko Sakkinen
2016-05-09  7:06               ` Jarkko Sakkinen
2016-04-25 17:34 ` [PATCH 4/6] intel_sgx: ptrace() support for the driver Jarkko Sakkinen
2016-04-25 17:34 ` [PATCH 5/6] intel_sgx: driver documentation Jarkko Sakkinen
2016-04-25 20:01   ` Andy Lutomirski
2016-05-05 22:45     ` Jarkko Sakkinen
2016-05-06  0:52       ` Andy Lutomirski
2016-05-06 11:35         ` Jarkko Sakkinen
2016-05-06 16:24           ` Andy Lutomirski [this message]
2016-04-25 17:34 ` [PATCH 6/6] intel_sgx: TODO file for the staging area Jarkko Sakkinen
2016-04-25 17:54   ` Greg KH
2016-04-25 18:56     ` Jarkko Sakkinen
2016-04-25 19:06   ` One Thousand Gnomes
2016-04-25 20:01   ` Andi Kleen
2016-04-26 11:23     ` Jarkko Sakkinen
2016-04-27  6:38       ` Jethro Beekman
2016-05-17  9:38         ` Jarkko Sakkinen
2016-04-25 17:53 ` [PATCH 0/6] Intel Secure Guard Extensions Greg KH
2016-04-25 19:03   ` Jarkko Sakkinen
2016-04-25 19:20     ` Andy Lutomirski
2016-04-26 19:00 ` Pavel Machek
2016-04-26 19:05   ` Andy Lutomirski
2016-04-26 19:41     ` Pavel Machek
2016-04-26 19:56       ` Andy Lutomirski
2016-04-26 20:11         ` Pavel Machek
2016-04-26 20:59           ` One Thousand Gnomes
2016-04-26 21:52             ` Pavel Machek
2016-04-26 22:35               ` Andy Lutomirski
2016-04-26 22:33           ` Andy Lutomirski
2016-04-27  7:32             ` Pavel Machek
2016-04-27  8:18             ` Ingo Molnar
2016-04-27 14:05               ` Andy Lutomirski
2016-05-06 11:23               ` Jarkko Sakkinen
2016-05-06 16:21                 ` Andy Lutomirski
2016-04-26 20:16         ` One Thousand Gnomes
2016-04-26 20:19       ` One Thousand Gnomes
2016-04-29 20:17   ` Jarkko Sakkinen
2016-05-01  9:40     ` Pavel Machek
2016-05-02 15:37     ` Austin S. Hemmelgarn
2016-05-03  9:06       ` Dr. Greg Wettstein
2016-05-03 15:38         ` Pavel Machek
2016-05-04  9:04           ` Dr. Greg Wettstein
2016-05-04 11:07             ` Pavel Machek
2016-05-06 11:39         ` Jarkko Sakkinen
2016-05-06 11:54           ` Thomas Gleixner
2016-05-09  5:38             ` Jarkko Sakkinen
2016-05-09  6:27               ` Thomas Gleixner
2016-05-09  9:20                 ` Jarkko Sakkinen
2016-05-12  8:50                 ` Dr. Greg Wettstein
2016-05-09  7:04               ` Greg KH
2016-05-09  9:13                 ` Jarkko Sakkinen
2016-05-08  9:58           ` Dr. Greg Wettstein
2016-05-09  1:32             ` Andy Lutomirski
2016-05-13  9:42               ` Dr. Greg Wettstein
2016-05-13 14:09                 ` Andy Lutomirski
2016-05-05 22:52       ` Jarkko Sakkinen
2016-05-06  7:14         ` Pavel Machek
2016-05-06 11:27           ` Jarkko Sakkinen
2016-04-29 22:08   ` Jarkko Sakkinen

Reply instructions:

You may reply publicly 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='CALCETrXZEtitPxeDOR9oMN-Ry=f5HZJVxy_7yXm3oEpryt+c2g@mail.gmail.com' \
    --to=luto@amacapital.net \
    --cc=bpetkov@suse.de \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).