Re: [PATCH V3 3/4] qemu: Implement the virDomainSetLaunchSecurityState API

[John Ferlan <jferlan@xxxxxxxxxx>]

On 12/14/21 23:46, Jim Fehlig wrote:
> Set a launch secret in guest memory using the sev-inject-launch-secret
> QMP API. Only supported for SEV-enabled guests in a paused state.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>   src/qemu/qemu_driver.c       | 88 ++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_monitor.c      | 14 ++++++
>   src/qemu/qemu_monitor.h      |  7 +++
>   src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++
>   src/qemu/qemu_monitor_json.h |  6 +++
>   tests/qemumonitorjsontest.c  |  3 ++
>   6 files changed, 163 insertions(+)

Just checking - not active day to day, but this technology is what some 
of my team works on now... I only realized this because Dan pointed it 
out and I was trying to help someone new on my team see how commands are 
created in libvirt. He's more familiar with sevctl processing, but will 
need to contribute to libvirt/qemu in the coming months.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index aae622ea5d..889892a097 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20070,6 +20070,93 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
>       return ret;
>   }
>   
> +
> +static int
> +qemuDomainSetLaunchSecurityState(virDomainPtr domain,
> +                                 virTypedParameterPtr params,
> +                                 int nparams,
> +                                 unsigned int flags)
> +{
> +    virQEMUDriver *driver = domain->conn->privateData;
> +    virDomainObj *vm;
> +    int ret = -1;
> +    int rc;
> +    size_t i;
> +    g_autofree char *secrethdr = NULL;
> +    g_autofree char *secret = NULL;
> +    unsigned long long setaddr = 0;
> +    bool hasSetaddr = false;
> +    int state;
> +
> +    virCheckFlags(0, -1);
> +    if (virTypedParamsValidate(params, nparams,
> +                               VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER,
> +                               VIR_TYPED_PARAM_STRING,
> +                               VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET,
> +                               VIR_TYPED_PARAM_STRING,
> +                               VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               NULL) < 0)
> +        return -1;
> +
> +    if (!(vm = qemuDomainObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    if (virDomainSetLaunchSecurityStateEnsureACL(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",
> +                       _("setting a launch secret is only supported in SEV-enabled domains"));
> +        goto cleanup;
> +    }

Would we need a specific capability check here before calling 
sev-inject-launch-secret since it's only available since qemu-6.0?

Seems to me VIR_DOMAIN_LAUNCH_SECURITY_SEV would be keyed off the 
presence of the sev-guest object only. That would seem to be a qemu 2.12 
type check. It's been a while since I looked at sources, so I'm rusty, 
but I think qemu_validate.c/qemuValidateDomainDef would check for 
QEMU_CAPS_SEV_GUEST only.

This is unlike the query-sev-capabilities and query-sev-launch-measure 
which are present in 2.12.

John

> +
> +    for (i = 0; i < nparams; i++) {
> +        virTypedParameterPtr param = &params[i];
> +
> +        if (STREQ(param->field, VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER)) {
> +            secrethdr = g_strdup(param->value.s);
> +        } else if (STREQ(param->field, VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET)) {
> +            secret = g_strdup(param->value.s);
> +        } else if (STREQ(param->field, VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS)) {
> +            setaddr =  param->value.ul;
> +            hasSetaddr = true;
> +        }
> +    }
> +
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto endjob;
> +
> +    state = virDomainObjGetState(vm, NULL);
> +    if (state != VIR_DOMAIN_PAUSED) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain must be in a paused state"));
> +        goto endjob;
> +    }
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    rc = qemuMonitorSetLaunchSecurityState(QEMU_DOMAIN_PRIVATE(vm)->mon,
> +                                           secrethdr, secret, setaddr, hasSetaddr);
> +    qemuDomainObjExitMonitor(driver, vm);
> +    if (rc < 0)
> +        goto endjob;
> +
> +    ret = 0;
> +
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}
> +
> +
>   static const unsigned int qemuDomainGetGuestInfoSupportedTypes =
>       VIR_DOMAIN_GUEST_INFO_USERS |
>       VIR_DOMAIN_GUEST_INFO_OS |
> @@ -20943,6 +21030,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
>       .domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */
>       .domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */
>       .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */
> +    .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */
>   };
>   
>   
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index dda6ae9796..5272d49c2e 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4379,6 +4379,20 @@ qemuMonitorGetSEVInfo(qemuMonitor *mon,
>   }
>   
>   
> +int
> +qemuMonitorSetLaunchSecurityState(qemuMonitor *mon,
> +                                  const char *secrethdr,
> +                                  const char *secret,
> +                                  unsigned long long setaddr,
> +                                  bool hasSetaddr)
> +{
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    return qemuMonitorJSONSetLaunchSecurityState(mon, secrethdr, secret,
> +                                                 setaddr, hasSetaddr);
> +}
> +
> +
>   int
>   qemuMonitorGetPRManagerInfo(qemuMonitor *mon,
>                               GHashTable **retinfo)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 29746f0b8e..87826e6268 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1454,6 +1454,13 @@ qemuMonitorGetSEVInfo(qemuMonitor *mon,
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>       ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
>   
> +int
> +qemuMonitorSetLaunchSecurityState(qemuMonitor *mon,
> +                                  const char *secrethdr,
> +                                  const char *secret,
> +                                  unsigned long long setaddr,
> +                                  bool hasSetaddr);
> +
>   typedef struct _qemuMonitorPRManagerInfo qemuMonitorPRManagerInfo;
>   struct _qemuMonitorPRManagerInfo {
>       bool connected;
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a3d6eca569..37ee859a33 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8262,6 +8262,51 @@ qemuMonitorJSONGetSEVInfo(qemuMonitor *mon,
>   }
>   
>   
> +/**
> + * Set a launch secret in guest memory
> + *
> + * Example JSON:
> + *
> + * { "execute" : "sev-inject-launch-secret",
> + *   "data": { "packet-header": "str", "secret": "str", "gpa": "uint64" } }
> + *
> + * The guest physical address (gpa) parameter is optional
> + */
> +int
> +qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon,
> +                                      const char *secrethdr,
> +                                      const char *secret,
> +                                      unsigned long long setaddr,
> +                                      bool hasSetaddr)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +
> +    if (hasSetaddr) {
> +        cmd = qemuMonitorJSONMakeCommand("sev-inject-launch-secret",
> +                                         "s:packet-header", secrethdr,
> +                                         "s:secret", secret,
> +                                         "U:gpa", setaddr,
> +                                         NULL);
> +    } else {
> +        cmd = qemuMonitorJSONMakeCommand("sev-inject-launch-secret",
> +                                         "s:packet-header", secrethdr,
> +                                         "s:secret", secret,
> +                                         NULL);
> +    }
> +    if (cmd == NULL)
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
>   /*
>    * Example return data
>    *
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index e88dfc9d50..64d9ebdaa3 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -476,6 +476,12 @@ qemuMonitorJSONGetVersion(qemuMonitor *mon,
>                             char **package)
>       ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
>   
> +int qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon,
> +                                          const char *secrethdr,
> +                                          const char *secret,
> +                                          unsigned long long setaddr,
> +                                          bool hasSetaddr);
> +
>   int
>   qemuMonitorJSONGetMachines(qemuMonitor *mon,
>                              qemuMonitorMachineInfo ***machines)
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 1b0bd0870d..48e2a457ab 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -1196,6 +1196,8 @@ GEN_TEST_FUNC(qemuMonitorJSONSetAction,
>                 QEMU_MONITOR_ACTION_REBOOT_RESET,
>                 QEMU_MONITOR_ACTION_WATCHDOG_SHUTDOWN,
>                 QEMU_MONITOR_ACTION_PANIC_SHUTDOWN)
> +GEN_TEST_FUNC(qemuMonitorJSONSetLaunchSecurityState, "sev_secret_header",
> +              "sev_secret", 0, true)
>   
>   static int
>   testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
> @@ -3067,6 +3069,7 @@ mymain(void)
>       DO_TEST_GEN(qemuMonitorJSONJobComplete);
>       DO_TEST_GEN(qemuMonitorJSONBlockJobCancel);
>       DO_TEST_GEN(qemuMonitorJSONSetAction);
> +    DO_TEST_GEN(qemuMonitorJSONSetLaunchSecurityState);
>       DO_TEST(qemuMonitorJSONGetBalloonInfo);
>       DO_TEST(qemuMonitorJSONGetBlockInfo);
>       DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo);