Hi John,
Thanks for taking a look!
On 12/17/21 12:48, John Ferlan wrote:
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(a)suse.com>
> ---
> 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?
Good point. I made an attempt at including a caps check in V4
https://listman.redhat.com/archives/libvir-list/2021-December/msg00800.html
Regards,
Jim
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 = ¶ms[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);
>