[25/30] security: selinux: Simplify SetImageLabelInternal

Message ID 0223a7618af9936a4db511d4ede7492a2313f905.1570482718.git.crobinso@redhat.com
State Accepted
Commit 6f1cd0a54e05552e089948735778f245b0593392
Headers show
Series
  • storagefile, security: qcow2 data_file support
Related show

Commit Message

Cole Robinson Oct. 7, 2019, 9:49 p.m.
All the SetFileCon calls only differ by the label they pass in.
Rework the conditionals to track what label we need, and use a
single SetFileCon call

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 src/security/security_selinux.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

-- 
2.23.0

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

Comments

Daniel Henrique Barboza Oct. 10, 2019, 4:02 p.m. | #1
On 10/7/19 6:49 PM, Cole Robinson wrote:
> All the SetFileCon calls only differ by the label they pass in.

> Rework the conditionals to track what label we need, and use a

> single SetFileCon call

>

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>   src/security/security_selinux.c | 31 ++++++++++---------------------

>   1 file changed, 10 insertions(+), 21 deletions(-)

>

> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c

> index e879fa39ab..9d28bc5773 100644

> --- a/src/security/security_selinux.c

> +++ b/src/security/security_selinux.c

> @@ -1822,6 +1822,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,

>       virSecurityLabelDefPtr secdef;

>       virSecurityDeviceLabelDefPtr disk_seclabel;

>       virSecurityDeviceLabelDefPtr parent_seclabel = NULL;

> +    char *use_label = NULL;

>       bool remember;

>       int ret;

>   

> @@ -1856,40 +1857,28 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,

>           if (!disk_seclabel->relabel)

>               return 0;

>   

> -        ret = virSecuritySELinuxSetFilecon(mgr, src->path,

> -                                           disk_seclabel->label, remember);

> +        use_label = disk_seclabel->label;

>       } else if (parent_seclabel && (!parent_seclabel->relabel || parent_seclabel->label)) {

>           if (!parent_seclabel->relabel)

>               return 0;

>   

> -        ret = virSecuritySELinuxSetFilecon(mgr, src->path,

> -                                           parent_seclabel->label, remember);

> +        use_label = parent_seclabel->label;

>       } else if (!parent || parent == src) {

>           if (src->shared) {

> -            ret = virSecuritySELinuxSetFilecon(mgr,

> -                                               src->path,

> -                                               data->file_context,

> -                                               remember);

> +            use_label = data->file_context;

>           } else if (src->readonly) {

> -            ret = virSecuritySELinuxSetFilecon(mgr,

> -                                               src->path,

> -                                               data->content_context,

> -                                               remember);

> +            use_label = data->content_context;

>           } else if (secdef->imagelabel) {

> -            ret = virSecuritySELinuxSetFilecon(mgr,

> -                                               src->path,

> -                                               secdef->imagelabel,

> -                                               remember);

> +            use_label = secdef->imagelabel;

>           } else {

> -            ret = 0;

> +            return 0;

>           }

>       } else {

> -        ret = virSecuritySELinuxSetFilecon(mgr,

> -                                           src->path,

> -                                           data->content_context,

> -                                           remember);

> +        use_label = data->content_context;

>       }

>   

> +    ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label, remember);

> +

>       if (ret == 1 && !disk_seclabel) {

>           /* If we failed to set a label, but virt_use_nfs let us

>            * proceed anyway, then we don't need to relabel later.  */


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Michal Prívozník Oct. 11, 2019, 1:04 p.m. | #2
On 10/7/19 11:49 PM, Cole Robinson wrote:
> All the SetFileCon calls only differ by the label they pass in.

> Rework the conditionals to track what label we need, and use a

> single SetFileCon call

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>   src/security/security_selinux.c | 31 ++++++++++---------------------

>   1 file changed, 10 insertions(+), 21 deletions(-)


Huh, I posted the same patch a while ago:

https://www.redhat.com/archives/libvir-list/2019-September/msg01240.html

Great minds think alike :-D

Michal

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

Patch

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e879fa39ab..9d28bc5773 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1822,6 +1822,7 @@  virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
     virSecurityLabelDefPtr secdef;
     virSecurityDeviceLabelDefPtr disk_seclabel;
     virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
+    char *use_label = NULL;
     bool remember;
     int ret;
 
@@ -1856,40 +1857,28 @@  virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
         if (!disk_seclabel->relabel)
             return 0;
 
-        ret = virSecuritySELinuxSetFilecon(mgr, src->path,
-                                           disk_seclabel->label, remember);
+        use_label = disk_seclabel->label;
     } else if (parent_seclabel && (!parent_seclabel->relabel || parent_seclabel->label)) {
         if (!parent_seclabel->relabel)
             return 0;
 
-        ret = virSecuritySELinuxSetFilecon(mgr, src->path,
-                                           parent_seclabel->label, remember);
+        use_label = parent_seclabel->label;
     } else if (!parent || parent == src) {
         if (src->shared) {
-            ret = virSecuritySELinuxSetFilecon(mgr,
-                                               src->path,
-                                               data->file_context,
-                                               remember);
+            use_label = data->file_context;
         } else if (src->readonly) {
-            ret = virSecuritySELinuxSetFilecon(mgr,
-                                               src->path,
-                                               data->content_context,
-                                               remember);
+            use_label = data->content_context;
         } else if (secdef->imagelabel) {
-            ret = virSecuritySELinuxSetFilecon(mgr,
-                                               src->path,
-                                               secdef->imagelabel,
-                                               remember);
+            use_label = secdef->imagelabel;
         } else {
-            ret = 0;
+            return 0;
         }
     } else {
-        ret = virSecuritySELinuxSetFilecon(mgr,
-                                           src->path,
-                                           data->content_context,
-                                           remember);
+        use_label = data->content_context;
     }
 
+    ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label, remember);
+
     if (ret == 1 && !disk_seclabel) {
         /* If we failed to set a label, but virt_use_nfs let us
          * proceed anyway, then we don't need to relabel later.  */