Re: [edk2-devel] [PATCH v5 00/11] Measured SEV boot with kernel/initrd/cmdline

["Yao, Jiewen" <jiewen.yao@intel.com>]

For OvmfPkg, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
For ArmVirtPkg, acked-by: Jiewen Yao <Jiewen.yao@intel.com>



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dov Murik
> Sent: Wednesday, July 28, 2021 3:07 AM
> To: devel@edk2.groups.io
> Cc: Dov Murik <dovmurik@linux.ibm.com>; Tobin Feldman-Fitzthum
> <tobin@linux.ibm.com>; Tobin Feldman-Fitzthum <tobin@ibm.com>; Jim
> Cadden <jcadden@ibm.com>; James Bottomley <jejb@linux.ibm.com>;
> Hubertus Franke <frankeh@us.ibm.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Ashish Kalra <ashish.kalra@amd.com>; Brijesh Singh <brijesh.singh@amd.com>;
> Erdem Aktas <erdemaktas@google.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Xu, Min M <min.m.xu@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Leif Lindholm <leif@nuviainc.com>; Sami
> Mujawar <sami.mujawar@arm.com>
> Subject: [edk2-devel] [PATCH v5 00/11] Measured SEV boot with
> kernel/initrd/cmdline
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3457
> 
> Booting with SEV prevented the loading of kernel, initrd, and kernel
> command-line via QEMU fw_cfg interface because they arrive from the VMM
> which is untrusted in SEV.
> 
> However, in some cases the kernel, initrd, and cmdline are not secret
> but should not be modified by the host.  In such a case, we want to
> verify inside the trusted VM that the kernel, initrd, and cmdline are
> indeed the ones expected by the Guest Owner, and only if that is the
> case go on and boot them up (removing the need for grub inside OVMF in
> that mode).
> 
> This patch series reserves an area in MEMFD (previously the last 1KB of
> the launch secret page) which will contain the hashes of these three
> blobs (kernel, initrd, cmdline), each under its own GUID entry.  This
> tables of hashes is populated by QEMU before launch, and encrypted as
> part of the initial VM memory; this makes sure these hashes are part of
> the SEV measurement (which has to be approved by the Guest Owner for
> secret injection, for example).  Note that populating the hashes table
> requires QEMU support [1].
> 
> OVMF parses the table of hashes populated by QEMU (patch 10), and as it
> reads the fw_cfg blobs from QEMU, it will verify each one against the
> expected hash.  This is all done inside the trusted VM context.  If all
> the hashes are correct, boot of the kernel is allowed to continue.
> 
> Any attempt by QEMU to modify the kernel, initrd, cmdline (including
> dropping one of them), or to modify the OVMF code that verifies those
> hashes, will cause the initial SEV measurement to change and therefore
> will be detectable by the Guest Owner during launch before secret
> injection.
> 
> Relevant part of OVMF serial log during boot with AmdSevX86 build and
> QEMU with -kernel/-initrd/-append:
> 
>   ...
>   BlobVerifierLibSevHashesConstructor: Found injected hashes table in secure
> location
>   Select Item: 0x17
>   Select Item: 0x8
>   FetchBlob: loading 7379328 bytes for "kernel"
>   Select Item: 0x18
>   Select Item: 0x11
>   VerifyBlob: Found GUID 4DE79437-ABD2-427F-B835-D5B172D2045B in table
>   VerifyBlob: Hash comparison succeeded for "kernel"
>   Select Item: 0xB
>   FetchBlob: loading 12483878 bytes for "initrd"
>   Select Item: 0x12
>   VerifyBlob: Found GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table
>   VerifyBlob: Hash comparison succeeded for "initrd"
>   Select Item: 0x14
>   FetchBlob: loading 86 bytes for "cmdline"
>   Select Item: 0x15
>   VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
>   VerifyBlob: Hash comparison succeeded for "cmdline"
>   ...
> 
> The patch series is organized as follows:
> 
> 1:     Simple comment fix in adjacent area in the code.
> 2:     Use GenericQemuLoadImageLib to gain one location for fw_cfg blob
>        fetching.
> 3:     Allow the (previously blocked) usage of -kernel in AmdSevX64.
> 4-7:   Add BlobVerifierLib with null implementation and use it in the correct
>        location in QemuKernelLoaderFsDxe.
> 8-9:   Reserve memory for hashes table, declare this area in the reset vector.
> 10-11: Add the secure implementation BlobVerifierLibSevHashes and use it in
>        AmdSevX64 builds.
> 
> [1] https://lore.kernel.org/qemu-devel/20210624102040.2015280-1-
> dovmurik@linux.ibm.com/
> 
> Code is at
> https://github.com/confidential-containers-demo/edk2/tree/sev-hashes-v5
> 
> v5 changes:
>  - rename the null implementation dir to OvmfPkg/Library/BlobVerifierLibNull
>    (note that I didn't remove the R-b tags on these patches; please let me
>    know if I should have acted otherwise)
>  - move the SevHashes implementation to
> OvmfPkg/AmdSev/BlobVerifierLibSevHashes
>  - BlobVerifierLib.h: fix #ifndef according to ECC warnings
>  - separate variable declaration and assignment in
>    BlobVerifierLibSevHashesConstructor (ECC warning)
> 
> v4: https://edk2.groups.io/g/devel/message/78075
> v4 changes:
>  - BlobVerifierSevHashes (patch 10): more comprehensive overflow tests
>    when parsing the SEV hashes table structure
> 
> v3: https://edk2.groups.io/g/devel/message/77955
> v3 changes:
>  - Rename to BlobVerifierLibNull, use decimal INF_VERSION, remove unused
>    DebugLib reference, fix doxygen comments, add missing IN attribute
>  - Rename to BlobVerifierLibSevHashes, use decimal INF_VERSION, fix
>    doxygen comments, add missing IN attribute,
>    calculate buffer hash only when the guid is found in hashes table
>  - SecretPei: use ALIGN_VALUE to round the hob size
>  - Coding style fixes
>  - Add missing 'Ref:' in patch 1 commit message
>  - Fix phrasing and typos in commit messages
>  - Remove Cc: Laszlo from series
> 
> v2: https://edk2.groups.io/g/devel/message/77505
> v2 changes:
>  - Use the last 1KB of the existing SEV launch secret page for hashes table
>    (instead of reserving a whole new MEMFD page).
>  - Build on top of commit cf203024745f ("OvmfPkg/GenericQemuLoadImageLib:
> Read
>    cmdline from QemuKernelLoaderFs", 2021-06-28) to have a single location in
>    which all of kernel/initrd/cmdline are fetched from QEMU.
>  - Use static linking of the two BlobVerifierLib implemenatations.
>  - Reorganize series.
> 
> v1: https://edk2.groups.io/g/devel/message/75567
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ashish Kalra <ashish.kalra@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> 
> Dov Murik (8):
>   OvmfPkg/AmdSev: use GenericQemuLoadImageLib in AmdSev builds
>   OvmfPkg: add library class BlobVerifierLib with null implementation
>   OvmfPkg: add BlobVerifierLibNull to DSC
>   ArmVirtPkg: add BlobVerifierLibNull to DSC
>   OvmfPkg/QemuKernelLoaderFsDxe: call VerifyBlob after fetch from fw_cfg
>   OvmfPkg/AmdSev/SecretPei: build hob for full page
>   OvmfPkg/AmdSev: add BlobVerifierLibSevHashes
>   OvmfPkg/AmdSev: Enforce hash verification of kernel blobs
> 
> James Bottomley (3):
>   OvmfPkg/AmdSev/SecretDxe: fix header comment to generic naming
>   OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
>   OvmfPkg/AmdSev: reserve MEMFD space for for firmware config hashes
> 
>  OvmfPkg/OvmfPkg.dec                                                                 |   9 +
>  ArmVirtPkg/ArmVirtQemu.dsc                                                          |   5 +-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                                                    |   5 +-
>  OvmfPkg/AmdSev/AmdSevX64.dsc                                                        |   9 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                                             |   5 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                                          |   5 +-
>  OvmfPkg/OvmfPkgX64.dsc                                                              |   5 +-
>  OvmfPkg/AmdSev/AmdSevX64.fdf                                                        |   5 +-
>  OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierLibSevHashes.inf
> |  37 ++++
>  OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf                         |  24
> +++
> 
> OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.
> inf           |   2 +
>  OvmfPkg/ResetVector/ResetVector.inf                                                 |   2 +
>  OvmfPkg/Include/Library/BlobVerifierLib.h                                           |  38 ++++
>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h                            |
> 11 ++
>  OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
> | 202 ++++++++++++++++++++
>  OvmfPkg/AmdSev/SecretDxe/SecretDxe.c                                                |   2 +-
>  OvmfPkg/AmdSev/SecretPei/SecretPei.c                                                |   3 +-
>  OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c                              |  33
> ++++
>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c                            |
> 5 +
>  OvmfPkg/Library/{PlatformBootManagerLib =>
> PlatformBootManagerLibGrub}/QemuKernel.c |   0
>  OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> |   9 +
>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm                                        |  20 ++
>  OvmfPkg/ResetVector/ResetVector.nasmb                                               |   2 +
>  23 files changed, 428 insertions(+), 10 deletions(-)
>  create mode 100644
> OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierLibSevHashes.inf
>  create mode 100644
> OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf
>  create mode 100644 OvmfPkg/Include/Library/BlobVerifierLib.h
>  create mode 100644
> OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
>  create mode 100644 OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c
>  copy OvmfPkg/Library/{PlatformBootManagerLib =>
> PlatformBootManagerLibGrub}/QemuKernel.c (100%)
> 
> --
> 2.25.1
> 
> 
> 
> 
>