Subject: Re: bug#22631: [PATCH] tests: support non-MLS SELinux systems in mkdir tests

From: Nicolas Iooss <[email-masked]>
Date: Sun, 18 Dec 2016 20:29:22 +0100

On 17/12/16 14:11, Pádraig Brady wrote:
> On 16/12/16 20:47, Nicolas Iooss wrote:
>> On 12/02/16 05:33, Pádraig Brady wrote:
>>> On 11/02/16 06:07, Nicolas Iooss wrote:
>>>> When running "make check" on a Linux system running SELinux with a
>>>> non-MLS policy, tests/mkdir/restorecon.sh test fails with:
>>>>
>>>>   chcon: invalid context: root:object_r:tmp_t:s0: Invalid argument
>>>>
>>>> Indeed in such a configuration, contexts cannot have ":s0" suffix.
>>>>
>>>> * tests/mkdir/restorecon.sh: detect non-MLS SELinux configurations by
>>>>   using sestatus and in this case use a valid context when calling
>>>>   runcon.  Update the sed pattern of get_selinux_type to always grab the
>>>>   SELinux type from the output of "ls -Zd" even with a non-MLS policy.
>>>> ---
>>>>  tests/mkdir/restorecon.sh | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/mkdir/restorecon.sh b/tests/mkdir/restorecon.sh
>>>> index 0e7f03bc93db..cfd3bdda9637 100755
>>>> --- a/tests/mkdir/restorecon.sh
>>>> +++ b/tests/mkdir/restorecon.sh
>>>> @@ -21,10 +21,14 @@ print_ver_ mkdir mknod mkfifo
>>>>  require_selinux_
>>>>  
>>>>  
>>>> -get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\):.*/\1/p'; }
>>>> +get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\)[: ].*/\1/p'; }
>>>>  
>>>>  mkdir subdir || framework_failure_
>>>> -chcon 'root:object_r:tmp_t:s0' subdir || framework_failure_
>>>> +if sestatus 2>&1 |grep 'Policy MLS status:.*enabled' > /dev/null; then
>>>> +  chcon 'root:object_r:tmp_t:s0' subdir || framework_failure_
>>>> +else
>>>> +  chcon 'root:object_r:tmp_t' subdir || framework_failure_
>>>> +fi
>>>>  cd subdir
>>>>  
>>>>  # --- mkdir -Z ---
>>>>
>>>
>>> +1
>>>
>>> thanks!
>>> Pádraig
>>
>> Hi,
>> This patch has not been included in coreutils 8.26, which makes
>> mkdir/restorecon.sh tests still fails on my system. What should I do for
>> this patch to be merged?
>>
>> Moreover the code which was modified in this patch has been copied in
>> tests/install/install-Z-selinux.sh. So this test also fails on systems
>> where SELinux is configured with a non-MLS policy. Do I need to send a
>> new patch which also modifies this file?
> 
> My bad. Sorry I missed this.
> 
> I presume these root tests have the same issue?
> 
>   $ git grep -l ':s0' tests | xargs grep -l require_root_
>   tests/cp/cp-a-selinux.sh
>   tests/misc/chcon.sh
>   tests/misc/selinux.sh
> 
> I updated those also which can be tested like:
> 
>   git am < cu-non-mls-tests.patch
>   sudo make TESTS="$(echo $(git show --name-only | grep ^tests))" check SUBDIRS=.
> 
> Do those pass on your system?
> 
> I'll apply the attached in your name if so.
> 
> thanks for the follow up.
> Pádraig

Thanks for your quick reply. With your patch I get 3 PASS and 2 SKIP on
my system instead of 3 ERROR and 2 SKIP:

PASS: tests/mkdir/restorecon.sh
chcon.sh: skipped test: unexpected context
'sysadm_u:object_r:user_home_t'; turn off mcstransd
SKIP: tests/misc/chcon.sh
PASS: tests/install/install-Z-selinux.sh
PASS: tests/cp/cp-a-selinux.sh
selinux.sh: skipped test: unexpected context
'sysadm_u:object_r:user_home_t'; turn off mcstransd
SKIP: tests/misc/selinux.sh

I am not running mcstransd on and the SKIP were due to incorrect
matching in skip_if_mcstransd_is_running_ function. I updated this
function to accept contexts with three components if MLS is disabled and
got one more PASS:

PASS: tests/mkdir/restorecon.sh
chcon.sh: skipped test: MLS is disabled
SKIP: tests/misc/chcon.sh
PASS: tests/install/install-Z-selinux.sh
PASS: tests/cp/cp-a-selinux.sh
PASS: tests/misc/selinux.sh

My updated patch is attached to this email.

Regards,
Nicolas
>From e462bc8d16cdec6c34fda61a3e01f9636152f0e3 Mon Sep 17 00:00:00 2001
From: Nicolas Iooss <[email-masked]>
Date: Thu, 11 Feb 2016 15:07:52 +0100
Subject: [PATCH 1/1] tests: support non-MLS enabled SELinux systems

When running "make check" on a Linux system running SELinux with a
non-MLS policy, tests/mkdir/restorecon.sh test fails with:

  chcon: invalid context: root:object_r:tmp_t:s0: Invalid argument

Indeed in such a configuration, contexts cannot have ":s0" suffix.

* init.cfg (get_selinux_type): Refactor this function to here
from various tests.  Update to work with a non-MLS policy.
(mls_enabled_): A new function to detect if MLS is enabled.
* tests/mkdir/restorecon.sh: Use a valid non-MLS context when needed.
* tests/install/install-Z-selinux.sh: Likewise.
* tests/cp/cp-a-selinux.sh: Likewise.
* tests/misc/selinux.sh: Likewise.
* tests/misc/chcon.sh: Skip if non-MLS as --range used throughout.
Fixes http://bugs.gnu.org/22631
---
 init.cfg                           | 11 +++++++++++
 tests/cp/cp-a-selinux.sh           |  4 ++--
 tests/install/install-Z-selinux.sh |  7 +++----
 tests/misc/chcon.sh                |  1 +
 tests/misc/selinux.sh              |  3 ++-
 tests/mkdir/restorecon.sh          |  7 +++----
 6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/init.cfg b/init.cfg
index db861944c6af..8bbf7812f520 100644
--- a/init.cfg
+++ b/init.cfg
@@ -128,6 +128,15 @@ require_selinux_()
   esac
 }
 
+# Return the SELinux type component if available
+get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\)[: ].*/\1/p'; }
+
+# Whether SELinux Multi Level Security is enabled
+mls_enabled_() {
+  sestatus 2>&1 |
+  grep 'Policy MLS status:.*enabled' > /dev/null
+}
+
 # Skip this test if we're not in SELinux "enforcing" mode.
 require_selinux_enforcing_()
 {
@@ -638,6 +647,8 @@ skip_if_mcstransd_is_running_()
   __ctx=$(stat --printf='%C\n' .) || framework_failure_
   case $__ctx in
     *:*:*:*) ;; # four components is ok
+    *:*:*)  # three components is ok too if ther is no MLS
+        mls_enabled_ && skip_ "unexpected context '$__ctx'; turn off mcstransd" ;;
     *) # anything else probably means mcstransd is running
         skip_ "unexpected context '$__ctx'; turn off mcstransd" ;;
   esac
diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
index 89735b65a832..3915952188dd 100755
--- a/tests/cp/cp-a-selinux.sh
+++ b/tests/cp/cp-a-selinux.sh
@@ -28,7 +28,8 @@ cwd=$(pwd)
 cleanup_() { cd /; umount "$cwd/mnt"; }
 
 # This context is special: it works even when mcstransd isn't running.
-ctx=root:object_r:tmp_t:s0
+ctx='root:object_r:tmp_t'
+mls_enabled_ && ctx="$ctx:s0"
 
 # Check basic functionality - before check on fixed context mount
 touch c || framework_failure_
@@ -62,7 +63,6 @@ grep $ctx ed_ctx &&
   { ls -lZd restore/existing_dir; fail=1; }
 
 # Check restorecon (-Z) functionality for file and directory
-get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\):.*/\1/p'; }
 # Also make a dir with our known context
 mkdir c_d || framework_failure_
 chcon $ctx c_d || framework_failure_
diff --git a/tests/install/install-Z-selinux.sh b/tests/install/install-Z-selinux.sh
index 9c3b6420bc95..c63a4786230a 100755
--- a/tests/install/install-Z-selinux.sh
+++ b/tests/install/install-Z-selinux.sh
@@ -21,11 +21,10 @@
 print_ver_ ginstall
 require_selinux_
 
-
-get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\):.*/\1/p'; }
-
 mkdir subdir || framework_failure_
-chcon 'root:object_r:tmp_t:s0' subdir || framework_failure_
+ctx='root:object_r:tmp_t'
+mls_enabled_ && ctx="$ctx:s0"
+chcon "$ctx" subdir || framework_failure_
 cd subdir
 
 # Since in a tmp_t dir, dirs can be created as user_tmp_t ...
diff --git a/tests/misc/chcon.sh b/tests/misc/chcon.sh
index bd40fbc7d314..c99021907172 100755
--- a/tests/misc/chcon.sh
+++ b/tests/misc/chcon.sh
@@ -21,6 +21,7 @@ print_ver_ chcon
 require_root_
 require_selinux_
 skip_if_mcstransd_is_running_
+mls_enabled_ || skip_ 'MLS is disabled'
 
 mkdir -p d/sub/s2 || framework_failure_
 touch f g d/sub/1 d/sub/2 || framework_failure_
diff --git a/tests/misc/selinux.sh b/tests/misc/selinux.sh
index a9515680a44f..28c05c4f82d7 100755
--- a/tests/misc/selinux.sh
+++ b/tests/misc/selinux.sh
@@ -30,7 +30,8 @@ mkfifo_or_skip_ p

 # special context that works both with and without mcstransd
-ctx=root:object_r:tmp_t:s0
+ctx='root:object_r:tmp_t'
+mls_enabled_ && ctx="$ctx:s0"
 
 chcon $ctx f d p ||
   skip_ '"chcon '$ctx' ..." failed'
diff --git a/tests/mkdir/restorecon.sh b/tests/mkdir/restorecon.sh
index 0e7f03bc93db..49e72196ff88 100755
--- a/tests/mkdir/restorecon.sh
+++ b/tests/mkdir/restorecon.sh
@@ -20,11 +20,10 @@
 print_ver_ mkdir mknod mkfifo
 require_selinux_
 
-
-get_selinux_type() { ls -Zd "$1" | sed -n 's/.*:\(.*_t\):.*/\1/p'; }
-
 mkdir subdir || framework_failure_
-chcon 'root:object_r:tmp_t:s0' subdir || framework_failure_
+ctx='root:object_r:tmp_t'
+mls_enabled_ && ctx="$ctx:s0"
+chcon "$ctx" subdir || framework_failure_
 cd subdir
 
 # --- mkdir -Z ---
-- 
2.11.0