[libvirt] [PATCH 6/6] Try much harder to restore disk file labels

[Daniel P. Berrange]

On Tue, Sep 01, 2009 at 01:00:13PM -0400, Stephen Smalley wrote:
> On Tue, 2009-09-01 at 16:28 +0100, Daniel P. Berrange wrote:
> > * src/security_selinux.c: matchpath() may well return NULL for many
> >   directories, to try and fallback to using parent directory label
> >   in that scenario.
> 
> When have you seen this happen?  matchpathcon() ultimately should fall
> back to the top-level regex (/.*) and map any otherwise unmatched files
> to default_t, and should generally have a fallback regex for each
> subtree (e.g. any file under /dev that isn't otherwise matched would get
> device_t).  So I wouldn't expect this to happen.

That describes what I always imagined would happen, but in my recent
testing it doesn't appear to be doing that in practice in some cases

eg, running matchpathcon("/media/usbdisk/virtual-images/demo.img") returned
an error. Likewise for /sys/bus/pci/devices/NNNN.NN.NN.NN/*.   I was testing
this on a Fedora 11 host. Perhaps the policy is simply missing some rules

> Also, files will inherit their SELinux type from the parent directory by
> default upon creation unless a type transition rule is specified, so it
> isn't clear why you need to replicate this copying from parent behavior
> in the application.

This code is being run upon VM shutdown, to get rid of the dynamic label
that was assigned at VM startup. THe file already exists, so the default
rule for file creation wouldn't come into effect at this point, so I was
aiming to replicate that behaviour for the existing file.

If we could guarentee that matchpathcon($PATH) would always return
something usable, I would happily kill this code

Regard,
Daniel

> 
> > ---
> >  src/security_selinux.c |   29 ++++++++++++++++++++++++++++-
> >  1 files changed, 28 insertions(+), 1 deletions(-)
> > 
> > diff --git a/src/security_selinux.c b/src/security_selinux.c
> > index bc295b1..0072360 100644
> > --- a/src/security_selinux.c
> > +++ b/src/security_selinux.c
> > @@ -366,8 +366,35 @@ SELinuxRestoreSecurityFileLabel(virConnectPtr conn,
> >      if (stat(newpath, &buf) != 0)
> >          goto err;
> >  
> > -    if (matchpathcon(newpath, buf.st_mode, &fcon) == 0)  {
> > +    /* We try real hard to reset the context
> > +     *
> > +     *   - Prefer an explicit context from policy for the file
> > +     *   - Otherwise copy from parent directory.
> > +     *
> > +     * NB this is not just for disk images - PCI/USB device/sysfs
> > +     * files here too
> > +     */
> > +    if (matchpathcon(newpath, buf.st_mode, &fcon) == 0) {
> >          rc = SELinuxSetFilecon(conn, newpath, fcon);
> > +    } else {
> > +        char *dir = strdup(newpath);
> > +        char *sep;
> > +        if (!dir) {
> > +            virReportOOMError(conn);
> > +            goto err;
> > +        }
> > +        VIR_WARN("Cannot find default context for %s, copying from parent", newpath);
> > +        sep = strrchr(dir, '/');
> > +        if (sep) {
> > +            *sep = '\0';
> > +            if (getfilecon(dir, &fcon) >= 0)
> > +                rc = SELinuxSetFilecon(conn, newpath, fcon);
> > +            else
> > +                VIR_ERROR("Unable to get security context for directory %s", dir);
> > +        } else {
> > +            VIR_ERROR("File %s did not contain a directory separator", newpath);
> > +        }
> > +        VIR_FREE(dir);
> >      }
> >  err:
> >      VIR_FREE(fcon);
> -- 
> Stephen Smalley
> National Security Agency
> 

-- 
|: 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 :|