[PATCH 1/3] libvirt: Introduce virDomainSetLaunchSecurityState public API

[Jim Fehlig]

On 12/3/21 08:06, Daniel P. Berrangé wrote:
> On Tue, Nov 30, 2021 at 04:51:58PM -0700, Jim Fehlig wrote:
>> This API allows setting a launch secret within a guests's memory. The
>> launch secret is created by the guest owner after retrieving and
>> verifying the launch measurement with virDomainGetLaunchSecurityInfo.
>>
>> The API uses virTypedParameter for input, allowing it to be expanded
>> to support other confidential computing technologies. In the case of
>> SEV, a basic guest launch workflow is described in the SEV API spec
>> in section "1.3.1 Launch"
>>
>> https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
>>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>   include/libvirt/libvirt-domain.h | 35 +++++++++++++++++++++
>>   src/driver-hypervisor.h          |  7 +++++
>>   src/libvirt-domain.c             | 52 ++++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms          |  5 +++
>>   4 files changed, 99 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index 2f017c5b68..7af634cfb2 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -5086,11 +5086,46 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>    */
>>   # define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
>>   
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER:
>> + *
>> + * A macro used to represent the SEV launch secret header. The secret header
>> + * is a base64-encoded VIR_TYPED_PARAM_STRING containing artifacts needed by
>> + * the SEV firmware to recover the plain text of the launch secret. See
>> + * section "6.6 LAUNCH_SECRET" in the SEV API specification for a detailed
>> + * description of the secret header.
>> + */
>> +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER "sev-secret-header"
>> +
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET:
>> + *
>> + * A macro used to represent the SEV launch secret. The secret is a
>> + * base64-encoded VIR_TYPED_PARAM_STRING containing an encrypted launch
>> + * secret. The secret is created by the domain owner after the SEV launch
>> + * measurement is retrieved and verified.
>> + */
>> +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET "sev-secret"
>> +
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS:
>> + *
>> + * A macro used to represent the physical address within the guest's memory
>> + * where the secret will be set, as VIR_TYPED_PARAM_LLONG. If not specified,
>> + * the address will be determined by the hypervisor.
>> + */
>> +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS "sev-secret-set-address"
> 
> Should we be using ULLONG here, since it probably doesn't make
> sense for an address to be negative ?

Yes. I let a problem I was trying to solve in 3/3 leak out here. I'll describe 
the problem when responding to 3/3.

> 
>> +
>>   int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>>                                      virTypedParameterPtr *params,
>>                                      int *nparams,
>>                                      unsigned int flags);
>>   
>> +int virDomainSetLaunchSecurityState(virDomainPtr domain,
>> +                                    virTypedParameterPtr params,
>> +                                    int nparams,
>> +                                    unsigned int flags);
>> +
> 
>> +/**
>> + * virDomainSetLaunchSecurityState:
>> + * @domain: a domain object
>> + * @params: pointer to launch security parameter objects
>> + * @nparams: number of launch security parameters
>> + * @flags: currently used, set to 0.
>> + *
>> + * Set a launch security secret in the guests's memory. The secret is created
>> + * by the guest owner after retrieving and verifying the launch measurement
>> + * with virDomainGetLaunchSecurityInfo.
>> + *
>> + * See VIR_DOMAIN_LAUNCH_SECURITY_* for a detailed description of accepted
>> + * launch security parameters.
> 
> Should we note something  about expected VM state/lifecycle
> 
>    When invoked, the VM must be running but with vCPUs paused.
>    This can be achieved by passing VIR_DOMAIN_START_PAUSED
>    to virDOmainCreate. If this method returns success, the
>    vCPUs can be started with virDomainResume. On error the
>    virtual machine should be destroy.

I'll include this and look at other doc improvements in V2.

 From libvirt perspective, I can document the entire workflow in the existing 
SEV kbase article, but will need to gain some hands on experience beforehand. 
That can be done independent of this series.

Regards,
Jim

>    
> 
>> + *
>> + * Returns -1 in case of failure, 0 in case of success.
>> + */
>> +int virDomainSetLaunchSecurityState(virDomainPtr domain,
>> +                                    virTypedParameterPtr params,
>> +                                    int nparams,
>> +                                    unsigned int flags)
>> +{
>> +    virConnectPtr 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);
> 
> Need a virCheckReadOnlyGoto too
> 
>> +    if (virTypedParameterValidateSet(conn, params, nparams) < 0)
>> +        goto error;
>> +
>> +    if (conn->driver->domainSetLaunchSecurityState) {
>> +        int ret;
>> +        ret = conn->driver->domainSetLaunchSecurityState(domain, params,
>> +                                                         nparams, flags);
>> +        if (ret < 0)
>> +            goto error;
>> +        return ret;
>> +    }
>> +    virReportUnsupportedError();
>> +
>> + error:
>> +    virDispatchError(domain->conn);
>> +    return -1;
>> +}
> 
> Regards,
> Daniel
>