[PATCH 1/2] common/quota: remove redundant SELinux detection code

[PATCH 1/2] common/quota: remove redundant SELinux detection code

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

SELINUX_MOUNT_OPTIONS is already set in common/config.  Setting it again
in common/quota is not necessary.  Nor is SELINUX_MOUNT_OPTIONS specific
to quota tests, so common/quota is not the right place for it.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/quota | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/common/quota b/common/quota
index c48cc70b..f49728c2 100644
--- a/common/quota
+++ b/common/quota
@@ -51,13 +51,6 @@ _require_quota()
 	_notrun "disk quotas not supported by this filesystem type: $FSTYP"
 	;;
     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.
-    if [ -x /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then
-        export SELINUX_MOUNT_OPTIONS="-o context=system_u:object_r:nfs_t:s0"
-    fi
 }
 
 #
-- 
2.12.0.246.ga2ecc84866-goog

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

[Eric Biggers]

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(-)

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
-- 
2.12.0.246.ga2ecc84866-goog

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

[Eryu Guan]

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

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

[Eric Biggers]

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

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

[Eryu Guan]

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