Re: [PATCH v22 02/24] x86/cpufeatures: x86/msr: Intel SGX Launch Control hardware bits

[Jarkko Sakkinen]

On Tue, Sep 24, 2019 at 05:52:32PM +0200, Borislav Petkov wrote:
> On Tue, Sep 03, 2019 at 05:26:33PM +0300, Jarkko Sakkinen wrote:
> > From: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
> > 
> > Add X86_FEATURE_SGX_LC, which informs whether or not the CPU supports SGX
> > Launch Control.
> > 
> > Add MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}, which when combined contain a
> > SHA256 hash of a 3072-bit RSA public key. SGX backed software packages, so
> > called enclaves, are always signed. All enclaves signed with the public key
> > are unconditionally allowed to initialize. [1]
> > 
> > Add FEATURE_CONTROL_SGX_LE_WR bit of the feature control MSR, which informs
> > whether the formentioned MSRs are writable or not. If the bit is off, the
> > public key MSRs are read-only for the OS.
> > 
> > If the MSRs are read-only, the platform must provide a launch enclave (LE).
> > LE can create cryptographic tokens for other enclaves that they can pass
> > together with their signature to the ENCLS(EINIT) opcode, which is used
> > to initialize enclaves.
> > 
> > Linux is unlikely to support the locked configuration because it takes away
> > the control of the launch decisions from the kernel.
> 
> Right, who has control over FEATURE_CONTROL_SGX_LE_WR? Can the
> kernel set it and put another hash in there or there will be locked
> configurations where setting that bit will trap?
> 
> I don't want to leave anything in the hands of the BIOS controlling
> whether the platform can set its own key because BIOS is known to f*ck
> it up almost every time. And so I'd like for us to be able to fix up
> things without depending on the mood of some OEM vendor's BIOS fixing
> desire.

The BIOS has control over the feature control bit because, as we know,
the feature control register is usually locked down before handover to
the OS.

The driver will support only the case where the bit is set i.e. that
it can freely write to the MSRs MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}.
It will refuse to initialize otherwise.

> > [1] Intel SDM: 38.1.4 Intel SGX Launch Control Configuration
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > Co-developed-by: Haim Cohen <haim.cohen@xxxxxxxxx>
> > Signed-off-by: Haim Cohen <haim.cohen@xxxxxxxxx>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> 
> This time checkpatch is right:
> 
> WARNING: Missing Signed-off-by: line by nominal patch author 'Kai Huang <kai.huang@xxxxxxxxxxxxxxx>'
> 
> And looking at the SOB chain, sounds like people need to make up their
> mind about authorship...

I'll make myself the sole author for this one as 98% of the effort in
this patch is really the commit message, which I rewrote for v22, and 2%
are the code changes (mechanical, peek at SDM).  This patch was squashed
from three patches, all like one line changes, and Kai was author of one
of them.

The next version will thus have only my SOB and author information will
be changed. I doubt anyone will complain if I do that.

/Jarkko