[libvirt] [PATCH 1/2] Re-label shared and readonly images

From: Daniel P. Berrange <berrange@redhat.com> This patch was posted ages ago here: https://bugzilla.redhat.com/493692 But was never posted upstream AFAICT. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/security_selinux.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/security_selinux.c b/src/security_selinux.c index 4fb7c86..87073d2 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -24,11 +24,12 @@ #include "virterror_internal.h" #include "util.h" #include "memory.h" - +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_SECURITY static char default_domain_context[1024]; +static char default_content_context[1024]; static char default_image_context[1024]; #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" @@ -148,8 +149,13 @@ SELinuxInitialize(virConnectPtr conn) close(fd); ptr = strchrnul(default_image_context, '\n'); - *ptr = '\0'; - + if (*ptr == '\n') { + *ptr = '\0'; + strcpy(default_content_context, ptr+1); + ptr = strchrnul(default_content_context, '\n'); + if (*ptr == '\n') + *ptr = '\0'; + } return 0; } @@ -313,6 +319,8 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) { char ebuf[1024]; + VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); + if(setfilecon(path, tcon) < 0) { virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: unable to set security context " @@ -337,9 +345,6 @@ SELinuxRestoreSecurityImageLabel(virConnectPtr conn, char *newpath = NULL; const char *path = disk->src; - if (disk->readonly || disk->shared) - return 0; - if ((err = virFileResolveLink(path, &newpath)) < 0) { virReportSystemError(conn, err, _("cannot resolve symlink %s"), path); @@ -366,8 +371,13 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; - if (secdef->imagelabel) + if (disk->shared) { + return SELinuxSetFilecon(conn, disk->src, default_image_context); + } else if (disk->readonly) { + return SELinuxSetFilecon(conn, disk->src, default_content_context); + } else if (secdef->imagelabel) { return SELinuxSetFilecon(conn, disk->src, secdef->imagelabel); + } return 0; } @@ -441,9 +451,6 @@ SELinuxSetSecurityLabel(virConnectPtr conn, if (secdef->imagelabel) { for (i = 0 ; i < vm->def->ndisks ; i++) { - if (vm->def->disks[i]->readonly || - vm->def->disks[i]->shared) continue; - if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0) return -1; } -- 1.6.2.5

From: Tim Waugh <twaugh@redhat.com> As pointed out by Tim Waugh here: https://bugzilla.redhat.com/507555 We shouldn't bother trying to set the context of a file if it already matches what we want. (Fixed to use STREQ() and not use tabs, as pointed out by danpb) Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/security_selinux.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/src/security_selinux.c b/src/security_selinux.c index 87073d2..174dd57 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -318,10 +318,19 @@ static int SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) { char ebuf[1024]; + security_context_t econ; VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); - if(setfilecon(path, tcon) < 0) { + if (setfilecon(path, tcon) < 0) { + if (getfilecon(path, &econ) >= 0) { + if (STREQ(tcon, econ)) { + freecon(econ); + /* It's alright, there's nothing to change anyway. */ + return 0; + } + freecon(econ); + } virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: unable to set security context " "'\%s\' on %s: %s."), __func__, -- 1.6.2.5

Fixes startup of guest's with sourceless cdrom devices. Patch originally posted here: https://bugzilla.redhat.com/499569 but never sent upstream. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/security_selinux.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/security_selinux.c b/src/security_selinux.c index 174dd57..80c1c85 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -380,6 +380,9 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + if (!disk->src) + return 0; + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) { -- 1.6.2.5

On Fri, Jul 03, 2009 at 10:43:12AM +0100, Mark McLoughlin wrote:
Fixes startup of guest's with sourceless cdrom devices.
Patch originally posted here:
https://bugzilla.redhat.com/499569
but never sent upstream.
Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/security_selinux.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
ACK.
diff --git a/src/security_selinux.c b/src/security_selinux.c index 174dd57..80c1c85 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -380,6 +380,9 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
+ if (!disk->src) + return 0; + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) { -- 1.6.2.5
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Jul 03, 2009 at 10:36:47AM +0100, Mark McLoughlin wrote:
From: Tim Waugh <twaugh@redhat.com>
As pointed out by Tim Waugh here:
https://bugzilla.redhat.com/507555
We shouldn't bother trying to set the context of a file if it already matches what we want.
(Fixed to use STREQ() and not use tabs, as pointed out by danpb)
Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/security_selinux.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
ACK
diff --git a/src/security_selinux.c b/src/security_selinux.c index 87073d2..174dd57 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -318,10 +318,19 @@ static int SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) { char ebuf[1024]; + security_context_t econ;
VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon);
- if(setfilecon(path, tcon) < 0) { + if (setfilecon(path, tcon) < 0) { + if (getfilecon(path, &econ) >= 0) { + if (STREQ(tcon, econ)) { + freecon(econ); + /* It's alright, there's nothing to change anyway. */ + return 0; + } + freecon(econ); + } virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: unable to set security context " "'\%s\' on %s: %s."), __func__, -- 1.6.2.5
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Jul 03, 2009 at 10:35:56AM +0100, Mark McLoughlin wrote:
From: Daniel P. Berrange <berrange@redhat.com>
This patch was posted ages ago here:
https://bugzilla.redhat.com/493692
But was never posted upstream AFAICT.
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Doh, I dropped the ball on that one. ACK Daniel
--- src/security_selinux.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/security_selinux.c b/src/security_selinux.c index 4fb7c86..87073d2 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -24,11 +24,12 @@ #include "virterror_internal.h" #include "util.h" #include "memory.h" - +#include "logging.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
static char default_domain_context[1024]; +static char default_content_context[1024]; static char default_image_context[1024]; #define SECURITY_SELINUX_VOID_DOI "0" #define SECURITY_SELINUX_NAME "selinux" @@ -148,8 +149,13 @@ SELinuxInitialize(virConnectPtr conn) close(fd);
ptr = strchrnul(default_image_context, '\n'); - *ptr = '\0'; - + if (*ptr == '\n') { + *ptr = '\0'; + strcpy(default_content_context, ptr+1); + ptr = strchrnul(default_content_context, '\n'); + if (*ptr == '\n') + *ptr = '\0'; + } return 0; }
@@ -313,6 +319,8 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) { char ebuf[1024];
+ VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); + if(setfilecon(path, tcon) < 0) { virSecurityReportError(conn, VIR_ERR_ERROR, _("%s: unable to set security context " @@ -337,9 +345,6 @@ SELinuxRestoreSecurityImageLabel(virConnectPtr conn, char *newpath = NULL; const char *path = disk->src;
- if (disk->readonly || disk->shared) - return 0; - if ((err = virFileResolveLink(path, &newpath)) < 0) { virReportSystemError(conn, err, _("cannot resolve symlink %s"), path); @@ -366,8 +371,13 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
- if (secdef->imagelabel) + if (disk->shared) { + return SELinuxSetFilecon(conn, disk->src, default_image_context); + } else if (disk->readonly) { + return SELinuxSetFilecon(conn, disk->src, default_content_context); + } else if (secdef->imagelabel) { return SELinuxSetFilecon(conn, disk->src, secdef->imagelabel); + }
return 0; } @@ -441,9 +451,6 @@ SELinuxSetSecurityLabel(virConnectPtr conn,
if (secdef->imagelabel) { for (i = 0 ; i < vm->def->ndisks ; i++) { - if (vm->def->disks[i]->readonly || - vm->def->disks[i]->shared) continue; - if (SELinuxSetSecurityImageLabel(conn, vm, vm->def->disks[i]) < 0) return -1; } -- 1.6.2.5
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange
-
Mark McLoughlin