[edk2-devel] [PATCH v3 6/6] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table

[James Bottomley]

On Wed, 2020-12-09 at 12:02 +0000, Yao, Jiewen wrote:
> Hi James
> I am not sure if this solution is only for AMD SEV or it is a generic
> solution to "pass a secret to grub and let grub decrypt the disk".

Well, being open source, it's driven by the implementation, which is
AMD.  However, if you look at how it works, it's composed of a secret
discovery piece and then a packaging piece which is this part.  It's
certainly true that any discover mechanism could feed this packaging
piece, so I agree that the SecretDXE could pass up the secret from any
discovery mechanism not just SEV.

> If it is only for AMD SEV, please stop reading and ignore my comment
> below.
> 
> If it is designed to be a generic solution to pass a secret to grub
> and let grub decrypt the disk. I have some thought below:
> Intel TDX (
> https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> ) have similar feature - a TDX virtual firmware may do attestation
> and get a key from remote key server.
> We might use same architecture to pass the secrete to grub.
> Initially, we define an ACPI 'SVKL' table to pass the secrete in
> intel-tdx-guest-hypervisor-communication-interface (
> https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface.pdf
> ), section 4.4 storage volume key data.
> But it is also OK if you want to use UEFI configuration table.

>From the coding point of view it's definitely easier to do a UEFI
configuration table.  If I had to insert something inside ACPI I'd be
hijacking a lot more of the guts of OVMF to achieve it.

> If we need a common API for both AMD SEV and Intel TDX, then I
> recommend some enhancement for SevLaunchSecret.h.
> 1) The file name (SevLaunchSecret.h) should be generic, such as
> TrustedVmSecret, StorageVolumeKey, etc. It should not include 'SEV'.
> Otherwise, we have to define a new GUID for 'TDX'.
> 2) The GUID name (gSevLaunchSecretGuid, SEV_LAUNCH_SECRET_GUID)
> should be generic. Same reason above.
> 3) The data structure name (SEV_LAUNCH_SECRET_LOCATION) should be
> generic. Same reason above.
> 4) The data structure field (SEV_LAUNCH_SECRET_LOCATION.Base) should
> use UINTN or EFI_PHYSICAL_ADDRESS to support above 4GB memory
> location.

I agree with trying to make it more generic.  what about replacing 

SEV_LAUNCH with CONFIDENTIAL_COMPUTING
SevLaunch with ConfidentialComputing

And obviously making the area a UINT64.

> 5) The internal data structure of the secret is not defined. Is it
> raw binary? Or ASCII string password? Or DER format certificate? Or
> PEM format key? At least, we shall describe it in the header file.

Since OVMF doesn't interpret the area, I didn't describe it in this
patch, but if you look at how the grub piece works:

https://lists.gnu.org/archive/html/grub-devel/2020-11/msg00081.html

It's another GUIDed list, so all I've defined is a GUID that means
"this is a disk password".  You can define a guid for any other content
type.

> 6) The might be a chance that a key server need input multiple keys
> to a trusted VM. How we handle this? Do we expect multiple UEFI
> configuration tables and each table support one key? or one table to
> support multiple keys?

The keys would each have their own guid which would be pieced up by the
receiver.

> Would you please take a look at intel-tdx-guest-hypervisor-
> communication-interface, section 4.4 storage volume key data.
> We defined multiple key layout, key type and key format. Please let
> us know if you have any thought.

I really think the standard GUIDed form:

GUID|len|data

Works best because a GUID is big enough to define for any number of
uses and it also means we don't have to define key types or anything,
because all a new consumer has to do is define their data structure and
give it a guid.  The single uefi config table is passed through to all
the elements until it gets to one that recognizes the GUID.

James




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68553): https://edk2.groups.io/g/devel/message/68553
Mute This Topic: https://groups.io/mt/78617882/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-