On Wed, 2022-10-26 at 10:59 +0100, Daniel P. Berrangé wrote:
> On Tue, Oct 25, 2022 at 07:38:43PM -0400, Cole Robinson wrote:
> > On 10/19/22 6:17 AM, Daniel P. Berrangé wrote:
> > > It is possible to build OVMF for SEV with an embedded Grub that
> > > can fetch LUKS disk secrets. This adds support for injecting
> > > secrets in the required format.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > > ---
> > > diff --git a/tools/virt-qemu-sev-validate b/tools/virt-qemu-sev-
> > > validate
> > > index 5ce5763d5b..2d15edb933 100755
> > > --- a/tools/virt-qemu-sev-validate
> > > +++ b/tools/virt-qemu-sev-validate
> > > @@ -36,16 +36,19 @@
> > >
> > > import abc
> > > import argparse
> > > -from base64 import b64decode
> > > +from base64 import b64decode, b64encode
> > > from hashlib import sha256
> > > import hmac
> > > import logging
> > > +import os
> > > import re
> > > import socket
> > > from struct import pack
> > > import sys
> > > import traceback
> > > from uuid import UUID
> > > +from cryptography.hazmat.primitives.ciphers import Cipher,
> > > algorithms, modes
> > > +
> > >
> > > from lxml import etree
> > > import libvirt
> > > @@ -573,7 +576,26 @@ class KernelTable(GUIDTable):
> > > return entries
> > >
> > >
> > > -class ConfidentialVM(object):
> > > +class SecretsTable(GUIDTable):
> > > +
> > > + TABLE_GUID = UUID('{1e74f542-71dd-4d66-963e-
> > > ef4287ff173b}').bytes_le
> > > + DISK_PW_GUID = UUID('{736869e5-84f0-4973-92ec-
> > > 06879ce3da0b}').bytes_le
> > > +
> > > + def __init__(self):
> > > + super().__init__(guid=self.TABLE_GUID,
> > > + lenlen=4)
> > > + self.disk_password = None
> > > +
> > > + def load_disk_password(self, path):
> > > + with open(path, 'rb') as fh:
> > > + self.disk_password = fh.read()
> > > +
> > > + def entries(self):
> > > + return self.build_entry(self.DISK_PW_GUID,
> > > + self.disk_password + bytes([0]),
> > > 4)
> > > +
> >
> > This bytes([0]) NUL byte ends up in the efi_secret /sys path.
> > Dropping
> > it doesn't seem to impact injecting the secret at all
>
> The specs/sev-guest-firmware.rst files does not specify anything in
> particular about the injected secret format. The rules for the format
> will vary according to the GUID used for the entry. In this case what
> I've called the DISK_PW_GUID is the same as the GUID used in James
> Bottomley's grub patch:
>
> https://lists.gnu.org/archive/html/grub-devel/2020-12/msg00260.html
That's way too old; the latest version is here:
https://lists.gnu.org/archive/html/grub-devel/2022-02/msg00064.html
The specific change was to update to use the new disk protector API.
>
> In particular note
>
> + /*
> + * the passphrase must be a zero terminated string because
> the
> + * password routines call grub_strlen () to find its size
> + */
>
> As written, the code just passes around a pointer to the data in the
> secret table, so it can't simply add a NUL terminator itself.
>
> See also James' python demo script for injection which adds a NUL:
>
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03691.html
Which makes all of this unnecessary. The disk protector API takes a
pointer and a length, so no longer needs to call grub_strlen on the
data. Thus if we don't need backwards compatibility (or older versions
of grub have the disk protector API backported), the trailing NULL can
be removed.
The patch here:
https://lists.gnu.org/archive/html/grub-devel/2022-02/msg00065.html
Still has the grub_strlen, but it's now in the patch series not grub
itself so it could be eliminated by expanding the get API to retrieve
the length as well as the secret.
James
