[libvirt PATCH v2 04/12] tools: support validating SEV direct kernel boot measurements
Daniel P. Berrangé
berrange at redhat.com
Wed Oct 26 12:24:21 UTC 2022
On Wed, Oct 26, 2022 at 02:57:33PM +0300, Dov Murik wrote:
> (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]).
Ah yes, I had known that, but completely forgot to address it
before sending.
I wonder if we should fully describe the SEV hashes table in
the docs/specs/sev-guest-firmware.rst file, to avoid others
making the same mistake.
> In any case, for direct boot kernel must be supplied. It doesn't make
> sense to pass initrd or cmdline without kernel.
Good point, will make that combination report an error.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list