[RFC PATCH v1 0/5] Add virDomainGetSevAttestationReport API
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection. My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API. This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both. This patch is not final, as I still would like some human-readable outputting and storage of the attestation report. Looking for thoughts on the design of this API, as well as suggested improvements. Tyler Fanelli (5): libvirt: Introduce virDomainGetSevAttestationReport public API remote: add RPC support for the virDomainGetSevAttestationReport API qemu_capabilities: Introduce QEMU_CAPS_SEV_GET_ATTESTATION_REPORT qemu: Implement the virDomainGetSevAttestationReport API tools: add domgetsevreport virsh command docs/manpages/virsh.rst | 18 ++++ include/libvirt/libvirt-domain.h | 22 +++++ src/driver-hypervisor.h | 7 ++ src/libvirt-domain.c | 63 ++++++++++++++ src/libvirt_public.syms | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 86 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 40 +++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/remote/remote_daemon_dispatch.c | 44 ++++++++++ src/remote/remote_driver.c | 55 ++++++++++++ src/remote/remote_protocol.x | 21 ++++- src/remote_protocol-structs | 12 +++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + tools/virsh-domain.c | 68 +++++++++++++++ 20 files changed, 466 insertions(+), 1 deletion(-) -- 2.34.1
This API allows getting an attestation report from a SEV-enabled guest. The API uses virTypedParameter for input. The details of an attestation report buffer are described in the SEV API spec in section "6.8.2 Parameters, Table 60". https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf Signed-off-by: Tyler Fanelli <tfanelli@redhat.com> --- include/libvirt/libvirt-domain.h | 14 +++++++ src/driver-hypervisor.h | 7 ++++ src/libvirt-domain.c | 63 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 4 ++ 4 files changed, 88 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2d5718301e..af8991dbd3 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5166,6 +5166,15 @@ int virDomainSetLifecycleAction(virDomainPtr domain, */ # define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS "sev-secret-set-address" +/** + * VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE: + * + * A macro used to represent a random 16 bytes value encoded in base64 + * that will be included in a SEV attestation report, as + * VIR_TYPED_PARAM_STRING. + */ +# define VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE "mnonce" + int virDomainGetLaunchSecurityInfo(virDomainPtr domain, virTypedParameterPtr *params, int *nparams, @@ -5176,6 +5185,11 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, int nparams, unsigned int flags); +int virDomainGetSevAttestationReport(virDomainPtr domain, + virTypedParameterPtr *params_ptr, + int *nparams, + unsigned int flags); + typedef enum { VIR_DOMAIN_GUEST_INFO_USERS = (1 << 0), /* return active users */ VIR_DOMAIN_GUEST_INFO_OS = (1 << 1), /* return OS information */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4423eb0885..568d8c9a26 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1348,6 +1348,12 @@ typedef int int nparams, unsigned int flags); +typedef int +(*virDrvDomainGetSevAttestationReport)(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef virDomainCheckpointPtr (*virDrvDomainCheckpointCreateXML)(virDomainPtr domain, const char *xmlDesc, @@ -1678,6 +1684,7 @@ struct _virHypervisorDriver { virDrvNodeGetSEVInfo nodeGetSEVInfo; virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo; virDrvDomainSetLaunchSecurityState domainSetLaunchSecurityState; + virDrvDomainGetSevAttestationReport domainGetSevAttestationReport; virDrvDomainCheckpointCreateXML domainCheckpointCreateXML; virDrvDomainCheckpointGetXMLDesc domainCheckpointGetXMLDesc; virDrvDomainListAllCheckpoints domainListAllCheckpoints; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a197618673..ebcba4a8b7 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12957,6 +12957,69 @@ int virDomainSetLaunchSecurityState(virDomainPtr domain, return -1; } +/** + * virDomainGetSevAttestationReport: + * @domain: a domain object + * @params_ptr: pointer to launch security parameter objects + * @nparams: pointer to number of launch security parameters + * @flags: currently used, set to 0 + * + * Get an attestation report from a SEV-enabled guest. On success, the guest + * attestation report can be obtained and the guest can be started. + * + * There is one parameter for receiving an attestation report, mnonce, which is + * a random 16-byte string to be included in the attestation report. + * + * Returns -1 in case of failure, 0 in case of success. + */ +int virDomainGetSevAttestationReport(virDomainPtr domain, + virTypedParameterPtr *params_ptr, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn; + virTypedParameterPtr params; + int rc; + + params = *params_ptr; + conn = domain->conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, *nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, *nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(params, error); + virCheckPositiveArgGoto(*nparams, error); + virCheckReadOnlyGoto(domain->conn->flags, error); + + rc = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING); + + if (rc < 0) + goto error; + if (rc) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + if (virTypedParameterValidateSet(conn, params, *nparams) < 0) + goto error; + + if (conn->driver->domainGetSevAttestationReport) { + int ret; + ret = conn->driver->domainGetSevAttestationReport(domain, params_ptr, + nparams, flags); + if (ret < 0) + goto error; + + return ret; + } + +error: + virDispatchError(domain->conn); + return -1; +} /** * virDomainAgentSetResponseTimeout: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f93692c427..f0cd5e7e55 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -916,4 +916,8 @@ LIBVIRT_8.0.0 { virDomainSetLaunchSecurityState; } LIBVIRT_7.8.0; +LIBVIRT_8.2.0 { + global: + virDomainGetSevAttestationReport; +} LIBVIRT_8.0.0; # .... define new API here using predicted next version number .... -- 2.34.1
Signed-off-by: Tyler Fanelli <tfanelli@redhat.com> --- src/remote/remote_daemon_dispatch.c | 44 +++++++++++++++++++++++ src/remote/remote_driver.c | 55 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 21 ++++++++++- src/remote_protocol-structs | 12 +++++++ 4 files changed, 131 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 2463386e39..dcb734ab09 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5305,6 +5305,50 @@ remoteDispatchNodeGetSevInfo(virNetServer *server G_GNUC_UNUSED, return rv; } +static int +remoteDispatchDomainGetSevAttestationReport(virNetServer *server G_GNUC_UNUSED, + virNetServerClient *client, + virNetMessage *msg G_GNUC_UNUSED, + struct virNetMessageError *rerr, + remote_domain_get_sev_attestation_report_args *args, + remote_domain_get_sev_attestation_report_ret *ret) +{ + virTypedParameterPtr params = NULL; + int nparams = 0; + int rv = -1; + virConnectPtr conn = remoteGetHypervisorConn(client); + virDomainPtr dom = NULL; + + if (!conn) + goto cleanup; + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virTypedParamsDeserialize((struct _virTypedParameterRemote *) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) + goto cleanup; + + if (virDomainGetSevAttestationReport(dom, ¶ms, &nparams, args->flags) < 0) + goto cleanup; + + if (virTypedParamsSerialize(params, nparams, + REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX, + (struct _virTypedParameterRemote **) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + + return rv; +} static int remoteDispatchNodeGetMemoryParameters(virNetServer *server G_GNUC_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7e7a21fcab..bfc5d6c874 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6775,6 +6775,60 @@ remoteNodeGetSEVInfo(virConnectPtr conn, return rv; } +static int +remoteDomainGetSevAttestationReport(virDomainPtr dom, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_sev_attestation_report_args args; + remote_domain_get_sev_attestation_report_ret ret; + struct private_data *priv = dom->conn->privateData; + virTypedParameterPtr ret_params = NULL; + int ret_nparams = 0; + + remoteDriverLock(priv); + + + make_nonnull_domain(&args.dom, dom); + args.flags = flags; + + if (virTypedParamsSerialize(*params, *nparams, + REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX, + (struct _virTypedParameterRemote **) &args.params.params_val, + &args.params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + goto cleanup; + } + + memset(&ret, 0, sizeof(ret)); + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_SEV_ATTESTATION_REPORT, + (xdrproc_t) xdr_remote_domain_get_sev_attestation_report_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_sev_attestation_report_ret, (char *) &ret) == -1) { + goto done; + } + + if (virTypedParamsDeserialize((struct _virTypedParameterRemote *) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX, + &ret_params, + &ret_nparams) < 0) + goto cleanup; + + virTypedParamsFree(*params, *nparams); + *params = g_steal_pointer(&ret_params); + *nparams = ret_nparams; + + rv = 0; + +cleanup: + virTypedParamsFree(ret_params, ret_nparams); + xdr_free((xdrproc_t) xdr_remote_domain_get_sev_attestation_report_ret, (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} static int remoteNodeGetCPUMap(virConnectPtr conn, @@ -8651,6 +8705,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ + .domainGetSevAttestationReport = remoteDomainGetSevAttestationReport, /* 8.1.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4f13cef662..4e5ce42bd5 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -275,6 +275,9 @@ const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64; /* Upper limit on number of launch security state entries */ const REMOTE_DOMAIN_LAUNCH_SECURITY_STATE_PARAMS_MAX = 64; +/* Upper limit on number of SEV attestation report entries */ +const REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX = 64; + /* Upper limit on number of parameters describing a guest */ const REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX = 2048; @@ -3651,6 +3654,16 @@ struct remote_domain_set_launch_security_state_args { unsigned int flags; }; +struct remote_domain_get_sev_attestation_report_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_sev_attestation_report_ret { + remote_typed_param params<REMOTE_DOMAIN_GET_SEV_ATTESTATION_REPORT_PARAMS_MAX>; +}; + /* nwfilter binding */ struct remote_nwfilter_binding_lookup_by_port_dev_args { @@ -6920,5 +6933,11 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439 + REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_SEV_ATTESTATION_REPORT = 440 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index d88176781d..67333284cd 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3013,6 +3013,17 @@ struct remote_domain_set_launch_security_state_args { } params; u_int flags; }; +struct remote_domain_get_sev_attestation_report_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_sev_attestation_report_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + int nparams; +}; struct remote_nwfilter_binding_lookup_by_port_dev_args { remote_nonnull_string name; }; @@ -3689,4 +3700,5 @@ enum remote_procedure { REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437, REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, + REMOTE_PROC_DOMAIN_GET_SEV_ATTESTATION_REPORT = 440, }; -- 2.34.1
The 'query-sev-attestation-report' qmp command is only available with qemu >= 6.1.0. Introduce a capability for query-sev-attestation-report. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 + 5 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 32980e7330..68ebbe6534 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -668,6 +668,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 425 */ "blockdev.nbd.tls-hostname", /* QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */ + "query-sev-attestation-report", /* QEMU_CAPS_SEV_GET_ATTESTATION_REPORT */ ); @@ -1235,6 +1236,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-dirty-rate", QEMU_CAPS_QUERY_DIRTY_RATE }, { "sev-inject-launch-secret", QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET }, { "calc-dirty-rate", QEMU_CAPS_CALC_DIRTY_RATE }, + { "query-sev-attestation-report", QEMU_CAPS_SEV_GET_ATTESTATION_REPORT }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0a215a11d5..6c0e8f40aa 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -643,6 +643,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 425 */ QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME, /* tls hostname can be overriden for NBD clients */ + QEMU_CAPS_SEV_GET_ATTESTATION_REPORT, /* 'query-sev-attestation-report' qmp command present */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index ba1aecc37e..63a46ed1e1 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -239,6 +239,7 @@ <flag name='rbd-encryption'/> <flag name='sev-inject-launch-secret'/> <flag name='calc-dirty-rate'/> + <flag name='query-sev-attestation-report'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index d77907af55..681dedb935 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -241,6 +241,7 @@ <flag name='sev-inject-launch-secret'/> <flag name='calc-dirty-rate'/> <flag name='dirtyrate-param.mode'/> + <flag name='query-sev-attestation-report'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index 0f34a341af..93ac38d04d 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -244,6 +244,7 @@ <flag name='calc-dirty-rate'/> <flag name='dirtyrate-param.mode'/> <flag name='blockdev.nbd.tls-hostname'/> + <flag name='query-sev-attestation-report'/> <version>6002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.34.1
Get a SEV attestation report using the query-sev-attestation-report QMP API. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com> --- include/libvirt/libvirt-domain.h | 8 +++ src/driver-hypervisor.h | 4 +- src/qemu/qemu_driver.c | 86 ++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 11 ++++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 40 +++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++ 7 files changed, 157 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index af8991dbd3..3f56da99cc 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5175,6 +5175,14 @@ int virDomainSetLifecycleAction(virDomainPtr domain, */ # define VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE "mnonce" +/** + * VIR_DOMAIN_SEV_ATTESTATION_REPORT_DATA: + * + * A macro used to represent the returned SEV attestation report (encoded in + * base64). + */ +# define VIR_DOMAIN_SEV_ATTESTATION_REPORT_DATA "sev-attestation-report" + int virDomainGetLaunchSecurityInfo(virDomainPtr domain, virTypedParameterPtr *params, int *nparams, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 568d8c9a26..8912e5d7ef 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1350,8 +1350,8 @@ typedef int typedef int (*virDrvDomainGetSevAttestationReport)(virDomainPtr domain, - virTypedParameterPtr params, - int nparams, + virTypedParameterPtr *params_ptr, + int *nparams, unsigned int flags); typedef virDomainCheckpointPtr diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b7e83c769a..a96a0b9f84 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20123,6 +20123,91 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain, return ret; } +static int +qemuDomainGetSevAttestationReport(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virQEMUDriver *driver; + virDomainObj *vm; + int ret = -1; + size_t i; + g_autofree char *mnonce = NULL; + g_autofree char *report = NULL; + int maxpar = 2; + g_autoptr(virQEMUCaps) qemucaps = NULL; + + driver = domain->conn->privateData; + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + if (virTypedParamsValidate(*params, *nparams, + VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE, + VIR_TYPED_PARAM_STRING, + NULL) < 0) + return -1; + + if (!(vm = qemuDomainObjFromDomain(domain))) + goto cleanup; + + if (virDomainGetSevAttestationReportEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + /* SEV must be enabled to get an attestation report */ + if (!vm->def->sec || + vm->def->sec->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("attestation report is only supported in SEV-enabled domains")); + goto cleanup; + } + + if (!(qemucaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL))) + goto cleanup; + + + if (!virQEMUCapsGet(qemucaps, QEMU_CAPS_SEV_GET_ATTESTATION_REPORT)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU does not support getting a SEV attestation report")); + goto cleanup; + } + + for (i = 0; i < *nparams; ++i) { + virTypedParameterPtr param = params[i]; + + if (STREQ(param->field, VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE)) + mnonce = g_strdup(param->value.s); + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorGetSevAttestationReport(QEMU_DOMAIN_PRIVATE(vm)->mon, + mnonce, + &report); + qemuDomainObjExitMonitor(vm); + if (ret < 0) + goto endjob; + + if (virTypedParamsAddString(params, nparams, &maxpar, + VIR_DOMAIN_SEV_ATTESTATION_REPORT_DATA, + report) < 0) + goto endjob; + + ret = 0; + +endjob: + qemuDomainObjEndJob(vm); + +cleanup: + virDomainObjEndAPI(&vm); + return ret; +} static const unsigned int qemuDomainGetGuestInfoSupportedTypes = VIR_DOMAIN_GUEST_INFO_USERS | @@ -21028,6 +21113,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ + .domainGetSevAttestationReport = qemuDomainGetSevAttestationReport, /* 8.1.0 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 316cff5b9b..284e4a0b01 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4330,6 +4330,17 @@ qemuMonitorSetLaunchSecurityState(qemuMonitor *mon, } +int +qemuMonitorGetSevAttestationReport(qemuMonitor *mon, + const char *mnonce, + char **report) +{ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetSevAttestationReport(mon, mnonce, report); +} + + int qemuMonitorGetPRManagerInfo(qemuMonitor *mon, GHashTable **retinfo) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 5c2a749282..2e6fb8bfe0 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1447,6 +1447,11 @@ qemuMonitorSetLaunchSecurityState(qemuMonitor *mon, unsigned long long setaddr, bool hasSetaddr); +int +qemuMonitorGetSevAttestationReport(qemuMonitor *mon, + const char *mnonce, + char **report); + 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 d5622bd6d9..45adf7a740 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8322,6 +8322,46 @@ qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon, return 0; } +/** + * Get a SEV attestation report + * + * Example JSON: + * + * {"execute" : "query-sev-attestation-report", + * "data" : { "mnonce": "str" } } + * {"return" : "data" : "mnonceNlSPUDlXPJG5966/8%YZ" } } + */ +int +qemuMonitorJSONGetSevAttestationReport(qemuMonitor *mon, + const char *mnonce, + char **report) +{ + const char *tmp; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + + cmd = qemuMonitorJSONMakeCommand("query-sev-attestation-report", + "s:mnonce", mnonce, + NULL); + if (cmd == 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 (!(tmp = virJSONValueObjectGetString(data, "data"))) + return -1; + + *report = g_strdup(tmp); + + return 0; +} /* * Example return data diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 982fbad44e..9a8e4ffd28 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -484,6 +484,11 @@ int qemuMonitorJSONSetLaunchSecurityState(qemuMonitor *mon, unsigned long long setaddr, bool hasSetaddr); +int +qemuMonitorJSONGetSevAttestationReport(qemuMonitor *mon, + const char *mnonce, + char **report); + int qemuMonitorJSONGetMachines(qemuMonitor *mon, qemuMonitorMachineInfo ***machines) -- 2.34.1
After domlaunchsecinfo is used to attest a VM, domgetsevreport can be used to get a full SEV attestation report from the guest. Signed-off-by: Tyler Fanelli <tfanelli@redhat.com> --- docs/manpages/virsh.rst | 18 +++++++++++ tools/virsh-domain.c | 68 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index d2e6528533..ce62551f91 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2119,6 +2119,24 @@ the guest's memory to set the secret. If not specified, the address will be determined by the hypervisor. +domgetsevreport +--------------- + +**Syntax:** + +:: + + domgetsevreport domain --mnonce mnonce-string + +Get an attestation report from a SEV-enabled guest. The guest must have a +launchSecurity type enabled in its configuration. On success, the attestation +report can be examined. On failure, guest may not be attested and should be +examined to confirm so. + +*--mnonce* specifies a random 16-byte value encoded in base64 to be included +in the attestation report + + dommemstat ---------- diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d5fd8be7c3..bd8f426596 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9715,6 +9715,68 @@ cmdDomSetLaunchSecState(vshControl * ctl, const vshCmd * cmd) return ret; } +/* + * "domgetsevreport" command + */ +static const vshCmdInfo info_domgetsevreport[] = { + {.name = "help", + .data = N_("Get domain SEV attestation report") + }, + {.name = "desc", + .data = N_("Get an attestation report from a SEV-enabled domain") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domgetsevreport[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "mnonce", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_REQ_OPT, + .help = N_("random 16 bytes value encoded in base64 to be included in report)"), + }, + {.name = NULL} +}; + +static bool +cmdDomGetSevAttestationReport(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + const char *mnonce = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0, maxparams = 0; + bool ret = false; + char *report_str; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "mnonce", &mnonce) < 0) + return false; + + if (mnonce == NULL) + return false; + + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_SEV_ATTESTATION_REPORT_MNONCE, + mnonce) < 0) + return false; + + if (virDomainGetSevAttestationReport(dom, ¶ms, &nparams, 0) != 0) { + vshError(ctl, "%s", _("Unable to get SEV attestation report")); + goto cleanup; + } + + report_str = vshGetTypedParamValue(ctl, ¶ms[1]); + vshPrint(ctl, "base64-encoded attestation report: %s\n", report_str); + + ret = true; + +cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + /* * "qemu-monitor-command" command */ @@ -13827,6 +13889,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_domsetlaunchsecstate, .flags = 0 }, + {.name = "domgetsevreport", + .handler = cmdDomGetSevAttestationReport, + .opts = opts_domgetsevreport, + .info = info_domgetsevreport, + .flags = 0 + }, {.name = "domname", .handler = cmdDomname, .opts = opts_domname, -- 2.34.1
Just a quick ping so this patchset doesn't get lost in the list -- may I receive a review on this? On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested improvements.
Tyler Fanelli (5): libvirt: Introduce virDomainGetSevAttestationReport public API remote: add RPC support for the virDomainGetSevAttestationReport API qemu_capabilities: Introduce QEMU_CAPS_SEV_GET_ATTESTATION_REPORT qemu: Implement the virDomainGetSevAttestationReport API tools: add domgetsevreport virsh command
docs/manpages/virsh.rst | 18 ++++ include/libvirt/libvirt-domain.h | 22 +++++ src/driver-hypervisor.h | 7 ++ src/libvirt-domain.c | 63 ++++++++++++++ src/libvirt_public.syms | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 86 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 40 +++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/remote/remote_daemon_dispatch.c | 44 ++++++++++ src/remote/remote_driver.c | 55 ++++++++++++ src/remote/remote_protocol.x | 21 ++++- src/remote_protocol-structs | 12 +++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + tools/virsh-domain.c | 68 +++++++++++++++ 20 files changed, 466 insertions(+), 1 deletion(-)
-- Tyler Fanelli (tfanelli)
On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested improvements.
The proposed API name contains Sev, when all the existing domain APIs have generic names: virDomainGetLaunchSecurityInfo, virDomainSetLaunchSecurityState. I was thinking it would be nice to avoid that Sev specific bit. So I looked at upcoming SNP and TDX qemu patches to see if they add any qmp commands that take input and return a lot of output. Then maybe it would make sense to name this something like virDomainGetLaunchSecurityInfoWithParams which we could use more in the future. qemu SNP patches: https://github.com/AMDESE/qemu/tree/snp-v3 - Only extend existing query-sev and query-sev-capabilities qemu TDX patches: https://github.com/intel/qemu-tdx - Adds query-tdx and query-tdx-capabilities, both with no input So, that doesn't help. After that, my question is, what query-sev-attestation-report adds. The kernel commit explains what it is: https://github.com/torvalds/linux/commit/2c07ded06427dd3339278487a1413d5e478...
The SEV FW version >= 0.23 added a new command that can be used to query the attestation report containing the SHA-256 digest of the guest memory encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and sign the report with the Platform Endorsement Key (PEK).
See the SEV FW API spec section 6.8 for more details.
Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be used to get the SHA-256 digest. The main difference between the KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter can be called while the guest is running and the measurement value is signed with PEK.
So it is LAUNCH_MEASURE, available at VM runtime, signed with an extra key. qemu caches the LAUNCH_MEASURE value at VM startup and lets us query it with qmp any time, so I don't think runtime access matters. Maybe the extra key signing is a security fix or something. I haven't figured it out. But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely. CCing danpb for his thoughts - Cole
On 4/11/22 10:57 AM, Cole Robinson wrote:
On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested improvements. The proposed API name contains Sev, when all the existing domain APIs have generic names: virDomainGetLaunchSecurityInfo, virDomainSetLaunchSecurityState.
I was thinking it would be nice to avoid that Sev specific bit. So I looked at upcoming SNP and TDX qemu patches to see if they add any qmp commands that take input and return a lot of output. Then maybe it would make sense to name this something like virDomainGetLaunchSecurityInfoWithParams which we could use more in the future.
qemu SNP patches: https://github.com/AMDESE/qemu/tree/snp-v3 - Only extend existing query-sev and query-sev-capabilities
qemu TDX patches: https://github.com/intel/qemu-tdx - Adds query-tdx and query-tdx-capabilities, both with no input
So, that doesn't help.
What about adding the attestation report to virDomainGetLaunchSecurityInfo? If that route is taken, there would probably need to be some extra logic added to decipher getting launch security info on SEV vs. SGX/TDX, right?
After that, my question is, what query-sev-attestation-report adds. The kernel commit explains what it is: https://github.com/torvalds/linux/commit/2c07ded06427dd3339278487a1413d5e478...
The SEV FW version >= 0.23 added a new command that can be used to query the attestation report containing the SHA-256 digest of the guest memory encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and sign the report with the Platform Endorsement Key (PEK).
See the SEV FW API spec section 6.8 for more details.
Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be used to get the SHA-256 digest. The main difference between the KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter can be called while the guest is running and the measurement value is signed with PEK.
So it is LAUNCH_MEASURE, available at VM runtime, signed with an extra key.
qemu caches the LAUNCH_MEASURE value at VM startup and lets us query it with qmp any time, so I don't think runtime access matters.
I'm surprised by this. I was under the impression that LAUNCH_MEASURE always had to be called when a VM is paused.
Maybe the extra key signing is a security fix or something. I haven't figured it out.
Signing with the PEK also allows a user to verify the root of trust between the owner and the platform.
But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely.
Interesting, although wouldn't hardcoding an nonce basically render it useless? User-specified nonce would allow a user to verify that their call was propagated to firmware at that instance. If they can't supply the nonce, they can't verify it's an attestation report from that specific call.
CCing danpb for his thoughts
- Cole
Tyler. -- Tyler Fanelli (tfanelli)
On 4/14/22 2:46 PM, Tyler Fanelli wrote:
On 4/11/22 10:57 AM, Cole Robinson wrote:
On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested improvements. The proposed API name contains Sev, when all the existing domain APIs have generic names: virDomainGetLaunchSecurityInfo, virDomainSetLaunchSecurityState.
I was thinking it would be nice to avoid that Sev specific bit. So I looked at upcoming SNP and TDX qemu patches to see if they add any qmp commands that take input and return a lot of output. Then maybe it would make sense to name this something like virDomainGetLaunchSecurityInfoWithParams which we could use more in the future.
qemu SNP patches: https://github.com/AMDESE/qemu/tree/snp-v3 - Only extend existing query-sev and query-sev-capabilities
qemu TDX patches: https://github.com/intel/qemu-tdx - Adds query-tdx and query-tdx-capabilities, both with no input
So, that doesn't help.
What about adding the attestation report to virDomainGetLaunchSecurityInfo? If that route is taken, there would probably need to be some extra logic added to decipher getting launch security info on SEV vs. SGX/TDX, right?
The problem is virDomainGetLaunchSecurityInfo doesn't have any way to pass in the nonce, so we can't reuse that API as is.
After that, my question is, what query-sev-attestation-report adds. The kernel commit explains what it is: https://github.com/torvalds/linux/commit/2c07ded06427dd3339278487a1413d5e478...
The SEV FW version >= 0.23 added a new command that can be used to query the attestation report containing the SHA-256 digest of the guest memory encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and sign the report with the Platform Endorsement Key (PEK). See the SEV FW API spec section 6.8 for more details. Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be used to get the SHA-256 digest. The main difference between the KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter can be called while the guest is running and the measurement value is signed with PEK. So it is LAUNCH_MEASURE, available at VM runtime, signed with an extra key.
And with a nonce passed in, I forgot to mention that. That's another bit I'm not sure what it adds over the traditional measurement
qemu caches the LAUNCH_MEASURE value at VM startup and lets us query it with qmp any time, so I don't think runtime access matters.
I'm surprised by this. I was under the impression that LAUNCH_MEASURE always had to be called when a VM is paused.
Maybe the extra key signing is a security fix or something. I haven't figured it out.
Signing with the PEK also allows a user to verify the root of trust between the owner and the platform.
But I don't understand what that means in practice. I figured key management via the session blob already took care of this, but I haven't tried to wrap my head around the details.
But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely.
Interesting, although wouldn't hardcoding an nonce basically render it useless? User-specified nonce would allow a user to verify that their call was propagated to firmware at that instance. If they can't supply the nonce, they can't verify it's an attestation report from that specific call.
Sorry if I was unclear, I didn't mean hardcoding a specific nonce value. I meant that, if we decide we there's not much value in making this API generic, then we can strip out the virTypedParams usage and make the signature closer to: int virDomainGetSevAttestationReport(virDomainPtr domain, const char *mnonce, char *report); Thanks, Cole
On Thu, Apr 14, 2022 at 02:46:38PM -0400, Tyler Fanelli wrote:
On 4/11/22 10:57 AM, Cole Robinson wrote:
On 3/23/22 3:36 PM, Tyler Fanelli wrote:
This an RFC discussing a new API, virDomainGetSevAttestationReport (along with a virsh command "domgetsevreport"), with initial QEMU support via the "query-sev-attestation-report" QAPI mechanism. "query-sev-attestation-report" is supplied a base64-encoded 16 byte "mnonce" string as input, with a purpose of being embedded into the attestation report to provide protection.
My main point of concern is the design/communication of the virTypedParameterPtr exchanged between the client and libvirtd and how they interact together, as I have seen no other API follow the method I used. Namely, the same virTypedParameterPtr is used for both input _AND_ output. The same virTypedParameterPtr containing the original mnonce string inputted to the API is also used to contain the attestation report upon being returned from the API.
This contrasts with much of the APIs I've noticed, which use a virTypedParameterPtr for either input or output, but not both.
This patch is not final, as I still would like some human-readable outputting and storage of the attestation report.
Looking for thoughts on the design of this API, as well as suggested improvements. The proposed API name contains Sev, when all the existing domain APIs have generic names: virDomainGetLaunchSecurityInfo, virDomainSetLaunchSecurityState.
I was thinking it would be nice to avoid that Sev specific bit. So I looked at upcoming SNP and TDX qemu patches to see if they add any qmp commands that take input and return a lot of output. Then maybe it would make sense to name this something like virDomainGetLaunchSecurityInfoWithParams which we could use more in the future.
qemu SNP patches: https://github.com/AMDESE/qemu/tree/snp-v3 - Only extend existing query-sev and query-sev-capabilities
qemu TDX patches: https://github.com/intel/qemu-tdx - Adds query-tdx and query-tdx-capabilities, both with no input
So, that doesn't help.
What about adding the attestation report to virDomainGetLaunchSecurityInfo? If that route is taken, there would probably need to be some extra logic added to decipher getting launch security info on SEV vs. SGX/TDX, right?
After that, my question is, what query-sev-attestation-report adds. The kernel commit explains what it is: https://github.com/torvalds/linux/commit/2c07ded06427dd3339278487a1413d5e478...
The SEV FW version >= 0.23 added a new command that can be used to query the attestation report containing the SHA-256 digest of the guest memory encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and sign the report with the Platform Endorsement Key (PEK). See the SEV FW API spec section 6.8 for more details. Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be used to get the SHA-256 digest. The main difference between the KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter can be called while the guest is running and the measurement value is signed with PEK.
So it is LAUNCH_MEASURE, available at VM runtime, signed with an extra key.
qemu caches the LAUNCH_MEASURE value at VM startup and lets us query it with qmp any time, so I don't think runtime access matters.
I'm surprised by this. I was under the impression that LAUNCH_MEASURE always had to be called when a VM is paused.
Yep, QEMU registers a callback that fetches the launch measurement from SEV platform when the machine init is done. The query-sev-launch-measure command always returns cached data, so there's no restriction on when we can call it from QEMU.
Maybe the extra key signing is a security fix or something. I haven't figured it out.
Signing with the PEK also allows a user to verify the root of trust between the owner and the platform.
The guest owner needs to acquire the PDH before they launch their guest, in order to generate the launch blob. The PDH is signed with the PEK, so they will have surely verified the root of trust with the platform before they even generate the launch blob for the VM. The generated launch blob can't be used on any other host, because the other host won't have the same PDH. If the launch measurement is signed with the PEK, the guest owner has the option of postponing their validation of the root of trust until after the VM is launched. Is that something we need to be able to deal with ? I'm not sure why one would want to postpone validation gven that we're already forced to do other stuff else upfront with SEV/SEV-ES. In the absence of a clearly stated requirement from an application I think we can ignore this. SEV-SNP is of course very different with its guest initiated post launch attestation.
But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely.
Interesting, although wouldn't hardcoding an nonce basically render it useless? User-specified nonce would allow a user to verify that their call was propagated to firmware at that instance. If they can't supply the nonce, they can't verify it's an attestation report from that specific call.
The launch blob contains a unique TIK/TEK pair, so if the launch measurement validates, the guest owner knows it is associated with a running VM that was created with their designated launch blob. A nonce is usually needed to avoid replay attacks, but I'm not seeing what attack vector is actually present in the SEV/SEV-ES scenario, since AFAIK, the attestation report content never changes once the VM is running. Overall I'm not seeing the need for this API with SEV/SEV-ES at least, and with SEV-SNP IIUC the attestation report is not available to the host, only to the guest ? With 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 4/20/22 5:45 AM, Daniel P. Berrangé wrote:
On Thu, Apr 14, 2022 at 02:46:38PM -0400, Tyler Fanelli wrote:
Maybe the extra key signing is a security fix or something. I haven't figured it out. Signing with the PEK also allows a user to verify the root of trust between
On 4/11/22 10:57 AM, Cole Robinson wrote: the owner and the platform. The guest owner needs to acquire the PDH before they launch their guest, in order to generate the launch blob. The PDH is signed with the PEK, so they will have surely verified the root of trust with the platform before they even generate the launch blob for the VM. The generated launch blob can't be used on any other host, because the other host won't have the same PDH.
If the launch measurement is signed with the PEK, the guest owner has the option of postponing their validation of the root of trust until after the VM is launched. Is that something we need to be able to deal with ? I'm not sure why one would want to postpone validation gven that we're already forced to do other stuff else upfront with SEV/SEV-ES. In the absence of a clearly stated requirement from an application I think we can ignore this.
SEV-SNP is of course very different with its guest initiated post launch attestation.
I see, then the use-case of postponing the validation until after VM launch doesn't make much sense for SEV/SEV-ES given that everything should be verified upfront.
But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely. Interesting, although wouldn't hardcoding an nonce basically render it useless? User-specified nonce would allow a user to verify that their call was propagated to firmware at that instance. If they can't supply the nonce, they can't verify it's an attestation report from that specific call. The launch blob contains a unique TIK/TEK pair, so if the launch measurement validates, the guest owner knows it is associated with a running VM that was created with their designated launch blob.
A nonce is usually needed to avoid replay attacks, but I'm not seeing what attack vector is actually present in the SEV/SEV-ES scenario, since AFAIK, the attestation report content never changes once the VM is running.
Overall I'm not seeing the need for this API with SEV/SEV-ES at least, and with SEV-SNP IIUC the attestation report is not available to the host, only to the guest ?
Realizing that my assumption of LAUNCH_MEASURE needing to be called while VM is paused is false, I tend to agree. With that in mind, what is the point of "query-sev-attestation-report" in QEMU? What was it's original purpose if it offers no real benefits compared to "query-sev-launch-measure"?
With regards, Daniel
-- Tyler Fanelli (tfanelli)
On Thu, Apr 21, 2022 at 12:35:27PM -0400, Tyler Fanelli wrote:
On 4/20/22 5:45 AM, Daniel P. Berrangé wrote:
But as is it's not clear what this buys us over the launch measurement we already report with virDomainGetLaunchSecurityInfo
If we figure out what the point of this is, IMO we can more easily reason about whether it makes sense to add a Sev specific libvirt API, and whether we need virTypedParams for both input and output. For example if the API really is specific to this one and only KVM ioctl/QMP command, we could hardcode the parameters and skip the virTypedParams question entirely. Interesting, although wouldn't hardcoding an nonce basically render it useless? User-specified nonce would allow a user to verify that their call was propagated to firmware at that instance. If they can't supply the nonce, they can't verify it's an attestation report from that specific call. The launch blob contains a unique TIK/TEK pair, so if the launch measurement validates, the guest owner knows it is associated with a running VM that was created with their designated launch blob.
A nonce is usually needed to avoid replay attacks, but I'm not seeing what attack vector is actually present in the SEV/SEV-ES scenario, since AFAIK, the attestation report content never changes once the VM is running.
Overall I'm not seeing the need for this API with SEV/SEV-ES at least, and with SEV-SNP IIUC the attestation report is not available to the host, only to the guest ?
Realizing that my assumption of LAUNCH_MEASURE needing to be called while VM is paused is false, I tend to agree. With that in mind, what is the point of "query-sev-attestation-report" in QEMU? What was it's original purpose if it offers no real benefits compared to "query-sev-launch-measure"?
I'm thinking the author didn't rememeber that we cached LAUNCH_MEASURE in QEMU. With 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 :|
participants (3)
-
Cole Robinson -
Daniel P. Berrangé -
Tyler Fanelli