An API inject a launch secret into the domain's memory.
Signed-off-by: Jim Fehlig <jfehlig@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
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@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.
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@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
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@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 :|
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@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
© 2016 - 2026 Red Hat, Inc.