GNU bug report logs - #8359
[PATCH] Unit tests: Properly detect whether SELinux is enabled or not.

Previous Next

Full log

Message #8 received at 8359 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Mathieu Bridon <bochecha <at> fedoraproject.org>
Cc: 8359 <at> debbugs.gnu.org
Subject: Re: bug#8359: [PATCH] Unit tests: Properly detect whether SELinux is
	enabled or not.
Date: Mon, 28 Mar 2011 09:54:19 +0200

Mathieu Bridon wrote:
> The unit tests would run ls to see if the files had an SELinux
> context, and would assume SELinux is enabled if they did.
>
> This is not ideal, and can cause test failures in some environments:
>     https://bugzilla.redhat.com/show_bug.cgi?id=573111#c26
>
> The problem in the case of the above bug report is that the host has
> SELinux enabled (and thus files have a context) but the chroot (mock)
> fakes SELinux being disabled. Unfortunately, it can't remove the
> context, which makes ls thinks that SELinux is enabled.
>
> Later on, when running certain unit tests (e.g id-context), they fail
> as they use the libselinux which (correctly) thinks SELinux is disabled
> (and in the case of id-context, id will not return the context of the
> user).
>
> A better way to test if SELinux is enabled is to search for the SELinux
> filesystem (see the above bug report). This is what this commit does.

Thank you for the diagnosis and patch.
However, I can't use that as-is, since removing the existing test would
mistakenly enable guaranteed-to-fail tests that are run from a file system
that does not support SELinux on a system for which it is enabled.

> diff --git a/tests/init.cfg b/tests/init.cfg
> index f74d50c..ca92297 100644
> --- a/tests/init.cfg
> +++ b/tests/init.cfg
> @@ -216,12 +216,9 @@ skip_if_()
>
>  require_selinux_()
>  {
> -  case `ls -Zd .` in
> -    '? .'|'unlabeled .')
> -      skip_test_ "this system (or maybe just" \
> -        "the current file system) lacks SELinux support"
> -    ;;
> -  esac
> +  grep selinux /proc/filesystems > /dev/null || \
> +    skip_test_ "this system (or maybe just" \
> +      "the current file system) lacks SELinux support"
>  }

I've adjusted it to address the above.
Also, I've tightened the regexp slightly, just in case,
and made the diagnostic more precise.
I've also rewritten the commit log.

Hmm... actually, I now have mixed feelings about this change.
Having SELinux enabled for id --context is conceptually a very
different thing from having an SELinux-enabled file system.
Now, I'm thinking that your new condition should guard only the id-context
test, rather than causing us to skip all FS-context-requiring tests.
In your environment, does any test other than id-context fail without
this patch?

From 1ff10c3073e2c20c9a7a9ff0e2cc93a3e16b41bd Mon Sep 17 00:00:00 2001
From: Mathieu Bridon <bochecha <at> fedoraproject.org>
Date: Mon, 28 Mar 2011 09:39:53 +0200
Subject: [PATCH] tests: avoid unwarranted failure in mock-simulated
 non-SELinux env.

* tests/init.cfg (require_selinux_): Skip the test also when
/proc/filesystems does not list selinuxfs.
Add comments.
Based on the patch by Mathieu Bridon in http://debbugs.gnu.org/8359.
More discussion in http://bugzilla.redhat.com/573111
---
 tests/init.cfg |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/tests/init.cfg b/tests/init.cfg
index f74d50c..0711455 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -216,6 +216,13 @@ skip_if_()

 require_selinux_()
 {
+  # When in a chroot of an SELinux-enabled system, but with a mock-simulated
+  # SELinux-*disabled* system, recognize that SELinux is disabled system wide:
+  grep 'selinuxfs$' /proc/filesystems > /dev/null \
+    || skip_test_ "this system lacks SELinux support"
+
+  # Independent of whether SELinux is enabled system-wide,
+  # the current file system may lack SELinux support.
   case `ls -Zd .` in
     '? .'|'unlabeled .')
       skip_test_ "this system (or maybe just" \
--
1.7.4.1.688.g95e3e

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.