| Message ID | 20190930131600.21473-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [testsuite] selinux-testsuite: Add submount test | expand |
On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > Add a test that verifies that SELinux permissions are not checked when > mounting submounts. The test sets up a simple local NFS export on a > directory which has another filesystem mounted on its subdirectory. > Since the export is set up with the crossmnt option enabled, any client > mount will try to transparently mount any subdirectory that has a > filesystem mounted on it on the server, triggering an internal mount. > The test tries to access the automounted part of this export via a > client mount without having a permission to mount filesystems, expecting > it to succeed. > > The original bug this test is checking for has been fixed in kernel > commit 892620bb3454 ("selinux: always allow mounting submounts"), which > has been backported to 4.9+ stable kernels. > > The test first checks whether it is able to export and mount directories > via NFS and skips the actual tests if e.g. NFS daemon is not running. > This means that the testsuite can still be run without having the NFS > server installed and running. 1) We have to manually start nfs-server in order for the test to run; else it will be skipped automatically. Do we want to start/stop the nfs-server as part of the test script? 2) I'm still getting test failures: submount/test ............... ls: cannot access '/home/sds/selinux-testsuite/tests/submount/nfs_import': Permission denied submount/test ............... 1/2 # Failed test at submount/test line 70. cat: /home/sds/selinux-testsuite/tests/submount/nfs_import/submount/file: Permission denied # Failed test at submount/test line 75. # Looks like you failed 2 tests of 2. submount/test ............... Dubious, test returned 2 (wstat 512, 0x200) Failed 2/2 subtests type=AVC msg=audit(1569852140.955:10938): avc: denied { search } for pid=20535 comm="ls" name="selinux-testsuite" dev="dm-2" ino=17170498 scontext=unconfined_u:unconfined_r:test_readnfs_t:s0-s0:c0.c1023 tcontext=unconfined_u:object_r:user_home_t:s0 tclass=dir permissive=0 It looks like we need more than userdom_search_generic_user_home_dirs() as presently defined (user_home_dir_t vs user_home_t search access?). > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > README.md | 24 ++++++++++++ > defconfig | 11 ++++++ > policy/Makefile | 2 +- > policy/test_submount.te | 31 +++++++++++++++ > tests/Makefile | 3 +- > tests/submount/Makefile | 2 + > tests/submount/test | 84 +++++++++++++++++++++++++++++++++++++++++ > 7 files changed, 155 insertions(+), 2 deletions(-) > create mode 100644 policy/test_submount.te > create mode 100644 tests/submount/Makefile > create mode 100755 tests/submount/test > > diff --git a/README.md b/README.md > index 1396c8e..f45f5b4 100644 > --- a/README.md > +++ b/README.md > @@ -114,6 +114,30 @@ the tests: > tests/infiniband_pkey/ibpkey_test.conf > tests/infiniband_endport/ibendport_test.conf > > +#### NFS server and client support > + > +The NFS automount test (tests/submount) requires NFS server and client support > +in the kernel. In addition, it requires the NFS utility programs and a running > +NFS daemon. On Fedora/RHEL you need the following package (other distributions > +should have a similar package): > + > +* nfs-utils _(to setup the NFS server and export directory trees)_ > + > +On a modern Fedora system you can install it with the following command: > + > + # dnf install nfs-utils > + > +The process to start the NFS server service varies across distributions, but > +usually you can start it by running: > + > + # service nfs-server start > + (or) > + # service nfs start > + (or) > + # systemctl start nfs-server > + (or) > + # systemctl start nfs > + > ## Running the Tests > > Create a shell with the `unconfined_r` or `sysadm_r` role and the Linux > diff --git a/defconfig b/defconfig > index cb57f22..2bcebbb 100644 > --- a/defconfig > +++ b/defconfig > @@ -67,3 +67,14 @@ CONFIG_ANDROID_BINDERFS=y > # They are not required for SELinux operation itself. > CONFIG_BPF=y > CONFIG_BPF_SYSCALL=y > + > +# NFS server and client. > +# This is enabled for testing NFS automount in tests/submount. > +# It is not required for SELinux operation itself. > +CONFIG_NETWORK_FILESYSTEMS=y > +CONFIG_FILE_LOCKING=y > +CONFIG_MULTIUSER=y > +CONFIG_NFSD=m > +CONFIG_NFSD_V3=y > +CONFIG_NFS_FS=m > +CONFIG_NFS_V3=m > diff --git a/policy/Makefile b/policy/Makefile > index a5942b3..dbef42d 100644 > --- a/policy/Makefile > +++ b/policy/Makefile > @@ -24,7 +24,7 @@ TARGETS = \ > test_task_getsid.te test_task_setpgid.te test_task_setsched.te \ > test_transition.te test_unix_socket.te \ > test_mmap.te test_overlayfs.te test_mqueue.te \ > - test_ibpkey.te test_atsecure.te test_cgroupfs.te > + test_ibpkey.te test_atsecure.te test_cgroupfs.te test_submount.te > > ifeq ($(shell [ $(POL_VERS) -ge 24 ] && echo true),true) > TARGETS += test_bounds.te test_nnp_nosuid.te > diff --git a/policy/test_submount.te b/policy/test_submount.te > new file mode 100644 > index 0000000..8694c6b > --- /dev/null > +++ b/policy/test_submount.te > @@ -0,0 +1,31 @@ > +################################# > +# > +# Policy for testing NFS crosmnt > +# > + > +# A domain that can access NFS files/directories > +type test_readnfs_t; > +domain_type(test_readnfs_t) > +unconfined_runs_test(test_readnfs_t) > +domain_obj_id_change_exemption(test_readnfs_t) > +typeattribute test_readnfs_t testdomain; > + > +# Allow execution of helper programs > +corecmd_exec_bin(test_readnfs_t) > +domain_exec_all_entry_files(test_readnfs_t) > +libs_use_ld_so(test_readnfs_t) > +libs_use_shared_libs(test_readnfs_t) > +libs_exec_ld_so(test_readnfs_t) > +libs_exec_lib_files(test_readnfs_t) > + > +# Allow this domain to be entered from sysadm domain > +miscfiles_domain_entry_test_files(test_readnfs_t) > +userdom_sysadm_entry_spec_domtrans_to(test_readnfs_t) > +corecmd_bin_entry_type(test_readnfs_t) > +sysadm_bin_spec_domtrans_to(test_readnfs_t) > + > +# Allow the domain to search home dirs so that ls works > +userdom_search_generic_user_home_dirs(test_readnfs_t) > + > +# Allow the domain to read NFS mounted files/directories > +fs_read_nfs_files(test_readnfs_t) > diff --git a/tests/Makefile b/tests/Makefile > index e5bdfff..e292be3 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -11,7 +11,8 @@ SUBDIRS:= domain_trans entrypoint execshare exectrace execute_no_trans \ > task_setnice task_setscheduler task_getscheduler task_getsid \ > task_getpgid task_setpgid file ioctl capable_file capable_net \ > capable_sys dyntrans dyntrace bounds nnp_nosuid mmap unix_socket \ > - inet_socket overlay checkreqprot mqueue mac_admin atsecure > + inet_socket overlay checkreqprot mqueue mac_admin atsecure \ > + submount > > ifeq ($(shell grep -q cap_userns $(POLDEV)/include/support/all_perms.spt && echo true),true) > ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1) > diff --git a/tests/submount/Makefile b/tests/submount/Makefile > new file mode 100644 > index 0000000..e7c006f > --- /dev/null > +++ b/tests/submount/Makefile > @@ -0,0 +1,2 @@ > +all: > +clean: > diff --git a/tests/submount/test b/tests/submount/test > new file mode 100755 > index 0000000..754b989 > --- /dev/null > +++ b/tests/submount/test > @@ -0,0 +1,84 @@ > +#!/usr/bin/perl > +# SPDX-License-Identifier: GPL-2.0 > +# > +# This test checks that NFS automounts do not trigger > +# unwanted SELinux checks for mount permission > +# > +# Author: Ondrej Mosnacek <omosnace@redhat.com> > +# Based on code by: Martin Frodl <mfrodl@redhat.com> > +# Copyright (c) 2018 Red Hat, Inc. > +# > + > +use Test::More; > +use File::Spec; > + > +my ( $basedir, $result ); > + > +BEGIN { > + $basedir = File::Spec->rel2abs($0); > + $basedir =~ s|(.*)/[^/]*|$1|; > + > + system "rm -rf $basedir/nfs_export 2>&1"; > + system "rm -rf $basedir/nfs_import 2>&1"; > + > + system "mkdir $basedir/nfs_export"; > + system "mkdir $basedir/nfs_import"; > + > + # Try exporting and mounting NFS. > + $result = > + system "exportfs -o ro,crossmnt,sync 127.0.0.1:$basedir/nfs_export 2>&1"; > + $result += system "mount -t nfs -o ro,timeo=5,retry=0,retrans=1 " > + . "127.0.0.1:$basedir/nfs_export $basedir/nfs_import 2>&1"; > + > + # Cleanup. > + system "umount $basedir/nfs_import 2>&1"; > + system "exportfs -u 127.0.0.1:$basedir/nfs_export 2>&1"; > + > + # If basic NFS workflow failed, then skip the test. > + if ( $result eq 0 ) { > + plan tests => 2; > + } > + else { > + system "rm -rf $basedir/nfs_export 2>&1"; > + system "rm -rf $basedir/nfs_import 2>&1"; > + plan skip_all => "Unable to use NFS server/client"; > + } > +} > + > +# Create an EXT2 image to mount as submount (NFS needs a fs with UUID). > +system > + "dd if=/dev/zero of=$basedir/nfs_export/image.ext2 bs=5M count=1 status=none"; > +system "mkfs.ext2 -q -F $basedir/nfs_export/image.ext2"; > + > +# Create the submount dir and mount the EXT2 image on it. > +system "mkdir $basedir/nfs_export/submount"; > +system "mount -t ext2 -o loop " > + . "$basedir/nfs_export/image.ext2 $basedir/nfs_export/submount"; > +system "touch $basedir/nfs_export/submount/file"; > + > +system "chcon -R -t test_file_t $basedir/nfs_export"; > + > +# Export the directory. > +system "exportfs -o ro,crossmnt,sync 127.0.0.1:$basedir/nfs_export"; > + > +# Mount the NFS export. > +system "mount -t nfs -o ro 127.0.0.1:$basedir/nfs_export $basedir/nfs_import"; > + > +# Sanity check if we can access the mounted filesystem. > +$result = > + system "runcon -t test_readnfs_t -- ls $basedir/nfs_import >/dev/null"; > +ok( $result eq 0 ); > + > +# Check that we can access the submounted filesystem. > +$result = system "runcon -t test_readnfs_t -- " > + . "cat $basedir/nfs_import/submount/file >/dev/null"; > +ok( $result eq 0 ); > + > +# Cleanup. > +system "umount $basedir/nfs_import 2>&1"; > +system "exportfs -u 127.0.0.1:$basedir/nfs_export 2>&1"; > +while ( system("umount $basedir/nfs_export/submount 2>&1") ne 0 ) { > + select( undef, undef, undef, 1 ); > +} > +system "rm -rf $basedir/nfs_export 2>&1"; > +system "rm -rf $basedir/nfs_import 2>&1"; >
On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > > Add a test that verifies that SELinux permissions are not checked when > > mounting submounts. The test sets up a simple local NFS export on a > > directory which has another filesystem mounted on its subdirectory. > > Since the export is set up with the crossmnt option enabled, any client > > mount will try to transparently mount any subdirectory that has a > > filesystem mounted on it on the server, triggering an internal mount. > > The test tries to access the automounted part of this export via a > > client mount without having a permission to mount filesystems, expecting > > it to succeed. > > > > The original bug this test is checking for has been fixed in kernel > > commit 892620bb3454 ("selinux: always allow mounting submounts"), which > > has been backported to 4.9+ stable kernels. > > > > The test first checks whether it is able to export and mount directories > > via NFS and skips the actual tests if e.g. NFS daemon is not running. > > This means that the testsuite can still be run without having the NFS > > server installed and running. > > 1) We have to manually start nfs-server in order for the test to run; > else it will be skipped automatically. Do we want to start/stop the > nfs-server as part of the test script? My two cents are that I'm not sure we want to automatically start/stop the NFS server with the usual "make test", perhaps we have a dedicated NFS test target that does the setup-test-shutdown? Other ideas are welcome.
On 10/8/19 5:30 PM, Paul Moore wrote: > On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: >>> Add a test that verifies that SELinux permissions are not checked when >>> mounting submounts. The test sets up a simple local NFS export on a >>> directory which has another filesystem mounted on its subdirectory. >>> Since the export is set up with the crossmnt option enabled, any client >>> mount will try to transparently mount any subdirectory that has a >>> filesystem mounted on it on the server, triggering an internal mount. >>> The test tries to access the automounted part of this export via a >>> client mount without having a permission to mount filesystems, expecting >>> it to succeed. >>> >>> The original bug this test is checking for has been fixed in kernel >>> commit 892620bb3454 ("selinux: always allow mounting submounts"), which >>> has been backported to 4.9+ stable kernels. >>> >>> The test first checks whether it is able to export and mount directories >>> via NFS and skips the actual tests if e.g. NFS daemon is not running. >>> This means that the testsuite can still be run without having the NFS >>> server installed and running. >> >> 1) We have to manually start nfs-server in order for the test to run; >> else it will be skipped automatically. Do we want to start/stop the >> nfs-server as part of the test script? > > My two cents are that I'm not sure we want to automatically start/stop > the NFS server with the usual "make test", perhaps we have a dedicated > NFS test target that does the setup-test-shutdown? Other ideas are > welcome. I guess my concern is that anything that doesn't run with the default make test probably won't get run at all with any regularity. For something that requires specialized hardware (e.g. InfiniBand), this is reasonable but that isn't true of NFS. For the more analogous cases of e.g. labeled IPSEC, NetLabel, SECMARK, we already load and unload network configurations for the testsuite during testing.
On Wed, Oct 9, 2019 at 9:53 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/8/19 5:30 PM, Paul Moore wrote: > > On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > >>> Add a test that verifies that SELinux permissions are not checked when > >>> mounting submounts. The test sets up a simple local NFS export on a > >>> directory which has another filesystem mounted on its subdirectory. > >>> Since the export is set up with the crossmnt option enabled, any client > >>> mount will try to transparently mount any subdirectory that has a > >>> filesystem mounted on it on the server, triggering an internal mount. > >>> The test tries to access the automounted part of this export via a > >>> client mount without having a permission to mount filesystems, expecting > >>> it to succeed. > >>> > >>> The original bug this test is checking for has been fixed in kernel > >>> commit 892620bb3454 ("selinux: always allow mounting submounts"), which > >>> has been backported to 4.9+ stable kernels. > >>> > >>> The test first checks whether it is able to export and mount directories > >>> via NFS and skips the actual tests if e.g. NFS daemon is not running. > >>> This means that the testsuite can still be run without having the NFS > >>> server installed and running. > >> > >> 1) We have to manually start nfs-server in order for the test to run; > >> else it will be skipped automatically. Do we want to start/stop the > >> nfs-server as part of the test script? > > > > My two cents are that I'm not sure we want to automatically start/stop > > the NFS server with the usual "make test", perhaps we have a dedicated > > NFS test target that does the setup-test-shutdown? Other ideas are > > welcome. > > I guess my concern is that anything that doesn't run with the default > make test probably won't get run at all with any regularity. FWIW, I think I'm the only one regularly running the tests on upstream kernels and reporting the results. RH was running the tests at one point, and may still be doing so, but I have no idea what kernels they are testing (maybe just RHEL, stable Fedora, etc.) and what their process is when they find failures. I also try to enable everything that I can enable for my test runs. Thanks to Mellanox I can even run the IB tests. > For > something that requires specialized hardware (e.g. InfiniBand), this is > reasonable but that isn't true of NFS. For the more analogous cases of > e.g. labeled IPSEC, NetLabel, SECMARK, we already load and unload > network configurations for the testsuite during testing. That's a good point about the other networking tests. My gut feeling tells me that NFS should be "different", but I guess I can't really justify that statement in an objectively meaningful way.
On Wed, Oct 9, 2019 at 4:01 PM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Oct 9, 2019 at 9:53 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On 10/8/19 5:30 PM, Paul Moore wrote: > > > On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > >> On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > > >>> Add a test that verifies that SELinux permissions are not checked when > > >>> mounting submounts. The test sets up a simple local NFS export on a > > >>> directory which has another filesystem mounted on its subdirectory. > > >>> Since the export is set up with the crossmnt option enabled, any client > > >>> mount will try to transparently mount any subdirectory that has a > > >>> filesystem mounted on it on the server, triggering an internal mount. > > >>> The test tries to access the automounted part of this export via a > > >>> client mount without having a permission to mount filesystems, expecting > > >>> it to succeed. > > >>> > > >>> The original bug this test is checking for has been fixed in kernel > > >>> commit 892620bb3454 ("selinux: always allow mounting submounts"), which > > >>> has been backported to 4.9+ stable kernels. > > >>> > > >>> The test first checks whether it is able to export and mount directories > > >>> via NFS and skips the actual tests if e.g. NFS daemon is not running. > > >>> This means that the testsuite can still be run without having the NFS > > >>> server installed and running. > > >> > > >> 1) We have to manually start nfs-server in order for the test to run; > > >> else it will be skipped automatically. Do we want to start/stop the > > >> nfs-server as part of the test script? > > > > > > My two cents are that I'm not sure we want to automatically start/stop > > > the NFS server with the usual "make test", perhaps we have a dedicated > > > NFS test target that does the setup-test-shutdown? Other ideas are > > > welcome. > > > > I guess my concern is that anything that doesn't run with the default > > make test probably won't get run at all with any regularity. > > FWIW, I think I'm the only one regularly running the tests on upstream > kernels and reporting the results. RH was running the tests at one > point, and may still be doing so, but I have no idea what kernels they > are testing (maybe just RHEL, stable Fedora, etc.) and what their > process is when they find failures. We do still run the selinux-testsuite nightly on Fedora Rawhide with your kernel-secnext kernel builds (I suppose we fetch them from COPR). I can't really describe what we do when they fail, because that hardly ever happens now :) But if we came across a failure that would suggest a bug, we would certainly investigate and report it. The testsuite is now also being run as part of CKI (https://github.com/cki-project), which AFAIK currently runs regularly on linux-stable kernels (the results are posted publicly to stable@vger.kernel.org). I don't follow these reports closely, so I'm not sure if there were any non-false-positive failures there... > > I also try to enable everything that I can enable for my test runs. > Thanks to Mellanox I can even run the IB tests. > > > For > > something that requires specialized hardware (e.g. InfiniBand), this is > > reasonable but that isn't true of NFS. For the more analogous cases of > > e.g. labeled IPSEC, NetLabel, SECMARK, we already load and unload > > network configurations for the testsuite during testing. > > That's a good point about the other networking tests. My gut feeling > tells me that NFS should be "different", but I guess I can't really > justify that statement in an objectively meaningful way. I think the main reason why I didn't include NFS server starting was that I don't know how to do it robustly across distros... Already on RHEL the service name varies ("nfs-server" vs. just "nfs") and then there is "service xyz start" vs. "systemctl start xyz"... -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
On Wed, Oct 9, 2019 at 10:53 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Wed, Oct 9, 2019 at 4:01 PM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Oct 9, 2019 at 9:53 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > On 10/8/19 5:30 PM, Paul Moore wrote: > > > > On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > >> On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > > > >>> Add a test that verifies that SELinux permissions are not checked when > > > >>> mounting submounts. The test sets up a simple local NFS export on a > > > >>> directory which has another filesystem mounted on its subdirectory. > > > >>> Since the export is set up with the crossmnt option enabled, any client > > > >>> mount will try to transparently mount any subdirectory that has a > > > >>> filesystem mounted on it on the server, triggering an internal mount. > > > >>> The test tries to access the automounted part of this export via a > > > >>> client mount without having a permission to mount filesystems, expecting > > > >>> it to succeed. > > > >>> > > > >>> The original bug this test is checking for has been fixed in kernel > > > >>> commit 892620bb3454 ("selinux: always allow mounting submounts"), which > > > >>> has been backported to 4.9+ stable kernels. > > > >>> > > > >>> The test first checks whether it is able to export and mount directories > > > >>> via NFS and skips the actual tests if e.g. NFS daemon is not running. > > > >>> This means that the testsuite can still be run without having the NFS > > > >>> server installed and running. > > > >> > > > >> 1) We have to manually start nfs-server in order for the test to run; > > > >> else it will be skipped automatically. Do we want to start/stop the > > > >> nfs-server as part of the test script? > > > > > > > > My two cents are that I'm not sure we want to automatically start/stop > > > > the NFS server with the usual "make test", perhaps we have a dedicated > > > > NFS test target that does the setup-test-shutdown? Other ideas are > > > > welcome. > > > > > > I guess my concern is that anything that doesn't run with the default > > > make test probably won't get run at all with any regularity. > > > > FWIW, I think I'm the only one regularly running the tests on upstream > > kernels and reporting the results. RH was running the tests at one > > point, and may still be doing so, but I have no idea what kernels they > > are testing (maybe just RHEL, stable Fedora, etc.) and what their > > process is when they find failures. > > We do still run the selinux-testsuite nightly on Fedora Rawhide with > your kernel-secnext kernel builds (I suppose we fetch them from COPR). > I can't really describe what we do when they fail, because that hardly > ever happens now :) I'm happy to hear that the tests are still running, but we must be looking at different test results ;) > But if we came across a failure that would suggest > a bug, we would certainly investigate and report it. Great, thank you. > The testsuite is now also being run as part of CKI > (https://github.com/cki-project), which AFAIK currently runs regularly > on linux-stable kernels (the results are posted publicly to > stable@vger.kernel.org). I don't follow these reports closely, so I'm > not sure if there were any non-false-positive failures there... That's good news. I assume CKI has some provision for emailing people when there are test failures? I don't really need to see every -stable kernel test, but it might be nice to see the failures. Alternatively, now that I think about it, this shouldn't be that hard to setup with the secnext stuff ... > > > For > > > something that requires specialized hardware (e.g. InfiniBand), this is > > > reasonable but that isn't true of NFS. For the more analogous cases of > > > e.g. labeled IPSEC, NetLabel, SECMARK, we already load and unload > > > network configurations for the testsuite during testing. > > > > That's a good point about the other networking tests. My gut feeling > > tells me that NFS should be "different", but I guess I can't really > > justify that statement in an objectively meaningful way. > > I think the main reason why I didn't include NFS server starting was > that I don't know how to do it robustly across distros... Already on > RHEL the service name varies ("nfs-server" vs. just "nfs") and then > there is "service xyz start" vs. "systemctl start xyz"... That's another good point. At this point in time I think it is relatively safe to stick with systemd/systemctl (and skip if systemctl is not found) as systemd appears to be eating the world; although this doesn't help with the service name problem.
On Thu, Oct 10, 2019 at 12:50 AM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Oct 9, 2019 at 10:53 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Wed, Oct 9, 2019 at 4:01 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Wed, Oct 9, 2019 at 9:53 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > > On 10/8/19 5:30 PM, Paul Moore wrote: > > > > > On Mon, Sep 30, 2019 at 10:07 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > > >> On 9/30/19 9:16 AM, Ondrej Mosnacek wrote: > > > > >>> Add a test that verifies that SELinux permissions are not checked when > > > > >>> mounting submounts. The test sets up a simple local NFS export on a > > > > >>> directory which has another filesystem mounted on its subdirectory. > > > > >>> Since the export is set up with the crossmnt option enabled, any client > > > > >>> mount will try to transparently mount any subdirectory that has a > > > > >>> filesystem mounted on it on the server, triggering an internal mount. > > > > >>> The test tries to access the automounted part of this export via a > > > > >>> client mount without having a permission to mount filesystems, expecting > > > > >>> it to succeed. > > > > >>> > > > > >>> The original bug this test is checking for has been fixed in kernel > > > > >>> commit 892620bb3454 ("selinux: always allow mounting submounts"), which > > > > >>> has been backported to 4.9+ stable kernels. > > > > >>> > > > > >>> The test first checks whether it is able to export and mount directories > > > > >>> via NFS and skips the actual tests if e.g. NFS daemon is not running. > > > > >>> This means that the testsuite can still be run without having the NFS > > > > >>> server installed and running. > > > > >> > > > > >> 1) We have to manually start nfs-server in order for the test to run; > > > > >> else it will be skipped automatically. Do we want to start/stop the > > > > >> nfs-server as part of the test script? > > > > > > > > > > My two cents are that I'm not sure we want to automatically start/stop > > > > > the NFS server with the usual "make test", perhaps we have a dedicated > > > > > NFS test target that does the setup-test-shutdown? Other ideas are > > > > > welcome. > > > > > > > > I guess my concern is that anything that doesn't run with the default > > > > make test probably won't get run at all with any regularity. > > > > > > FWIW, I think I'm the only one regularly running the tests on upstream > > > kernels and reporting the results. RH was running the tests at one > > > point, and may still be doing so, but I have no idea what kernels they > > > are testing (maybe just RHEL, stable Fedora, etc.) and what their > > > process is when they find failures. > > > > We do still run the selinux-testsuite nightly on Fedora Rawhide with > > your kernel-secnext kernel builds (I suppose we fetch them from COPR). > > I can't really describe what we do when they fail, because that hardly > > ever happens now :) > > I'm happy to hear that the tests are still running, but we must be > looking at different test results ;) Well, we pin the testsuite to a fixed commit and bump it manually as needed/wanted, so we generally don't see failures that are fixed quickly. But I don't recall many (non-false-positive) failures appearing on kernel-secnext in the past few months either. > > > But if we came across a failure that would suggest > > a bug, we would certainly investigate and report it. > > Great, thank you. > > > The testsuite is now also being run as part of CKI > > (https://github.com/cki-project), which AFAIK currently runs regularly > > on linux-stable kernels (the results are posted publicly to > > stable@vger.kernel.org). I don't follow these reports closely, so I'm > > not sure if there were any non-false-positive failures there... > > That's good news. I assume CKI has some provision for emailing people > when there are test failures? I don't really need to see every > -stable kernel test, but it might be nice to see the failures. > Alternatively, now that I think about it, this shouldn't be that hard > to setup with the secnext stuff ... I don't know if it is possible to subscribe to test failures (I'd guess not). There is a notion of test maintainers, who are defined internally, who receive reports of failures for review before they are sent to relevant recipients, but this is an internal-only facility... Maybe you can try filing a feature request at [1]? [1] https://github.com/CKI-project/meta/issues > > > > > For > > > > something that requires specialized hardware (e.g. InfiniBand), this is > > > > reasonable but that isn't true of NFS. For the more analogous cases of > > > > e.g. labeled IPSEC, NetLabel, SECMARK, we already load and unload > > > > network configurations for the testsuite during testing. > > > > > > That's a good point about the other networking tests. My gut feeling > > > tells me that NFS should be "different", but I guess I can't really > > > justify that statement in an objectively meaningful way. > > > > I think the main reason why I didn't include NFS server starting was > > that I don't know how to do it robustly across distros... Already on > > RHEL the service name varies ("nfs-server" vs. just "nfs") and then > > there is "service xyz start" vs. "systemctl start xyz"... > > That's another good point. At this point in time I think it is > relatively safe to stick with systemd/systemctl (and skip if systemctl > is not found) as systemd appears to be eating the world; although this > doesn't help with the service name problem. It seems that wherever the service can be started by systemctl, it is called "nfs-server" (on RHEL-7 there is also an alias "nfs"), so I think we can stick to just 'systemctl start nfs-server' and it should work most (if not all) of the time. -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
diff --git a/README.md b/README.md index 1396c8e..f45f5b4 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,30 @@ the tests: tests/infiniband_pkey/ibpkey_test.conf tests/infiniband_endport/ibendport_test.conf +#### NFS server and client support + +The NFS automount test (tests/submount) requires NFS server and client support +in the kernel. In addition, it requires the NFS utility programs and a running +NFS daemon. On Fedora/RHEL you need the following package (other distributions +should have a similar package): + +* nfs-utils _(to setup the NFS server and export directory trees)_ + +On a modern Fedora system you can install it with the following command: + + # dnf install nfs-utils + +The process to start the NFS server service varies across distributions, but +usually you can start it by running: + + # service nfs-server start + (or) + # service nfs start + (or) + # systemctl start nfs-server + (or) + # systemctl start nfs + ## Running the Tests Create a shell with the `unconfined_r` or `sysadm_r` role and the Linux diff --git a/defconfig b/defconfig index cb57f22..2bcebbb 100644 --- a/defconfig +++ b/defconfig @@ -67,3 +67,14 @@ CONFIG_ANDROID_BINDERFS=y # They are not required for SELinux operation itself. CONFIG_BPF=y CONFIG_BPF_SYSCALL=y + +# NFS server and client. +# This is enabled for testing NFS automount in tests/submount. +# It is not required for SELinux operation itself. +CONFIG_NETWORK_FILESYSTEMS=y +CONFIG_FILE_LOCKING=y +CONFIG_MULTIUSER=y +CONFIG_NFSD=m +CONFIG_NFSD_V3=y +CONFIG_NFS_FS=m +CONFIG_NFS_V3=m diff --git a/policy/Makefile b/policy/Makefile index a5942b3..dbef42d 100644 --- a/policy/Makefile +++ b/policy/Makefile @@ -24,7 +24,7 @@ TARGETS = \ test_task_getsid.te test_task_setpgid.te test_task_setsched.te \ test_transition.te test_unix_socket.te \ test_mmap.te test_overlayfs.te test_mqueue.te \ - test_ibpkey.te test_atsecure.te test_cgroupfs.te + test_ibpkey.te test_atsecure.te test_cgroupfs.te test_submount.te ifeq ($(shell [ $(POL_VERS) -ge 24 ] && echo true),true) TARGETS += test_bounds.te test_nnp_nosuid.te diff --git a/policy/test_submount.te b/policy/test_submount.te new file mode 100644 index 0000000..8694c6b --- /dev/null +++ b/policy/test_submount.te @@ -0,0 +1,31 @@ +################################# +# +# Policy for testing NFS crosmnt +# + +# A domain that can access NFS files/directories +type test_readnfs_t; +domain_type(test_readnfs_t) +unconfined_runs_test(test_readnfs_t) +domain_obj_id_change_exemption(test_readnfs_t) +typeattribute test_readnfs_t testdomain; + +# Allow execution of helper programs +corecmd_exec_bin(test_readnfs_t) +domain_exec_all_entry_files(test_readnfs_t) +libs_use_ld_so(test_readnfs_t) +libs_use_shared_libs(test_readnfs_t) +libs_exec_ld_so(test_readnfs_t) +libs_exec_lib_files(test_readnfs_t) + +# Allow this domain to be entered from sysadm domain +miscfiles_domain_entry_test_files(test_readnfs_t) +userdom_sysadm_entry_spec_domtrans_to(test_readnfs_t) +corecmd_bin_entry_type(test_readnfs_t) +sysadm_bin_spec_domtrans_to(test_readnfs_t) + +# Allow the domain to search home dirs so that ls works +userdom_search_generic_user_home_dirs(test_readnfs_t) + +# Allow the domain to read NFS mounted files/directories +fs_read_nfs_files(test_readnfs_t) diff --git a/tests/Makefile b/tests/Makefile index e5bdfff..e292be3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -11,7 +11,8 @@ SUBDIRS:= domain_trans entrypoint execshare exectrace execute_no_trans \ task_setnice task_setscheduler task_getscheduler task_getsid \ task_getpgid task_setpgid file ioctl capable_file capable_net \ capable_sys dyntrans dyntrace bounds nnp_nosuid mmap unix_socket \ - inet_socket overlay checkreqprot mqueue mac_admin atsecure + inet_socket overlay checkreqprot mqueue mac_admin atsecure \ + submount ifeq ($(shell grep -q cap_userns $(POLDEV)/include/support/all_perms.spt && echo true),true) ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1) diff --git a/tests/submount/Makefile b/tests/submount/Makefile new file mode 100644 index 0000000..e7c006f --- /dev/null +++ b/tests/submount/Makefile @@ -0,0 +1,2 @@ +all: +clean: diff --git a/tests/submount/test b/tests/submount/test new file mode 100755 index 0000000..754b989 --- /dev/null +++ b/tests/submount/test @@ -0,0 +1,84 @@ +#!/usr/bin/perl +# SPDX-License-Identifier: GPL-2.0 +# +# This test checks that NFS automounts do not trigger +# unwanted SELinux checks for mount permission +# +# Author: Ondrej Mosnacek <omosnace@redhat.com> +# Based on code by: Martin Frodl <mfrodl@redhat.com> +# Copyright (c) 2018 Red Hat, Inc. +# + +use Test::More; +use File::Spec; + +my ( $basedir, $result ); + +BEGIN { + $basedir = File::Spec->rel2abs($0); + $basedir =~ s|(.*)/[^/]*|$1|; + + system "rm -rf $basedir/nfs_export 2>&1"; + system "rm -rf $basedir/nfs_import 2>&1"; + + system "mkdir $basedir/nfs_export"; + system "mkdir $basedir/nfs_import"; + + # Try exporting and mounting NFS. + $result = + system "exportfs -o ro,crossmnt,sync 127.0.0.1:$basedir/nfs_export 2>&1"; + $result += system "mount -t nfs -o ro,timeo=5,retry=0,retrans=1 " + . "127.0.0.1:$basedir/nfs_export $basedir/nfs_import 2>&1"; + + # Cleanup. + system "umount $basedir/nfs_import 2>&1"; + system "exportfs -u 127.0.0.1:$basedir/nfs_export 2>&1"; + + # If basic NFS workflow failed, then skip the test. + if ( $result eq 0 ) { + plan tests => 2; + } + else { + system "rm -rf $basedir/nfs_export 2>&1"; + system "rm -rf $basedir/nfs_import 2>&1"; + plan skip_all => "Unable to use NFS server/client"; + } +} + +# Create an EXT2 image to mount as submount (NFS needs a fs with UUID). +system + "dd if=/dev/zero of=$basedir/nfs_export/image.ext2 bs=5M count=1 status=none"; +system "mkfs.ext2 -q -F $basedir/nfs_export/image.ext2"; + +# Create the submount dir and mount the EXT2 image on it. +system "mkdir $basedir/nfs_export/submount"; +system "mount -t ext2 -o loop " + . "$basedir/nfs_export/image.ext2 $basedir/nfs_export/submount"; +system "touch $basedir/nfs_export/submount/file"; + +system "chcon -R -t test_file_t $basedir/nfs_export"; + +# Export the directory. +system "exportfs -o ro,crossmnt,sync 127.0.0.1:$basedir/nfs_export"; + +# Mount the NFS export. +system "mount -t nfs -o ro 127.0.0.1:$basedir/nfs_export $basedir/nfs_import"; + +# Sanity check if we can access the mounted filesystem. +$result = + system "runcon -t test_readnfs_t -- ls $basedir/nfs_import >/dev/null"; +ok( $result eq 0 ); + +# Check that we can access the submounted filesystem. +$result = system "runcon -t test_readnfs_t -- " + . "cat $basedir/nfs_import/submount/file >/dev/null"; +ok( $result eq 0 ); + +# Cleanup. +system "umount $basedir/nfs_import 2>&1"; +system "exportfs -u 127.0.0.1:$basedir/nfs_export 2>&1"; +while ( system("umount $basedir/nfs_export/submount 2>&1") ne 0 ) { + select( undef, undef, undef, 1 ); +} +system "rm -rf $basedir/nfs_export 2>&1"; +system "rm -rf $basedir/nfs_import 2>&1";
Add a test that verifies that SELinux permissions are not checked when mounting submounts. The test sets up a simple local NFS export on a directory which has another filesystem mounted on its subdirectory. Since the export is set up with the crossmnt option enabled, any client mount will try to transparently mount any subdirectory that has a filesystem mounted on it on the server, triggering an internal mount. The test tries to access the automounted part of this export via a client mount without having a permission to mount filesystems, expecting it to succeed. The original bug this test is checking for has been fixed in kernel commit 892620bb3454 ("selinux: always allow mounting submounts"), which has been backported to 4.9+ stable kernels. The test first checks whether it is able to export and mount directories via NFS and skips the actual tests if e.g. NFS daemon is not running. This means that the testsuite can still be run without having the NFS server installed and running. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- README.md | 24 ++++++++++++ defconfig | 11 ++++++ policy/Makefile | 2 +- policy/test_submount.te | 31 +++++++++++++++ tests/Makefile | 3 +- tests/submount/Makefile | 2 + tests/submount/test | 84 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 policy/test_submount.te create mode 100644 tests/submount/Makefile create mode 100755 tests/submount/test