[libvirt PATCH v2 09/12] tools: support generating SEV secret injection tables

Daniel P. Berrangé berrange at redhat.com
Wed Oct 26 13:16:10 UTC 2022


On Wed, Oct 26, 2022 at 08:52:24AM -0400, James Bottomley wrote:
> 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:
> > > 
> > > 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.

Ah thanks, I should have googled harder !

> > 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.

Assuming no distros have shipped with the not-yet-merged grub patches
applied to their builds, I'd suggest we can ignore backwards compatibility
concerns, and just focus on what will eventually be merged in grub
upstream....

> 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.

....which suggests we can just go for eliminating the strlen call
and document that the existing GUID is intended to be 8-bit clean
for binary keys, with no trailing NUL present.

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