8:23 p.m.
Hi All!
This series is an RFC impl for the missing API needed for SEV attestation as
discussed here
https://listman.redhat.com/archives/libvir-list/2021-May/msg00196.html
I pinged about the status a few weeks back, found it had stalled, and agreed
to work on an impl after returning from vacation
https://listman.redhat.com/archives/libvir-list/2021-October/msg01052.html
Although the series is only compile tested, I wanted to share it early in
case others are considering the task.
While discussing the missing API, Daniel suggested virDomainSetLaunchSecurityInfo
https://listman.redhat.com/archives/libvir-list/2021-October/msg01074.html
but noted the asymmetry with virDomainGetLaunchSecurityInfo. I decided to go
with virDomainInjectLaunchSecret, which better describes the function IMO.
I also decided to go with an explicit set of parameters, following in the
footsteps of virDrvDomainAuthorizedSSHKeys*. It wasn't until patch 3 that I
realized virTypedParameter is definitely a better approach for an API that
may need future support for other types of secrets. I'll make that change in
a V1 after collecting feedback on this RFC.
Regards,
Jim
Jim Fehlig (3):
libvirt: Introduce virDomainInjectLaunchSecret public API
remote: Implement domain inject launch secret API
qemu: Implement the virDomainInjectLaunchSecret API
include/libvirt/libvirt-domain.h | 6 ++++
src/driver-hypervisor.h | 8 +++++
src/libvirt-domain.c | 50 +++++++++++++++++++++++++++
src/libvirt_public.syms | 5 +++
src/qemu/qemu_driver.c | 53 +++++++++++++++++++++++++++++
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 +++
src/remote/remote_daemon_dispatch.c | 27 +++++++++++++++
src/remote/remote_driver.c | 32 +++++++++++++++++
src/remote/remote_protocol.x | 16 ++++++++-
src/remote_protocol-structs | 8 +++++
13 files changed, 261 insertions(+), 1 deletion(-)
--
2.33.0
8:23 p.m.
New subject: [RFC PATCH 1/3] libvirt: Introduce virDomainInjectLaunchSecret public API
An API inject a launch secret into the domain's memory.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
include/libvirt/libvirt-domain.h | 6 ++++
src/driver-hypervisor.h | 8 +++++
src/libvirt-domain.c | 50 ++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 69 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 2f017c5b68..418ee4bd2d 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5091,6 +5091,12 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
int *nparams,
unsigned int flags);
+int virDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ 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 d642af8a37..a308754d5b 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1333,6 +1333,13 @@ typedef int
int *nparams,
unsigned int flags);
+typedef int
+(*virDrvDomainInjectLaunchSecret)(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags);
+
typedef virDomainCheckpointPtr
(*virDrvDomainCheckpointCreateXML)(virDomainPtr domain,
const char *xmlDesc,
@@ -1661,6 +1668,7 @@ struct _virHypervisorDriver {
virDrvConnectBaselineHypervisorCPU connectBaselineHypervisorCPU;
virDrvNodeGetSEVInfo nodeGetSEVInfo;
virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
+ virDrvDomainInjectLaunchSecret domainInjectLaunchSecret;
virDrvDomainCheckpointCreateXML domainCheckpointCreateXML;
virDrvDomainCheckpointGetXMLDesc domainCheckpointGetXMLDesc;
virDrvDomainListAllCheckpoints domainListAllCheckpoints;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ce7cafde36..877c65c04f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12818,6 +12818,56 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
}
+/**
+ * virDomainInjectLaunchSecret:
+ * @domain: a domain object
+ * @secrethdr: Base64 encoded secret header
+ * @secret: Base64 encoded secret
+ * @injectaddr: Domain memory address where the secret will be injected
+ * @flags: currently used, set to 0.
+ *
+ * Inject a launch secret in the domain's memory. secrethdr and secret are
+ * passed to the underlying hypervisor as is. injectaddr can be used to
+ * specify an address in the domain memory where the secret will be injected.
+ * It can be set to 0 for the hypervisor default.
+ *
+ * Returns -1 in case of failure, 0 in case of success.
+ */
+int virDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags)
+{
+ virConnectPtr conn = domain->conn;
+
+ VIR_DOMAIN_DEBUG(domain, "secrethdr=%p, secret=%p injectaddr=%llu
flags=0x%x",
+ secrethdr, secret, injectaddr, flags);
+
+ virResetLastError();
+
+ virCheckDomainReturn(domain, -1);
+ virCheckNonNullArgGoto(secrethdr, error);
+ virCheckNonNullArgGoto(secret, error);
+ virCheckPositiveArgGoto(injectaddr, error);
+ virCheckReadOnlyGoto(conn->flags, error);
+
+ if (conn->driver->domainInjectLaunchSecret) {
+ int ret;
+ ret = conn->driver->domainInjectLaunchSecret(domain, secrethdr,
+ secret, injectaddr, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+ virReportUnsupportedError();
+
+ error:
+ virDispatchError(domain->conn);
+ return -1;
+}
+
+
/**
* virDomainAgentSetResponseTimeout:
* @domain: a domain object
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 788a967df7..c5e708d475 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -911,4 +911,9 @@ LIBVIRT_7.8.0 {
virNetworkCreateXMLFlags;
} LIBVIRT_7.7.0;
+LIBVIRT_7.10.0 {
+ global:
+ virDomainInjectLaunchSecret;
+} LIBVIRT_7.8.0;
+
# .... define new API here using predicted next version number ....
--
2.33.0
4:33 a.m.
New subject: [RFC PATCH 1/3] libvirt: Introduce virDomainInjectLaunchSecret public API
On Tue, Nov 16, 2021 at 19:23:52 -0700, Jim Fehlig wrote:
An API inject a launch secret into the domain's memory.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
include/libvirt/libvirt-domain.h | 6 ++++
src/driver-hypervisor.h | 8 +++++
src/libvirt-domain.c | 50 ++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 69 insertions(+)
Note that I don't know enough about SEV to do a full review. These are
merely observations based on the interface of qemu and the documentation
for them.
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ce7cafde36..877c65c04f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12818,6 +12818,56 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
}
+/**
+ * virDomainInjectLaunchSecret:
+ * @domain: a domain object
+ * @secrethdr: Base64 encoded secret header
+ * @secret: Base64 encoded secret
+ * @injectaddr: Domain memory address where the secret will be injected
+ * @flags: currently used, set to 0.
+ *
+ * Inject a launch secret in the domain's memory. secrethdr and secret are
+ * passed to the underlying hypervisor as is.
While this might be true for qemu IMO it doesn't make much sense to
mention this in the documentation because it's not exactly important to
the user that it's not modified.
injectaddr can be used to
+ * specify an address in the domain memory where the secret will be injected.
+ * It can be set to 0 for the hypervisor default.
This makes an assumption that address 0 will never be used to inject the
secret. Note that qemu actually supports that as the 'gpa' attribute of
the command is optional, so you can populate it with a 0 to inject into
address 0.
In general the API feels too-much tied to the implementation of sev used
here and IMO isn't future proof, but I don't have enough examples to
prove my point.
11:47 a.m.
New subject: [RFC PATCH 1/3] libvirt: Introduce virDomainInjectLaunchSecret public API
On 11/17/21 03:33, Peter Krempa wrote:
On Tue, Nov 16, 2021 at 19:23:52 -0700, Jim Fehlig wrote:
> An API inject a launch secret into the domain's memory.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> include/libvirt/libvirt-domain.h | 6 ++++
> src/driver-hypervisor.h | 8 +++++
> src/libvirt-domain.c | 50 ++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 ++++
> 4 files changed, 69 insertions(+)
Note that I don't know enough about SEV to do a full review. These are
merely observations based on the interface of qemu and the documentation
for them.
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ce7cafde36..877c65c04f 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12818,6 +12818,56 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> }
>
>
> +/**
> + * virDomainInjectLaunchSecret:
> + * @domain: a domain object
> + * @secrethdr: Base64 encoded secret header
> + * @secret: Base64 encoded secret
> + * @injectaddr: Domain memory address where the secret will be injected
> + * @flags: currently used, set to 0.
> + *
> + * Inject a launch secret in the domain's memory. secrethdr and secret are
> + * passed to the underlying hypervisor as is.
While this might be true for qemu IMO it doesn't make much sense to
mention this in the documentation because it's not exactly important to
the user that it's not modified.
Ok. I need to improve the description in a V1.
> injectaddr can be used to
> + * specify an address in the domain memory where the secret will be injected.
> + * It can be set to 0 for the hypervisor default.
This makes an assumption that address 0 will never be used to inject the
secret. Note that qemu actually supports that as the 'gpa' attribute of
the command is optional, so you can populate it with a 0 to inject into
address 0.
Good point, thanks!
In general the API feels too-much tied to the implementation of sev
used
here and IMO isn't future proof, but I don't have enough examples to
prove my point.
Using virTypedParams for input will help with future-proofness. I'm still not
fond of the API name. I see Daniel has suggested a better name. I'll respond to
his post with some other ideas I had.
Regards,
Jim
11:28 a.m.
New subject: [RFC PATCH 1/3] libvirt: Introduce virDomainInjectLaunchSecret public API
On Tue, Nov 16, 2021 at 07:23:52PM -0700, Jim Fehlig wrote:
An API inject a launch secret into the domain's memory.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
include/libvirt/libvirt-domain.h | 6 ++++
src/driver-hypervisor.h | 8 +++++
src/libvirt-domain.c | 50 ++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 69 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 2f017c5b68..418ee4bd2d 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5091,6 +5091,12 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
int *nparams,
unsigned int flags);
+int virDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags);
I thought of a better name at last, that shows its relation
to virDomainGetLaunchSecurityInfo without implying that they
are the direct inverse of each other:
virDomainSetLaunchSecurityState(...)
Also, we whould bear in mind that the set of state parameters
may be differnt for vendors other than AMD, and even later
generations of AMD SEV might want more parameters.
So lets use a 'virTypedParameter' array for this methodeg
virDomainSetLaunchSecurityState(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
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 :|
12:02 p.m.
New subject: [RFC PATCH 1/3] libvirt: Introduce virDomainInjectLaunchSecret public API
On 11/23/21 10:28, Daniel P. Berrangé wrote:
On Tue, Nov 16, 2021 at 07:23:52PM -0700, Jim Fehlig wrote:
> An API inject a launch secret into the domain's memory.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> include/libvirt/libvirt-domain.h | 6 ++++
> src/driver-hypervisor.h | 8 +++++
> src/libvirt-domain.c | 50 ++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 ++++
> 4 files changed, 69 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 2f017c5b68..418ee4bd2d 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5091,6 +5091,12 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> int *nparams,
> unsigned int flags);
>
> +int virDomainInjectLaunchSecret(virDomainPtr domain,
> + const char *secrethdr,
> + const char *secret,
> + unsigned long long injectaddr,
> + unsigned int flags);
I thought of a better name at last, that shows its relation
to virDomainGetLaunchSecurityInfo without implying that they
are the direct inverse of each other:
virDomainSetLaunchSecurityState(...)
I need to get over my distaste for 'launch' in the API name.
virDomainGetLaunchSecurityInfo already exists, so no changing that. And not
including 'launch' in the Set API would be a source of confusion. If we were
creating the names anew, I'd prefer something like virDomain{Get,Set}PrestartSecret.
Also, we whould bear in mind that the set of state parameters
may be differnt for vendors other than AMD, and even later
generations of AMD SEV might want more parameters.
Nod.
So lets use a 'virTypedParameter' array for this methodeg
Right. I mentioned that in the cover letter. While hacking on patch3 I realized
explicit params was a no-go :-).
virDomainSetLaunchSecurityState(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
Thanks! I'll include this in a V1.
Regards,
Jim
8:23 p.m.
New subject: [RFC PATCH 2/3] remote: Implement domain inject launch secret API
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/remote/remote_daemon_dispatch.c | 27 ++++++++++++++++++++++++
src/remote/remote_driver.c | 32 +++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 16 ++++++++++++++-
src/remote_protocol-structs | 8 ++++++++
4 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 689001889e..f2f7b35f53 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3451,6 +3451,33 @@ remoteDispatchDomainGetLaunchSecurityInfo(virNetServer *server
G_GNUC_UNUSED,
return rv;
}
+static int
+remoteDispatchDomainInjectLaunchSecret(virNetServer *server G_GNUC_UNUSED,
+ virNetServerClient *client,
+ virNetMessage *msg G_GNUC_UNUSED,
+ struct virNetMessageError *rerr,
+ remote_domain_inject_launch_secret_args *args)
+{
+ int rv = -1;
+ virConnectPtr conn = remoteGetHypervisorConn(client);
+ virDomainPtr dom = NULL;
+
+ if (!conn)
+ goto cleanup;
+
+ if (!(dom = get_nonnull_domain(conn, args->dom)))
+ goto cleanup;
+
+ rv = virDomainInjectLaunchSecret(dom, args->secrethdr, args->secret,
+ args->injectaddr, args->flags);
+
+ cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ virObjectUnref(dom);
+ return rv;
+}
+
static int
remoteDispatchDomainGetPerfEvents(virNetServer *server G_GNUC_UNUSED,
virNetServerClient *client,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 235c406a5a..4fbb3c5bad 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1893,6 +1893,37 @@ remoteDomainGetLaunchSecurityInfo(virDomainPtr domain,
return rv;
}
+static int
+remoteDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags)
+{
+ int rv = -1;
+ struct private_data *priv = domain->conn->privateData;
+ remote_domain_inject_launch_secret_args args;
+
+ remoteDriverLock(priv);
+
+ make_nonnull_domain(&args.dom, domain);
+ args.secrethdr = (char *) secrethdr;
+ args.secret = (char *) secret;
+ args.injectaddr = injectaddr;
+ args.flags = flags;
+
+ if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_INJECT_LAUNCH_SECRET,
+ (xdrproc_t) xdr_remote_domain_inject_launch_secret_args, (char *)
&args,
+ (xdrproc_t) xdr_void, (char *) NULL) == -1)
+ goto done;
+
+ rv = 0;
+
+ done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
static int
remoteDomainGetPerfEvents(virDomainPtr domain,
virTypedParameterPtr *params,
@@ -8574,6 +8605,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0 */
.domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */
.domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */
+ .domainInjectLaunchSecret = remoteDomainInjectLaunchSecret, /* 7.10.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 60010778ca..fb0da81e9a 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3900,6 +3900,14 @@ struct remote_domain_event_memory_device_size_change_msg {
unsigned hyper size;
};
+struct remote_domain_inject_launch_secret_args {
+ remote_nonnull_domain dom;
+ remote_nonnull_string secrethdr;
+ remote_nonnull_string secret;
+ unsigned hyper injectaddr;
+ unsigned int flags;
+};
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -6905,5 +6913,11 @@ enum remote_procedure {
* @generate: both
* @acl: none
*/
- REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438
+ REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
+
+ /**
+ * @generate: none
+ * @acl: domain:write
+ */
+ REMOTE_PROC_DOMAIN_INJECT_LAUNCH_SECRET = 439
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index dbef4ace79..c9e26b0ce1 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -3241,6 +3241,13 @@ struct remote_domain_event_memory_device_size_change_msg {
remote_nonnull_string alias;
uint64_t size;
};
+struct remote_domain_inject_launch_secret_args {
+ remote_nonnull_domain dom;
+ remote_nonnull_string secrethdr;
+ remote_nonnull_string secret;
+ uint64_t injectaddr;
+ u_int flags;
+};
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3680,4 +3687,5 @@ enum remote_procedure {
REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 436,
REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437,
REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
+ REMOTE_PROC_DOMAIN_INJECT_LAUNCH_SECRET = 439,
};
--
2.33.0
8:23 p.m.
New subject: [RFC PATCH 3/3] qemu: Implement the virDomainInjectLaunchSecret API
Inject a launch secret in domain memory using the sev-inject-launch-secret
QMP API. Only supported for SEV-enabed domains.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++
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, 110 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d954635dde..58e3f08afe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20104,6 +20104,58 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
return ret;
}
+
+static int
+qemuDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags)
+{
+ virQEMUDriver *driver = domain->conn->privateData;
+ virDomainObj *vm;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomainObjFromDomain(domain)))
+ goto cleanup;
+
+ if (virDomainInjectLaunchSecretEnsureACL(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",
+ _("injecting a launch secret is only supported in SEV-enabled
domains"));
+ goto cleanup;
+ }
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+ goto endjob;
+
+ if (qemuMonitorInjectLaunchSecret(QEMU_DOMAIN_PRIVATE(vm)->mon,
+ secrethdr, secret, injectaddr) < 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 |
@@ -20981,6 +21033,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */
.domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */
.domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */
+ .domainInjectLaunchSecret = qemuDomainInjectLaunchSecret, /* 7.10.0 */
};
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 810dac209d..c64469a03b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4383,6 +4383,18 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon)
}
+int
+qemuMonitorInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr)
+{
+ QEMU_CHECK_MONITOR(mon);
+
+ return qemuMonitorJSONInjectLaunchSecret(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 0dd7b1c4e2..2dec2b57bb 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1445,6 +1445,12 @@ int qemuMonitorBlockdevMediumInsert(qemuMonitor *mon,
char *
qemuMonitorGetSEVMeasurement(qemuMonitor *mon);
+int
+qemuMonitorInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned 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 4669b9135d..69aef078ec 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8124,6 +8124,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
+qemuMonitorJSONInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned 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)))
+ 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..95758cdc6e 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 qemuMonitorJSONInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr);
+
int qemuMonitorJSONGetVersion(qemuMonitor *mon,
int *major,
int *minor,
--
2.33.0
4:39 a.m.
New subject: [RFC PATCH 3/3] qemu: Implement the virDomainInjectLaunchSecret API
On Tue, Nov 16, 2021 at 19:23:54 -0700, Jim Fehlig wrote:
Inject a launch secret in domain memory using the
sev-inject-launch-secret
QMP API. Only supported for SEV-enabed domains.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++
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, 110 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d954635dde..58e3f08afe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20104,6 +20104,58 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
return ret;
}
+
+static int
+qemuDomainInjectLaunchSecret(virDomainPtr domain,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr,
+ unsigned int flags)
+{
+ virQEMUDriver *driver = domain->conn->privateData;
+ virDomainObj *vm;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomainObjFromDomain(domain)))
+ goto cleanup;
+
+ if (virDomainInjectLaunchSecretEnsureACL(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",
+ _("injecting a launch secret is only supported in
SEV-enabled domains"));
+ goto cleanup;
+ }
Missing check that the VM is running.
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+ goto endjob;
No need for async monitor without an async job
+
+ if (qemuMonitorInjectLaunchSecret(QEMU_DOMAIN_PRIVATE(vm)->mon,
+ secrethdr, secret, injectaddr) < 0)
+ goto endjob;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto endjob;
+
+ ret = 0;
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
[...]
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 810dac209d..c64469a03b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4383,6 +4383,18 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon)
}
+int
+qemuMonitorInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned long long injectaddr)
+{
+ QEMU_CHECK_MONITOR(mon);
You cleverly avoid logging the secret here ...
+
+ return qemuMonitorJSONInjectLaunchSecret(mon, secrethdr, secret, injectaddr);
+}
+
+
int
qemuMonitorGetPRManagerInfo(qemuMonitor *mon,
GHashTable **retinfo)
[...]
diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
index 4669b9135d..69aef078ec 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8124,6 +8124,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
+qemuMonitorJSONInjectLaunchSecret(qemuMonitor *mon,
+ const char *secrethdr,
+ const char *secret,
+ unsigned 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)))
+ return -1;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
... but once this is sent to the monitor it can be logged if QMP traffic
logging is enabled.
Thus either the secret is not so secret, but the qemu documentation nor
the documentation you've added seem to be lessening the secrecy of the
passed string in any way, or the 'sev-inject-launch-secret' is broken by
design as it doesn't support passing an encrypted object via qcrypto.
In case when the secret must really be kept secret the QMP command must
be fixed first, otherwise libvirt is not able to guarantee any secrecy.
+ return -1;
+
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ return -1;
+
+ return 0;
+}
12:15 p.m.
New subject: [RFC PATCH 3/3] qemu: Implement the virDomainInjectLaunchSecret API
On 11/17/21 03:39, Peter Krempa wrote:
On Tue, Nov 16, 2021 at 19:23:54 -0700, Jim Fehlig wrote:
> Inject a launch secret in domain memory using the sev-inject-launch-secret
> QMP API. Only supported for SEV-enabed domains.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++
> 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, 110 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d954635dde..58e3f08afe 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20104,6 +20104,58 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
> return ret;
> }
>
> +
> +static int
> +qemuDomainInjectLaunchSecret(virDomainPtr domain,
> + const char *secrethdr,
> + const char *secret,
> + unsigned long long injectaddr,
> + unsigned int flags)
> +{
> + virQEMUDriver *driver = domain->conn->privateData;
> + virDomainObj *vm;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + if (!(vm = qemuDomainObjFromDomain(domain)))
> + goto cleanup;
> +
> + if (virDomainInjectLaunchSecretEnsureACL(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",
> + _("injecting a launch secret is only supported in
SEV-enabled domains"));
> + goto cleanup;
> + }
Missing check that the VM is running.
> +
> + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> + goto cleanup;
> +
> + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> + goto endjob;
No need for async monitor without an async job
> +
> + if (qemuMonitorInjectLaunchSecret(QEMU_DOMAIN_PRIVATE(vm)->mon,
> + secrethdr, secret, injectaddr) < 0)
> + goto endjob;
> +
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto endjob;
> +
> + ret = 0;
> +
> + endjob:
> + qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:
[...]
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 810dac209d..c64469a03b 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4383,6 +4383,18 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon)
> }
>
>
> +int
> +qemuMonitorInjectLaunchSecret(qemuMonitor *mon,
> + const char *secrethdr,
> + const char *secret,
> + unsigned long long injectaddr)
> +{
> + QEMU_CHECK_MONITOR(mon);
You cleverly avoid logging the secret here ...
> +
> + return qemuMonitorJSONInjectLaunchSecret(mon, secrethdr, secret, injectaddr);
> +}
> +
> +
> int
> qemuMonitorGetPRManagerInfo(qemuMonitor *mon,
> GHashTable **retinfo)
[...]
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 4669b9135d..69aef078ec 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8124,6 +8124,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
> +qemuMonitorJSONInjectLaunchSecret(qemuMonitor *mon,
> + const char *secrethdr,
> + const char *secret,
> + unsigned 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)))
> + return -1;
> +
> + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
... but once this is sent to the monitor it can be logged if QMP traffic
logging is enabled.
Thus either the secret is not so secret, but the qemu documentation nor
the documentation you've added seem to be lessening the secrecy of the
passed string in any way, or the 'sev-inject-launch-secret' is broken by
design as it doesn't support passing an encrypted object via qcrypto.
AFAICT, the secret is an encrypted object. See section "6.6 LAUNCH_SECRET" in
the following doc
https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
I'll need to do more snooping so I can improve the API documentation in 1/3. A
clearer description of the API and its parameters would help with questions like
this.
Regards,
Jim
In case when the secret must really be kept secret the QMP command must
be fixed first, otherwise libvirt is not able to guarantee any secrecy.
> + return -1;
> +
> + if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> + return -1;
> +
> + return 0;
> +}
1095
days inactive
1101
days old
9 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Jim Fehlig -
Peter Krempa