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

[Laszlo Ersek]

Hi Jiewen,

On 12/09/20 13:02, 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".
> 
> 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.
> 
> 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.
> 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.
> 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?
> 
> 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.

These are several change requests. I do not disagree with them, but I
strongly propose we implement them separately.

James's current v3 series presents a state that has been tested and
reviewed. I'd like to commit it as-is. If for nothing else, then because
I'd like the edk2 git commit history to have a snapshot of the work at
this stage.

We have ample time until the next edk2 stable tag. Right after this v3
series is committed, you guys can start generalizing it (e.g., renaming
files and variables). Working from special case to general case is not
uncommon. The feature need not be ready as soon as it is committed; it
needs to be stable and externally compatible at the next stable tag.

If some changes from your list above would be incompatible with other
software (which is also in the making, to my understanding), then I
would request / propose that patches for those other projects be held
back, until the generalization of the edk2 patches reaches a certain
maturity.

Basically, I wouldn't like to do an incremental review on this series
that introduces the above-described *amount* of changes, from v3 to v4.
And I would like to perform a from-the-scratch review even less, on this
series. I believe your requests have merit, I'd just like to see patches
"on top" that implement them. We're right after the last stable tag,
this is the time for some incompatibility, until things settle.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68645): https://edk2.groups.io/g/devel/message/68645
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]
-=-=-=-=-=-=-=-=-=-=-=-