[libvirt PATCH 0/6] Improve AMD SEV support

This addresses a few issues in the AMD SEV support - Neither host or domain level SEV metadata is exposed in virsh commands - The domain launch security parameters don't expose enough info to validate the measurement The second point was the real purpose of my work. Per the SEV API guide to calculate the measurement we need measurement = HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK) The API_MINOR, API_MAJOR, BUILD values are things that are available from 'query-sev' QMP command and libvirt does not expose this info. This patch series adds them to virDomainGetLaunchSecurityParams alongside the measurement that we already report. So now the client can fetch this info and calculate an expected measurement to compare with the actual measurement they got. They will thus know if the guest is safe to inject secrets into, which is where Jim's recent patches come into play. Daniel P. Berrangé (6): include: add new launch security parameters qemu: report error querying launch params for inactive guest qemu: add monitor APIs for query-sev qemu: report new launch security parameters tools: add 'domlaunchsecinfo' virsh command tools: add 'nodesevinfo' virsh command include/libvirt/libvirt-domain.h | 32 +++++++++++++++++++ src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++---- src/qemu/qemu_monitor.c | 13 ++++++++ src/qemu/qemu_monitor.h | 9 ++++++ src/qemu/qemu_monitor_json.c | 45 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++++ tools/virsh-domain.c | 53 ++++++++++++++++++++++++++++++++ tools/virsh-host.c | 45 +++++++++++++++++++++++++++ 8 files changed, 246 insertions(+), 6 deletions(-) -- 2.33.1

Three more parameters are required in order that clients can perform a launch attestation on the SEV guest. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-domain.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index d0dd11ab01..5d3e15766e 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5101,6 +5101,38 @@ int virDomainSetLifecycleAction(virDomainPtr domain, */ # define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement" +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR: + * + * Macro represents the API major version of the SEV host, + * as VIR_TYPED_PARAM_UINT. + */ +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR "sev-api-major" + +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR: + * + * Macro represents the API minor version of the SEV guest, + * as VIR_TYPED_PARAM_UINT. + */ +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR "sev-api-minor" + +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID: + * + * Macro represents the build ID of the SEV host, + * as VIR_TYPED_PARAM_UINT. + */ +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID "sev-build-id" + +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY: + * + * Macro represents the policy of the SEV guest, + * as VIR_TYPED_PARAM_UINT. + */ +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY "sev-policy" + int virDomainGetLaunchSecurityInfo(virDomainPtr domain, virTypedParameterPtr *params, int *nparams, -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:29 +0000, Daniel P. Berrangé wrote:
Three more parameters are required in order that clients can perform a launch attestation on the SEV guest.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-domain.h | 32 ++++++++++++++++++++++++++++++++
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Querying launch params on a inactive guest currently triggers a warning about the monitor being NULL. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8093b8f69b..5bacf73003 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19992,6 +19992,12 @@ qemuDomainGetSEVMeasurement(virQEMUDriver *driver, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) return -1; + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + qemuDomainObjEnterMonitor(driver, vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:30 +0000, Daniel P. Berrangé wrote:
Querying launch params on a inactive guest currently triggers a warning about the monitor being NULL.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 6 ++++++ 1 file changed, 6 insertions(+)
https://bugzilla.redhat.com/show_bug.cgi?id=2030437 Reviewed-by: Peter Krempa <pkrempa@redhat.com>

We're only returning the set of fields needed to perform an attestation, per the SEV API docs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 +++++++++++ src/qemu/qemu_monitor.h | 9 ++++++++ src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++++++ 4 files changed, 75 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 75e0e4ed92..dda6ae9796 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4366,6 +4366,19 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon) } +int +qemuMonitorGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) +{ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetSEVInfo(mon, apiMajor, apiMinor, buildID, policy); +} + + int qemuMonitorGetPRManagerInfo(qemuMonitor *mon, GHashTable **retinfo) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index edc2b01a66..29746f0b8e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1445,6 +1445,15 @@ int qemuMonitorBlockdevMediumInsert(qemuMonitor *mon, char * qemuMonitorGetSEVMeasurement(qemuMonitor *mon); +int +qemuMonitorGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + 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 e00d785c20..423bae49d2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8216,6 +8216,51 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) } +/** + * Retrive info about the SEV setup, returning those fields that + * are required to do a launch attestation, as per + * + * HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK) + * + * specified in section 6.5.1 of AMD Secure Encrypted + * Virtualization API. + * + * { "execute": "query-sev" } + * { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0, + * "build-id" : 0, "policy" : 0, "state" : "running", + * "handle" : 1 } } + */ +int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || + virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || + virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || + virJSONValueObjectGetNumberUint(data, "policy", policy) < 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 0984717675..163be25c32 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -369,6 +369,14 @@ int qemuMonitorJSONSystemWakeup(qemuMonitor *mon); char *qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon); +int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + int qemuMonitorJSONGetVersion(qemuMonitor *mon, int *major, int *minor, -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:31 +0000, Daniel P. Berrangé wrote:
We're only returning the set of fields needed to perform an attestation, per the SEV API docs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 +++++++++++ src/qemu/qemu_monitor.h | 9 ++++++++ src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++++++ 4 files changed, 75 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e00d785c20..423bae49d2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8216,6 +8216,51 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) }
+/** + * Retrive info about the SEV setup, returning those fields that + * are required to do a launch attestation, as per + * + * HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK) + * + * specified in section 6.5.1 of AMD Secure Encrypted + * Virtualization API. + * + * { "execute": "query-sev" } + * { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0, + * "build-id" : 0, "policy" : 0, "state" : "running", + * "handle" : 1 } } + */ +int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy)
Please use consistent (with what you've added in the header file) and modern header formatting.
+{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || + virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || + virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || + virJSONValueObjectGetNumberUint(data, "policy", policy) < 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 0984717675..163be25c32 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -369,6 +369,14 @@ int qemuMonitorJSONSystemWakeup(qemuMonitor *mon);
char *qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon);
+int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Preferrably use modern header formatting.
+ int qemuMonitorJSONGetVersion(qemuMonitor *mon, int *major, int *minor,
qemumonitorjsontest? Reviewed-by: Peter Krempa <pkrempa@redhat.com>

On Thu, Dec 09, 2021 at 09:36:03AM +0100, Peter Krempa wrote:
On Wed, Dec 08, 2021 at 18:44:31 +0000, Daniel P. Berrangé wrote:
We're only returning the set of fields needed to perform an attestation, per the SEV API docs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 +++++++++++ src/qemu/qemu_monitor.h | 9 ++++++++ src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++++++ 4 files changed, 75 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e00d785c20..423bae49d2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8216,6 +8216,51 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) }
+/** + * Retrive info about the SEV setup, returning those fields that + * are required to do a launch attestation, as per + * + * HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK) + * + * specified in section 6.5.1 of AMD Secure Encrypted + * Virtualization API. + * + * { "execute": "query-sev" } + * { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0, + * "build-id" : 0, "policy" : 0, "state" : "running", + * "handle" : 1 } } + */ +int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy)
Please use consistent (with what you've added in the header file) and modern header formatting.
+{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || + virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || + virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || + virJSONValueObjectGetNumberUint(data, "policy", policy) < 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 0984717675..163be25c32 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -369,6 +369,14 @@ int qemuMonitorJSONSystemWakeup(qemuMonitor *mon);
char *qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon);
+int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Preferrably use modern header formatting.
Almost everything in this header uses the style matching this patch. IMHO divering in style is worse.
int qemuMonitorJSONGetVersion(qemuMonitor *mon, int *major, int *minor,
qemumonitorjsontest?
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, Dec 09, 2021 at 16:24:04 +0000, Daniel P. Berrangé wrote:
On Thu, Dec 09, 2021 at 09:36:03AM +0100, Peter Krempa wrote:
On Wed, Dec 08, 2021 at 18:44:31 +0000, Daniel P. Berrangé wrote:
We're only returning the set of fields needed to perform an attestation, per the SEV API docs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 +++++++++++ src/qemu/qemu_monitor.h | 9 ++++++++ src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 8 +++++++ 4 files changed, 75 insertions(+)
[...]
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 0984717675..163be25c32 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -369,6 +369,14 @@ int qemuMonitorJSONSystemWakeup(qemuMonitor *mon);
char *qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon);
+int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, + unsigned int *apiMajor, + unsigned int *apiMinor, + unsigned int *buildID, + unsigned int *policy) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Preferrably use modern header formatting.
Almost everything in this header uses the style matching this patch. IMHO divering in style is worse.
Fair enough: https://listman.redhat.com/archives/libvir-list/2021-December/msg00362.html

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5bacf73003..e1296d3723 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19977,14 +19977,19 @@ qemuNodeGetSEVInfo(virConnectPtr conn, static int -qemuDomainGetSEVMeasurement(virQEMUDriver *driver, - virDomainObj *vm, - virTypedParameterPtr *params, - int *nparams, - unsigned int flags) +qemuDomainGetSEVInfo(virQEMUDriver *driver, + virDomainObj *vm, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) { int ret = -1; + int rv; g_autofree char *tmp = NULL; + unsigned int apiMajor = 0; + unsigned int apiMinor = 0; + unsigned int buildID = 0; + unsigned int policy = 0; int maxpar = 0; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -20006,10 +20011,34 @@ qemuDomainGetSEVMeasurement(virQEMUDriver *driver, if (!tmp) goto endjob; + qemuDomainObjEnterMonitor(driver, vm); + rv = qemuMonitorGetSEVInfo(QEMU_DOMAIN_PRIVATE(vm)->mon, + &apiMajor, &apiMinor, &buildID, &policy); + qemuDomainObjExitMonitor(driver, vm); + + if (rv < 0) + goto endjob; + if (virTypedParamsAddString(params, nparams, &maxpar, VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT, tmp) < 0) goto endjob; + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR, + apiMajor) < 0) + goto endjob; + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR, + apiMinor) < 0) + goto endjob; + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID, + buildID) < 0) + goto endjob; + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY, + policy) < 0) + goto endjob; ret = 0; @@ -20037,7 +20066,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, if (vm->def->sec && vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (qemuDomainGetSEVMeasurement(driver, vm, params, nparams, flags) < 0) + if (qemuDomainGetSEVInfo(driver, vm, params, nparams, flags) < 0) goto cleanup; } -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:32 +0000, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> ---
https://bugzilla.redhat.com/show_bug.cgi?id=2030435 Also a rather sparse commit message. For justification you can use what you've put into the comment for the function calling query-sev.
src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5bacf73003..e1296d3723 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19977,14 +19977,19 @@ qemuNodeGetSEVInfo(virConnectPtr conn,
static int -qemuDomainGetSEVMeasurement(virQEMUDriver *driver, - virDomainObj *vm, - virTypedParameterPtr *params, - int *nparams, - unsigned int flags) +qemuDomainGetSEVInfo(virQEMUDriver *driver, + virDomainObj *vm, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) { int ret = -1; + int rv; g_autofree char *tmp = NULL; + unsigned int apiMajor = 0; + unsigned int apiMinor = 0; + unsigned int buildID = 0; + unsigned int policy = 0; int maxpar = 0;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -20006,10 +20011,34 @@ qemuDomainGetSEVMeasurement(virQEMUDriver *driver, if (!tmp) goto endjob;
+ qemuDomainObjEnterMonitor(driver, vm); + rv = qemuMonitorGetSEVInfo(QEMU_DOMAIN_PRIVATE(vm)->mon, + &apiMajor, &apiMinor, &buildID, &policy); + qemuDomainObjExitMonitor(driver, vm);
You don't have to enter monitor twice to do two calls.
+ + if (rv < 0) + goto endjob; + if (virTypedParamsAddString(params, nparams, &maxpar, VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT, tmp) < 0)
Once you merge the monitor blocks: Reviewed-by: Peter Krempa <pkrempa@redhat.com>

This command reports the launch security parameters for a guest, allowing an external tool to perform a launch attestation. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh-domain.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8379f9f135..1560a8ea0d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9525,6 +9525,53 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) goto cleanup; } +/* + * "domlaunchsecinfo" command + */ +static const vshCmdInfo info_domlaunchsecinfo[] = { + {.name = "help", + .data = N_("Get domain launch security info") + }, + {.name = "desc", + .data = N_("Get the launch security parameters for a guest domain") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domlaunchsecinfo[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = NULL} +}; + +static bool +cmdDomLaunchSecInfo(vshControl * ctl, const vshCmd * cmd) +{ + g_autoptr(virshDomain) dom = NULL; + size_t i; + int nparams = 0; + virTypedParameterPtr params = NULL; + bool ret = false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, 0) != 0) { + vshError(ctl, "%s", _("Unable to get launch security parameters")); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + g_autofree char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + /* * "qemu-monitor-command" command */ @@ -14544,6 +14591,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_domjobinfo, .flags = 0 }, + {.name = "domlaunchsecinfo", + .handler = cmdDomLaunchSecInfo, + .opts = opts_domlaunchsecinfo, + .info = info_domlaunchsecinfo, + .flags = 0 + }, {.name = "domname", .handler = cmdDomname, .opts = opts_domname, -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:33 +0000, Daniel P. Berrangé wrote:
This command reports the launch security parameters for a guest, allowing an external tool to perform a launch attestation.
https://bugzilla.redhat.com/show_bug.cgi?id=2030438
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh-domain.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
Missing manpage addition. The code looks okay.

While some SEV info is reported in the domain capabilities, for reasons of size, this excludes the certificates. The nodesevinfo command provides the full set of information. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh-host.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 5da1346a9c..5ee3834de2 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -888,6 +888,45 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * "nodesevinfo" command + */ +static const vshCmdInfo info_nodesevinfo[] = { + {.name = "help", + .data = N_("node SEV information") + }, + {.name = "desc", + .data = N_("Returns basic SEV information about the node.") + }, + {.name = NULL} +}; + +static bool +cmdNodeSEVInfo(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) +{ + virshControl *priv = ctl->privData; + size_t i; + int nparams = 0; + virTypedParameterPtr params = NULL; + bool ret = false; + + if (virNodeGetSEVInfo(priv->conn, ¶ms, &nparams, 0) != 0) { + vshError(ctl, "%s", _("Unable to get host SEV information")); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + g_autofree char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-18s: %s\n", params[i].field, str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + /* * "nodesuspend" command */ @@ -1828,6 +1867,12 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_nodememstats, .flags = 0 }, + {.name = "nodesevinfo", + .handler = cmdNodeSEVInfo, + .opts = NULL, + .info = info_nodesevinfo, + .flags = 0 + }, {.name = "nodesuspend", .handler = cmdNodeSuspend, .opts = opts_node_suspend, -- 2.33.1

On Wed, Dec 08, 2021 at 18:44:34 +0000, Daniel P. Berrangé wrote:
While some SEV info is reported in the domain capabilities, for reasons of size, this excludes the certificates. The nodesevinfo command provides the full set of information.
https://bugzilla.redhat.com/show_bug.cgi?id=2030438
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh-host.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
Missing manpage addition.
participants (2)
-
Daniel P. Berrangé
-
Peter Krempa