[libvirt] [PATCH] selinux: Fix incorrect file label generation.

This is an ad-hoc fix for the file label generation. It uses the base context role to determine whether to use the libvirt process context role. If this is object_r we don't touch it. It might be better to add a new flag to virSecuritySELinuxGenNewContext that specifies the context type (process or file) in the future. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/security/security_selinux.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 48fd78b..34b9aad 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -176,7 +176,9 @@ virSecuritySELinuxGenNewContext(const char *basecontext, const char *mcs) goto cleanup; } - if (context_role_set(context, + /* don't exchange role context if object_r as this is a file context */ + if (strcmp("object_r", context_role_get(context)) && + context_role_set(context, context_role_get(ourContext)) != 0) { virReportSystemError(errno, _("Unable to set SELinux context user '%s'"), -- 1.7.0.4

On 2012年08月17日 20:53, Viktor Mihajlovski wrote:
This is an ad-hoc fix for the file label generation. It uses the base context role to determine whether to use the libvirt process context role. If this is object_r we don't touch it. It might be better to add a new flag to virSecuritySELinuxGenNewContext that specifies the context type (process or file) in the future.
Signed-off-by: Viktor Mihajlovski<mihajlov@linux.vnet.ibm.com> --- src/security/security_selinux.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 48fd78b..34b9aad 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -176,7 +176,9 @@ virSecuritySELinuxGenNewContext(const char *basecontext, const char *mcs) goto cleanup; }
- if (context_role_set(context, + /* don't exchange role context if object_r as this is a file context */ + if (strcmp("object_r", context_role_get(context))&&
No strcmp directly, it should be STREQ instead. Good to read HACKING before making patches. :-)
+ context_role_set(context, context_role_get(ourContext)) != 0) { virReportSystemError(errno, _("Unable to set SELinux context user '%s'"),

On 08/17/2012 08:18 AM, Osier Yang wrote:
On 2012年08月17日 20:53, Viktor Mihajlovski wrote:
This is an ad-hoc fix for the file label generation. It uses the base context role to determine whether to use the libvirt process context role. If this is object_r we don't touch it. It might be better to add a new flag to virSecuritySELinuxGenNewContext that specifies the context type (process or file) in the future.
I'd rather Daniel Berrange chimed in on the approach.
+ if (strcmp("object_r", context_role_get(context))&&
No strcmp directly, it should be STREQ instead. Good to read HACKING before making patches. :-)
Also, 'make syntax-check' will catch coding style abuse like this. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/17/2012 04:28 PM, Eric Blake wrote:
On 08/17/2012 08:18 AM, Osier Yang wrote:
On 2012年08月17日 20:53, Viktor Mihajlovski wrote:
This is an ad-hoc fix for the file label generation. It uses the base context role to determine whether to use the libvirt process context role. If this is object_r we don't touch it. It might be better to add a new flag to virSecuritySELinuxGenNewContext that specifies the context type (process or file) in the future.
I'd rather Daniel Berrange chimed in on the approach.
sure ... this is why I wrote "ad-hoc" :-) -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Fri, Aug 17, 2012 at 02:53:29PM +0200, Viktor Mihajlovski wrote:
This is an ad-hoc fix for the file label generation. It uses the base context role to determine whether to use the libvirt process context role. If this is object_r we don't touch it. It might be better to add a new flag to virSecuritySELinuxGenNewContext that specifies the context type (process or file) in the future.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/security/security_selinux.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 48fd78b..34b9aad 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -176,7 +176,9 @@ virSecuritySELinuxGenNewContext(const char *basecontext, const char *mcs) goto cleanup; }
- if (context_role_set(context, + /* don't exchange role context if object_r as this is a file context */ + if (strcmp("object_r", context_role_get(context)) && + context_role_set(context, context_role_get(ourContext)) != 0) { virReportSystemError(errno, _("Unable to set SELinux context user '%s'"),
Depending on the role name is a bit hacky & potentially unreliable. We should add a 'bool isObject' parameter to this method to indicate whether the label being generated is for an object or a process and conditionalize based on that. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Osier Yang
-
Viktor Mihajlovski