[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