[libvirt] [PATCH 4/4] libvirt-domain: add new virDomainGetSevVmMeasurement() API

[Daniel P. Berrangé]

On Mon, Feb 26, 2018 at 11:53:36AM -0600, Brijesh Singh wrote:
> The virDomainGetSevVmMeasurement() can be used to retrieve the measurement
> of encrypted VM launched using AMD SEV feature. The measurement is a
> signature of the memory contents that can be sent to the guest owner as
> an attestation that the memory was encrypted correctly by the firmware
> before booting the guest.
> 
> Signed-off-by: Xiaogang Chen <Xiaogang.Chen at amd.com>
> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> ---
>  include/libvirt/libvirt-domain.h |  4 +++
>  src/driver-hypervisor.h          |  4 +++
>  src/libvirt-domain.c             | 41 +++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  1 +
>  src/qemu/qemu_driver.c           | 57 ++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c          |  8 ++++++
>  src/qemu/qemu_monitor.h          |  3 +++
>  src/qemu/qemu_monitor_json.c     | 33 +++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h     |  2 ++
>  src/remote/remote_driver.c       |  3 ++-
>  src/remote/remote_protocol.x     | 17 +++++++++++-
>  11 files changed, 171 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4048acf38aaf..c0bcfea4723c 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4756,4 +4756,8 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>                                  unsigned int action,
>                                  unsigned int flags);
>  
> +char *
> +virDomainGetSevVmMeasurement(virDomainPtr domain,
> +                             unsigned int flags);

I'm a little concerned with introducing an API like this that is tied to the
specific implementation that we have in AMD chips today, as it is unlikely
to be usable when other vendors provide equivalent functionality.

At the same time it is hard (impossible) to predict what information future
impelementations will need to provide. So I wonder if we would be better
making the API generic by using the virTypedParameters design. eg

    virDomaniGetLaunchSecurityInfo(virDOmaniPtr domain,
                                   virTypedParametersPtr *params,
				   unsigned int *nparams,
				   unsigned int flags);

And then to start with we could just define a single named parameter

  #define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"


> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0dbc7b..6e956d965a26 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -783,6 +783,7 @@ LIBVIRT_3.9.0 {
>  LIBVIRT_4.1.0 {
>      global:
>          virStoragePoolLookupByTargetPath;
> +        virDomainGetSevVmMeasurement;
>  } LIBVIRT_3.9.0;

Since we're in freeze for 4.1.0, it is going to be 4.2.0 at the earliest now.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 313d730c791f..852d1f0fd2f7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -21254,6 +21254,62 @@ qemuDomainSetLifecycleAction(virDomainPtr dom,
>      return ret;
>  }
>  
> +static char *
> +qemuDomainGetSevVmMeasurement(virDomainPtr dom,
> +                              unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    char *ret = NULL, *tmp;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (!(vm = qemuDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("domain is not running"));
> +       goto endjob;
> +    }
> +
> +    if (virDomainGetSevVmMeasurementEnsureACL(dom->conn, vm->def) < 0){
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("get sev vm measurement is not allowed"));
> +        goto cleanup;
> +    }
> +
> +    if (vm->def->sev) {
> +        goto endjob;
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("domain is not SEV guest"));

Hehe, unreachable code due to inverted order :-)

> +    }
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +        goto endjob;
> +
> +    VIR_DEBUG("query sev launch measurement");
> +    if(!(tmp = qemuMonitorGetSevMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon))){
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("failed to get measurement"));
> +        goto endjob;
> +    }
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto endjob;
> +
> +    ret = tmp;
> +
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}

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