Re: [libvirt PATCH v2 04/12] tools: support validating SEV direct kernel boot measurements

(sorry in advance for missing CCs, I tried to download the mbox from https://listman.redhat.com/archives/libvir-list/ but it doesn't include the To and Cc lines of the messages.) On 19/10/2022 13:17, berrange at redhat.com (Daniel P. Berrangé) wrote: > When doing direct kernel boot we need to include the kernel, initrd and > cmdline in the measurement. > > Signed-off-by: Daniel P. Berrang? <berrange at redhat.com> > --- > docs/manpages/virt-qemu-sev-validate.rst | 43 ++++++++++ > tools/virt-qemu-sev-validate | 102 ++++++++++++++++++++++- > 2 files changed, 144 insertions(+), 1 deletion(-) > > diff --git a/docs/manpages/virt-qemu-sev-validate.rst b/docs/manpages/virt-qemu-sev-validate.rst > index 2c02a27103..da804ae6a0 100644 > --- a/docs/manpages/virt-qemu-sev-validate.rst > +++ b/docs/manpages/virt-qemu-sev-validate.rst > @@ -102,6 +102,20 @@ initialize AMD SEV. For the validation to be trustworthy it important that the > firmware build used has no support for loading non-volatile variables from > NVRAM, even if NVRAM is expose to the guest. > > +``-k PATH``, ``--kernel=PATH`` > + > +Path to the kernel binary if doing direct kernel boot. > + > +``-r PATH``, ``--initrd=PATH`` > + > +Path to the initrd binary if doing direct kernel boot. Defaults to zero length > +content if omitted. > + > +``-e STRING``, ``--cmdline=STRING`` > + > +String containing any kernel command line parameters used during boot of the > +domain. Defaults to the empty string if omitted. > + > ``--tik PATH`` > > TIK file for domain. This file must be exactly 16 bytes in size and contains the > @@ -180,6 +194,22 @@ Validate the measurement of a SEV guest booting from disk: > --build-id 13 \ > --policy 3 > > +Validate the measurement of a SEV guest with direct kernel boot: > + > +:: > + > + # virt-dom-sev-validate \ > + --firmware OVMF.sev.fd \ > + --kernel vmlinuz-5.11.12 \ > + --initrd initramfs-5.11.12 \ > + --cmdline "root=/dev/vda1" \ > + --tk this-guest-tk.bin \ > + --measurement Zs2pf19ubFSafpZ2WKkwquXvACx9Wt/BV+eJwQ/taO8jhyIj/F8swFrybR1fZ2ID \ > + --api-major 0 \ > + --api-minor 24 \ > + --build-id 13 \ > + --policy 3 > + > Fetch from remote libvirt > ------------------------- > > @@ -200,6 +230,19 @@ Validate the measurement of a SEV guest booting from disk: > --tk this-guest-tk.bin \ > --domain fedora34x86_64 > > +Validate the measurement of a SEV guest with direct kernel boot: > + > +:: > + > + # virt-dom-sev-validate \ > + --connect qemu+ssh://root at some.remote.host/system \ > + --firmware OVMF.sev.fd \ > + --kernel vmlinuz-5.11.12 \ > + --initrd initramfs-5.11.12 \ > + --cmdline "root=/dev/vda1" \ > + --tk this-guest-tk.bin \ > + --domain fedora34x86_64 > + > Fetch from local libvirt > ------------------------ > > diff --git a/tools/virt-qemu-sev-validate b/tools/virt-qemu-sev-validate > index eb9485c6ed..062f9545f8 100755 > --- a/tools/virt-qemu-sev-validate > +++ b/tools/virt-qemu-sev-validate > @@ -34,6 +34,7 @@ > # firmware versions with known flaws. > # > > +import abc > import argparse > from base64 import b64decode > from hashlib import sha256 > @@ -43,6 +44,7 @@ import re > import socket > import sys > import traceback > +from uuid import UUID > > from lxml import etree > import libvirt > @@ -70,6 +72,85 @@ class InvalidStateException(Exception): > pass > > > +class GUIDTable(abc.ABC): > + GUID_LEN = 16 > + > + def __init__(self, guid, lenlen=2): > + self.guid = guid > + self.lenlen = lenlen > + > + @abc.abstractmethod > + def entries(self): > + pass > + > + def build_entry(self, guid, payload, lenlen): > + dummylen = int(0).to_bytes(lenlen, 'little') > + entry = bytearray(guid + dummylen + payload) > + > + lenle = len(entry).to_bytes(lenlen, 'little') > + entry[self.GUID_LEN:(self.GUID_LEN + lenlen)] = lenle > + > + return bytes(entry) > + > + def build(self): > + payload = self.entries() > + > + if len(payload) == 0: > + return bytes([]) > + > + dummylen = int(0).to_bytes(self.lenlen, 'little') > + table = bytearray(self.guid + dummylen + payload) > + > + guidlen = len(table).to_bytes(self.lenlen, 'little') > + table[self.GUID_LEN:(self.GUID_LEN + self.lenlen)] = guidlen > + > + pad = 16 - (len(table) % 16) > + table += bytes([0]) * pad > + > + log.debug("Table: %s", bytes(table).hex()) > + return bytes(table) > + > + > +class KernelTable(GUIDTable): > + > + TABLE_GUID = UUID('{9438d606-4f22-4cc9-b479-a793-d411fd21}').bytes_le > + KERNEL_GUID = UUID('{4de79437-abd2-427f-b835-d5b1-72d2045b}').bytes_le > + INITRD_GUID = UUID('{44baf731-3a2f-4bd7-9af1-41e2-9169781d}').bytes_le > + CMDLINE_GUID = UUID('{97d02dd8-bd20-4c94-aa78-e771-4d36ab2a}').bytes_le > + > + def __init__(self): > + super().__init__(guid=self.TABLE_GUID, > + lenlen=2) > + > + self.kernel = None > + self.initrd = None > + self.cmdline = None > + > + def load_kernel(self, path): > + with open(path, "rb") as fh: > + self.kernel = sha256(fh.read()).digest() > + log.debug("Kernel: %s", self.kernel.hex()) > + > + def load_initrd(self, path): > + with open(path, "rb") as fh: > + self.initrd = sha256(fh.read()).digest() > + log.debug("Initrd: %s", self.initrd.hex()) > + > + def load_cmdline(self, val): > + self.cmdline = sha256(val.encode("utf8") + bytes([0])).digest() > + log.debug("Cmdline: %s", self.cmdline.hex()) > + > + def entries(self): > + entries = bytes([]) > + if self.cmdline is not None: > + entries += self.build_entry(self.CMDLINE_GUID, self.cmdline, 2) > + if self.initrd is not None: > + entries += self.build_entry(self.INITRD_GUID, self.initrd, 2) I think this will not work correctly if cmdline and/or initrd are not supplied. The QEMU behaviour is to always include all three entries in the table. If initrd is not supplied then its entry should be sha256(""). If cmdline is not supplied then its entry should be sha256(bytes[0]).

In any case, for direct boot kernel must be supplied. It doesn't make sense to pass initrd or cmdline without kernel.