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

Jim Fehlig <jfehlig@xxxxxxxx> · Fri, 3 Dec 2021 17:44:24 -0700

On 12/3/21 08:10, Daniel P. Berrangé wrote:
On Tue, Nov 30, 2021 at 04:52:00PM -0700, Jim Fehlig wrote:
Set a launch secret in guest memory using the sev-inject-launch-secret
QMP API. Only supported for SEV-enabled guests.

Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
  src/qemu/qemu_driver.c       | 78 ++++++++++++++++++++++++++++++++++++
  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, 135 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4e680bc0a7..b6ee41b29e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20056,6 +20056,83 @@ 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;
+    size_t i;
+    g_autofree char *secrethdr = NULL;
+    g_autofree char *secret = NULL;
+    long long setaddr = -1;

unsigned, but would need to defaut to 0 then i guess.

As Peter mentioned, 0 may be a valid injection address

https://listman.redhat.com/archives/libvir-list/2021-November/msg00463.html

The address in optional in the qemu API, in which case it is retrieved from ovmf 
IIUC. I used a signed variable here to distinguish between user not providing an 
address and providing one, including a valid value of 0.

+
+    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_LLONG,

UULONG

+                               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;
+    }
+
+    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.l;
+    }
+
+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+        goto cleanup;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto endjob;
+
+    qemuDomainObjEnterMonitor(driver, vm);
+
+    if (qemuMonitorSetLaunchSecurityState(QEMU_DOMAIN_PRIVATE(vm)->mon,
+                                          secrethdr, secret, setaddr) < 0)
+        goto endjob;
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 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 |
@@ -20930,6 +21007,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 26b59801b8..7f31a53231 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4372,6 +4372,18 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon)
  }
  
  
+int
+qemuMonitorSetLaunchSecurityState(qemuMonitor *mon,
+                                  const char *secrethdr,
+                                  const char *secret,
+                                  long long injectaddr)
+{
+    QEMU_CHECK_MONITOR(mon);
+
+    return qemuMonitorJSONSetLaunchSecurityState(mon, secrethdr, secret, injectaddr);
+}
+
+
  int
  qemuMonitorGetPRManagerInfo(qemuMonitor *mon,
                              GHashTable **retinfo)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 99ecebc648..c485e403fd 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1444,6 +1444,12 @@ int qemuMonitorBlockdevMediumInsert(qemuMonitor *mon,
  char *
  qemuMonitorGetSEVMeasurement(qemuMonitor *mon);
  
+int
+qemuMonitorSetLaunchSecurityState(qemuMonitor *mon,
+                                  const char *secrethdr,
+                                  const char *secret,
+                                  long long injectaddr);
+
  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 56f0b22b2a..35aea34b5f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8174,6 +8174,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
+qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon,
+                                      const char *secrethdr,
+                                      const char *secret,
+                                      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)))

Heh, and I was supposed to handle that here :-). If injectaddr is -1, don't 
include the 'gpa' parameter to sev-inject-launch-secret. Will fix in V2, 
although alternative suggestions for handling the issue are welcomed.

Regards,
Jim

+        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 f7fb13f56c..4d2d73c661 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -368,6 +368,11 @@ int qemuMonitorJSONSystemWakeup(qemuMonitor *mon);
  
  char *qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon);
  
+int qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon,
+                                          const char *secrethdr,
+                                          const char *secret,
+                                          long long injectaddr);
+
  int qemuMonitorJSONGetVersion(qemuMonitor *mon,
                                int *major,
                                int *minor,
--
2.33.0



Regards,
Daniel









