[libvirt] [PATCH v6 8/9] qemu: Add support to launch security info

[Erik Skultety]

On Wed, May 23, 2018 at 04:18:33PM -0500, Brijesh Singh wrote:
> This patch implements the internal driver API for launch event into
> qemu driver. When SEV is enabled, execute 'query-sev-launch-measurement'
> to get the measurement of memory encrypted through launch sequence.
>
> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> ---

Apart from what I'm writing below, naming comments from patch 6 apply here
too...

>  src/qemu/qemu_driver.c       | 68 ++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c      |  8 ++++++
>  src/qemu/qemu_monitor.h      |  3 ++
>  src/qemu/qemu_monitor_json.c | 42 +++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  2 ++
>  5 files changed, 123 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3a328e5d4679..6569dea32fce 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -21194,6 +21194,73 @@ qemuDomainSetLifecycleAction(virDomainPtr dom,
>  }
>
>
> +static int
> +qemuDomainGetSevMeasurement(virQEMUDriverPtr driver,
> +                            virDomainObjPtr vm,
> +                            virTypedParameterPtr *params,
> +                            int *nparams,
> +                            unsigned int flags)
> +{
> +    int ret = -1;
> +    char *tmp;
> +    int maxpar = 0;
> +
> +    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +        return -1;
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +        goto endjob;
> +
> +    tmp = qemuMonitorGetSevMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon);
> +    if (tmp == NULL)
> +        goto endjob;
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto endjob;
> +
> +    if (virTypedParamsAddString(params, nparams, &maxpar,
> +                                VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
> +                                tmp) < 0)
> +        goto endjob;
> +
> +    ret = 0;
> +
> + endjob:

You're going to leak @tmp here, since virTypedParamsAddString makes its own
copy of tmp.

> +    qemuDomainObjEndJob(driver, vm);
> +    return ret;
> +}
> +
> +
> +static int
> +qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
> +                                virTypedParameterPtr *params,
> +                                int *nparams,
> +                                unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = domain->conn->privateData;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +
> +    if (!(vm = qemuDomObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (vm->def->sev) {
> +        if (qemuDomainGetSevMeasurement(driver, vm, params, nparams, flags) < 0)
> +            goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}
> +
>  static virHypervisorDriver qemuHypervisorDriver = {
>      .name = QEMU_DRIVER_NAME,
>      .connectURIProbe = qemuConnectURIProbe,
> @@ -21414,6 +21481,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
>      .domainSetVcpu = qemuDomainSetVcpu, /* 3.1.0 */
>      .domainSetBlockThreshold = qemuDomainSetBlockThreshold, /* 3.2.0 */
>      .domainSetLifecycleAction = qemuDomainSetLifecycleAction, /* 3.9.0 */
> +    .domainGetLaunchSecurityInfo = qemuDomainGetLaunchSecurityInfo, /* 4.2.0 */
>  };
>
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 3b034930408c..977cbe5a41f8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4226,3 +4226,11 @@ qemuMonitorBlockdevDel(qemuMonitorPtr mon,
>
>      return qemuMonitorJSONBlockdevDel(mon, nodename);
>  }
> +
> +char *
> +qemuMonitorGetSevMeasurement(qemuMonitorPtr mon)
> +{
> +    QEMU_CHECK_MONITOR_NULL(mon);
> +
> +    return qemuMonitorJSONGetSevMeasurement(mon);
> +}
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index b1b7ef09c929..8a64ae5f3d96 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1137,4 +1137,7 @@ int qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
>  int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
>                             const char *nodename);
>
> +char *
> +qemuMonitorGetSevMeasurement(qemuMonitorPtr mon);
> +
>  #endif /* QEMU_MONITOR_H */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 24d3a2ff412f..041f595ca1e4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8024,3 +8024,45 @@ qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
>      virJSONValueFree(reply);
>      return ret;
>  }
> +
> +/**
> + * The function is used to retrieve the measurement of SEV guest.

s/of SEV/of a SEV

> + * The measurement is signature of the memory contents that was encrypted
> + * through the SEV launch flow.
> + *
> + * A example jason output:

s/jason/JSON

> + *
> + * { "execute" : "query-sev-launch-measure" }
> + * { "return" : { "data" : "4l8LXeNlSPUDlXPJG5966/8%YZ" } }
> + */
> +char *
> +qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon)
> +{
> +    const char *tmp;
> +    char *measurement = NULL;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr data;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-launch-measure", NULL)))
> +         return NULL;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        goto cleanup;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)

We should call qemuMonitorJSONCheckReply instead, not only does it call
JSONCheckError, but checks the return type as well which is suited for callers
who actually care about the data returned, not just whether the query passed or
failed.

> +        goto cleanup;
> +
> +    data = virJSONValueObjectGetObject(reply, "return");
> +
> +    if (!(tmp = virJSONValueObjectGetString(data, "data")))
> +        goto cleanup;
> +
> +    if (VIR_STRDUP(measurement, tmp) < 0)
> +        goto cleanup;
> +
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return measurement;

Erik