[PATCH] qemu: Report sev measurement value and nonce explicitly

Cole Robinson posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/fc3aa1f8b77d45e06c41bbc5a95e18d9bbd80a3f.1665950774.git.crobinso@redhat.com
include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
2 files changed, 45 insertions(+)
[PATCH] qemu: Report sev measurement value and nonce explicitly
Posted by Cole Robinson 1 year, 6 months ago
The value returned by qemu's query-sev-launch-measure comes
straight from the LAUNCH_MEASURE SEV firmware command. It's two
values packed together: first 32 bytes is the launch measurement,
last 16 bytes is the nonce.

This combined value is really just an artifact of the return value
of the firmware command, it has no direct usage. Users want the two
individual values. But because qemu and libvirt do not separate them
apart, every app that wants to process this value will have to do
it manually.

This performs the split for the user, and delivers the values in two
new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
 src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8357aea797..55723ba150 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -6317,6 +6317,28 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
  */
 # define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS "sev-secret-set-address"
 
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE:
+ *
+ * Macro represents the measurement value of the SEV guest,
+ * extracted from the compound VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT value,
+ * as VIR_TYPED_PARAM_STRING.
+ *
+ * Since: 8.9.0
+ */
+# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE "sev-measurement-value"
+
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE:
+ *
+ * Macro represents the measurement nonce of the SEV guest,
+ * extracted from the compound VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT value,
+ * as VIR_TYPED_PARAM_STRING.
+ *
+ * Since: 8.9.0
+ */
+# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE "sev-measurement-nonce"
+
 int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
                                    virTypedParameterPtr *params,
                                    int *nparams,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 40d23b5723..590e8f3fab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19951,10 +19951,14 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
     int ret = -1;
     int rv;
     g_autofree char *tmp = NULL;
+    g_autofree char *measurement = NULL;
+    g_autofree char *measurement_val = NULL;
+    g_autofree char *nonce = NULL;
     unsigned int apiMajor = 0;
     unsigned int apiMinor = 0;
     unsigned int buildID = 0;
     unsigned int policy = 0;
+    size_t measurement_size = 0;
     int maxpar = 0;
 
     virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -19982,6 +19986,17 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
     if (rv < 0)
         goto endjob;
 
+    measurement = (char *) g_base64_decode(tmp, &measurement_size);
+    if (measurement_size != 48) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected SEV measurement size %zu, expected 48"),
+                       measurement_size);
+        goto endjob;
+    }
+
+    measurement_val = g_base64_encode((unsigned char *) measurement, 32);
+    nonce = g_base64_encode((unsigned char *) measurement + 32, 16);
+
     if (virTypedParamsAddString(params, nparams, &maxpar,
                                 VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
                                 tmp) < 0)
@@ -20002,6 +20017,14 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
                               VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY,
                               policy) < 0)
         goto endjob;
+    if (virTypedParamsAddString(params, nparams, &maxpar,
+                                VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_VALUE,
+                                measurement_val) < 0)
+        goto endjob;
+    if (virTypedParamsAddString(params, nparams, &maxpar,
+                                VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT_NONCE,
+                                nonce) < 0)
+        goto endjob;
 
     ret = 0;
 
-- 
2.37.3
Re: [PATCH] qemu: Report sev measurement value and nonce explicitly
Posted by Michal Prívozník 1 year, 6 months ago
On 10/16/22 22:06, Cole Robinson wrote:
> The value returned by qemu's query-sev-launch-measure comes
> straight from the LAUNCH_MEASURE SEV firmware command. It's two
> values packed together: first 32 bytes is the launch measurement,
> last 16 bytes is the nonce.
> 
> This combined value is really just an artifact of the return value
> of the firmware command, it has no direct usage. Users want the two
> individual values. But because qemu and libvirt do not separate them
> apart, every app that wants to process this value will have to do
> it manually.
> 
> This performs the split for the user, and delivers the values in two
> new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
>  include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
>  src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
Re: [PATCH] qemu: Report sev measurement value and nonce explicitly
Posted by Cole Robinson 1 year, 6 months ago
On 10/17/22 3:42 AM, Michal Prívozník wrote:
> On 10/16/22 22:06, Cole Robinson wrote:
>> The value returned by qemu's query-sev-launch-measure comes
>> straight from the LAUNCH_MEASURE SEV firmware command. It's two
>> values packed together: first 32 bytes is the launch measurement,
>> last 16 bytes is the nonce.
>>
>> This combined value is really just an artifact of the return value
>> of the firmware command, it has no direct usage. Users want the two
>> individual values. But because qemu and libvirt do not separate them
>> apart, every app that wants to process this value will have to do
>> it manually.
>>
>> This performs the split for the user, and delivers the values in two
>> new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>>  include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
>>  src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
>>  2 files changed, 45 insertions(+)
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 

Thanks, but thinking more, I'll propose this at the qemu layer and make
sure it's acceptable there first. Otherwise long term tools will may
still need to handle the sev-measurement format to cover both qemu and
libvirt cases.

- Cole

Re: [PATCH] qemu: Report sev measurement value and nonce explicitly
Posted by Daniel P. Berrangé 1 year, 6 months ago
On Tue, Oct 25, 2022 at 03:03:46PM -0400, Cole Robinson wrote:
> On 10/17/22 3:42 AM, Michal Prívozník wrote:
> > On 10/16/22 22:06, Cole Robinson wrote:
> >> The value returned by qemu's query-sev-launch-measure comes
> >> straight from the LAUNCH_MEASURE SEV firmware command. It's two
> >> values packed together: first 32 bytes is the launch measurement,
> >> last 16 bytes is the nonce.
> >>
> >> This combined value is really just an artifact of the return value
> >> of the firmware command, it has no direct usage. Users want the two
> >> individual values. But because qemu and libvirt do not separate them
> >> apart, every app that wants to process this value will have to do
> >> it manually.
> >>
> >> This performs the split for the user, and delivers the values in two
> >> new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce
> >>
> >> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> >> ---
> >>  include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
> >>  src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
> >>  2 files changed, 45 insertions(+)
> >>
> > 
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> > 
> 
> Thanks, but thinking more, I'll propose this at the qemu layer and make
> sure it's acceptable there first. Otherwise long term tools will may
> still need to handle the sev-measurement format to cover both qemu and
> libvirt cases.

I'm not entirely convinced we should split them apart at either
libvirt or QEMU level.

I think I would tend to view CVM launch measurement data as being
an opaque blob from the POV of libvirt/QEMU. In the case of SEV/SEV-ES
it does represent a tuple of nonce+hash, but that encoding is an
artifact of the current firmware implementation. The firmware <->
userspace API for SEV treats it as opaque, which means the firmware
has the freedom to change its contents at will. I expect this is
unlikely to happen in practice, but it is allowed for by the current
design, as we transmit the firmware major/minor version, alongside
the measurement.  If we split them apart then it makes QMEU and
libvirt aware of the specific firmware implementation which is
undesirable.

Overall I'd say the only 2 parties who should try to interpret the
measurement are the firmware and the attestation software client,
all the pieces inbetween should remain dumb transports.

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

Re: [PATCH] qemu: Report sev measurement value and nonce explicitly
Posted by Cole Robinson 1 year, 5 months ago
On 10/26/22 7:24 AM, Daniel P. Berrangé wrote:
> On Tue, Oct 25, 2022 at 03:03:46PM -0400, Cole Robinson wrote:
>> On 10/17/22 3:42 AM, Michal Prívozník wrote:
>>> On 10/16/22 22:06, Cole Robinson wrote:
>>>> The value returned by qemu's query-sev-launch-measure comes
>>>> straight from the LAUNCH_MEASURE SEV firmware command. It's two
>>>> values packed together: first 32 bytes is the launch measurement,
>>>> last 16 bytes is the nonce.
>>>>
>>>> This combined value is really just an artifact of the return value
>>>> of the firmware command, it has no direct usage. Users want the two
>>>> individual values. But because qemu and libvirt do not separate them
>>>> apart, every app that wants to process this value will have to do
>>>> it manually.
>>>>
>>>> This performs the split for the user, and delivers the values in two
>>>> new TYPED_PARAM fields: sev-measurement-value, sev-measurement-nonce
>>>>
>>>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>>>> ---
>>>>  include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++
>>>>  src/qemu/qemu_driver.c           | 23 +++++++++++++++++++++++
>>>>  2 files changed, 45 insertions(+)
>>>>
>>>
>>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>>>
>>
>> Thanks, but thinking more, I'll propose this at the qemu layer and make
>> sure it's acceptable there first. Otherwise long term tools will may
>> still need to handle the sev-measurement format to cover both qemu and
>> libvirt cases.
> 
> I'm not entirely convinced we should split them apart at either
> libvirt or QEMU level.
> 
> I think I would tend to view CVM launch measurement data as being
> an opaque blob from the POV of libvirt/QEMU. In the case of SEV/SEV-ES
> it does represent a tuple of nonce+hash, but that encoding is an
> artifact of the current firmware implementation. The firmware <->
> userspace API for SEV treats it as opaque, which means the firmware
> has the freedom to change its contents at will. I expect this is
> unlikely to happen in practice, but it is allowed for by the current
> design, as we transmit the firmware major/minor version, alongside
> the measurement.  If we split them apart then it makes QMEU and
> libvirt aware of the specific firmware implementation which is
> undesirable.
> 
> Overall I'd say the only 2 parties who should try to interpret the
> measurement are the firmware and the attestation software client,
> all the pieces inbetween should remain dumb transports.

Ok sounds good, I'll leave it as is

Thanks,
Cole