1. Patchew
  2. Libvirt
  3. [PATCH 0/9] Fix image labels lost after migration with shared fs
  4. View patch

[PATCH 2/9] security: add virSecurityManagerUpdateImageLabel

Peng Liang posted 9 patches 4 years, 5 months ago
[PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
Posted by Peng Liang 4 years, 5 months ago 
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
---
 src/libvirt_private.syms        |  1 +
 src/security/security_driver.h  |  5 +++++
 src/security/security_manager.c | 29 +++++++++++++++++++++++++++++
 src/security/security_manager.h |  5 +++++
 4 files changed, 40 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 090ac80691cb..50c4d0fd000a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1707,6 +1707,7 @@ virSecurityManagerStackAddNested;
 virSecurityManagerTransactionAbort;
 virSecurityManagerTransactionCommit;
 virSecurityManagerTransactionStart;
+virSecurityManagerUpdateImageLabel;
 virSecurityManagerVerify;
 
 
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 07f8def3d3c6..82ef0ddd9801 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -123,6 +123,10 @@ typedef int (*virSecurityDomainMoveImageMetadata) (virSecurityManager *mgr,
                                                    pid_t pid,
                                                    virStorageSource *src,
                                                    virStorageSource *dst);
+typedef int (*virSecurityDomainUpdateImageLabel) (virSecurityManager *mgr,
+                                                  virDomainDef *def,
+                                                  virStorageSource *src,
+                                                  virSecurityDomainImageLabelFlags flags);
 typedef int (*virSecurityDomainSetMemoryLabel) (virSecurityManager *mgr,
                                                 virDomainDef *def,
                                                 virDomainMemoryDef *mem);
@@ -180,6 +184,7 @@ struct _virSecurityDriver {
     virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
     virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel;
     virSecurityDomainMoveImageMetadata domainMoveImageMetadata;
+    virSecurityDomainUpdateImageLabel domainUpdateSecurityImageLabel;
 
     virSecurityDomainSetMemoryLabel domainSetSecurityMemoryLabel;
     virSecurityDomainRestoreMemoryLabel domainRestoreSecurityMemoryLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 9906c1691d0f..b580704d3abf 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
 }
 
 
+/**
+ * virSecurityManagerUpdateImageLabel:
+ * @mgr: security manager object
+ * @vm: domain definition object
+ * @src: disk source definition to operate on
+ * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
+ *
+ * Update security label from @src according to @flags.
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
+                                   virDomainDef *vm,
+                                   virStorageSource *src,
+                                   virSecurityDomainImageLabelFlags flags)
+{
+    if (mgr->drv->domainUpdateSecurityImageLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
+        virObjectUnlock(mgr);
+        return ret;
+    }
+
+    return 0;
+}
+
+
 int
 virSecurityManagerSetDaemonSocketLabel(virSecurityManager *mgr,
                                        virDomainDef *vm)
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 57047ccb137d..00bbb255538f 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -175,6 +175,11 @@ int virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
                                         pid_t pid,
                                         virStorageSource *src,
                                         virStorageSource *dst);
+int
+virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
+                                   virDomainDef *vm,
+                                   virStorageSource *src,
+                                   virSecurityDomainImageLabelFlags flags);
 
 int virSecurityManagerSetMemoryLabel(virSecurityManager *mgr,
                                      virDomainDef *vm,
-- 
2.31.1
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
Posted by Michal Prívozník 4 years, 5 months ago 
On 8/23/21 4:41 AM, Peng Liang wrote:
> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> ---
>  src/libvirt_private.syms        |  1 +
>  src/security/security_driver.h  |  5 +++++
>  src/security/security_manager.c | 29 +++++++++++++++++++++++++++++
>  src/security/security_manager.h |  5 +++++
>  4 files changed, 40 insertions(+)
> 


> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 9906c1691d0f..b580704d3abf 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
>  }
>  
>  
> +/**
> + * virSecurityManagerUpdateImageLabel:
> + * @mgr: security manager object
> + * @vm: domain definition object
> + * @src: disk source definition to operate on
> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
> + *
> + * Update security label from @src according to @flags.
> + *
> + * Returns: 0 on success, -1 on error.
> + */
> +int
> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
> +                                   virDomainDef *vm,
> +                                   virStorageSource *src,
> +                                   virSecurityDomainImageLabelFlags flags)
> +{
> +    if (mgr->drv->domainUpdateSecurityImageLabel) {
> +        int ret;
> +        virObjectLock(mgr);
> +        ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
> +        virObjectUnlock(mgr);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +

Is there a reason why this needs to be inside virSecurityManager? We
already have virSecurityMoveRememberedLabel() that lives outside of it,
in security_util.c and conceptually this function belongs there.

Michal
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
Posted by Peng Liang 4 years, 5 months ago 
On 9/9/2021 7:01 PM, Michal Prívozník wrote:
> On 8/23/21 4:41 AM, Peng Liang wrote:
>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
>> ---
>>  src/libvirt_private.syms        |  1 +
>>  src/security/security_driver.h  |  5 +++++
>>  src/security/security_manager.c | 29 +++++++++++++++++++++++++++++
>>  src/security/security_manager.h |  5 +++++
>>  4 files changed, 40 insertions(+)
>>
> 
> 
>> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
>> index 9906c1691d0f..b580704d3abf 100644
>> --- a/src/security/security_manager.c
>> +++ b/src/security/security_manager.c
>> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
>>  }
>>  
>>  
>> +/**
>> + * virSecurityManagerUpdateImageLabel:
>> + * @mgr: security manager object
>> + * @vm: domain definition object
>> + * @src: disk source definition to operate on
>> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
>> + *
>> + * Update security label from @src according to @flags.
>> + *
>> + * Returns: 0 on success, -1 on error.
>> + */
>> +int
>> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
>> +                                   virDomainDef *vm,
>> +                                   virStorageSource *src,
>> +                                   virSecurityDomainImageLabelFlags flags)
>> +{
>> +    if (mgr->drv->domainUpdateSecurityImageLabel) {
>> +        int ret;
>> +        virObjectLock(mgr);
>> +        ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
>> +        virObjectUnlock(mgr);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
> 
> Is there a reason why this needs to be inside virSecurityManager? We
> already have virSecurityMoveRememberedLabel() that lives outside of it,
> in security_util.c and conceptually this function belongs there.
> 
> Michal
> 
> .
> 
Maybe all security managers' labels need to be updated during migration,
so I add it here.

Thanks,
Peng
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
Posted by Michal Prívozník 4 years, 5 months ago 
On 9/9/21 1:45 PM, Peng Liang wrote:
> On 9/9/2021 7:01 PM, Michal Prívozník wrote:
>> On 8/23/21 4:41 AM, Peng Liang wrote:
>>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
>>> ---
>>>  src/libvirt_private.syms        |  1 +
>>>  src/security/security_driver.h  |  5 +++++
>>>  src/security/security_manager.c | 29 +++++++++++++++++++++++++++++
>>>  src/security/security_manager.h |  5 +++++
>>>  4 files changed, 40 insertions(+)
>>>
>>
>>
>>> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
>>> index 9906c1691d0f..b580704d3abf 100644
>>> --- a/src/security/security_manager.c
>>> +++ b/src/security/security_manager.c
>>> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
>>>  }
>>>  
>>>  
>>> +/**
>>> + * virSecurityManagerUpdateImageLabel:
>>> + * @mgr: security manager object
>>> + * @vm: domain definition object
>>> + * @src: disk source definition to operate on
>>> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
>>> + *
>>> + * Update security label from @src according to @flags.
>>> + *
>>> + * Returns: 0 on success, -1 on error.
>>> + */
>>> +int
>>> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
>>> +                                   virDomainDef *vm,
>>> +                                   virStorageSource *src,
>>> +                                   virSecurityDomainImageLabelFlags flags)
>>> +{
>>> +    if (mgr->drv->domainUpdateSecurityImageLabel) {
>>> +        int ret;
>>> +        virObjectLock(mgr);
>>> +        ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
>>> +        virObjectUnlock(mgr);
>>> +        return ret;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +
>>
>> Is there a reason why this needs to be inside virSecurityManager? We
>> already have virSecurityMoveRememberedLabel() that lives outside of it,
>> in security_util.c and conceptually this function belongs there.
>>
>> Michal
>>
>> .
>>
> Maybe all security managers' labels need to be updated during migration,
> so I add it here.

Ah, you are correct. The timestamp XATTR is specific to secdriver so DAC
and SELinux have their own timestamps. So your approach is in fact
correct. For your v2 can you please also implement SELinux? I think it's
going to be 1:1 copy of DAC code.

Michal
Re: [PATCH 2/9] security: add virSecurityManagerUpdateImageLabel
Posted by Peng Liang 4 years, 5 months ago 
On 9/9/2021 10:47 PM, Michal Prívozník wrote:
> On 9/9/21 1:45 PM, Peng Liang wrote:
>> On 9/9/2021 7:01 PM, Michal Prívozník wrote:
>>> On 8/23/21 4:41 AM, Peng Liang wrote:
>>>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
>>>> ---
>>>>  src/libvirt_private.syms        |  1 +
>>>>  src/security/security_driver.h  |  5 +++++
>>>>  src/security/security_manager.c | 29 +++++++++++++++++++++++++++++
>>>>  src/security/security_manager.h |  5 +++++
>>>>  4 files changed, 40 insertions(+)
>>>>
>>>
>>>
>>>> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
>>>> index 9906c1691d0f..b580704d3abf 100644
>>>> --- a/src/security/security_manager.c
>>>> +++ b/src/security/security_manager.c
>>>> @@ -476,6 +476,35 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
>>>>  }
>>>>  
>>>>  
>>>> +/**
>>>> + * virSecurityManagerUpdateImageLabel:
>>>> + * @mgr: security manager object
>>>> + * @vm: domain definition object
>>>> + * @src: disk source definition to operate on
>>>> + * @flags: bitwise or of 'virSecurityDomainImageLabelFlags'
>>>> + *
>>>> + * Update security label from @src according to @flags.
>>>> + *
>>>> + * Returns: 0 on success, -1 on error.
>>>> + */
>>>> +int
>>>> +virSecurityManagerUpdateImageLabel(virSecurityManager *mgr,
>>>> +                                   virDomainDef *vm,
>>>> +                                   virStorageSource *src,
>>>> +                                   virSecurityDomainImageLabelFlags flags)
>>>> +{
>>>> +    if (mgr->drv->domainUpdateSecurityImageLabel) {
>>>> +        int ret;
>>>> +        virObjectLock(mgr);
>>>> +        ret = mgr->drv->domainUpdateSecurityImageLabel(mgr, vm, src, flags);
>>>> +        virObjectUnlock(mgr);
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +
>>>
>>> Is there a reason why this needs to be inside virSecurityManager? We
>>> already have virSecurityMoveRememberedLabel() that lives outside of it,
>>> in security_util.c and conceptually this function belongs there.
>>>
>>> Michal
>>>
>>> .
>>>
>> Maybe all security managers' labels need to be updated during migration,
>> so I add it here.
> 
> Ah, you are correct. The timestamp XATTR is specific to secdriver so DAC
> and SELinux have their own timestamps. So your approach is in fact
> correct. For your v2 can you please also implement SELinux? I think it's
> going to be 1:1 copy of DAC code.
> 
> Michal
> 
> .
> 

OK，I'll add and test it in v2.
Thanks for your reviewing!

Peng

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.