[libvirt] [PATCH] libvirt writes an mcs translation file to /run/setrans directory

[Daniel P. Berrange]

On Tue, May 21, 2013 at 09:12:49AM -0400, dwalsh at redhat.com wrote:
> From: Dan Walsh <dwalsh at redhat.com>
> 
> mcstransd is a translation tool that can translate MCS Labels into human
> understandable code.  I have patched it to watch for translation files in the
> /run/setrans directory.  This allows us to run commands like ps -eZ and see
> system_u:system_r:svirt_t:Fedora18 rather then system_u:system_r:svirt_t:s0:c1,c2.
> When used with containers it would make an easy way to list all processes within
> a container using ps -eZ | grep Fedora18
> 
> Pass in privileged field into Security Manager so this is only attempted on privileged
> machines

Did you actually test this patch, because it doesn't work at all ?

An LXC guest fails to start:

  2013-05-21 16:26:30.894+0000: 1: error : virSecuritySELinuxAddMCSFile:107 : unable to create MCS file /var/run/setrans/busy: No such file or directory

If I create that directory inside the container, it at least starts,
but doesn't have any effect because you're trying to write to /var/run
directory inside the container, rather than in the host.

With a QEMU guest this does nothing at all, because the QEMU driver
uses virSecurityManagerSetChildProcessLabel instead of
virSecurityManagerSetProcessLabel so this new code simply never
runs.


Trying todo this from the virSecurityManagerSetProcessLabel method
is just wrong. As I said last time, virSecurityManagerGenProcessLabel
is a better place IMHO.

> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 5d108b9..c416666 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -83,6 +83,57 @@ virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr,
>                                                   virDomainTPMDefPtr tpm);
>  
>  
> +static int
> +virSecuritySELinuxAddMCSFile(const char *name,
> +                             const char *label)
> +{
> +    int ret = -1;
> +    char *tmp = NULL;
> +    context_t con = NULL;
> +
> +    if (virAsprintf(&tmp, "%s/%s", SELINUX_TRANS_DIR, name) < 0) {

SELINUX_TRANS_DIR doesn't appear to exist in any libselinux package
prior to Fedora 19, so this breaks the build on all RHEL distros
and Fedora < 18. This code needs to be made conditional on this
constant existing in the headers.

> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (!(con = context_new(label))) {
> +        virReportSystemError(errno, "%s",
> +                             _("unable to allocate security context"));
> +        goto cleanup;
> +    }
> +    if (virFileWriteStr(tmp, context_range_get(con),  S_IRUSR|S_IWUSR) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to create MCS file %s"), tmp);
> +        goto cleanup;
> +    }
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(tmp);
> +    context_free(con);
> +    return ret;
> +}
> +
> +static int
> +virSecuritySELinuxRemoveMCSFile(const char *name)
> +{
> +    char *tmp = NULL;
> +    int ret = -1;
> +    if (virAsprintf(&tmp, "%s/%s", SELINUX_TRANS_DIR, name) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (unlink(tmp) < 0 && errno != ENOENT) {
> +        virReportSystemError(errno,
> +                             _("Unable to remove MCS file %s"), tmp);
> +        goto cleanup;
> +    }
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(tmp);
> +    return ret;
> +}
> +
>  /*
>   * Returns 0 on success, 1 if already reserved, or -1 on fatal error
>   */
> @@ -1953,7 +2004,7 @@ virSecuritySELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr,
>      }
>      VIR_FREE(secdef->imagelabel);
>  
> -    return 0;
> +    return virSecuritySELinuxRemoveMCSFile(def->name);
>  }
>  
>  
> @@ -2047,10 +2098,14 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN
>              return -1;
>      }
>  
> +    if (virSecurityManagerGetPrivileged(mgr) && (virSecuritySELinuxAddMCSFile(def->name, secdef->label) < 0))
> +        return -1;

As I said last time, failure to create the MCS file should not be treated
as a fatal error IMHO.

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