[edk2-devel] [PATCH v3 6/6] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table
Yao, Jiewen
jiewen.yao at intel.com
Wed Dec 9 12:02:24 UTC 2020
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.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of James
> Bottomley
> Sent: Tuesday, December 1, 2020 4:28 AM
> To: devel at edk2.groups.io
> Cc: dovmurik at linux.vnet.ibm.com; Dov.Murik1 at il.ibm.com;
> ashish.kalra at amd.com; brijesh.singh at amd.com; tobin at ibm.com;
> david.kaplan at amd.com; jon.grimm at amd.com; thomas.lendacky at amd.com;
> jejb at linux.ibm.com; frankeh at us.ibm.com; Dr . David Alan Gilbert
> <dgilbert at redhat.com>; Laszlo Ersek <lersek at redhat.com>; Justen, Jordan L
> <jordan.l.justen at intel.com>; Ard Biesheuvel <ard.biesheuvel at arm.com>
> Subject: [edk2-devel] [PATCH v3 6/6] OvmfPkg/AmdSev: Expose the Sev
> Secret area using a configuration table
>
> Now that the secret area is protected by a boot time HOB, extract its
> location details into a configuration table referenced by
> gSevLaunchSecretGuid so the boot loader or OS can locate it before a
> call to ExitBootServices().
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3077
> Signed-off-by: James Bottomley <jejb at linux.ibm.com>
> Reviewed-by: Laszlo Ersek <lersek at redhat.com>
> ---
> OvmfPkg/OvmfPkg.dec | 1 +
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf | 37
> ++++++++++++++++++++++++++
> OvmfPkg/Include/Guid/SevLaunchSecret.h | 28 +++++++++++++++++++
> OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 26 ++++++++++++++++++
> 6 files changed, 94 insertions(+)
> create mode 100644 OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> create mode 100644 OvmfPkg/Include/Guid/SevLaunchSecret.h
> create mode 100644 OvmfPkg/AmdSev/SecretDxe/SecretDxe.c
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 7d27f8e16040..8a294116efaa 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -117,6 +117,7 @@ [Guids]
> gLinuxEfiInitrdMediaGuid = {0x5568e427, 0x68fc, 0x4f3d, {0xac,
> 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> gQemuKernelLoaderFsMediaGuid = {0x1428f772, 0xb64a, 0x441e,
> {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
> gGrubFileGuid = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62,
> 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
> + gSevLaunchSecretGuid = {0xadf956ad, 0xe98c, 0x484c, {0xae,
> 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
>
> [Ppis]
> # PPI whose presence in the PPI database signals that the TPM base
> address
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index e9c522bedad9..bb7697eb324b 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -778,6 +778,7 @@ [Components]
> gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> }
> !endif
> + OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> OvmfPkg/AmdSev/Grub/Grub.inf
> !if $(BUILD_SHELL) == TRUE
> ShellPkg/Application/Shell/Shell.inf {
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf
> b/OvmfPkg/AmdSev/AmdSevX64.fdf
> index b2656a1cf6fc..e8fd4b8c7b89 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf
> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
> @@ -269,6 +269,7 @@ [FV.DXEFV]
> !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> INF
> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellComma
> nd.inf
> !endif
> +INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> INF OvmfPkg/AmdSev/Grub/Grub.inf
> !if $(BUILD_SHELL) == TRUE
> INF ShellPkg/Application/Shell/Shell.inf
> diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> new file mode 100644
> index 000000000000..62ab00a3d382
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> @@ -0,0 +1,37 @@
> +## @file
> +# Sev Secret configuration Table installer
> +#
> +# Copyright (C) 2020 James Bottomley, IBM Corporation.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = SecretDxe
> + FILE_GUID = 6e2b9619-8810-4e9d-a177-d432bb9abeda
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = InitializeSecretDxe
> +
> +[Sources]
> + SecretDxe.c
> +
> +[Packages]
> + OvmfPkg/OvmfPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> +
> +[Guids]
> + gSevLaunchSecretGuid
> +
> +[FixedPcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
> + gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
> +
> +[Depex]
> + TRUE
> diff --git a/OvmfPkg/Include/Guid/SevLaunchSecret.h
> b/OvmfPkg/Include/Guid/SevLaunchSecret.h
> new file mode 100644
> index 000000000000..fa5f3830bc2b
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/SevLaunchSecret.h
> @@ -0,0 +1,28 @@
> + /** @file
> + UEFI Configuration Table for exposing the SEV Launch Secret location to
> UEFI
> + applications (boot loaders).
> +
> + Copyright (C) 2020 James Bottomley, IBM Corporation.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> + **/
> +
> +#ifndef SEV_LAUNCH_SECRET_H_
> +#define SEV_LAUNCH_SECRET_H_
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +#define SEV_LAUNCH_SECRET_GUID \
> + { 0xadf956ad, \
> + 0xe98c, \
> + 0x484c, \
> + { 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47 }, \
> + }
> +
> +typedef struct {
> + UINT32 Base;
> + UINT32 Size;
> +} SEV_LAUNCH_SECRET_LOCATION;
> +
> +extern EFI_GUID gSevLaunchSecretGuid;
> +
> +#endif // SEV_LAUNCH_SECRET_H_
> diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c
> b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c
> new file mode 100644
> index 000000000000..d8cc9b00946a
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c
> @@ -0,0 +1,26 @@
> +/** @file
> + SEV Secret configuration table constructor
> +
> + Copyright (C) 2020 James Bottomley, IBM Corporation.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +#include <PiDxe.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Guid/SevLaunchSecret.h>
> +
> +STATIC SEV_LAUNCH_SECRET_LOCATION mSecretDxeTable = {
> + FixedPcdGet32 (PcdSevLaunchSecretBase),
> + FixedPcdGet32 (PcdSevLaunchSecretSize),
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +InitializeSecretDxe(
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + return gBS->InstallConfigurationTable (&gSevLaunchSecretGuid,
> + &mSecretDxeTable
> + );
> +}
> --
> 2.26.2
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68546): https://edk2.groups.io/g/devel/message/68546
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list