[edk2-devel] [PATCH v1 0/8] Measured SEV boot with kernel/initrd/cmdline
Ard Biesheuvel
ardb at kernel.org
Tue Jun 1 13:20:13 UTC 2021
On Tue, 1 Jun 2021 at 14:12, Laszlo Ersek <lersek at redhat.com> wrote:
>
...
> - A major complication for hashing all three of: kernel, initrd,
> cmdline, is that the *fetching* of this triplet is split between two
> places. (Well, it is split between *three* places in fact, but I'm going
> to ignore LinuxInitrdDynamicShellCommand for now, because the AmdSevX64
> platform sets BUILD_SHELL to FALSE for production.)
>
> The kernel and the initrd are fetched in QemuKernelLoaderFsDxe, but the
> command line is fetched in (both) QemuLoadImageLib instances. This
> requires that all these modules be littered with hashing as well, which
> I find *really bad*. Even if we factor out the actual logic, I strongly
> dislike having *just hooks* for hashing in multiple modules.
>
> Now, please refer to efc52d67e157 ("OvmfPkg/QemuKernelLoaderFsDxe: don't
> expose kernel command line", 2020-03-05). If we first
>
> (a) reverted that commit, and
>
> (b) modified *both* QemuLoadImageLib instances, to load the kernel
> command line from the *synthetic filesystem* (rather than directly from
> fw_cfg),
>
> then we could centralize the hashing to just QemuKernelLoaderFsDxe.
>
> Ard -- what's your thought on this?
>
I don't have any problems with that - I take it this means we can drop
the QemuFwCfgLib dependency from GenericQemuLoadImageLib altogether,
right?
>
> And then, we could eliminate the dynamic callback registration, plus the
> separate SevFwCfgVerifier, SevHashFinderLib, and SevQemuLoadImageLib stuff.
>
> We'd only need one new lib class, with *statically linked* hooks for
> QemuKernelLoaderFsDxe, and two instances of this new class, a Null one,
> and an actual (SEV hash verifier) one. The latter instance would locate
> the hash values, calculate the fresh hashes, and perform the
> comparisons. Only the AmdSevX64 platform would use the non-Null instance
> of this library class.
>
> (NB QemuKernelLoaderFsDxe is used by some ArmVirtPkg platforms, so
> resolutions to the Null instance would be required there too.)
>
This sounds like a good approach to me.
>
> >
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> > Cc: Jordan Justen <jordan.l.justen at intel.com>
> > Cc: Ashish Kalra <ashish.kalra at amd.com>
> > Cc: Brijesh Singh <brijesh.singh at amd.com>
> > Cc: Erdem Aktas <erdemaktas at google.com>
> > Cc: James Bottomley <jejb at linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > Cc: Min Xu <min.m.xu at intel.com>
> > Cc: Tom Lendacky <thomas.lendacky at amd.com>
> >
> > James Bottomley (8):
> > OvmfPkg/AmdSev/SecretDxe: fix header comment to generic naming
> > OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
> > OvmfPkg/AmdSev: add a page to the MEMFD for firmware config hashes
> > OvmfPkg/QemuKernelLoaderFsDxe: Add ability to verify loaded items
> > OvmfPkg/AmdSev: Add library to find encrypted hashes for the FwCfg
> > device
> > OvmfPkg/AmdSev: Add firmware file plugin to verifier
> > OvmfPkg: GenericQemuLoadImageLib: Allow verifying fw_cfg command line
> > OvmfPkg/AmdSev: add SevQemuLoadImageLib
> >
> > OvmfPkg/OvmfPkg.dec | 10 ++
> > OvmfPkg/AmdSev/AmdSevX64.dsc | 9 +-
> > OvmfPkg/AmdSev/AmdSevX64.fdf | 3 +
> > OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.inf | 30 +++++
> > OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.inf | 34 ++++++
> > OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.inf | 30 +++++
> > OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf | 2 +
> > OvmfPkg/ResetVector/ResetVector.inf | 2 +
> > OvmfPkg/AmdSev/Include/Library/SevHashFinderLib.h | 47 ++++++++
> > OvmfPkg/Include/Library/QemuFwCfgLib.h | 35 ++++++
> > OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h | 11 ++
> > OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.c | 60 ++++++++++
> > OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.c | 126 ++++++++++++++++++++
> > OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.c | 52 ++++++++
> > OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 2 +-
> > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 29 +++++
> > OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c | 5 +
> > OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c | 50 ++++++++
> > OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 31 +++++
> > OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 20 ++++
> > OvmfPkg/ResetVector/ResetVector.nasmb | 2 +
> > 21 files changed, 587 insertions(+), 3 deletions(-)
> > create mode 100644 OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.inf
> > create mode 100644 OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.inf
> > create mode 100644 OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.inf
> > create mode 100644 OvmfPkg/AmdSev/Include/Library/SevHashFinderLib.h
> > create mode 100644 OvmfPkg/AmdSev/Library/SevFwCfgVerifier/SevFwCfgVerifier.c
> > create mode 100644 OvmfPkg/AmdSev/Library/SevHashFinderLib/SevHashFinderLib.c
> > create mode 100644 OvmfPkg/AmdSev/Library/SevQemuLoadImageLib/SevQemuLoadImageLib.c
> > create mode 100644 OvmfPkg/Library/PlatformBootManagerLibGrub/QemuKernel.c
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75908): https://edk2.groups.io/g/devel/message/75908
Mute This Topic: https://groups.io/mt/83074450/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