Re: [PATCH] qapi, i386/sev: Add debug-launch-digest to launch-measure response

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Jan 31, 2022 at 11:15:39AM +0000, Dov Murik wrote:
> Currently the responses of QMP commands query-sev-launch-measure and
> query-sev-attestation-report return just the signed measurement. In
> order to validate it, the Guest Owner must know the exact guest launch
> digest, besides other host and guest properties which are included in
> the measurement.
> 
> The other host and guest details (SEV API major, SEV API minor, SEV
> build, and guest policy) are all available via query-sev QMP command.
> However, the launch digest is not available.  This makes checking the
> measurement harder for the Guest Owner, as it has to iterate through all
> allowed launch digests and compute the expected measurement.

So more specifically to validate the measurement, the guest owner is
currently calculating:

   expected_measurement = HMAC(0x04 || API_MAJOR || API_MINOR || BUILD ||
                               GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)

where GCTX.LD is

    SHA256(FIRMWARE-CODE || KERNEL-HASHES-TABLE || VMSA-FOREACH-VCPU)

and comparing expected_measurement to the actual measurement from
query-sev-launch-measure.

> Add a new field debug-launch-digest to the response of
> query-sev-launch-measure and query-sev-attestation-report which includes
> the guest launch digest, which is the SHA256 of all initial memory added
> to the guest via sev_launch_update_data().

So this new 'debug-launch-digest' field is returning GCTX.LD value
above.

> Note that the value of debug-launch-digest should not be used for
> verifying the measurement, because it is not signed.  Hence the choice
> of the 'debug-' prefix for the field's name.

The earlier paragraph talks about making it easier for the guest
owner to verify the measurement, but here is saying this new field
should not be used to verify the measurement.

So I'm confused as to what is the benefit of returning this info ?

Due to the lack of guarantees about this data, it can't be used
for a real production use case. AFAIK it can only be useful if
debugging your code logic used for validating measurwements.
Am I missing something about the motivation here ?


If the guest owner is comparing expected and actual measurements
and they get a mis-match, all they'll see is two big hex strings
representing the HMAC value, and they'll have to work backwards
to figure out whether one of their expected inputs had a mistake,
or their algorithm was buggy.

If the guest owner is comparing the expectd and actual launch
digest and they get a mis-match, again they'll just huave two
big hex strings representing the SHA256 value, and they'll have
to work backwards to figure out whether one of their expected
inputs had a mistake, or their algorithm was buggy.

By having this 'debug-launch-digest' field, you can slightly
more quickly determine whether your mistake lies in your impl
of the HMAC alg, or API_MAJOR || API_MINOR || BUILD || GCTX.POLICY
values, vs a mistake in your calc of the debug-launch-digest
field. Basically it gives you one step in bisecting the mistake
location.

Is that really compelling enough to justify adding this extra
info to the QAPI commands ? IME of writing code to verify the SEV
measurement, there were many little ways I screwed up at first
and having this field would not have made a significant difference
to my debugging efforts.

What was/would have been useful was having a known good reference
implementation of the algorithm which dumped out all the key
values for the different steps of process. I used James Bottomley's
python script as my reference and hacked it to dump out key values
so I could see what step I went wrong in. I was still working blind
for doing the SEV-ES VMSA and kernel hashes table work.

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