Re: [edk2-devel] [PATCH v1 0/8] Measured SEV boot with kernel/initrd/cmdline

["Laszlo Ersek" <lersek@redhat.com>]

Hi James,

thanks for the answer, one comment below:

On 06/02/21 20:10, James Bottomley wrote:
> On Tue, 2021-06-01 at 14:11 +0200, Laszlo Ersek wrote:
>> Ard,
>>
>> I'll have a specific question for you below; please feel free to jump
>> forward (search for your name). Thanks.
>>
>> Dov, my comments below:
>>
>> On 05/25/21 07:31, Dov Murik wrote:
>>> 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 declares a new page in MEMFD 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 a new QEMU patch which will
>>> be submitted soon.
>>>
>>> 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.
>>
>> Please catch the error in my reasoning below.
>>
>> The goal is for the guest firmware to ensure the authenticity
>> (integrity) of kernel, initrd, cmdline.
> 
> That's right
> 
>> This is not really different from a normal Secure Boot setup, where
>> the guest firmware verifies the kernel image (presented as a UEFI
>> application, i.e. with the UEFI stub). It is up to the kernel to
>> verify the integrity of the initrd.
> 
> Right, but this doesn't happen today (there's no initrd measure in the
> kernel), so the only current choice you have is to combine the kernel
> and initrd then sign the whole combination, which gives one signature
> for both.
> 
>>  The command line is not particularly verified (as far as I know?), 
> 
> Right, it's not, which means secure boot can't replace the
> verification, because the command line is an essential thing to measure
> given the things it can do to the booting kernel.
> 
>> but if that's a problem, it should be solved even for bare metal
>> Secure Boot use cases. (Because, if the "root" user is compromised on
>> a running Linux system, they can modify the kernel params for next
>> boot in the grub config.)
> 
> The usual statement to this is that secure boot doesn't need to do this
> because the commandline is a measured boot problem, so grub measures
> its entire execution to PCR 8.  However, then you have to grope around
> in the measurement log and verify it via a TPM quote, which is
> incredibly sophisticated stuff compared to taking a single measurement
> hash.  Plus, we have no secure TPM currently for virtual machines, so
> there's no measured boot measurement the guest owner can trust.
> 
>> The AmdSevX64 platform uses a unified firmware image (executable +
>> varstore are presented as one blob, no separate CODE and VARS). There
>> is one pflash chip, and the initial guest-owner-side measurement
>> covers the whole blob, including the varstore.
> 
> That's right, except being part of a single rom volume, the varstore is
> read only.  This is a deliberate design choice: the absence of SMM and
> the fact that a R/W interface wouldn't get measured properly and also
> because NV  config changes can be used to effect secret leakage means
> it needs to be immutable.
> 
> If it's mutable, you need to store it separately, protect it with
> something like SMM and be aware of how the measurement would change
> when the store was updated ... which is another thing that's not that
> easy because of the way flash operates on overwrite.
> 
> Finally there's the secure write problem: the DMA for the variable
> write would have to go through an unencrypted buffer (because that's
> the only way DMA works in confidential computing today).  For disks,
> including boot, we cope with this by using an encrypted disk, so we
> take the hardware protected page, encrypt it and place it in the clear
> DMA buffer for disk write, meaning confidentiality and integrity are
> preserved.  We'd have to add an encryption mechanism to pflash or
> something to match this process.
> 
>> This suggests that the guest owner could boot the unified firmware
>> image in a trusted guest environment first, and use UEFI-level tools
>> to enroll various SB certificates. Then this modified image would be
>> deployed every time to the untrusted cloud.
> 
> No, because of the read only nature of the NV store.  You could alter
> it from outside using EDK tools, but you can't update it from inside. 
> There's also the trust problem: in a current boot OVMF is provided by
> the cloud service provider (this could be changed by re-engineering the
> control plane, but it's the way it works today), we were thinking
> having a single trusted OVMF provided by an outside entity (i.e. Red
> Hat for the IBM cloud) would give higher confidence in the solution. 
> There may be cases where customers insist on rolling their own, but the
> hope is most of them would use a standard package.

Here I meant, by "booting the unified firmware image in a trusted guest
environment first", that the guest owner would do that on their own
premises, not on an untrusted cloud. In other words, in that initial
run, they wouldn't use any kind of memory encryption. "Trusted
environment" meant that the guest owner would trust their own
environment not to compromise their image in any way. And in this
environment, they could use EnrollDefaultKeys.efi or some other
guest-side tool to pre-enroll certificates.

In the actual production (cloud) environment, read-only access to the
varstore would be sufficient (with the certificates already existing in it).

But, anyway, the rest of your explanation is convincing; and I agree the
general idea of this series is fine. We just need to sort out the
technical details / solution structure, per the second half of my review.

Thanks!
Laszlo


> 
>> The AmdSevX64 platform could adopt a PlatformBootManagerLib instance
>> where the TryRunningQemuKernel() call is reinstated, backed by the
>> usual QemuLoadImageLib class APIs QemuLoadKernelImage() and
>> QemuStartKernelImage().
>>
>> edk2 offers two QemuLoadImageLib instances, GenericQemuLoadImageLib
>> and X86QemuLoadImageLib. The former strictly enforces SB
>> verification. That was in fact a *problem* for the traditional
>> OvmfPkg platforms; please refer to commit 82808b422617 ("Revert
>> "OvmfPkg: use generic QEMU image loader for secure boot enabled
>> ..."", 2020-06-16). But the same rigor seems just right here, for the
>> AmdSevX64 platform.
>>
>> Where I see a gap in all this myself -- and of course there could be
>> plenty other gaps that I just don't see -- is the varstore's
>> protection from the hypervisor, once the guest is up and running. Can
>> we discuss that perhaps?
> 
> I think it could possibly be done, but we're missing the encrypted
> pflash, the measurement computation, and the kernel measuring initrd
> and cmdline, all of which have to be controlled by the guest owner.
> 
> I did consider the secure boot variable paths, but given the complexity
> explosion and all the missing pieces it looked like a bit of a non
> starter compared to adding the hashes, which is fairly simple.
> 
>> If necessary, we could perhaps rework the AmdSevX64 platform to drop
>> the pflash-backed variable driver stack, and use in-RAM (memory-only)
>> variable emulation. Actual persistence / non-volatility of UEFI
>> variables may not really be relevant for the remotely attested
>> platform, but keeping all the variables in RAM would subject the
>> varstore to memory encryption / protection. And perhaps we could
>> integrate the enrollment of SB certificates into the *code* part of
>> the firmware, with gRT->SetVariable() calls. (Normally this would be
>> absolutely horrible, but for the remotely attested platform, anything
>> goes.)
>>
>> I simply dislike adding brand new code for a use case which at least
>> *appears* to significantly overlap with that of Secure Boot. Secure
>> Boot is about image verification, and it's rooted in tamper-resistant
>> storage of certificates and/or image hashes. If we can figure out
>> "tamper resistant" in the current context, we could perhaps reuse
>> much of the existent SB infrastructure.
> 
> The problem even with sharing code paths is secure boot it about
> authenticode hashes.  Most of what we're hashing for confidential
> computing isn't even authenticode, so we'd have to ream the SecurityPkg
> fairly comprehensively to get it to take either authenticode on the
> image path or linear hashes for the other elements.  The way grub does
> this is to have a separate verifier plugin for the measurements of any
> opened file, but even in the grub case that does a linear hash of the
> kernel rather than an authenticode one.
> 
> I'll let Dov answer the implementation details.
> 
> Regards,
> 
> James
> 
>