On 09/10/2015 08:47 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1124841 > > If running in session mode it may happen that we fail to set > correct SELinux label, but the image may still be readable to > the qemu process. Take this into account. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/security/security_selinux.c | 126 +++++++++++++++++++++++++--------------- > 1 file changed, 78 insertions(+), 48 deletions(-) > I was just poking around in this code trying to get back up to speed so I can post patches for the ideas I proposed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01400.html > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index c6da6b0..c2464c2 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -886,7 +886,8 @@ virSecuritySELinuxGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN > * return 1 if labelling was not possible. Otherwise, require a label > * change, and return 0 for success, -1 for failure. */ > static int > -virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, bool optional) > +virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, > + bool optional, bool privileged) > { > security_context_t econ; > > @@ -915,7 +916,10 @@ virSecuritySELinuxSetFileconHelper(const char *path, char *tcon, bool optional) > virReportSystemError(setfilecon_errno, > _("unable to set security context '%s' on '%s'"), > tcon, path); > - if (security_getenforce() == 1) > + /* However, don't claim error if SELinux is in Enforcing mode and > + * we are running as unprivileged user and we really did see EPERM. > + * Otherwise we want to return error if SELinux is Enforcing. */ > + if (security_getenforce() == 1 && (setfilecon_errno != EPERM || privileged)) > return -1; Ignoring a labelling EPERM failure for readonly media for qemu:///session seems fine to me (like the initial bug report), but this also ignores failure for RW disk images, so it throws out main security benefit of svirt if the source permissions are in the wrong state, which is worrying. I suggest amending this to only skip the !privileged error if disk->readonly. Thoughts? - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list