[RFC PATCH 3/3] qemu: Implement the virDomainInjectLaunchSecret API

[Peter Krempa]

On Tue, Nov 16, 2021 at 19:23:54 -0700, Jim Fehlig wrote:
> Inject a launch secret in domain memory using the sev-inject-launch-secret
> QMP API. Only supported for SEV-enabed domains.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
>  src/qemu/qemu_driver.c       | 53 ++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c      | 12 ++++++++
>  src/qemu/qemu_monitor.h      |  6 ++++
>  src/qemu/qemu_monitor_json.c | 34 +++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  5 ++++
>  5 files changed, 110 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d954635dde..58e3f08afe 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20104,6 +20104,58 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
>      return ret;
>  }
>  
> +
> +static int
> +qemuDomainInjectLaunchSecret(virDomainPtr domain,
> +                             const char *secrethdr,
> +                             const char *secret,
> +                             unsigned long long injectaddr,
> +                             unsigned int flags)
> +{
> +    virQEMUDriver *driver = domain->conn->privateData;
> +    virDomainObj *vm;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = qemuDomainObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    if (virDomainInjectLaunchSecretEnsureACL(domain->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    /* Currently only SEV is supported */
> +    if (!vm->def->sec ||
> +        vm->def->sec->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("injecting a launch secret is only supported in SEV-enabled domains"));
> +        goto cleanup;
> +    }

Missing check that the VM is running.

> +
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +        goto endjob;

No need for async monitor without an async job

> +
> +    if (qemuMonitorInjectLaunchSecret(QEMU_DOMAIN_PRIVATE(vm)->mon,
> +                                      secrethdr, secret, injectaddr) < 0)
> +        goto endjob;
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto endjob;
> +
> +    ret = 0;
> +
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:


[...]

> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 810dac209d..c64469a03b 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4383,6 +4383,18 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon)
>  }
>  
>  
> +int
> +qemuMonitorInjectLaunchSecret(qemuMonitor *mon,
> +                              const char *secrethdr,
> +                              const char *secret,
> +                              unsigned long long injectaddr)
> +{
> +    QEMU_CHECK_MONITOR(mon);

You cleverly avoid logging the secret here ...

> +
> +    return qemuMonitorJSONInjectLaunchSecret(mon, secrethdr, secret, injectaddr);
> +}
> +
> +
>  int
>  qemuMonitorGetPRManagerInfo(qemuMonitor *mon,
>                              GHashTable **retinfo)


[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 4669b9135d..69aef078ec 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8124,6 +8124,40 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon)
>  }
>  
>  
> +/**
> + * The function is used to inject a launch secret in an SEV guest.
> + *
> + * Example JSON:
> + *
> + * { "execute" : "sev-inject-launch-secret",
> + *   "data": { "packet-header": "str", "secret": "str", "gpa": "uint64" } }
> + */
> +int
> +qemuMonitorJSONInjectLaunchSecret(qemuMonitor *mon,
> +                                  const char *secrethdr,
> +                                  const char *secret,
> +                                  unsigned long long injectaddr)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("sev-inject-launch-secret",
> +                                           "s:packet-header", secrethdr,
> +                                           "s:secret", secret,
> +                                           "U:gpa", injectaddr,
> +                                           NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)

... but once this is sent to the monitor it can be logged if QMP traffic
logging is enabled.

Thus either the secret is not so secret, but the qemu documentation nor
the documentation you've added seem to be lessening the secrecy of the
passed string in any way, or the 'sev-inject-launch-secret' is broken by
design as it doesn't support passing an encrypted object via qcrypto.

In case when the secret must really be kept secret the QMP command must
be fixed first, otherwise libvirt is not able to guarantee any secrecy.

> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        return -1;
> +
> +    return 0;
> +}