diff mbox

[2/2] common/config: don't hard-code SELinux context

Message ID 20170311005048.128477-2-ebiggers3@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Biggers March 11, 2017, 12:50 a.m. UTC
From: Eric Biggers <ebiggers@google.com>

If SELinux is enabled, xfstests mounts its filesystems with
"-o context=system_u:object_r:nfs_t:s0" so that no SELinux xattrs get
created and interfere with tests.  However, this particular context is
not guaranteed to be available because the context names are a detail of
the SELinux policy.  The SELinux policy on Android systems, for example,
does not have a context with this name.

To fix this, just grab the SELinux context of the root directory.  This
is arbitrary, but it should always provide a valid context.  And any
valid context *should* be okay (i.e. we don't necessarily need a
"liberal" one), since one would likely encounter many other problems if
they were to run xfstests in a confined context with SELinux in
enforcing mode.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/config | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Eryu Guan March 13, 2017, 4:02 a.m. UTC | #1
On Fri, Mar 10, 2017 at 04:50:48PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If SELinux is enabled, xfstests mounts its filesystems with
> "-o context=system_u:object_r:nfs_t:s0" so that no SELinux xattrs get
> created and interfere with tests.  However, this particular context is
> not guaranteed to be available because the context names are a detail of
> the SELinux policy.  The SELinux policy on Android systems, for example,
> does not have a context with this name.
> 
> To fix this, just grab the SELinux context of the root directory.  This
> is arbitrary, but it should always provide a valid context.  And any
> valid context *should* be okay (i.e. we don't necessarily need a
> "liberal" one), since one would likely encounter many other problems if
> they were to run xfstests in a confined context with SELinux in
> enforcing mode.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

SELINUX_MOUNT_OPTIONS has just been updated to be configurable, you can
set your own SELINUX_MOUNT_OPTIONS to override the default one, does
this work for you?

d8b1dc1 common/config: make SELinux protection conditional

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Biggers March 13, 2017, 5:59 p.m. UTC | #2
On Mon, Mar 13, 2017 at 12:02:26PM +0800, Eryu Guan wrote:
> On Fri, Mar 10, 2017 at 04:50:48PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > If SELinux is enabled, xfstests mounts its filesystems with
> > "-o context=system_u:object_r:nfs_t:s0" so that no SELinux xattrs get
> > created and interfere with tests.  However, this particular context is
> > not guaranteed to be available because the context names are a detail of
> > the SELinux policy.  The SELinux policy on Android systems, for example,
> > does not have a context with this name.
> > 
> > To fix this, just grab the SELinux context of the root directory.  This
> > is arbitrary, but it should always provide a valid context.  And any
> > valid context *should* be okay (i.e. we don't necessarily need a
> > "liberal" one), since one would likely encounter many other problems if
> > they were to run xfstests in a confined context with SELinux in
> > enforcing mode.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> SELINUX_MOUNT_OPTIONS has just been updated to be configurable, you can
> set your own SELINUX_MOUNT_OPTIONS to override the default one, does
> this work for you?
> 
> d8b1dc1 common/config: make SELinux protection conditional
> 
> Thanks,
> Eryu

Oh, I didn't notice this.  It looks like Gwendal ran into the same problem, but
on ChromeOS instead of Android.

The problem can indeed be solved by overriding SELINUX_MOUNT_OPTIONS.  But I
think auto-detecting a valid context is better because then xfstests will just
work without having to override SELINUX_MOUNT_OPTIONS.

An exception would be that if for some reason someone actually wants to run
xfstests in some particular SELinux context (maybe one they've set up
specifically for xfstests), then they'd likely need to specify a particular
context when mounting.

How about just doing it both ways: use SELINUX_MOUNT_OPTIONS in the environment
if set, otherwise mount with an auto-detected valid context?

Eric
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan March 14, 2017, 1:06 p.m. UTC | #3
On Mon, Mar 13, 2017 at 10:59:35AM -0700, Eric Biggers wrote:
> On Mon, Mar 13, 2017 at 12:02:26PM +0800, Eryu Guan wrote:
> > On Fri, Mar 10, 2017 at 04:50:48PM -0800, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > If SELinux is enabled, xfstests mounts its filesystems with
> > > "-o context=system_u:object_r:nfs_t:s0" so that no SELinux xattrs get
> > > created and interfere with tests.  However, this particular context is
> > > not guaranteed to be available because the context names are a detail of
> > > the SELinux policy.  The SELinux policy on Android systems, for example,
> > > does not have a context with this name.
> > > 
> > > To fix this, just grab the SELinux context of the root directory.  This
> > > is arbitrary, but it should always provide a valid context.  And any
> > > valid context *should* be okay (i.e. we don't necessarily need a
> > > "liberal" one), since one would likely encounter many other problems if
> > > they were to run xfstests in a confined context with SELinux in
> > > enforcing mode.
> > > 
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > 
> > SELINUX_MOUNT_OPTIONS has just been updated to be configurable, you can
> > set your own SELINUX_MOUNT_OPTIONS to override the default one, does
> > this work for you?
> > 
> > d8b1dc1 common/config: make SELinux protection conditional
> > 
> > Thanks,
> > Eryu
> 
> Oh, I didn't notice this.  It looks like Gwendal ran into the same problem, but
> on ChromeOS instead of Android.
> 
> The problem can indeed be solved by overriding SELINUX_MOUNT_OPTIONS.  But I
> think auto-detecting a valid context is better because then xfstests will just
> work without having to override SELINUX_MOUNT_OPTIONS.
> 
> An exception would be that if for some reason someone actually wants to run
> xfstests in some particular SELinux context (maybe one they've set up
> specifically for xfstests), then they'd likely need to specify a particular
> context when mounting.
> 
> How about just doing it both ways: use SELINUX_MOUNT_OPTIONS in the environment
> if set, otherwise mount with an auto-detected valid context?

This looks reasonable to me, and I tested ext4 ext3 and xfs with auto
group tests with selinux mount option set to `stat -c %C /`, and didn't
see any new failures.

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/common/config b/common/config
index fb60216c..ab635767 100644
--- a/common/config
+++ b/common/config
@@ -259,11 +259,16 @@  case "$HOSTOS" in
 esac
 
 # SELinux adds extra xattrs which can mess up our expected output.
-# So, mount with a context, and they won't be created
-# # nfs_t is a "liberal" context so we can use it.
+# So, mount with a context, and they won't be created.
+#
+# Since the context= option only accepts contexts defined in the
+# SELinux policy, and different systems may have different policies
+# with different context names, use the context of an existing
+# directory.  (Assume that any valid context is fine, since xfstests
+# should really only be run from an "unconfined" process, or with
+# SELinux in permissive mode.)
 if [ -x /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then
-	SELINUX_MOUNT_OPTIONS="-o context=system_u:object_r:nfs_t:s0"
-	export SELINUX_MOUNT_OPTIONS
+	export SELINUX_MOUNT_OPTIONS="-o context=$(stat -c %C /)"
 fi
 
 # check if mkfs.xfs supports v5 xfs