[libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info

Brijesh Singh posted 9 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Brijesh Singh 7 years, 2 months ago
The API can be used outside the libvirt to get the launch security
information. When SEV is enabled, the API can be used to get the
measurement of the launch process.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 include/libvirt/libvirt-domain.h | 17 ++++++++++++++
 src/driver-hypervisor.h          |  7 ++++++
 src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
 src/libvirt_public.syms          |  5 ++++
 4 files changed, 79 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4048acf38aaf..11c3fec92bfa 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
                                 unsigned int action,
                                 unsigned int flags);
 
+/**
+ * Launch Security API
+ */
+
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
+ *
+ * Macro represents the launch measurement of the SEV guest,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
+
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                   virTypedParameterPtr params,
+                                   int *nparams,
+                                   unsigned int flags);
+
 #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index ce0e2b252552..dc4873a8ad1c 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1283,6 +1283,12 @@ typedef int
                                   unsigned int action,
                                   unsigned int flags);
 
+typedef int
+(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
+                                     virTypedParameterPtr params,
+                                     int *nparams,
+                                     unsigned int flags);
+
 
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
     virDrvDomainSetVcpu domainSetVcpu;
     virDrvDomainSetBlockThreshold domainSetBlockThreshold;
     virDrvDomainSetLifecycleAction domainSetLifecycleAction;
+    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
 };
 
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index eaec0979ad49..21356bb92894 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
     virDispatchError(domain->conn);
     return -1;
 }
+
+/**
+ * virDomainGetLaunchSecurityInfo:
+ * @domain: a domain object
+ * @params: where to store security info
+ * @nparams: number of items in @params
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Get the launch security info. In case of the SEV guest, this will
+ * return the launch measurement.
+ *
+ * Returns -1 in case of failure, 0 in case of success.
+ */
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+                                   virTypedParameterPtr params,
+                                   int *nparams,
+                                   unsigned int flags)
+{
+    virConnectPtr conn;
+
+    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
+                     params, nparams, flags);
+
+    virResetLastError();
+
+    virCheckDomainReturn(domain, -1);
+    virCheckNonNegativeArgGoto(*nparams, error);
+    if (*nparams != 0)
+        virCheckNonNullArgGoto(params, error);
+
+    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+        flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+    conn = domain->conn;
+
+    if (conn->driver->domainGetLaunchSecurityInfo) {
+        int ret;
+        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
+                                                        nparams, flags);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }
+    virReportUnsupportedError();
+
+ error:
+    virDispatchError(domain->conn);
+    return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 95df3a0dbc7b..caba2862d371 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
         virStoragePoolLookupByTargetPath;
 } LIBVIRT_3.9.0;
 
+LIBVIRT_4.2.0 {
+    global:
+        virDomainGetLaunchSecurityInfo;
+} LIBVIRT_4.1.0;
+
 # .... define new API here using predicted next version number ....
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Thu, Mar 08, 2018 at 11:12:04AM -0600, Brijesh Singh wrote:
> The API can be used outside the libvirt to get the launch security
> information. When SEV is enabled, the API can be used to get the
> measurement of the launch process.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  include/libvirt/libvirt-domain.h | 17 ++++++++++++++
>  src/driver-hypervisor.h          |  7 ++++++
>  src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  5 ++++
>  4 files changed, 79 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4048acf38aaf..11c3fec92bfa 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>                                  unsigned int action,
>                                  unsigned int flags);
>  
> +/**
> + * Launch Security API
> + */
> +
> +/**
> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
> + *
> + * Macro represents the launch measurement of the SEV guest,
> + * as VIR_TYPED_PARAM_STRING.
> + */
> +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
> +
> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> +                                   virTypedParameterPtr params,
> +                                   int *nparams,
> +                                   unsigned int flags);

These days we prefer new APIs to use

       virTypedParameterPtr *params,

and have the API implementation allocate the right number of
elements for the array, so the caller doesn't have to allocate
anything itself - only free the returned memory.

See virDomainGetJobStats for an example.

> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index ce0e2b252552..dc4873a8ad1c 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1283,6 +1283,12 @@ typedef int
>                                    unsigned int action,
>                                    unsigned int flags);
>  
> +typedef int
> +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
> +                                     virTypedParameterPtr params,
> +                                     int *nparams,
> +                                     unsigned int flags);
> +
>  
>  typedef struct _virHypervisorDriver virHypervisorDriver;
>  typedef virHypervisorDriver *virHypervisorDriverPtr;
> @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
>      virDrvDomainSetVcpu domainSetVcpu;
>      virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>      virDrvDomainSetLifecycleAction domainSetLifecycleAction;
> +    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
>  };
>  
>  
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index eaec0979ad49..21356bb92894 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>      virDispatchError(domain->conn);
>      return -1;
>  }
> +
> +/**
> + * virDomainGetLaunchSecurityInfo:
> + * @domain: a domain object
> + * @params: where to store security info
> + * @nparams: number of items in @params
> + * @flags: bitwise-OR of virDomainModificationImpact

This API doesn't use virDomainModificationImpact. So just say

   @flags currently used, set to 0

> + *
> + * Get the launch security info. In case of the SEV guest, this will
> + * return the launch measurement.
> + *
> + * Returns -1 in case of failure, 0 in case of success.
> + */
> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
> +                                   virTypedParameterPtr params,
> +                                   int *nparams,
> +                                   unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
> +                     params, nparams, flags);
> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(domain, -1);
> +    virCheckNonNegativeArgGoto(*nparams, error);
> +    if (*nparams != 0)
> +        virCheckNonNullArgGoto(params, error);

We should require both args to be non-null, when we do allocation
ourselves.

I think we probably want to forbid this method for read-only
users, so add

  virCheckReadOnlyGoto(conn->flags, error);



> +
> +    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> +                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
> +        flags |= VIR_TYPED_PARAM_STRING_OKAY;
> +
> +    conn = domain->conn;
> +
> +    if (conn->driver->domainGetLaunchSecurityInfo) {
> +        int ret;
> +        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
> +                                                        nparams, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +    virReportUnsupportedError();
> +
> + error:
> +    virDispatchError(domain->conn);
> +    return -1;
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0dbc7b..caba2862d371 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
>          virStoragePoolLookupByTargetPath;
>  } LIBVIRT_3.9.0;
>  
> +LIBVIRT_4.2.0 {
> +    global:
> +        virDomainGetLaunchSecurityInfo;
> +} LIBVIRT_4.1.0;
> +
>  # .... define new API here using predicted next version number ....
> -- 
> 2.14.3
> 

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info
Posted by Brijesh Singh 7 years, 2 months ago

On 03/12/2018 07:01 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:04AM -0600, Brijesh Singh wrote:
>> The API can be used outside the libvirt to get the launch security
>> information. When SEV is enabled, the API can be used to get the
>> measurement of the launch process.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   include/libvirt/libvirt-domain.h | 17 ++++++++++++++
>>   src/driver-hypervisor.h          |  7 ++++++
>>   src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms          |  5 ++++
>>   4 files changed, 79 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index 4048acf38aaf..11c3fec92bfa 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>                                   unsigned int action,
>>                                   unsigned int flags);
>>   
>> +/**
>> + * Launch Security API
>> + */
>> +
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
>> + *
>> + * Macro represents the launch measurement of the SEV guest,
>> + * as VIR_TYPED_PARAM_STRING.
>> + */
>> +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
>> +
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags);
> 
> These days we prefer new APIs to use
> 
>         virTypedParameterPtr *params,
> 
> and have the API implementation allocate the right number of
> elements for the array, so the caller doesn't have to allocate
> anything itself - only free the returned memory.
> 
> See virDomainGetJobStats for an example.
> 

Ah okay, I will take a look and change the API to allocate the array 
elements instead of forcing caller to allocate the arrays.



>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index ce0e2b252552..dc4873a8ad1c 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -1283,6 +1283,12 @@ typedef int
>>                                     unsigned int action,
>>                                     unsigned int flags);
>>   
>> +typedef int
>> +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
>> +                                     virTypedParameterPtr params,
>> +                                     int *nparams,
>> +                                     unsigned int flags);
>> +
>>   
>>   typedef struct _virHypervisorDriver virHypervisorDriver;
>>   typedef virHypervisorDriver *virHypervisorDriverPtr;
>> @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
>>       virDrvDomainSetVcpu domainSetVcpu;
>>       virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>>       virDrvDomainSetLifecycleAction domainSetLifecycleAction;
>> +    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
>>   };
>>   
>>   
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index eaec0979ad49..21356bb92894 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>       virDispatchError(domain->conn);
>>       return -1;
>>   }
>> +
>> +/**
>> + * virDomainGetLaunchSecurityInfo:
>> + * @domain: a domain object
>> + * @params: where to store security info
>> + * @nparams: number of items in @params
>> + * @flags: bitwise-OR of virDomainModificationImpact
> 
> This API doesn't use virDomainModificationImpact. So just say
> 
>     @flags currently used, set to 0
> 

Noted. thanks


>> + *
>> + * Get the launch security info. In case of the SEV guest, this will
>> + * return the launch measurement.
>> + *
>> + * Returns -1 in case of failure, 0 in case of success.
>> + */
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags)
>> +{
>> +    virConnectPtr conn;
>> +
>> +    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
>> +                     params, nparams, flags);
>> +
>> +    virResetLastError();
>> +
>> +    virCheckDomainReturn(domain, -1);
>> +    virCheckNonNegativeArgGoto(*nparams, error);
>> +    if (*nparams != 0)
>> +        virCheckNonNullArgGoto(params, error);
> 
> We should require both args to be non-null, when we do allocation
> ourselves.
> 
> I think we probably want to forbid this method for read-only
> users, so add
> 
>    virCheckReadOnlyGoto(conn->flags, error);
> 
> 
> 

Okay, got it.


>> +
>> +    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>> +                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
>> +        flags |= VIR_TYPED_PARAM_STRING_OKAY;
>> +
>> +    conn = domain->conn;
>> +
>> +    if (conn->driver->domainGetLaunchSecurityInfo) {
>> +        int ret;
>> +        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
>> +                                                        nparams, flags);
>> +        if (ret < 0)
>> +            goto error;
>> +        return ret;
>> +    }
>> +    virReportUnsupportedError();
>> +
>> + error:
>> +    virDispatchError(domain->conn);
>> +    return -1;
>> +}
>> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
>> index 95df3a0dbc7b..caba2862d371 100644
>> --- a/src/libvirt_public.syms
>> +++ b/src/libvirt_public.syms
>> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
>>           virStoragePoolLookupByTargetPath;
>>   } LIBVIRT_3.9.0;
>>   
>> +LIBVIRT_4.2.0 {
>> +    global:
>> +        virDomainGetLaunchSecurityInfo;
>> +} LIBVIRT_4.1.0;
>> +
>>   # .... define new API here using predicted next version number ....
>> -- 
>> 2.14.3
>>
> 
> Regards,
> Daniel
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list