Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ondrej Mosnacek <omosnace@redhat.com>
To:     Paul Moore <paul@paul-moore.com>
Cc:     selinux@vger.kernel.org, linux-nfs@vger.kernel.org,
        Scott Mayhew <smayhew@redhat.com>
Subject: [PATCH] selinux: make labeled NFS work when mounted before policy load
Date:   Mon, 29 May 2023 16:05:27 +0200
Message-Id: <20230529140527.901242-1-omosnace@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Currently, when an NFS filesystem that supports passing LSM/SELinux
labels is mounted during early boot (before the SELinux policy is
loaded), it ends up mounted without the labeling support (i.e. with
Fedora policy all files get the generic NFS label
system_u:object_r:nfs_t:s0).

This is because the information that the NFS mount supports passing
labels (communicated to the LSM layer via the kern_flags argument of
security_set_mnt_opts()) gets lost and when the policy is loaded the
mount is initialized as if the passing is not supported.

Fix this by noting the "native labeling" in newsbsec->flags (using a new
SE_SBNATIVE flag) on the pre-policy-loaded call of
selinux_set_mnt_opts() and then making sure it is respected on the
second call from delayed_superblock_init().

Additionally, make inode_doinit_with_dentry() initialize the inode's
label from its extended attributes whenever it doesn't find it already
intitialized by the filesystem. This is needed to properly initialize
pre-existing inodes when delayed_superblock_init() is called. It should
not trigger in any other cases (and if it does, it's still better to
initialize the correct label instead of leaving the inode unlabeled).

Fixes: eb9ae686507bc5 ("SELinux: Add new labeling type native labels")
Tested-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 security/selinux/hooks.c            | 58 ++++++++++++++++++++---------
 security/selinux/include/security.h |  1 +
 2 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 99ded60a6b911..d600616a9e6d1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -605,6 +605,13 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	u32 defcontext_sid = 0;
 	int rc = 0;
 
+	/*
+	 * Specifying internal flags without providing a place to
+	 * place the results is not allowed
+	 */
+	if (kern_flags && !set_kern_flags)
+		return -EINVAL;
+
 	mutex_lock(&sbsec->lock);
 
 	if (!selinux_initialized()) {
@@ -612,6 +619,10 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 			/* Defer initialization until selinux_complete_init,
 			   after the initial policy is loaded and the security
 			   server is ready to handle calls. */
+			if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
+				sbsec->flags |= SE_SBNATIVE;
+				*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+			}
 			goto out;
 		}
 		rc = -EINVAL;
@@ -619,12 +630,6 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 			"before the security server is initialized\n");
 		goto out;
 	}
-	if (kern_flags && !set_kern_flags) {
-		/* Specifying internal flags without providing a place to
-		 * place the results is not allowed */
-		rc = -EINVAL;
-		goto out;
-	}
 
 	/*
 	 * Binary mount data FS will come through this function twice.  Once
@@ -757,7 +762,17 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	 * sets the label used on all file below the mountpoint, and will set
 	 * the superblock context if not already set.
 	 */
-	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !context_sid) {
+	if (sbsec->flags & SE_SBNATIVE) {
+		/*
+		 * This means we are initializing a superblock that has been
+		 * mounted before the SELinux was initialized and the
+		 * filesystem requested native labeling. We had already
+		 * returned SECURITY_LSM_NATIVE_LABELS in *set_kern_flags
+		 * in the original mount attempt, so now we just need to set
+		 * the SECURITY_FS_USE_NATIVE behavior.
+		 */
+		sbsec->behavior = SECURITY_FS_USE_NATIVE;
+	} else if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !context_sid) {
 		sbsec->behavior = SECURITY_FS_USE_NATIVE;
 		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
 	}
@@ -868,13 +883,6 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	int set_context =	(oldsbsec->flags & CONTEXT_MNT);
 	int set_rootcontext =	(oldsbsec->flags & ROOTCONTEXT_MNT);
 
-	/*
-	 * if the parent was able to be mounted it clearly had no special lsm
-	 * mount options.  thus we can safely deal with this superblock later
-	 */
-	if (!selinux_initialized())
-		return 0;
-
 	/*
 	 * Specifying internal flags without providing a place to
 	 * place the results is not allowed.
@@ -882,18 +890,31 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	if (kern_flags && !set_kern_flags)
 		return -EINVAL;
 
+	mutex_lock(&newsbsec->lock);
+
+	/*
+	 * if the parent was able to be mounted it clearly had no special lsm
+	 * mount options.  thus we can safely deal with this superblock later
+	 */
+	if (!selinux_initialized()) {
+		if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
+			newsbsec->flags |= SE_SBNATIVE;
+			*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+		}
+		goto out;
+	}
+
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
 	/* if fs is reusing a sb, make sure that the contexts match */
 	if (newsbsec->flags & SE_SBINITIALIZED) {
+		mutex_unlock(&newsbsec->lock);
 		if ((kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context)
 			*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
 		return selinux_cmp_sb_context(oldsb, newsb);
 	}
 
-	mutex_lock(&newsbsec->lock);
-
 	newsbsec->flags = oldsbsec->flags;
 
 	newsbsec->sid = oldsbsec->sid;
@@ -1394,8 +1415,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 	spin_unlock(&isec->lock);
 
 	switch (sbsec->behavior) {
+	/*
+	 * In case of SECURITY_FS_USE_NATIVE we need to re-fetch the labels
+	 * via xattr when called from delayed_superblock_init().
+	 */
 	case SECURITY_FS_USE_NATIVE:
-		break;
 	case SECURITY_FS_USE_XATTR:
 		if (!(inode->i_opflags & IOP_XATTR)) {
 			sid = sbsec->def_sid;
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 8746fafeb7789..0b9c2c25413ba 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -65,6 +65,7 @@
 #define SE_SBPROC		0x0200
 #define SE_SBGENFS		0x0400
 #define SE_SBGENFS_XATTR	0x0800
+#define SE_SBNATIVE		0x1000
 
 #define CONTEXT_STR	"context"
 #define FSCONTEXT_STR	"fscontext"
-- 
2.40.1