On Wed, May 15, 2013 at 02:36:32PM -0400, dwalsh@redhat.com wrote:

From: Dan Walsh <dwalsh@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 --- src/security/security_selinux.c | 59 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d108b9..cbcd013 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) { + virReportOOMError(); + return -1; + } + if (! (con = context_new(label))) {

Still has bogus whitespace after the '!'

+ virReportSystemError(errno, "%s", + _("unable to allocate security context")); + goto cleanup; + } + if (virFileWriteStr(tmp, context_range_get(con), 0) < 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;

Space needed either side of the =

+ 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,16 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN return -1; }

+ if (virSecuritySELinuxAddMCSFile(def->name, secdef->label) < 0) { + if (security_getenforce() == 1) + return -1;

This call to security_getenforce() must go - we should be unconditonally reporting errors.

+ } + if (setexeccon_raw(secdef->label) == -1) { virReportSystemError(errno, _("unable to set security context '%s'"), secdef->label); + virSecuritySELinuxRemoveMCSFile(def->name); if (security_getenforce() == 1) return -1; }

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

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/17/2013 05:52 AM, Daniel P. Berrange wrote:

On Wed, May 15, 2013 at 02:36:32PM -0400, dwalsh@redhat.com wrote:

...

From: Dan Walsh <dwalsh@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 --- src/security/security_selinux.c | 59 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d108b9..cbcd013 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) { + 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), 0) < 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,16 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN return -1; }

+ if (virSecuritySELinuxAddMCSFile(def->name, secdef->label) < 0) { + if (security_getenforce() == 1) + return -1; + } +

As you mentioned offlist, this is not going to work because the SetProcessLabel function is called in a child process, where you can't guarantee to see the host's /run directory.

Instead it should be done in the GenSecurityLabel function which is called from a safe context.

Daniel

Fine, but what about the case where the user is running libvirt and libvirt is not allowed to write to /run/setrans. Should we just silently fail in this case? -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlGWLNQACgkQrlYvE4MpobOcJgCgm7m8uxm4Fu4sJptduOo1PI7Y gawAoMFCdG45r3vZMpi9XpmF6Y/4SHgz =9c6r -----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/17/2013 05:52 AM, Daniel P. Berrange wrote:

On Wed, May 15, 2013 at 02:36:32PM -0400, dwalsh@redhat.com wrote:

...

From: Dan Walsh <dwalsh@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 --- src/security/security_selinux.c | 59 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d108b9..cbcd013 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) { + 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), 0) < 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,16 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN return -1; }

+ if (virSecuritySELinuxAddMCSFile(def->name, secdef->label) < 0) { + if (security_getenforce() == 1) + return -1; + } +

As you mentioned offlist, this is not going to work because the SetProcessLabel function is called in a child process, where you can't guarantee to see the host's /run directory.

Instead it should be done in the GenSecurityLabel function which is called from a safe context.

Daniel

I did get this to work last night by moving the location of the virSecurityManagerSetProcessLabel to happen in the PivorRoot code before calling lxcContainerMountAllFS Which overmounts the /run directory. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlGWMYQACgkQrlYvE4MpobO9LgCePeIBlJuCTONdoAgeRk11EFE1 saYAnjX5ViWMMTXDI9qDlk59wlE6+3F8 =ju8u -----END PGP SIGNATURE-----