Re: [intel-sgx-kernel-dev] [PATCH 08/10] kvm: vmx: add guest's IA32_SGXLEPUBKEYHASHn runtime switch support

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Wed, Jun 14, 2017 at 03:11:34PM +0000, Christopherson, Sean J wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> > On Tue, Jun 13, 2017 at 01:13:04PM -0700, Sean Christopherson wrote:
> >  
> > > The MSRs need to be written to run a LE in the guest, EINITTOKEN can't be
> > > used to EINIT an enclave that is requesting access to the EINITTOKENKEY,
> > > i.e. a LE. Preventing the guest from running its own LE is not an option,
> > > as the owner of the LE, e.g. guest kernel or userspace daemon, will likely
> > > disable SGX if its LE fails to run (including any ECALLS into the LE).
> > > Allowing a guest to run a LE doesn't mean the host can't ignore/discard the
> > > guest's EINITTOKENs, assuming the host traps EINIT.
> > 
> > [I started one week leave today but will peek MLs seldomly so except
> > some delay in my follow up responses]
> > 
> > Please, lets not use the term ECALL in these discussions. It's neither
> > hardware nor kernel specific concept. It's abstraction that exists only
> > in the Intel SDK. I have neither ECALLs nor OCALLs in my LE for example.
> > There are enough moving parts without such abstraction.
> > 
> > I'm looking at the section "EINIT - Initialize an Enclave for Execution"
> > from the SDM. I'm not seeing a branch in the pseudo code that checks for
> > ATTRIBUTES.EINITTOKENKEY.
> 
> (* if controlled ATTRIBUTES are set, SIGSTRUCT must be signed using an authorized key *)
> CONTROLLED_ATTRIBUTES <- 0000000000000020H;
> IF (((DS:RCX.ATTRIBUTES & CONTROLLED_ATTRIBUTES) != 0) and (TMP_MRSIGNER != IA32_SGXLEPUBKEYHASH))
>     RFLAG.ZF <- 1;
>     RAX <- SGX_INVALID_ATTRIBUTE;
>     GOTO EXIT;
> FI;
> 
> Bit 5, i.e. 20H, corresponds to the EINITTOKENKEY.  This is also covered in the
> text description under Intel SGX Launch Control Configuration - "The hash of the
> public key used to sign the SIGSTRUCT of the Launch Enclave must equal the value
> in the IA32_SGXLEPUBKEYHASH MSRs."

Thanks. I wonder by the naming is ambiguous (the value is exactly the
same as the value of ATTRIBUTES.EINITTOKENKEY but the name is different)
but there it is.

> > 39.1.4 states that "Only Launch Enclaves are allowed to launch without a
> > valid token." I'm not sure what I should deduce from that because that
> > statement is *incorrect*. If you control the MSRs, you can launch
> > anything you want to launch. I guess we should make a bug report of this
> > section as it's complete nonsense?
> 
> I wouldn't call it complete nonsense, there are far more egregious ambiguities
> in the SDM.  If you read the statement in the context of someone learning about
> SGX, it makes perfect sense: if it's not a launch enclave, it needs a token.
> Sure, rewording the statement to something like "Only enclaves whose public key
> hash equals the value in the IA32_SGXLEPUBKEYHASH MSRs are allowed to launch
> without a token." is technically more accurate, but I wouldn't describe the
> current wording as "complete nonsense".  

Agreed! That was a harsh overstatement.

I think that in this kind of stuff the accurancy still would make sense
when cryptography is involved.

I'll make updates to intel_sgx.rst. It's good to have it documented when
virtualization stuff is upstreamed.

/Jarkko