From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web09.674.1626455509264432747 for ; Fri, 16 Jul 2021 10:11:49 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=XnHu3BA9; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 5E661613FB for ; Fri, 16 Jul 2021 17:11:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1626455508; bh=/+xgm28shbDm97gTMH7cRVjvmfzDdVbD0iLDgniNVV8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=XnHu3BA9rPpdh1MVoIQAdO5XUlrIO2EOfhwe3lF+2hvqSu64Gfyg/ovnR9cts9vFD WaXNAegOrfpkf9x40TIhr1UnX6C6Gf8jJZW+huVj/NJWtB3z0eNNWcldUw3xwuTwLI jhygOMtn2koaJeiTteZHLJdwN7TIlgXhb454W8cNsWFfeZtjMF9Fyto7Uwh19FtAw2 os+HlZrM0oYVXBOcD3i7hFmPygZvdzWxn4BI0GC5MIzwYh0r2O2J6kHWpbU18h7eK2 ofvNnAJ3AjP1MLO2NfYRhh/agk7VTpPXd2itEhJvO1mSW5AnrsPg5kD+XE/NqF2zXo gdzlMWPrdtNWw== Received: by mail-oi1-f178.google.com with SMTP id y66so2244037oie.7 for ; Fri, 16 Jul 2021 10:11:48 -0700 (PDT) X-Gm-Message-State: AOAM533UNNSn0G70UT08j6CSDFVKLQeO9ft5nrOmmtsA1h70Tek/5cHb iT146EoRdJXjrTkMpuYZzXkivnJ4Byah0dwDYQ4= X-Google-Smtp-Source: ABdhPJxo+D7uO/g3BDv0YenFDApkG+ci6g5QFqQvh7LqpxJ0XbWAMMN5+TP9wgAaSt7Yi5hhDC7KBac/Mjn/uNo8P1Y= X-Received: by 2002:aca:5a04:: with SMTP id o4mr8616155oib.33.1626455507663; Fri, 16 Jul 2021 10:11:47 -0700 (PDT) MIME-Version: 1.0 References: <20210706085501.1260662-1-dovmurik@linux.ibm.com> In-Reply-To: <20210706085501.1260662-1-dovmurik@linux.ibm.com> From: "Ard Biesheuvel" Date: Fri, 16 Jul 2021 19:11:36 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v2 00/11] Measured SEV boot with kernel/initrd/cmdline To: edk2-devel-groups-io , Dov Murik Cc: Tobin Feldman-Fitzthum , Tobin Feldman-Fitzthum , Jim Cadden , James Bottomley , Hubertus Franke , Laszlo Ersek , Ard Biesheuvel , Jordan Justen , Ashish Kalra , Brijesh Singh , Erdem Aktas , Jiewen Yao , Min Xu , Tom Lendacky , Leif Lindholm , Sami Mujawar Content-Type: text/plain; charset="UTF-8" On Tue, 6 Jul 2021 at 10:55, Dov Murik wrote: > > 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 > theses hashes are part of the SEV measurement (which has to be approved > by the Guest Owner for secret injection, for example). Note that this > requires QEMU support [1]. > > OVMF parses the table of hashes populated by QEMU (patch 5), and as it > reads the fw_cfg blobs from QEMU, it will verify each one against the > expected hash (kernel and initrd verifiers are introduced in patch 6, > and command-line verifier is introduced in patches 7+8). 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: > > ... > SevHashesBlobVerifierLibConstructor: 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 entry '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 entry '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 entry '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 SevHashesBlobVerifierLib 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-v2 > > 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: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Ashish Kalra > Cc: Brijesh Singh > Cc: Erdem Aktas > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Min Xu > Cc: Tom Lendacky > Cc: Leif Lindholm > Cc: Sami Mujawar > Anyone on the cc list care to review this? > Dov Murik (8): > OvmfPkg/AmdSev: use GenericQemuLoadImageLib in AmdSev builds > OvmfPkg: add library class BlobVerifierLib with null implementation > OvmfPkg: add NullBlobVerifierLib to DSC > ArmVirtPkg: add NullBlobVerifierLib to DSC > OvmfPkg/QemuKernelLoaderFsDxe: call VerifyBlob after fetch from fw_cfg > OvmfPkg/AmdSev/SecretPei: build hob for full page > OvmfPkg: add SevHashesBlobVerifierLib > 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/Library/BlobVerifierLib/NullBlobVerifierLib.inf | 27 +++ > OvmfPkg/Library/BlobVerifierLib/SevHashesBlobVerifierLib.inf | 36 ++++ > 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/SecretDxe/SecretDxe.c | 2 +- > OvmfPkg/AmdSev/SecretPei/SecretPei.c | 9 +- > OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c | 34 ++++ > OvmfPkg/Library/BlobVerifierLib/SevHashesBlobVerifier.c | 199 ++++++++++++++++++++ > 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, 434 insertions(+), 10 deletions(-) > create mode 100644 OvmfPkg/Library/BlobVerifierLib/NullBlobVerifierLib.inf > create mode 100644 OvmfPkg/Library/BlobVerifierLib/SevHashesBlobVerifierLib.inf > create mode 100644 OvmfPkg/Include/Library/BlobVerifierLib.h > create mode 100644 OvmfPkg/Library/BlobVerifierLib/NullBlobVerifier.c > create mode 100644 OvmfPkg/Library/BlobVerifierLib/SevHashesBlobVerifier.c > copy OvmfPkg/Library/{PlatformBootManagerLib => PlatformBootManagerLibGrub}/QemuKernel.c (100%) > > -- > 2.25.1 > > > > > >