Re: [PATCH v5 14/24] hw/uefi: add var-service-json.c + qapi for NV vars.

[Markus Armbruster <armbru@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> > +# @data: variable value, encoded as hex string.
>> 
>> I understand this is a blob.  We commonly use base64 for that.  Why not
>> here?
>
> It's an existing format already supported by other tools.

Having a second format for blobs is unfortunate.

I can't judge whether the convenience of sharing the format with other
tools is worth the additional interface complexity and code.  It could
be if it avoids having to re-encode from hex to base64 in multiple
places.

Got to trust your judgement here.

>                                                            Guess I
> should add that to the preamble.

Commit messages are usually a good home for rationale.

>> > +# @digest: variable certificate digest.  Used to verify the signature
>> > +#     of updates for authenticated variables.
>> 
>> How to create and verify these digests will be obvious enough to users
>> of this interface?
>
> Well, no.  It's a somewhat complicated story ...
>
> UEFI has two kinds of authenticated variables.  First, the secure boot
> variables (PK, KEK, db, dbx).  These have hard-coded rules, the rule
> most relevant in practice is that signed update requests for the 'db'
> and 'dbx' variables are checked against the 'KEK' certificate database.
>
> For other authenticated variables UEFI essentially remembers the
> certificate when the variable is created, and any update / delete
> requests need a signature from the same certificate (simplified a
> bit to keep it short, the actual rules are more complicated, details
> are in the UEFI spec).
>
> This digest handles the "remembers the certificate" part.
>
> In practice the second kind of authenticated variables is rarely used.
> I have yet to see a piece of software actually using that in practice
> (other than the test case I have written).
>
>
> Also note that this is *not* part of the QMP interface.  The driver uses
> this to store the efi variable store in json format on disk (see
> var-service-json.c in this patch).
>
> Adding support for reading/writing uefi variables is something we might
> add to the monitor in the future.   Should that happen it is not clear
> whenever such an interface would actually use the raw 'UefiVariable'
> struct or if higher-level interfaces would be more useful.  For example
> a command to query whenever the guest has secure boot turned on.

I wonder how much of this, if anything, should be worked into the doc
comment.  You decide :)

With the typo I pointed out fixed:
Acked-by: Markus Armbruster <armbru@redhat.com>