Re: [PATCH] NFS: Ensure security label is set for root inode

From: Stephen Smalley <stephen.smalley.work@gmail.com>
To: Richard Haines <richard_c_haines@btinternet.com>
Cc: Scott Mayhew <smayhew@redhat.com>,
	trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
	bfields@fieldses.org, Paul Moore <paul@paul-moore.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	linux-nfs@vger.kernel.org, SElinux list <selinux@vger.kernel.org>
Subject: Re: [PATCH] NFS: Ensure security label is set for root inode
Date: Mon, 9 Mar 2020 09:35:22 -0400	[thread overview]
Message-ID: <CAEjxPJ6pLLGQ2ywfjkanDNZc1isVV8=6sJmoYFy8shaSGr972A@mail.gmail.com> (raw)
In-Reply-To: <dc704637496883ac7c21c196aeae4e1ab37f76fa.camel@btinternet.com>

On Sun, Mar 8, 2020 at 1:47 PM Richard Haines
<richard_c_haines@btinternet.com> wrote:
>
> On Fri, 2020-03-06 at 17:01 -0500, Scott Mayhew wrote:
> > On Wed, 04 Mar 2020, Stephen Smalley wrote:
> > > I should note that we are getting similar errors though when trying
> > > to
> > > specify any context-related
> > > mount options on NFS via the new fsconfig(2) system call, see
> > > https://github.com/SELinuxProject/selinux-kernel/issues/49
> I've done further testing and found that with this patch the
> fsconfig(2) problem is also resolved for nfs (provided the rootcontext
> is not specified).

Excellent, two bugs fixed with one patch!

>>> It still needs some further
> > > enhancements as per
> > > https://github.com/SELinuxProject/selinux-testsuite/issues/32#issuecomment-582992492
> > > but it at least provides some degree of regression testing.
> Could you describe how I could test these, also are there any other
> SELinux tests that may be useful (with howto's). I'm almost ready to
> post another set of RFC test patches, but can add more.

The ones identified in that github issue comment would simply be
additional tests in tools/nfs.sh
unless they happen to already be covered by your fs/filesystem tests
once those are applied to the
host/native filesystem instead of just ext4.  The test cases are:

0. Test the bug fixed by this patch, i.e. perform mount of a
security_label exported filesystem, check the label of the mounted
directory to confirm it isn't unlabeled.
That's a NFS-specific test, goes in tools/nfs.sh.

1. Mount the same filesystem twice with two different sets of context
mount options, check that mount(2) fails with errno EINVAL.
Test cases might include a) mount first without any context mount
options, then try mounting a 2nd time with context mount options and
vice versa, b) mounting
with the same set of context options (e.g. both using context=) but
different context values, c) mounting with different sets of context
options (e.g. one uses
context=, the other uses fscontext=).  This test could be done in
fs/filesystem for any filesystem type, not NFS-specific.

2. Mount a security_label exported NFS filesystem twice, confirm that
NFS security labeling support isn't silently disabled by trying to
set a label on a file and confirm it is set (fixed by  kernel commit
3815a245b50124f0865415dcb606a034e97494d4).  This would go in
tools/nfs.sh
since it is NFS-specific.

3. Perform a context= mount of a security_label exported NFS
filesystem, check that pre-existing files within the mount show up
with the context= value
not the underlying xattr value (fixed by kernel commit
0b4d3452b8b4a5309b4445b900e3cec022cca95a). My original version of
tools/nfs.sh actually would have caught this because it was testing
the context of the nfs.sh script file itself within the context mount
but I dropped it back to only checking the top-level mount directory
when I moved tools/nfs.sh to avoid depending on a fixed location for
it, so it won't be caught currently. We could just change it back to
testing the context of a pre-existing file within the mount; any file
will do.  This would go in tools/nfs.sh, NFS-specific.

4. Ensuring that all of the tests/filesystem and tests/fs_filesystem
tests that make sense for NFS are being run on the labeled NFS mount
itself when run via tools/nfs.sh and not just on an ext4 mount created
by the test script.