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.

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

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.

Suggested-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>

---

Implementation note: I keep a buffer (sev->launch_memory) with the
content of all the memory added using sev_launch_update_data().  The
buffer is freed when the measurement is calculated (at which point no
more memory can be added by sev_launch_update_data()).  Ideally
it would be enough to keep just a SHA256 context struct, but I found no
such abstraction in crypto/hash.h -- only functions for calculating
finalized hashes of given buffers.
---
 qapi/misc-target.json    | 18 ++++++++++++++----
 target/i386/sev.c        | 40 ++++++++++++++++++++++++++++++++++++++--
 target/i386/trace-events |  2 +-
 3 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4bc45d2474..43e659a7d2 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -142,10 +142,15 @@
 #
 # @data: the measurement value encoded in base64
 #
+# @debug-launch-digest: SHA256 of launch memory (base64 encoded) (since: 6.3)
+#
 # Since: 2.12
 #
 ##
-{ 'struct': 'SevLaunchMeasureInfo', 'data': {'data': 'str'},
+{ 'struct': 'SevLaunchMeasureInfo',
+  'data': { 'data': 'str',
+            'debug-launch-digest': 'str'
+          },
   'if': 'TARGET_I386' }
 
 ##
@@ -160,7 +165,8 @@
 # Example:
 #
 # -> { "execute": "query-sev-launch-measure" }
-# <- { "return": { "data": "4l8LXeNlSPUDlXPJG5966/8%YZ" } }
+# <- { "return": { "data": "4l8LXeNlSPUDlXPJG5966/8%YZ",
+#                  "debug-launch-digest": "abcdef1234567890" } }
 #
 ##
 { 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
@@ -237,11 +243,14 @@
 #
 # @data:  guest attestation report (base64 encoded)
 #
+# @debug-launch-digest: SHA256 of launch memory (base64 encoded) (since: 6.3)
 #
 # Since: 6.1
 ##
 { 'struct': 'SevAttestationReport',
-  'data': { 'data': 'str'},
+  'data': { 'data': 'str',
+            'debug-launch-digest': 'str'
+          },
   'if': 'TARGET_I386' }
 
 ##
@@ -261,7 +270,8 @@
 #
 # -> { "execute" : "query-sev-attestation-report",
 #                  "arguments": { "mnonce": "aaaaaaa" } }
-# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
+# <- { "return" : { "data": "aaaaaaaabbbddddd",
+#                   "debug-launch-digest": "abcdef1234567890" } }
 #
 ##
 { 'command': 'query-sev-attestation-report',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 025ff7a6f8..1c8d8a9966 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -21,6 +21,7 @@
 #include "qapi/error.h"
 #include "qom/object_interfaces.h"
 #include "qemu/base64.h"
+#include "qemu/buffer.h"
 #include "qemu/module.h"
 #include "qemu/uuid.h"
 #include "crypto/hash.h"
@@ -73,6 +74,8 @@ struct SevGuestState {
     int sev_fd;
     SevState state;
     gchar *measurement;
+    gchar *debug_launch_digest;
+    Buffer launch_memory;
 
     uint32_t reset_cs;
     uint32_t reset_ip;
@@ -643,6 +646,7 @@ static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
 
     report = g_new0(SevAttestationReport, 1);
     report->data = g_base64_encode(data, input.len);
+    report->debug_launch_digest = g_strdup(sev->debug_launch_digest);
 
     trace_kvm_sev_attestation_report(mnonce, report->data);
 
@@ -709,6 +713,7 @@ sev_launch_start(SevGuestState *sev)
 
     sev_set_guest_state(sev, SEV_STATE_LAUNCH_UPDATE);
     sev->handle = start.handle;
+    buffer_init(&sev->launch_memory, "sev-guest-launch-memory");
     ret = 0;
 
 out:
@@ -727,6 +732,9 @@ sev_launch_update_data(SevGuestState *sev, uint8_t *addr, uint64_t len)
         return 1;
     }
 
+    buffer_reserve(&sev->launch_memory, len);
+    buffer_append(&sev->launch_memory, addr, len);
+
     update.uaddr = (__u64)(unsigned long)addr;
     update.len = len;
     trace_kvm_sev_launch_update_data(addr, len);
@@ -799,7 +807,17 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
 
     /* encode the measurement value and emit the event */
     sev->measurement = g_base64_encode(data, measurement.len);
-    trace_kvm_sev_launch_measurement(sev->measurement);
+    if (qcrypto_hash_base64(QCRYPTO_HASH_ALG_SHA256,
+                            (const char *)sev->launch_memory.buffer,
+                            sev->launch_memory.offset,
+                            &sev->debug_launch_digest,
+                            NULL) < 0) {
+        error_report("%s: failed hashing launch memory", __func__);
+        return;
+    }
+    buffer_free(&sev->launch_memory);
+
+    trace_kvm_sev_launch_measurement(sev->measurement, sev->debug_launch_digest);
 }
 
 static char *sev_get_launch_measurement(void)
@@ -812,9 +830,19 @@ static char *sev_get_launch_measurement(void)
     return NULL;
 }
 
+static char *sev_get_debug_launch_digest(void)
+{
+    if (sev_guest &&
+        sev_guest->state >= SEV_STATE_LAUNCH_SECRET) {
+        return g_strdup(sev_guest->debug_launch_digest);
+    }
+
+    return NULL;
+}
+
 SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 {
-    char *data;
+    char *data, *debug_launch_digest;
     SevLaunchMeasureInfo *info;
 
     data = sev_get_launch_measurement();
@@ -823,8 +851,16 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
         return NULL;
     }
 
+    debug_launch_digest = sev_get_debug_launch_digest();
+    if (!debug_launch_digest) {
+        error_setg(errp, "SEV launch digest is not available");
+        return NULL;
+    }
+
+
     info = g_malloc0(sizeof(*info));
     info->data = data;
+    info->debug_launch_digest = debug_launch_digest;
 
     return info;
 }
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 2cd8726eeb..6a24bb00f0 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -7,7 +7,7 @@ kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%zx"
 kvm_sev_change_state(const char *old, const char *new) "%s -> %s"
 kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p"
 kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIx64
-kvm_sev_launch_measurement(const char *value) "data %s"
+kvm_sev_launch_measurement(const char *value, const char *debug_launch_digest) "data %s debug_launch_digest %s"
 kvm_sev_launch_finish(void) ""
 kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
 kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s"
-- 
2.25.1

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

On 31/01/2022 13:44, Daniel P. Berrangé wrote:
> 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.
> 

Yes. Maybe rename it to gctx-ld ?

hmmm, except at the moment it doesn't include the VMSAs, because I don't
know an easy way to do it within QEMU :-( .  So the content of
debug-launch-digest is currently incorrect for SEV-ES guests.



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

The scenario we are encountering is that there's not one allowed
assignment to the parameters, but rather a more relaxed policy along the
lines of:

API_MAJOR.API_MINOR should be >= 12.34
BUILD should be >= 66
GCTX.POLICY should be 0x0 or 0x2
GCTX.LD can be one of these allowed OVMFs: {hash-a, hash-b, hash-c}


Having the values of API*, BUILD, POLICY from query-sev is very
comfortable for verifying they are in the allowed ranges.  But since the
Guest Owner doesn't have GCTX.LD, they have to compute the measurement
for each of the allowed OVMF images.  Once the values are known and
"allowed", then the HMAC must be computed to see that the signed
measurement does indeed match.

Note: I guess that adding the hashes of kernel/initrd/cmdline here might
help too (for direct boot with kernel-hashes=on), and maybe the hash of
OVMF itself (on its own).

More generally: within space/network constraints, give the Guest Owner
all the information it needs to compute the launch measurement.  There's
a problem with OVMF there (we don't want to send the whole 4 MB over the
QMP response, but given its hash we can't "extend" it to include the
kernel-hashes struct).



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

I think that adding the extra field is not that bad, even if it's useful
just for some cases.

I'll be glad to get rid of the launch_memory buffer in my implementation
and replace it with a SHA256-context variable (but found none in QEMU).
 Other than that I don't think the suggested addition is too bad (in
fact, I would add some more info like kernel hashes.).


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

Another point of reference is the session::verify() function in the sev
crate [1], but it doesn't deal with VMSAs and kernel-hashes either.

[1] https://github.com/enarx/sev/blob/main/src/session/mod.rs#L122


-Dov

On Mon, Jan 31, 2022 at 03:38:47PM +0200, Dov Murik wrote:
> 
> 
> On 31/01/2022 13:44, Daniel P. Berrangé wrote:
> > 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.
> > 
> 
> Yes. Maybe rename it to gctx-ld ?
> 
> hmmm, except at the moment it doesn't include the VMSAs, because I don't
> know an easy way to do it within QEMU :-( .  So the content of
> debug-launch-digest is currently incorrect for SEV-ES guests.

Yes, that comes back to the problem we've discussed previously about
how to determine the correct expected VMSA content which has no easy
answer. 


> The scenario we are encountering is that there's not one allowed
> assignment to the parameters, but rather a more relaxed policy along the
> lines of:
> 
> API_MAJOR.API_MINOR should be >= 12.34
> BUILD should be >= 66
> GCTX.POLICY should be 0x0 or 0x2
> GCTX.LD can be one of these allowed OVMFs: {hash-a, hash-b, hash-c}
> 
> 
> Having the values of API*, BUILD, POLICY from query-sev is very
> comfortable for verifying they are in the allowed ranges.  But since the
> Guest Owner doesn't have GCTX.LD, they have to compute the measurement
> for each of the allowed OVMF images.  Once the values are known and
> "allowed", then the HMAC must be computed to see that the signed
> measurement does indeed match.

Ok, so the usage scenario is that the platform owner is deciding 
which OVMF build in use, not the guest owner. That guest owner just
knows that it is an OVMF build from a set of builds published by the
platform owner. Good enough if you trust the cloud owner in general,
but want confidence that their compute host isn't compromised. Would
need  an independantly reproducible build, if you don't trust the
cloud owner at all.


Assuming we've got 5 possible OVMF builds, currently we would need
to calculate 5 HMACs over the inpuot data.

With this extra piece of info, we only need to calculate 1 HMAC.

So this is enabling a performance optimization, that could indeed
be used in a production deployment.  The HMAC ops are not exactly
performance intensive though until we get to the point of choosing
between a huge number of possible OVMFs.

If we can't get the VMSA info included, then the guest owner still
needs a local copy of every possible OVMF binary that is valid. IOW
this digest is essentially no more than a filename to identify which
OVMF binary to calc the HMAC over.

> Note: I guess that adding the hashes of kernel/initrd/cmdline here might
> help too (for direct boot with kernel-hashes=on), and maybe the hash of
> OVMF itself (on its own).

IOW, I think there's only two scenarios that make sense

1. The combined launch digest over firmware, kernel hashes
   and VMSA state.

2. Individual hashes for each of firmware, kernel hashes table and
   VMSA state

I think we should assume that anyone who has access to SEV-ES hardware
is likely to run in SEV-ES policy, not SEV policy. So without VMSA
state I think that (1) is of quite limited usefulness. (2) would
be nicer to allow optimization of picking which OVMF blob to use,
as you wouldn't need to figure out the cross-product of very valid
OVMF and every valid kernel hashes table - the latter probably isn't
even a finite bounded set.

> More generally: within space/network constraints, give the Guest Owner
> all the information it needs to compute the launch measurement.  There's
> a problem with OVMF there (we don't want to send the whole 4 MB over the
> QMP response, but given its hash we can't "extend" it to include the
> kernel-hashes struct).

Yeah its a shame we aren't just measuring the digest of each piece
of information in  GCTX.LD, instead of measuring the raw information
directly.


I wonder if we're thinking of this at the wrong level though. Does
it actually need to be QEMU providing this info to the guest owner ?

Guest owners aren't going to be interacting with QEMU / QMP directly,
nor are they likely to be interacting with libvirt directly. Their
way into the public cloud will be via some high level API. eg the
OpenStack Nova REST API, or the IBM Cloud API (whatever that may
be). This high level mgmt infra is likely what is deciding which
of the 'N' possible OVMF builds to pick for a given VM launch. It
could easily just expose the full OVMF data to the user via its
own API regardless of what query-sev does.

Similarly if the cloud is choosing which kernel, out of N possible
kernels to boot with, they could expose the raw kernel data somewhere
in their API - we don't neccessarily need to expose that from QEMU.

If we don't find a way to extract the VMSA data blob on the fly,
then the cloud owner will need to public VMSA data somewhere out
of band regardless.



> > 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.
> 
> I think that adding the extra field is not that bad, even if it's useful
> just for some cases.
> 
> I'll be glad to get rid of the launch_memory buffer in my implementation
> and replace it with a SHA256-context variable (but found none in QEMU).
>  Other than that I don't think the suggested addition is too bad (in
> fact, I would add some more info like kernel hashes.).

Given we can't actually emit a valid GCTX.LD for SEV, my preference
would be to just report the hashes of each piece of data that is a
input to the GCTX.LD. eg report 'firmware-digest', 'kernel-digest',
'initrd-digest', 'cmdline-digest'.

They can use firmware-digest as a identifier to locate the raw
firmware data blob.

They can use kernel-digest, initrd-digest, and cmdline-digest to
compare against their list of expected kernel/initrd binaries, if
the cloud owner controls them, and also to then build the kernl
hashes table.

They still have to figure out the VMSAs separately somehow, or
the cloud mgmt layer can use all of this info to expose the
raw data in their own higher level API.


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

On 1/31/22 9:26 AM, Daniel P. Berrangé wrote:

> 
> Ok, so the usage scenario is that the platform owner is deciding 
> which OVMF build in use, not the guest owner. That guest owner just
> knows that it is an OVMF build from a set of builds published by the
> platform owner. Good enough if you trust the cloud owner in general,
> but want confidence that their compute host isn't compromised. Would
> need  an independantly reproducible build, if you don't trust the
> cloud owner at all.
> 
> 
> Assuming we've got 5 possible OVMF builds, currently we would need
> to calculate 5 HMACs over the inpuot data.
> 
> With this extra piece of info, we only need to calculate 1 HMAC.
> 
> So this is enabling a performance optimization, that could indeed
> be used in a production deployment.  The HMAC ops are not exactly
> performance intensive though until we get to the point of choosing
> between a huge number of possible OVMFs.
> 
> If we can't get the VMSA info included, then the guest owner still
> needs a local copy of every possible OVMF binary that is valid. IOW
> this digest is essentially no more than a filename to identify which
> OVMF binary to calc the HMAC over.

For us the guest owner isn't monolithic. The guest owner will rely on a
Key Broker Service (KBS) running in some trusted domain to process
individual measurements and secret requests. The guest owner must
provision the KBS at the start of the day. I mention this because while
the guest owner will definitely need to build their own version of OVMF
and the components accounted for in the hashes table, ideally the
binaries won't need to be uploaded to the KBS.

If we forget about SEV-ES, the flow is pretty easy. At the start of the
day the guest owner builds the firmware, kernel, etc that they expect
the VM to be started with. They use a script to hash all the components,
formulate the hashes table, and ultimately produce the launch digest.
The guest owner uploads the launch digest to the KBS. If multiple
configurations are supported, they upload multiple digests.

We could do something very similar for SEV-ES, but there are some drawbacks.
> 
> IOW, I think there's only two scenarios that make sense
> 
> 1. The combined launch digest over firmware, kernel hashes
>    and VMSA state.
> 
> 2. Individual hashes for each of firmware, kernel hashes table and
>    VMSA state
> 
> I think we should assume that anyone who has access to SEV-ES hardware
> is likely to run in SEV-ES policy, not SEV policy. So without VMSA
> state I think that (1) is of quite limited usefulness. (2) would
> be nicer to allow optimization of picking which OVMF blob to use,
> as you wouldn't need to figure out the cross-product of very valid
> OVMF and every valid kernel hashes table - the latter probably isn't
> even a finite bounded set.
> 
I see something very similar. We could support -ES by doing the same
thing described above for SEV. The catch is that the guest owner would
have to calculate a firmware digest for every possible VMSA. That said,
if we assume that the VMSAs aren't going to change much, this just comes
down to a different VMSA and launch digest for each allowed cpu count.

Generating these digests could probably be automatically handled by the
same script that generates the secret table and calculates the launch
digest. No matter what we do, the guest owner will need to come up with
an expected VMSA. We would probably have to extend QEMU to include the
VMSA in the debug hash field for approach this to work.

>> More generally: within space/network constraints, give the Guest Owner
>> all the information it needs to compute the launch measurement.  There's
>> a problem with OVMF there (we don't want to send the whole 4 MB over the
>> QMP response, but given its hash we can't "extend" it to include the
>> kernel-hashes struct).
> 
> Yeah its a shame we aren't just measuring the digest of each piece
> of information in  GCTX.LD, instead of measuring the raw information
> directly.
Exactly. This is annoying because it complicates the other option, which
is to have the guest owner upload hashes of each individual component to
the KBS, which would generate the final hash when checking a launch
measurement. Unfortunately the launch digest is the hash of the firmware
binary itself, not the hash of the hash of the firmware. This means that
the guest owner would have to upload the firmware binary to the KBS. If
we took this approach the guest owner wouldn't have to generate a bunch
of launch digests, although they might need to provide the KBS with
complex instructions about which components can be used together.

Anyway, I think either of these options is fine. The first is more work
for the guest owner at configuration time and the second is more work
for the KBS at runtime. Like I said, the guest owner will need to know
an expected VMSA either way so maybe we should think of that as a
separate issue.
> 
> 
> I wonder if we're thinking of this at the wrong level though. Does
> it actually need to be QEMU providing this info to the guest owner ?
> 
The CSP could generate the hash that it expects to boot without the help
of QEMU, although it might be more complicated for SEV-ES. Even so, it
would be convenient if the CSP could ask QEMU/libvirt for the expected
hashes via the same interface that it gets the measurement. The CSP will
have to report the real launch measurement to the KBS. It would be handy
if the debug measurement were available at the same time with no extra
bookkeeping.

-Tobin

On 31/01/2022 16:26, Daniel P. Berrangé wrote:
[...]
> 
> IOW, I think there's only two scenarios that make sense
> 
> 1. The combined launch digest over firmware, kernel hashes
>    and VMSA state.
> 
> 2. Individual hashes for each of firmware, kernel hashes table and
>    VMSA state
> 


Just one more data point relevant to this discussion: in SNP the guest
asks the PSP for a signed attestation report (MSG_REPORT_REQ).  The
returned report (ATTESTATION_REPORT structure; see section 7.3 of [1])
includes a MEASUREMENT field which is the measurement calculated at
launch (it's a SHA384-based chain of hashes and not a hash of the entire
content as in SEV-ES; and GPAs are also included. Details in section
8.17).  The entire report is signed with the signature appearing in a
separate SIGNATURE field.



Mimicking that in QEMU for SEV-ES would be in my opinion closer to
option (1) above.

Again, the proposed patch here doesn't yet include the VMSAs in the
GCTX.LD and therefore is lacking.  Dave mentioned adding ioctl in KVM; I
think that Daniel once suggested adding a virtual file like
/sys/kernel/debug/kvm/617063-12/vcpu0/launch_vmsa with the 4KB VMSA content.


Note that AFAIK measured direct boot with -kernel is not yet supported
in SNP but we plan to add it (with similar hashes table) after the SNP
patches are accepted in OVMF.


[1] https://www.amd.com/system/files/TechDocs/56860.pdf


-Dov