[PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Scott Mayhew]

Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
The problem is related to how filesystems are mounted in NFSv4.

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.

NFS servers with a modern nfs-utils package will automatically create a
pseudo fs to fill in the gaps (including the root itself) leading up to
the actual export, so it is uncommon these days for an NFS server to
have an explicit fsid=0 export.

Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
flag on an initialized superblock would ensure that SBLABEL_MNT is set
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option.

Scott Mayhew (2):
  selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
    initialized superblock
  nfs: update labeling behavior on a superblock when submounting

 fs/nfs/super.c           | 23 ++++++++++++++++++++++-
 security/selinux/hooks.c |  4 ++--
 2 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.9.3

[PATCH RFC 1/2] selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already initialized superblock

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.

Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
flag on an initialized superblock will ensure that SBLABEL_MNT is set
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 security/selinux/hooks.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..366ab86 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -730,7 +730,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	 * will be used for both mounts)
 	 */
 	if ((sbsec->flags & SE_SBINITIALIZED) && (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)
-	    && (num_opts == 0))
+	    && (num_opts == 0) && !(kern_flags & SECURITY_LSM_NATIVE_LABELS))
 		goto out;
 
 	root_isec = backing_inode_security_novalidate(root);
@@ -797,7 +797,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 		}
 	}
 
-	if (sbsec->flags & SE_SBINITIALIZED) {
+	if (sbsec->flags & SE_SBINITIALIZED && !(kern_flags & SECURITY_LSM_NATIVE_LABELS)) {
 		/* previously mounted with options, but not on this attempt? */
 		if ((sbsec->flags & SE_MNTMASK) && !num_opts)
 			goto out_double_mount;
-- 
2.9.3

[PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Scott Mayhew]

When the client traverses from filesystem exported without the
"security_label" option to one exported with the "security_label"
option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
set in its security mount options.  Otherwise, attempts to set security
labels via setxattr over NFSv4.2 will fail.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..d7a3b89 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+	struct security_mnt_opts opts;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+	if (error)
+		goto err;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(NFS_SB(mount_info->cloned->sb)->caps & NFS_CAP_SECURITY_LABEL)) {
+		memset(&opts, 0, sizeof(opts));
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+		error = security_sb_set_mnt_opts(s, &opts, kflags, &kflags_out);
+		if (error)
+			goto err;
+
+		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	}
+err:
+	return error;
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
-- 
2.9.3

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> When the client traverses from filesystem exported without the
> "security_label" option to one exported with the "security_label"
> option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
> set in its security mount options.  Otherwise, attempts to set
> security
> labels via setxattr over NFSv4.2 will fail.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..d7a3b89 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +	struct security_mnt_opts opts;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +	if (error)
> +		goto err;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(NFS_SB(mount_info->cloned->sb)->caps &
> NFS_CAP_SECURITY_LABEL)) {
> +		memset(&opts, 0, sizeof(opts));
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +		error = security_sb_set_mnt_opts(s, &opts, kflags,
> &kflags_out);
> +		if (error)
> +			goto err;
> +
> +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	}
> +err:
> +	return error;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);

Could this clobber a context set via context= mount option?

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Scott Mayhew]

On Fri, 26 May 2017, Stephen Smalley wrote:

> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > When the client traverses from filesystem exported without the
> > "security_label" option to one exported with the "security_label"
> > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > security_sb_set_mnt_opts() so that the new superblock has SBLABEL_MNT
> > set in its security mount options.  Otherwise, attempts to set
> > security
> > labels via setxattr over NFSv4.2 will fail.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..d7a3b89 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +	struct security_mnt_opts opts;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +	if (error)
> > +		goto err;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > NFS_CAP_SECURITY_LABEL)) {
> > +		memset(&opts, 0, sizeof(opts));
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +		error = security_sb_set_mnt_opts(s, &opts, kflags,
> > &kflags_out);
> > +		if (error)
> > +			goto err;
> > +
> > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +			NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +	}
> > +err:
> > +	return error;
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> 
> Could this clobber a context set via context= mount option?

Argh, yes I suppose it could.  In my first attempt to fix this, I added
a security_sb_get_mnt_opts() hook to get the original mount options and
then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
security_sb_set_mnt_opts().  When I saw that security_sb_set_mnt_opts()
wouldn't allow me to change a superblock that had already been
initialized, I got rid of the hook and added the check in patch 1...
maybe a combination of the two is needed?

Testing it again now, I'm not sure the context= mount option is working
correctly with the latest kernel.

-Scott

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> On Fri, 26 May 2017, Stephen Smalley wrote:
> 
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > When the client traverses from filesystem exported without the
> > > "security_label" option to one exported with the "security_label"
> > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > security_sb_set_mnt_opts() so that the new superblock has
> > > SBLABEL_MNT
> > > set in its security mount options.  Otherwise, attempts to set
> > > security
> > > labels via setxattr over NFSv4.2 will fail.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..d7a3b89 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +	struct security_mnt_opts opts;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new
> > > sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > NFS_CAP_SECURITY_LABEL)) {
> > > +		memset(&opts, 0, sizeof(opts));
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > kflags,
> > > &kflags_out);
> > > +		if (error)
> > > +			goto err;
> > > +
> > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +			NFS_SB(s)->caps &=
> > > ~NFS_CAP_SECURITY_LABEL;
> > > +	}
> > > +err:
> > > +	return error;
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > 
> > Could this clobber a context set via context= mount option?
> 
> Argh, yes I suppose it could.  In my first attempt to fix this, I
> added
> a security_sb_get_mnt_opts() hook to get the original mount options
> and
> then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> security_sb_set_mnt_opts().  When I saw that
> security_sb_set_mnt_opts()
> wouldn't allow me to change a superblock that had already been
> initialized, I got rid of the hook and added the check in patch 1...
> maybe a combination of the two is needed?

Could we instead extend the security_sb_clone_mnt_opts() hook interface
to pass flags, and just handle it inside of the
selinux_sb_clone_mnt_opts() hook?  Then we don't have to change
selinux_sb_set_mnt_opts() handling, which may be called for mount(2)
from userspace, not just from NFS client code.  You might need to
refactor it to share common code, but we shouldn't alter its logic when
called normally.


> Testing it again now, I'm not sure the context= mount option is
> working
> correctly with the latest kernel.

Hmm...if not, then that's another regression that will need to be
addressed.

[PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 18 +++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 43 ++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..6a11535 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags, &kflags_out);
+	if (error)
+		goto err;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+err:
+	return error;
+
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..2f54bfb 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1388,7 +1388,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..a55ae9c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index b9fea39..7b70ea2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..80d9acf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block *sb)
 		       sb->s_id, sb->s_type->name);
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	   leave the flag untouched because sb_clone_mnt_opts might be handing
+	   us a superblock that needs the flag to be cleared. */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	 * mount options.  thus we can safely deal with this superblock later
 	 */
 	if (!ss_initialized)
-		return 0;
+		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;
+	}
 
 	/* 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)
-		return selinux_cmp_sb_context(oldsb, newsb);
+	if (newsbsec->flags & SE_SBINITIALIZED) {
+		rc = selinux_cmp_sb_context(oldsb, newsb);
+		goto out;
+	}
 
 	mutex_lock(&newsbsec->lock);
 
@@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
+			&& !(kern_flags & SECURITY_LSM_NATIVE_LABELS)
+			&& !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out_unlock;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out_unlock:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+out:
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Scott Mayhew wrote:

> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 43 ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 69 insertions(+), 11 deletions(-)

With this patch, the behavior and flags fields of the superblock_security_struct
and the nfs_server->caps all have the correct values when crossing from an
export without the "security_label" option to one with the "security_label"
option and vice versa.

Here /export/export1 and /export/export1/export2/export3 have the
"security_label" option; /export and /export/export1/export2 do not:

[root@rhel7u4 ~]# exportfs -v
/export       	<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1
		<world>(rw,sync,wdelay,hide,no_subtree_check,security_label,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1/export2
		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,secure,no_root_squash,no_all_squash)
/export/export1/export2/export3
		<world>(rw,sync,wdelay,hide,no_subtree_check,security_label,sec=sys,secure,no_root_squash,no_all_squash)


[root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
[root@fedora25 ~]# ls -lZ /mnt/t
total 8
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55 test
[root@fedora25 ~]# ls -lZ /mnt/t/export1
total 20
drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
total 20
drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
total 16
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30 14:02 lost+found

I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
but not /mnt/t or /mnt/t/export1/export2:

[root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
chcon: failed to change context of '/mnt/t/file1' to ‘system_u:object_r:usr_t:s0’: Operation not supported
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
chcon: failed to change context of '/mnt/t/export1/export2/file1' to ‘system_u:object_r:usr_t:s0’: Operation not supported
[root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/export3/file1
[root@fedora25 ~]# ls -lZ /mnt/t/file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09 /mnt/t/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09 /mnt/t/export1/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09 /mnt/t/export1/export2/file1
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08 /mnt/t/export1/export2/export3/file1
[root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
[root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/export2/export3/file1

/mnt/t/export1 and /mnt/t/export1/export2/export3 have the "seclabel"
mount option; /mnt/t and /mnt/t/export1/export do not:

[root@fedora25 ~]# cat /proc/mounts
...
rhel7u4:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1 /mnt/t/export1 nfs4 rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4 rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2/export3 /mnt/t/export1/export2/export3 nfs4 rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0

The capabilities flags has NFS_CAP_SECURITY_LABEL set on /mnt/t/export1 and
/mnt/t/export1/export2/export3; it is not set on /mnt/t and /mnt/t/export1/export2:

[root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
device rhel7u4:/export mounted on /mnt/t with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype nfs4 statvers=1.1
	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2 mounted on /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2/export3 mounted on /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
[root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
0
[root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
40000
[root@fedora25 ~]# 

The context= mount option now works correctly:

[root@fedora25 ~]# mount -o v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export /mnt/t
[root@fedora25 ~]# ls -lZ /mnt/t
total 8
drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr 21 12:55 test
[root@fedora25 ~]# ls -lZ /mnt/t/export1
total 20
drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
total 20
drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found
[root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
total 16
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30 14:02 lost+found

[root@fedora25 ~]# cat /proc/mounts
...
rhel7u4:/export /mnt/t nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1 /mnt/t/export1 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0
rhel7u4:/export/export1/export2/export3 /mnt/t/export1/export2/export3 nfs4 rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.122.83 0 0

[root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
device rhel7u4:/export mounted on /mnt/t with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2 mounted on /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255
device rhel7u4:/export/export1/export2/export3 mounted on /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,namlen=255

-Scott

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..6a11535 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags, &kflags_out);
> +	if (error)
> +		goto err;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +err:
> +	return error;
> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					      struct super_block *newsb)
> +					      struct super_block *newsb,
> +					      unsigned long kern_flags,
> +					      unsigned long *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..80d9acf 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +	   leave the flag untouched because sb_clone_mnt_opts might be handing
> +	   us a superblock that needs the flag to be cleared. */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>  	struct superblock_security_struct *newsbsec = newsb->s_security;
>  
> @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	 * mount options.  thus we can safely deal with this superblock later
>  	 */
>  	if (!ss_initialized)
> -		return 0;
> +		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;
> +	}
>  
>  	/* 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)
> -		return selinux_cmp_sb_context(oldsb, newsb);
> +	if (newsbsec->flags & SE_SBINITIALIZED) {
> +		rc = selinux_cmp_sb_context(oldsb, newsb);
> +		goto out;
> +	}
>  
>  	mutex_lock(&newsbsec->lock);
>  
> @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> +			&& !(kern_flags & SECURITY_LSM_NATIVE_LABELS)
> +			&& !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out_unlock;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out_unlock:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +out:
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> On Thu, 01 Jun 2017, Scott Mayhew wrote:
> 
> > When an NFSv4 client performs a mount operation, it first mounts
> > the
> > NFSv4 root and then does path walk to the exported path and
> > performs a
> > submount on that, cloning the security mount options from the
> > root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct
> > value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if
> > SECURITY_LSM_NATIVE_LABELS is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 43
> > ++++++++++++++++++++++++++++++++++++++-----
> >  5 files changed, 69 insertions(+), 11 deletions(-)
> 
> With this patch, the behavior and flags fields of the
> superblock_security_struct
> and the nfs_server->caps all have the correct values when crossing
> from an
> export without the "security_label" option to one with the
> "security_label"
> option and vice versa.
> 
> Here /export/export1 and /export/export1/export2/export3 have the
> "security_label" option; /export and /export/export1/export2 do not:
> 
> [root@rhel7u4 ~]# exportfs -v
> /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,se
> c=sys,secure,no_root_squash,no_all_squash)
> /export/export1
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> abel,sec=sys,secure,no_root_squash,no_all_squash)
> /export/export1/export2
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> cure,no_root_squash,no_all_squash)
> /export/export1/export2/export3
> 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> abel,sec=sys,secure,no_root_squash,no_all_squash)
> 
> 
> [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> [root@fedora25 ~]# ls -lZ /mnt/t
> total 8
> drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09
> export1

Shouldn't this be the label of export1 from the server, since export1
is exported with security_label?

> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file2
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09
> file3
> drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58
> scratch
> drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55
> test
> [root@fedora25 ~]# ls -lZ /mnt/t/export1
> total 20
> drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May 30
> 14:09 export2

Shouldn't this be system_u:object_r:nfs_t:s0, like the rest of export2?

> -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> total 20
> drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08
> export3

Shouldn't this be the label of export3 from the server, since it is
exported with security_label?

> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file2
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09
> file3
> drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02
> lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> total 16
> -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May 30
> 14:08 file1
> -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May 30
> 14:08 file2
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May 30
> 14:08 file3
> drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May 30
> 14:02 lost+found
> 
> I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
> but not /mnt/t or /mnt/t/export1/export2:
> 
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
> chcon: failed to change context of '/mnt/t/file1' to
> ‘system_u:object_r:usr_t:s0’: Operation not supported
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
> [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
> chcon: failed to change context of '/mnt/t/export1/export2/file1' to
> ‘system_u:object_r:usr_t:s0’: Operation not supported
> [root@fedora25 ~]# chcon -t usr_t
> /mnt/t/export1/export2/export3/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> /mnt/t/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09
> /mnt/t/export1/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
> -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> /mnt/t/export1/export2/file1
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
> -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08
> /mnt/t/export1/export2/export3/file1
> [root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
> [root@fedora25 ~]# chcon -t etc_t
> /mnt/t/export1/export2/export3/file1
> 
> /mnt/t/export1 and /mnt/t/export1/export2/export3 have the "seclabel"
> mount option; /mnt/t and /mnt/t/export1/export do not:
> 
> [root@fedora25 ~]# cat /proc/mounts
> ...
> rhel7u4:/export /mnt/t nfs4
> rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=
> tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,loc
> al_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1 /mnt/t/export1 nfs4
> rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,ha
> rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.12
> 2.236,local_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,proto=
> tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,loc
> al_lock=none,addr=192.168.122.83 0 0
> rhel7u4:/export/export1/export2/export3
> /mnt/t/export1/export2/export3 nfs4
> rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,ha
> rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.12
> 2.236,local_lock=none,addr=192.168.122.83 0 0
> 
> The capabilities flags has NFS_CAP_SECURITY_LABEL set on
> /mnt/t/export1 and
> /mnt/t/export1/export2/export3; it is not set on /mnt/t and
> /mnt/t/export1/export2:
> 
> [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype
> nfs4 statvers=1.1
> 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2 mounted on
> /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2/export3 mounted on
> /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> [root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
> 0
> [root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
> 40000
> [root@fedora25 ~]# 
> 
> The context= mount option now works correctly:
> 
> [root@fedora25 ~]# mount -o
> v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export
> /mnt/t
> [root@fedora25 ~]# ls -lZ /mnt/t
> total 8
> drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May 30
> 14:09 export1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May 30
> 14:09 file3
> drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr 21
> 12:58 scratch
> drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr 21
> 12:55 test
> [root@fedora25 ~]# ls -lZ /mnt/t/export1
> total 20
> drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May 30
> 14:09 export2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> total 20
> drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May 30
> 14:08 export3
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:09 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> total 16
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file1
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file2
> -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May 30
> 14:08 file3
> drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May 30
> 14:02 lost+found
> 
> [root@fedora25 ~]# cat /proc/mounts
> ...
> rhel7u4:/export /mnt/t nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1 /mnt/t/export1 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> rhel7u4:/export/export1/export2/export3
> /mnt/t/export1/export2/export3 nfs4
> rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize=1
> 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans
> =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.12
> 2.83 0 0
> 
> [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1 mounted on /mnt/t/export1 with fstype
> nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2 mounted on
> /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> device rhel7u4:/export/export1/export2/export3 mounted on
> /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> len=255
> 
> -Scott
> 
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..6a11535 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the
> > new sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, 
> > s, kflags, &kflags_out);
> > +	if (error)
> > +		goto err;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +err:
> > +	return error;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block
> > *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int
> > mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int
> > security_sb_set_mnt_opts(struct super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..80d9acf 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct
> > super_block *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/* Explicitly set or clear SBLABEL_MNT.  It's not
> > sufficient to simply
> > +	   leave the flag untouched because sb_clone_mnt_opts
> > might be handing
> > +	   us a superblock that needs the flag to be cleared. */
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	 * mount options.  thus we can safely deal with this
> > superblock later
> >  	 */
> >  	if (!ss_initialized)
> > -		return 0;
> > +		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;
> > +	}
> >  
> >  	/* 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)
> > -		return selinux_cmp_sb_context(oldsb, newsb);
> > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > +		goto out;
> > +	}
> >  
> >  	mutex_lock(&newsbsec->lock);
> >  
> > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > +			&& !(kern_flags &
> > SECURITY_LSM_NATIVE_LABELS)
> > +			&& !set_context) {
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out_unlock;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS &&
> > !set_context) {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out_unlock:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +out:
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,
> > -- 
> > 2.9.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 14:08 -0400, Stephen Smalley wrote:
> On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> > On Thu, 01 Jun 2017, Scott Mayhew wrote:
> > 
> > > When an NFSv4 client performs a mount operation, it first mounts
> > > the
> > > NFSv4 root and then does path walk to the exported path and
> > > performs a
> > > submount on that, cloning the security mount options from the
> > > root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > > mounted
> > > with the context= mount option will not show the correct labels
> > > because
> > > the nfs_server->caps flags of the cloned superblock will still
> > > have
> > > NFS_CAP_SECURITY_LABEL set.
> > > 
> > > Allowing the NFSv4 client to enable or disable
> > > SECURITY_LSM_NATIVE_LABELS
> > > behavior will ensure that the SBLABEL_MNT flag has the correct
> > > value
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option
> > > and
> > > vice versa.  Similarly, checking to see if
> > > SECURITY_LSM_NATIVE_LABELS is
> > > set upon return from security_sb_clone_mnt_opts() and clearing
> > > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > > to
> > > be displayed for NFSv4.2 mounts mounted with the context= mount
> > > option.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c            | 18 +++++++++++++++++-
> > >  include/linux/lsm_hooks.h |  4 +++-
> > >  include/linux/security.h  |  8 ++++++--
> > >  security/security.c       |  7 +++++--
> > >  security/selinux/hooks.c  | 43
> > > ++++++++++++++++++++++++++++++++++++++-----
> > >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > With this patch, the behavior and flags fields of the
> > superblock_security_struct
> > and the nfs_server->caps all have the correct values when crossing
> > from an
> > export without the "security_label" option to one with the
> > "security_label"
> > option and vice versa.
> > 
> > Here /export/export1 and /export/export1/export2/export3 have the
> > "security_label" option; /export and /export/export1/export2 do
> > not:
> > 
> > [root@rhel7u4 ~]# exportfs -v
> > /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,
> > se
> > c=sys,secure,no_root_squash,no_all_squash)
> > /export/export1
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > /export/export1/export2
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> > cure,no_root_squash,no_all_squash)
> > /export/export1/export2/export3
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > 
> > 
> > [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30
> > 14:09
> > export1
> 
> Shouldn't this be the label of export1 from the server, since export1
> is exported with security_label?

BTW, I don't think your patch introduces this bug (assuming it is a
bug), so this isn't an obstacle to your patch proceeding, but unless I
misunderstand, it is another bug that needs to be fixed (in this case,
with the initialization of the root inode label for a NFS mount).

> 
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file2
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30
> > 14:09
> > file3
> > drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21
> > 12:58
> > scratch
> > drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21
> > 12:55
> > test
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1
> > total 20
> > drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0  4096 May
> > 30
> > 14:09 export2
> 
> Shouldn't this be system_u:object_r:nfs_t:s0, like the rest of
> export2?
> 
> > -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> > total 20
> > drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30
> > 14:08
> > export3
> 
> Shouldn't this be the label of export3 from the server, since it is
> exported with security_label?
> 
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file2
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30
> > 14:09
> > file3
> > drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30
> > 14:02
> > lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> > total 16
> > -rw-r--r--. 1 root root system_u:object_r:etc_t:s0           0 May
> > 30
> > 14:08 file1
> > -rw-r--r--. 1 root root system_u:object_r:bin_t:s0           0 May
> > 30
> > 14:08 file2
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0           0 May
> > 30
> > 14:08 file3
> > drwx------. 2 root root system_u:object_r:unlabeled_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > 
> > I can do chcon on /mnt/t/export1 and /mnt/t/export1/export2/export3
> > but not /mnt/t or /mnt/t/export1/export2:
> > 
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/file1
> > chcon: failed to change context of '/mnt/t/file1' to
> > ‘system_u:object_r:usr_t:s0’: Operation not supported
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/file1
> > [root@fedora25 ~]# chcon -t usr_t /mnt/t/export1/export2/file1
> > chcon: failed to change context of '/mnt/t/export1/export2/file1'
> > to
> > ‘system_u:object_r:usr_t:s0’: Operation not supported
> > [root@fedora25 ~]# chcon -t usr_t
> > /mnt/t/export1/export2/export3/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> > /mnt/t/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/file1
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:09
> > /mnt/t/export1/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/file1
> > -rw-r--r--. 1 root root system_u:object_r:nfs_t:s0 0 May 30 14:09
> > /mnt/t/export1/export2/file1
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3/file1
> > -rw-r--r--. 1 root root system_u:object_r:usr_t:s0 0 May 30 14:08
> > /mnt/t/export1/export2/export3/file1
> > [root@fedora25 ~]# chcon -t etc_t /mnt/t/export1/file1
> > [root@fedora25 ~]# chcon -t etc_t
> > /mnt/t/export1/export2/export3/file1
> > 
> > /mnt/t/export1 and /mnt/t/export1/export2/export3 have the
> > "seclabel"
> > mount option; /mnt/t and /mnt/t/export1/export do not:
> > 
> > [root@fedora25 ~]# cat /proc/mounts
> > ...
> > rhel7u4:/export /mnt/t nfs4
> > rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,prot
> > o=
> > tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,l
> > oc
> > al_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1 /mnt/t/export1 nfs4
> > rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,
> > ha
> > rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.
> > 12
> > 2.236,local_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> > rw,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,hard,prot
> > o=
> > tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.122.236,l
> > oc
> > al_lock=none,addr=192.168.122.83 0 0
> > rhel7u4:/export/export1/export2/export3
> > /mnt/t/export1/export2/export3 nfs4
> > rw,seclabel,relatime,vers=4.2,rsize=131072,wsize=131072,namlen=255,
> > ha
> > rd,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.
> > 12
> > 2.236,local_lock=none,addr=192.168.122.83 0 0
> > 
> > The capabilities flags has NFS_CAP_SECURITY_LABEL set on
> > /mnt/t/export1 and
> > /mnt/t/export1/export2/export3; it is not set on /mnt/t and
> > /mnt/t/export1/export2:
> > 
> > [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> > device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> > statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1 mounted on /mnt/t/export1 with
> > fstype
> > nfs4 statvers=1.1
> > 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2 mounted on
> > /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2/export3 mounted on
> > /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1ffffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > [root@fedora25 ~]# printf "%x\n" $(( 0x1fbffdf & (1<<18) ))
> > 0
> > [root@fedora25 ~]# printf "%x\n" $(( 0x1ffffdf & (1<<18) ))
> > 40000
> > [root@fedora25 ~]# 
> > 
> > The context= mount option now works correctly:
> > 
> > [root@fedora25 ~]# mount -o
> > v4.2,context="system_u:object_r:user_home_t:s0" rhel7u4:/export
> > /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0 4096 May
> > 30
> > 14:09 export1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0    0 May
> > 30
> > 14:09 file3
> > drwxrwxrwx. 3 root root system_u:object_r:user_home_t:s0   22 Apr
> > 21
> > 12:58 scratch
> > drwxrwxrwx. 7 root root system_u:object_r:user_home_t:s0 4096 Apr
> > 21
> > 12:55 test
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1
> > total 20
> > drwxr-xr-x. 4 root root system_u:object_r:user_home_t:s0  4096 May
> > 30
> > 14:09 export2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2
> > total 20
> > drwxr-xr-x. 3 root root system_u:object_r:user_home_t:s0  4096 May
> > 30
> > 14:08 export3
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:09 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > [root@fedora25 ~]# ls -lZ /mnt/t/export1/export2/export3
> > total 16
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file1
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file2
> > -rw-r--r--. 1 root root system_u:object_r:user_home_t:s0     0 May
> > 30
> > 14:08 file3
> > drwx------. 2 root root system_u:object_r:user_home_t:s0 16384 May
> > 30
> > 14:02 lost+found
> > 
> > [root@fedora25 ~]# cat /proc/mounts
> > ...
> > rhel7u4:/export /mnt/t nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1 /mnt/t/export1 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1/export2 /mnt/t/export1/export2 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > rhel7u4:/export/export1/export2/export3
> > /mnt/t/export1/export2/export3 nfs4
> > rw,context=system_u:object_r:user_home_t:s0,relatime,vers=4.2,rsize
> > =1
> > 31072,wsize=131072,namlen=255,hard,proto=tcp,port=0,timeo=600,retra
> > ns
> > =2,sec=sys,clientaddr=192.168.122.236,local_lock=none,addr=192.168.
> > 12
> > 2.83 0 0
> > 
> > [root@fedora25 ~]# egrep "rhel7u4|caps" /proc/self/mountstats
> > device rhel7u4:/export mounted on /mnt/t with fstype nfs4
> > statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1 mounted on /mnt/t/export1 with
> > fstype
> > nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2 mounted on
> > /mnt/t/export1/export2 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > device rhel7u4:/export/export1/export2/export3 mounted on
> > /mnt/t/export1/export2/export3 with fstype nfs4 statvers=1.1
> > 	caps:	caps=0x1fbffdf,wtmult=512,dtsize=32768,bsize=0,nam
> > len=255
> > 
> > -Scott
> > 
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..6a11535 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb, 
> > > s, kflags, &kflags_out);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > > +err:
> > > +	return error;
> > > +
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > >  
> > > diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h
> > > index 080f34e..2f54bfb 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1388,7 +1388,9 @@ union security_list_options {
> > >  				unsigned long kern_flags,
> > >  				unsigned long *set_kern_flags);
> > >  	int (*sb_clone_mnt_opts)(const struct super_block
> > > *oldsb,
> > > -					struct super_block
> > > *newsb);
> > > +					struct super_block
> > > *newsb,
> > > +					unsigned long
> > > kern_flags,
> > > +					unsigned long
> > > *set_kern_flags);
> > >  	int (*sb_parse_opts_str)(char *options, struct
> > > security_mnt_opts *opts);
> > >  	int (*dentry_init_security)(struct dentry *dentry, int
> > > mode,
> > >  					const struct qstr *name,
> > > void **ctx,
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index af675b5..a55ae9c 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct
> > > super_block
> > > *sb,
> > >  				unsigned long kern_flags,
> > >  				unsigned long *set_kern_flags);
> > >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > > -				struct super_block *newsb);
> > > +				struct super_block *newsb,
> > > +				unsigned long kern_flags,
> > > +				unsigned long *set_kern_flags);
> > >  int security_sb_parse_opts_str(char *options, struct
> > > security_mnt_opts *opts);
> > >  int security_dentry_init_security(struct dentry *dentry, int
> > > mode,
> > >  					const struct qstr *name,
> > > void **ctx,
> > > @@ -581,7 +583,9 @@ static inline int
> > > security_sb_set_mnt_opts(struct super_block *sb,
> > >  }
> > >  
> > >  static inline int security_sb_clone_mnt_opts(const struct
> > > super_block *oldsb,
> > > -					      struct super_block
> > > *newsb)
> > > +					      struct super_block
> > > *newsb,
> > > +					      unsigned long
> > > kern_flags,
> > > +					      unsigned long
> > > *set_kern_flags)
> > >  {
> > >  	return 0;
> > >  }
> > > diff --git a/security/security.c b/security/security.c
> > > index b9fea39..7b70ea2 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct
> > > super_block *sb,
> > >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> > >  
> > >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > > -				struct super_block *newsb)
> > > +				struct super_block *newsb,
> > > +				unsigned long kern_flags,
> > > +				unsigned long *set_kern_flags)
> > >  {
> > > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb,
> > > newsb);
> > > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > > +				kern_flags, set_kern_flags);
> > >  }
> > >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> > >  
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index e67a526..80d9acf 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct
> > > super_block *sb)
> > >  		       sb->s_id, sb->s_type->name);
> > >  
> > >  	sbsec->flags |= SE_SBINITIALIZED;
> > > +
> > > +	/* Explicitly set or clear SBLABEL_MNT.  It's not
> > > sufficient to simply
> > > +	   leave the flag untouched because sb_clone_mnt_opts
> > > might be handing
> > > +	   us a superblock that needs the flag to be cleared. */
> > >  	if (selinux_is_sblabel_mnt(sb))
> > >  		sbsec->flags |= SBLABEL_MNT;
> > > +	else
> > > +		sbsec->flags &= ~SBLABEL_MNT;
> > >  
> > >  	/* Initialize the root inode. */
> > >  	rc = inode_doinit_with_dentry(root_inode, root);
> > > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const
> > > struct
> > > super_block *oldsb,
> > >  }
> > >  
> > >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > > *oldsb,
> > > -					struct super_block
> > > *newsb)
> > > +					struct super_block
> > > *newsb,
> > > +					unsigned long
> > > kern_flags,
> > > +					unsigned long
> > > *set_kern_flags)
> > >  {
> > > +	int rc = 0;
> > >  	const struct superblock_security_struct *oldsbsec =
> > > oldsb-
> > > > s_security;
> > > 
> > >  	struct superblock_security_struct *newsbsec = newsb-
> > > > s_security;
> > > 
> > >  
> > > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	 * mount options.  thus we can safely deal with this
> > > superblock later
> > >  	 */
> > >  	if (!ss_initialized)
> > > -		return 0;
> > > +		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;
> > > +	}
> > >  
> > >  	/* 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)
> > > -		return selinux_cmp_sb_context(oldsb, newsb);
> > > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > > +		goto out;
> > > +	}
> > >  
> > >  	mutex_lock(&newsbsec->lock);
> > >  
> > > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	newsbsec->def_sid = oldsbsec->def_sid;
> > >  	newsbsec->behavior = oldsbsec->behavior;
> > >  
> > > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > > +			&& !(kern_flags &
> > > SECURITY_LSM_NATIVE_LABELS)
> > > +			&& !set_context) {
> > > +		rc = security_fs_use(newsb);
> > > +		if (rc)
> > > +			goto out_unlock;
> > > +	}
> > > +
> > > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS &&
> > > !set_context) {
> > > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > > +	}
> > > +
> > >  	if (set_context) {
> > >  		u32 sid = oldsbsec->mntpoint_sid;
> > >  
> > > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > > struct super_block *oldsb,
> > >  	}
> > >  
> > >  	sb_finish_set_opts(newsb);
> > > +out_unlock:
> > >  	mutex_unlock(&newsbsec->lock);
> > > -	return 0;
> > > +out:
> > > +	return rc;
> > >  }
> > >  
> > >  static int selinux_parse_opts_str(char *options,
> > > -- 
> > > 2.9.3
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> > > nfs" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.htm
> > > l

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 10:55 -0400, Scott Mayhew wrote:
> > On Thu, 01 Jun 2017, Scott Mayhew wrote:
> > 
> > > When an NFSv4 client performs a mount operation, it first mounts
> > > the
> > > NFSv4 root and then does path walk to the exported path and
> > > performs a
> > > submount on that, cloning the security mount options from the
> > > root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts
> > > mounted
> > > with the context= mount option will not show the correct labels
> > > because
> > > the nfs_server->caps flags of the cloned superblock will still have
> > > NFS_CAP_SECURITY_LABEL set.
> > > 
> > > Allowing the NFSv4 client to enable or disable
> > > SECURITY_LSM_NATIVE_LABELS
> > > behavior will ensure that the SBLABEL_MNT flag has the correct
> > > value
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option and
> > > vice versa.  Similarly, checking to see if
> > > SECURITY_LSM_NATIVE_LABELS is
> > > set upon return from security_sb_clone_mnt_opts() and clearing
> > > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels
> > > to
> > > be displayed for NFSv4.2 mounts mounted with the context= mount
> > > option.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c            | 18 +++++++++++++++++-
> > >  include/linux/lsm_hooks.h |  4 +++-
> > >  include/linux/security.h  |  8 ++++++--
> > >  security/security.c       |  7 +++++--
> > >  security/selinux/hooks.c  | 43
> > > ++++++++++++++++++++++++++++++++++++++-----
> > >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > With this patch, the behavior and flags fields of the
> > superblock_security_struct
> > and the nfs_server->caps all have the correct values when crossing
> > from an
> > export without the "security_label" option to one with the
> > "security_label"
> > option and vice versa.
> > 
> > Here /export/export1 and /export/export1/export2/export3 have the
> > "security_label" option; /export and /export/export1/export2 do not:
> > 
> > [root@rhel7u4 ~]# exportfs -v
> > /export       	<world>(rw,sync,wdelay,hide,no_subtree_check,se
> > c=sys,secure,no_root_squash,no_all_squash)
> > /export/export1
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > /export/export1/export2
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,sec=sys,se
> > cure,no_root_squash,no_all_squash)
> > /export/export1/export2/export3
> > 		<world>(rw,sync,wdelay,hide,no_subtree_check,security_l
> > abel,sec=sys,secure,no_root_squash,no_all_squash)
> > 
> > 
> > [root@fedora25 ~]# mount -o v4.2 rhel7u4:/export /mnt/t
> > [root@fedora25 ~]# ls -lZ /mnt/t
> > total 8
> > drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09
> > export1
> 
> Shouldn't this be the label of export1 from the server, since export1
> is exported with security_label?
> 
<snip/>

That's my fault, I actually didn't have labels on all the directories on
the server.

[root@rhel7u4 ~]# ls -laZ /export/export1
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxrwxrwx. root root unconfined_u:object_r:usr_t:s0   ..
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 export2
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 ..
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 export3
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2/export3
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 .
drwxr-xr-x. root root system_u:object_r:unlabeled_t:s0 ..
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:unlabeled_t:s0 lost+found

Setting the missing labels...

[root@rhel7u4 ~]# chcon -t usr_t /export/export1
[root@rhel7u4 ~]# chcon -t usr_t /export/export1/export2
[root@rhel7u4 ~]# chcon -t usr_t /export/export1/export2/export3
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/lost+found
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/export2/lost+found
[root@rhel7u4 ~]# chcon -t lost_found_t /export/export1/export2/export3/lost+found

[root@rhel7u4 ~]# ls -laZ /export/export1
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxrwxrwx. root root unconfined_u:object_r:usr_t:s0   ..
drwxr-xr-x. root root system_u:object_r:usr_t:s0       export2
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxr-xr-x. root root system_u:object_r:usr_t:s0       ..
drwxr-xr-x. root root system_u:object_r:usr_t:s0       export3
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found
[root@rhel7u4 ~]# ls -laZ /export/export1/export2/export3
drwxr-xr-x. root root system_u:object_r:usr_t:s0       .
drwxr-xr-x. root root system_u:object_r:usr_t:s0       ..
-rw-r--r--. root root system_u:object_r:etc_t:s0       file1
-rw-r--r--. root root system_u:object_r:bin_t:s0       file2
-rw-r--r--. root root system_u:object_r:usr_t:s0       file3
drwx------. root root system_u:object_r:lost_found_t:s0 lost+found

Even after setting the labels on the NFS server, the behavior on the client
appears strange until you remember that the client doesn't actually know about
label support on the export until it steps on the mountpoint and triggers the
submount.  I don't know if I'd call it a bug or not... more of a peculiarity :)

Here we're doing READDIR with the NFS filehandle of /export.  Since /export is
not exported with the security_label option, the attrmask in the READDIR is not
requesting security labels... so export1 shows nfs_t even though
/export/export1 is exported with the security_label option:

[root@fedora25 ~]# ls -laZ /mnt/t
total 8
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0   87 Jun  1 09:10 .
drwxr-xr-x. 5 root root system_u:object_r:mnt_t:s0   42 May 22 08:45 ..
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 export1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0    0 May 30 14:09 file3
drwxrwxrwx. 3 root root system_u:object_r:nfs_t:s0   22 Apr 21 12:58 scratch
drwxrwxrwx. 7 root root system_u:object_r:nfs_t:s0 4096 Apr 21 12:55 test

Here we're doing a READDIR with the NFS filehandle of /export/export1, which
triggers the mount of /export/export1.  Since /export/export1 is exported with
the security_label option, the attrmask in the READDIR is requesting security
labels.  Note that '.' now shows usr_t instead of nfs_t.  However, since
/export/export1/export2 is not exported with the security_label option, the NFS
server does not return a security label in the attributes for export2 in the
READDIR reply... so it's displayed as unlabeled_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1
total 24
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0         4096 May 30 14:09 .
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0           87 Jun  1 09:10 ..
drwxr-xr-x. 4 root root system_u:object_r:unlabeled_t:s0   4096 May 30 14:09 export2
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0            0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0            0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0            0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:lost_found_t:s0 16384 May 30 14:02 lost+found

Here we're doing doing a READDIR with the NFS filehandle of /export/export1/export2,
which triggers the mount of /export/export1/export2.  Since /export/export1/export2
is not exported with the security_label option, the attrmask in the READDIR is not
requesting security labels, and everything (including both '.' and 'export3') is
displayed as nfs_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1/export2
total 28
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0  4096 May 30 14:09 .
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0  4096 May 30 14:09 ..
drwxr-xr-x. 3 root root system_u:object_r:nfs_t:s0  4096 May 30 14:08 export3
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file1
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file2
-rw-r--r--. 1 root root system_u:object_r:nfs_t:s0     0 May 30 14:09 file3
drwx------. 2 root root system_u:object_r:nfs_t:s0 16384 May 30 14:02 lost+found

Here we're doing a READDIR with the NFS filehandle of /export1/export2/export3,
which triggers the mount of /export1/export2/export3.  Since /export1/export2/export3
is exported with the security_label option, the attrmask in the READDIR is
requesting security labels, and '.' now shows as usr_t:

[root@fedora25 ~]# ls -laZ /mnt/t/export1/export2/export3
total 24
drwxr-xr-x. 3 root root system_u:object_r:usr_t:s0         4096 May 30 14:08 .
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0         4096 May 30 14:09 ..
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0            0 May 30 14:08 file1
-rw-r--r--. 1 root root system_u:object_r:bin_t:s0            0 May 30 14:08 file2
-rw-r--r--. 1 root root system_u:object_r:usr_t:s0            0 May 30 14:08 file3
drwx------. 2 root root system_u:object_r:lost_found_t:s0 16384 May 30 14:02 lost+found

After triggering all those mounts, the labels for export1 and export3 match
what I set on the server:

[root@fedora25 ~]# ls -ldZ /mnt/t
drwxrwxrwx. 5 root root system_u:object_r:nfs_t:s0 87 Jun  1 09:10 /mnt/t
[root@fedora25 ~]# ls -ldZ /mnt/t/export1
drwxr-xr-x. 4 root root system_u:object_r:usr_t:s0 4096 May 30 14:09 /mnt/t/export1
[root@fedora25 ~]# ls -ldZ /mnt/t/export1/export2
drwxr-xr-x. 4 root root system_u:object_r:nfs_t:s0 4096 May 30 14:09 /mnt/t/export1/export2
[root@fedora25 ~]# ls -ldZ /mnt/t/export1/export2/export3
drwxr-xr-x. 3 root root system_u:object_r:usr_t:s0 4096 May 30 14:08 /mnt/t/export1/export2/export3
[root@fedora25 ~]# 

-Scott

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 10:46 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 43
> ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 69 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..6a11535 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags, &kflags_out);
> +	if (error)
> +		goto err;

Not sure this is justified; coding style says to just return directly
if no cleanup is required.

> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +err:
> +	return error;
> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..80d9acf 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	   leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	   us a superblock that needs the flag to be cleared. */

Coding style (and checkpatch.pl) prefer a different style for multi-
line comments.

>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	 * mount options.  thus we can safely deal with this
> superblock later
>  	 */
>  	if (!ss_initialized)
> -		return 0;
> +		goto out;

Likewise, don't see the point of this since no cleanup is required, and
as per coding style.

> +
> +	if (kern_flags && !set_kern_flags) {
> +		/* Specifying internal flags without providing a
> place to
> +		 * place the results is not allowed */
> +		rc = -EINVAL;
> +		goto out;

Ditto, just return directly.

> +	}
>  
>  	/* 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)
> -		return selinux_cmp_sb_context(oldsb, newsb);
> +	if (newsbsec->flags & SE_SBINITIALIZED) {
> +		rc = selinux_cmp_sb_context(oldsb, newsb);
> +		goto out;
> +	}

And again.

>  
>  	mutex_lock(&newsbsec->lock);
>  
> @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> +			&& !(kern_flags &
> SECURITY_LSM_NATIVE_LABELS)
> +			&& !set_context) {

I prefer ending the prior line with &&, not beginning the next one with
it. Also indentation of the continuation lines seems excessive.

> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out_unlock;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out_unlock:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +out:
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Thu, 01 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 10:46 -0400, Scott Mayhew wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> > is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 43
> > ++++++++++++++++++++++++++++++++++++++-----
> >  5 files changed, 69 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..6a11535 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s, kflags, &kflags_out);
> > +	if (error)
> > +		goto err;
> 
> Not sure this is justified; coding style says to just return directly
> if no cleanup is required.
> 
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +err:
> > +	return error;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..80d9acf 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,14 @@ static int sb_finish_set_opts(struct super_block
> > *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/* Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> > to simply
> > +	   leave the flag untouched because sb_clone_mnt_opts might
> > be handing
> > +	   us a superblock that needs the flag to be cleared. */
> 
> Coding style (and checkpatch.pl) prefer a different style for multi-
> line comments.
> 
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +969,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -977,14 +986,23 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	 * mount options.  thus we can safely deal with this
> > superblock later
> >  	 */
> >  	if (!ss_initialized)
> > -		return 0;
> > +		goto out;
> 
> Likewise, don't see the point of this since no cleanup is required, and
> as per coding style.
> 
> > +
> > +	if (kern_flags && !set_kern_flags) {
> > +		/* Specifying internal flags without providing a
> > place to
> > +		 * place the results is not allowed */
> > +		rc = -EINVAL;
> > +		goto out;
> 
> Ditto, just return directly.
> 
> > +	}
> >  
> >  	/* 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)
> > -		return selinux_cmp_sb_context(oldsb, newsb);
> > +	if (newsbsec->flags & SE_SBINITIALIZED) {
> > +		rc = selinux_cmp_sb_context(oldsb, newsb);
> > +		goto out;
> > +	}
> 
> And again.
> 
> >  
> >  	mutex_lock(&newsbsec->lock);
> >  
> > @@ -994,6 +1012,19 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE
> > +			&& !(kern_flags &
> > SECURITY_LSM_NATIVE_LABELS)
> > +			&& !set_context) {
> 
> I prefer ending the prior line with &&, not beginning the next one with
> it. Also indentation of the continuation lines seems excessive.
> 
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out_unlock;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> > {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1044,10 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out_unlock:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +out:
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,

Again, my fault... I should've run checkpatch.pl.

I'll address all these and re-send.

-Scott

[PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 18 +++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..ffded39 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
+			&kflags_out);
+	if (error)
+		return error;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	return error;
+
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..2f54bfb 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1388,7 +1388,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..a55ae9c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index b9fea39..7b70ea2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..d8d5d35 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block *sb)
 		       sb->s_id, sb->s_type->name);
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/*
+	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	 * leave the flag untouched because sb_clone_mnt_opts might be handing
+	 * us a superblock that needs the flag to be cleared.
+	 */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	if (!ss_initialized)
 		return 0;
 
+	/*
+	 * Specifying internal flags without providing a place to
+	 * place the results is not allowed.
+	 */
+	if (kern_flags && !set_kern_flags)
+		return -EINVAL;
+
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
@@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
+		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Thu, 2017-06-01 at 16:59 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 18 +++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 64 insertions(+), 8 deletions(-)

What tree is this against? Doesn't apply cleanly on selinux #next.

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..ffded39 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return error;

This can just be return 0, right?

> +
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..2f54bfb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1388,7 +1388,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..a55ae9c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..7b70ea2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..d8d5d35 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  		       sb->s_id, sb->s_type->name);
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH v2] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Fri, 02 Jun 2017, Stephen Smalley wrote:

> On Thu, 2017-06-01 at 16:59 -0400, Scott Mayhew wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels
> > because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> > 
> > Allowing the NFSv4 client to enable or disable
> > SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> > is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount
> > option.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 18 +++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
> >  5 files changed, 64 insertions(+), 8 deletions(-)
> 
> What tree is this against? Doesn't apply cleanly on selinux #next.

It's against Linus' mainline tree.
> 
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..ffded39 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,26 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > *mntroot,
> >  			  struct nfs_mount_info *mount_info)
> >  {
> > +	int error;
> > +	unsigned long kflags = 0, kflags_out = 0;
> > +
> >  	/* clone any lsm security options from the parent to the new
> > sb */
> >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > >rpc_ops->dir_inode_ops)
> >  		return -ESTALE;
> > -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s);
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> > s, kflags,
> > +			&kflags_out);
> > +	if (error)
> > +		return error;
> > +
> > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +	return error;
> 
> This can just be return 0, right?

Yes.
> 
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >  
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..2f54bfb 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1388,7 +1388,9 @@ union security_list_options {
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -					struct super_block *newsb);
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags);
> >  	int (*sb_parse_opts_str)(char *options, struct
> > security_mnt_opts *opts);
> >  	int (*dentry_init_security)(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..a55ae9c 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -240,7 +240,9 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  				unsigned long kern_flags,
> >  				unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb);
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct
> > security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >  					const struct qstr *name,
> > void **ctx,
> > @@ -581,7 +583,9 @@ static inline int security_sb_set_mnt_opts(struct
> > super_block *sb,
> >  }
> >  
> >  static inline int security_sb_clone_mnt_opts(const struct
> > super_block *oldsb,
> > -					      struct super_block
> > *newsb)
> > +					      struct super_block
> > *newsb,
> > +					      unsigned long
> > kern_flags,
> > +					      unsigned long
> > *set_kern_flags)
> >  {
> >  	return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..7b70ea2 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -380,9 +380,12 @@ int security_sb_set_mnt_opts(struct super_block
> > *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >  
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -				struct super_block *newsb)
> > +				struct super_block *newsb,
> > +				unsigned long kern_flags,
> > +				unsigned long *set_kern_flags)
> >  {
> > -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +				kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >  
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..d8d5d35 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -529,8 +529,16 @@ static int sb_finish_set_opts(struct super_block
> > *sb)
> >  		       sb->s_id, sb->s_type->name);
> >  
> >  	sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +	/*
> > +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> > to simply
> > +	 * leave the flag untouched because sb_clone_mnt_opts might
> > be handing
> > +	 * us a superblock that needs the flag to be cleared.
> > +	 */
> >  	if (selinux_is_sblabel_mnt(sb))
> >  		sbsec->flags |= SBLABEL_MNT;
> > +	else
> > +		sbsec->flags &= ~SBLABEL_MNT;
> >  
> >  	/* Initialize the root inode. */
> >  	rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -963,8 +971,11 @@ static int selinux_cmp_sb_context(const struct
> > super_block *oldsb,
> >  }
> >  
> >  static int selinux_sb_clone_mnt_opts(const struct super_block
> > *oldsb,
> > -					struct super_block *newsb)
> > +					struct super_block *newsb,
> > +					unsigned long kern_flags,
> > +					unsigned long
> > *set_kern_flags)
> >  {
> > +	int rc = 0;
> >  	const struct superblock_security_struct *oldsbsec = oldsb-
> > >s_security;
> >  	struct superblock_security_struct *newsbsec = newsb-
> > >s_security;
> >  
> > @@ -979,6 +990,13 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	if (!ss_initialized)
> >  		return 0;
> >  
> > +	/*
> > +	 * Specifying internal flags without providing a place to
> > +	 * place the results is not allowed.
> > +	 */
> > +	if (kern_flags && !set_kern_flags)
> > +		return -EINVAL;
> > +
> >  	/* how can we clone if the old one wasn't set up?? */
> >  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >  
> > @@ -994,6 +1012,18 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	newsbsec->def_sid = oldsbsec->def_sid;
> >  	newsbsec->behavior = oldsbsec->behavior;
> >  
> > +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> > +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> > !set_context) {
> > +		rc = security_fs_use(newsb);
> > +		if (rc)
> > +			goto out;
> > +	}
> > +
> > +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> > {
> > +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +	}
> > +
> >  	if (set_context) {
> >  		u32 sid = oldsbsec->mntpoint_sid;
> >  
> > @@ -1013,8 +1043,9 @@ static int selinux_sb_clone_mnt_opts(const
> > struct super_block *oldsb,
> >  	}
> >  
> >  	sb_finish_set_opts(newsb);
> > +out:
> >  	mutex_unlock(&newsbsec->lock);
> > -	return 0;
> > +	return rc;
> >  }
> >  
> >  static int selinux_parse_opts_str(char *options,

[PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.

Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options.  As a result, setxattr's of security labels
over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.

Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/nfs/super.c            | 17 ++++++++++++++++-
 include/linux/lsm_hooks.h |  4 +++-
 include/linux/security.h  |  8 ++++++--
 security/security.c       |  7 +++++--
 security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
 5 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2f3822a..b8e0735 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
 int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
 			  struct nfs_mount_info *mount_info)
 {
+	int error;
+	unsigned long kflags = 0, kflags_out = 0;
+
 	/* clone any lsm security options from the parent to the new sb */
 	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
 		return -ESTALE;
-	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
+		kflags |= SECURITY_LSM_NATIVE_LABELS;
+
+	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
+			&kflags_out);
+	if (error)
+		return error;
+
+	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
+		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
+		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 68d91e4..3cc9d77 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1409,7 +1409,9 @@ union security_list_options {
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
-					struct super_block *newsb);
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags);
 	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
 	int (*dentry_init_security)(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
diff --git a/include/linux/security.h b/include/linux/security.h
index 549cb82..b44e954 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 				unsigned long kern_flags,
 				unsigned long *set_kern_flags);
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb);
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags);
 int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
 int security_dentry_init_security(struct dentry *dentry, int mode,
 					const struct qstr *name, void **ctx,
@@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
 }
 
 static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-					      struct super_block *newsb)
+					      struct super_block *newsb,
+					      unsigned long kern_flags,
+					      unsigned long *set_kern_flags)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index 714433e..3013237 100644
--- a/security/security.c
+++ b/security/security.c
@@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
 EXPORT_SYMBOL(security_sb_set_mnt_opts);
 
 int security_sb_clone_mnt_opts(const struct super_block *oldsb,
-				struct super_block *newsb)
+				struct super_block *newsb,
+				unsigned long kern_flags,
+				unsigned long *set_kern_flags)
 {
-	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
+	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
+				kern_flags, set_kern_flags);
 }
 EXPORT_SYMBOL(security_sb_clone_mnt_opts);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9926adb..9cc042d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
 	}
 
 	sbsec->flags |= SE_SBINITIALIZED;
+
+	/*
+	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
+	 * leave the flag untouched because sb_clone_mnt_opts might be handing
+	 * us a superblock that needs the flag to be cleared.
+	 */
 	if (selinux_is_sblabel_mnt(sb))
 		sbsec->flags |= SBLABEL_MNT;
+	else
+		sbsec->flags &= ~SBLABEL_MNT;
 
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
@@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
 }
 
 static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
-					struct super_block *newsb)
+					struct super_block *newsb,
+					unsigned long kern_flags,
+					unsigned long *set_kern_flags)
 {
+	int rc = 0;
 	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
 	struct superblock_security_struct *newsbsec = newsb->s_security;
 
@@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	if (!ss_initialized)
 		return 0;
 
+	/*
+	 * Specifying internal flags without providing a place to
+	 * place the results is not allowed.
+	 */
+	if (kern_flags && !set_kern_flags)
+		return -EINVAL;
+
 	/* how can we clone if the old one wasn't set up?? */
 	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
 
@@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	newsbsec->def_sid = oldsbsec->def_sid;
 	newsbsec->behavior = oldsbsec->behavior;
 
+	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
+		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
+		rc = security_fs_use(newsb);
+		if (rc)
+			goto out;
+	}
+
+	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
+		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
+		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
+	}
+
 	if (set_context) {
 		u32 sid = oldsbsec->mntpoint_sid;
 
@@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 	}
 
 	sb_finish_set_opts(newsb);
+out:
 	mutex_unlock(&newsbsec->lock);
-	return 0;
+	return rc;
 }
 
 static int selinux_parse_opts_str(char *options,
-- 
2.9.3

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Scott Mayhew]

On Mon, 05 Jun 2017, Scott Mayhew wrote:

> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)

This is against the "next" branch of
git@github.com:SELinuxProject/selinux-kernel.git.

-Scott

> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name, void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					      struct super_block *newsb)
> +					      struct super_block *newsb,
> +					      unsigned long kern_flags,
> +					      unsigned long *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
>  	}
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>  	struct superblock_security_struct *newsbsec = newsb->s_security;
>  
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Stephen Smalley]

On Mon, 2017-06-05 at 11:45 -0400, Scott Mayhew wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels
> because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
> 
> Allowing the NFSv4 client to enable or disable
> SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS
> is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount
> option.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>

Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
Resolves: https://github.com/SELinuxProject/selinux-kernel/issues/35

> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry
> *mntroot,
>  			  struct nfs_mount_info *mount_info)
>  {
> +	int error;
> +	unsigned long kflags = 0, kflags_out = 0;
> +
>  	/* clone any lsm security options from the parent to the new
> sb */
>  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> >rpc_ops->dir_inode_ops)
>  		return -ESTALE;
> -	return security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s);
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +	error = security_sb_clone_mnt_opts(mount_info->cloned->sb,
> s, kflags,
> +			&kflags_out);
> +	if (error)
> +		return error;
> +
> +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +		!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +		NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  	int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -					struct super_block *newsb);
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags);
>  	int (*sb_parse_opts_str)(char *options, struct
> security_mnt_opts *opts);
>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  				unsigned long kern_flags,
>  				unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb);
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct
> security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>  					const struct qstr *name,
> void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct
> super_block *sb,
>  }
>  
>  static inline int security_sb_clone_mnt_opts(const struct
> super_block *oldsb,
> -					      struct super_block
> *newsb)
> +					      struct super_block
> *newsb,
> +					      unsigned long
> kern_flags,
> +					      unsigned long
> *set_kern_flags)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block
> *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>  
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -				struct super_block *newsb)
> +				struct super_block *newsb,
> +				unsigned long kern_flags,
> +				unsigned long *set_kern_flags)
>  {
> -	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +	return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +				kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block
> *sb)
>  	}
>  
>  	sbsec->flags |= SE_SBINITIALIZED;
> +
> +	/*
> +	 * Explicitly set or clear SBLABEL_MNT.  It's not sufficient
> to simply
> +	 * leave the flag untouched because sb_clone_mnt_opts might
> be handing
> +	 * us a superblock that needs the flag to be cleared.
> +	 */
>  	if (selinux_is_sblabel_mnt(sb))
>  		sbsec->flags |= SBLABEL_MNT;
> +	else
> +		sbsec->flags &= ~SBLABEL_MNT;
>  
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct
> super_block *oldsb,
>  }
>  
>  static int selinux_sb_clone_mnt_opts(const struct super_block
> *oldsb,
> -					struct super_block *newsb)
> +					struct super_block *newsb,
> +					unsigned long kern_flags,
> +					unsigned long
> *set_kern_flags)
>  {
> +	int rc = 0;
>  	const struct superblock_security_struct *oldsbsec = oldsb-
> >s_security;
>  	struct superblock_security_struct *newsbsec = newsb-
> >s_security;
>  
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	if (!ss_initialized)
>  		return 0;
>  
> +	/*
> +	 * Specifying internal flags without providing a place to
> +	 * place the results is not allowed.
> +	 */
> +	if (kern_flags && !set_kern_flags)
> +		return -EINVAL;
> +
>  	/* how can we clone if the old one wasn't set up?? */
>  	BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>  
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	newsbsec->def_sid = oldsbsec->def_sid;
>  	newsbsec->behavior = oldsbsec->behavior;
>  
> +	if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +		!(kern_flags & SECURITY_LSM_NATIVE_LABELS) &&
> !set_context) {
> +		rc = security_fs_use(newsb);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context)
> {
> +		newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +		*set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +	}
> +
>  	if (set_context) {
>  		u32 sid = oldsbsec->mntpoint_sid;
>  
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const
> struct super_block *oldsb,
>  	}
>  
>  	sb_finish_set_opts(newsb);
> +out:
>  	mutex_unlock(&newsbsec->lock);
> -	return 0;
> +	return rc;
>  }
>  
>  static int selinux_parse_opts_str(char *options,

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Paul Moore]

On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
>
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after cloning
> the security mount options.  As a result, setxattr's of security labels
> over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> with the context= mount option will not show the correct labels because
> the nfs_server->caps flags of the cloned superblock will still have
> NFS_CAP_SECURITY_LABEL set.
>
> Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> behavior will ensure that the SBLABEL_MNT flag has the correct value
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option and
> vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> set upon return from security_sb_clone_mnt_opts() and clearing
> NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> be displayed for NFSv4.2 mounts mounted with the context= mount option.
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  fs/nfs/super.c            | 17 ++++++++++++++++-
>  include/linux/lsm_hooks.h |  4 +++-
>  include/linux/security.h  |  8 ++++++--
>  security/security.c       |  7 +++++--
>  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>  5 files changed, 63 insertions(+), 8 deletions(-)

Thanks for sorting this out Scott and Stephen.

NFS folks, any objections to this patch?  If not, I'd like to pull
this into the SELinux tree but I'd like to have an ACK from you before
I do.

> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 2f3822a..b8e0735 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
>  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
>                           struct nfs_mount_info *mount_info)
>  {
> +       int error;
> +       unsigned long kflags = 0, kflags_out = 0;
> +
>         /* clone any lsm security options from the parent to the new sb */
>         if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
>                 return -ESTALE;
> -       return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> +
> +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> +               kflags |= SECURITY_LSM_NATIVE_LABELS;
> +
> +       error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> +                       &kflags_out);
> +       if (error)
> +               return error;
> +
> +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> +               !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> +               NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 68d91e4..3cc9d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1409,7 +1409,9 @@ union security_list_options {
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
>         int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> -                                       struct super_block *newsb);
> +                                       struct super_block *newsb,
> +                                       unsigned long kern_flags,
> +                                       unsigned long *set_kern_flags);
>         int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
>         int (*dentry_init_security)(struct dentry *dentry, int mode,
>                                         const struct qstr *name, void **ctx,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 549cb82..b44e954 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                               struct super_block *newsb);
> +                               struct super_block *newsb,
> +                               unsigned long kern_flags,
> +                               unsigned long *set_kern_flags);
>  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>                                         const struct qstr *name, void **ctx,
> @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
>  }
>
>  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                                             struct super_block *newsb)
> +                                             struct super_block *newsb,
> +                                             unsigned long kern_flags,
> +                                             unsigned long *set_kern_flags)
>  {
>         return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index 714433e..3013237 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
>  EXPORT_SYMBOL(security_sb_set_mnt_opts);
>
>  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                               struct super_block *newsb)
> +                               struct super_block *newsb,
> +                               unsigned long kern_flags,
> +                               unsigned long *set_kern_flags)
>  {
> -       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> +       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> +                               kern_flags, set_kern_flags);
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9926adb..9cc042d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
>         }
>
>         sbsec->flags |= SE_SBINITIALIZED;
> +
> +       /*
> +        * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> +        * leave the flag untouched because sb_clone_mnt_opts might be handing
> +        * us a superblock that needs the flag to be cleared.
> +        */
>         if (selinux_is_sblabel_mnt(sb))
>                 sbsec->flags |= SBLABEL_MNT;
> +       else
> +               sbsec->flags &= ~SBLABEL_MNT;
>
>         /* Initialize the root inode. */
>         rc = inode_doinit_with_dentry(root_inode, root);
> @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
>  }
>
>  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> -                                       struct super_block *newsb)
> +                                       struct super_block *newsb,
> +                                       unsigned long kern_flags,
> +                                       unsigned long *set_kern_flags)
>  {
> +       int rc = 0;
>         const struct superblock_security_struct *oldsbsec = oldsb->s_security;
>         struct superblock_security_struct *newsbsec = newsb->s_security;
>
> @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         if (!ss_initialized)
>                 return 0;
>
> +       /*
> +        * Specifying internal flags without providing a place to
> +        * place the results is not allowed.
> +        */
> +       if (kern_flags && !set_kern_flags)
> +               return -EINVAL;
> +
>         /* how can we clone if the old one wasn't set up?? */
>         BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
>
> @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         newsbsec->def_sid = oldsbsec->def_sid;
>         newsbsec->behavior = oldsbsec->behavior;
>
> +       if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +               !(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> +               rc = security_fs_use(newsb);
> +               if (rc)
> +                       goto out;
> +       }
> +
> +       if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> +               newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> +               *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> +       }
> +
>         if (set_context) {
>                 u32 sid = oldsbsec->mntpoint_sid;
>
> @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
>         }
>
>         sb_finish_set_opts(newsb);
> +out:
>         mutex_unlock(&newsbsec->lock);
> -       return 0;
> +       return rc;
>  }
>
>  static int selinux_parse_opts_str(char *options,
> --
> 2.9.3
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[J . Bruce Fields]

On Mon, Jun 05, 2017 at 05:21:55PM -0400, Paul Moore wrote:
> On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> >
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after cloning
> > the security mount options.  As a result, setxattr's of security labels
> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
> > with the context= mount option will not show the correct labels because
> > the nfs_server->caps flags of the cloned superblock will still have
> > NFS_CAP_SECURITY_LABEL set.
> >
> > Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
> > behavior will ensure that the SBLABEL_MNT flag has the correct value
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option and
> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
> > set upon return from security_sb_clone_mnt_opts() and clearing
> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
> > be displayed for NFSv4.2 mounts mounted with the context= mount option.
> >
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  fs/nfs/super.c            | 17 ++++++++++++++++-
> >  include/linux/lsm_hooks.h |  4 +++-
> >  include/linux/security.h  |  8 ++++++--
> >  security/security.c       |  7 +++++--
> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
> >  5 files changed, 63 insertions(+), 8 deletions(-)
> 
> Thanks for sorting this out Scott and Stephen.
> 
> NFS folks, any objections to this patch?  If not, I'd like to pull
> this into the SELinux tree but I'd like to have an ACK from you before
> I do.

Looks OK to me, but I think it's Trond or Anna (added to cc) that you
want the ACK from.

--b.

> 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 2f3822a..b8e0735 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2544,10 +2544,25 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> >  int nfs_clone_sb_security(struct super_block *s, struct dentry *mntroot,
> >                           struct nfs_mount_info *mount_info)
> >  {
> > +       int error;
> > +       unsigned long kflags = 0, kflags_out = 0;
> > +
> >         /* clone any lsm security options from the parent to the new sb */
> >         if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client->rpc_ops->dir_inode_ops)
> >                 return -ESTALE;
> > -       return security_sb_clone_mnt_opts(mount_info->cloned->sb, s);
> > +
> > +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> > +               kflags |= SECURITY_LSM_NATIVE_LABELS;
> > +
> > +       error = security_sb_clone_mnt_opts(mount_info->cloned->sb, s, kflags,
> > +                       &kflags_out);
> > +       if (error)
> > +               return error;
> > +
> > +       if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > +               !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > +               NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> > +       return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 68d91e4..3cc9d77 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1409,7 +1409,9 @@ union security_list_options {
> >                                 unsigned long kern_flags,
> >                                 unsigned long *set_kern_flags);
> >         int (*sb_clone_mnt_opts)(const struct super_block *oldsb,
> > -                                       struct super_block *newsb);
> > +                                       struct super_block *newsb,
> > +                                       unsigned long kern_flags,
> > +                                       unsigned long *set_kern_flags);
> >         int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts);
> >         int (*dentry_init_security)(struct dentry *dentry, int mode,
> >                                         const struct qstr *name, void **ctx,
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index 549cb82..b44e954 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -249,7 +249,9 @@ int security_sb_set_mnt_opts(struct super_block *sb,
> >                                 unsigned long kern_flags,
> >                                 unsigned long *set_kern_flags);
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                               struct super_block *newsb);
> > +                               struct super_block *newsb,
> > +                               unsigned long kern_flags,
> > +                               unsigned long *set_kern_flags);
> >  int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
> >  int security_dentry_init_security(struct dentry *dentry, int mode,
> >                                         const struct qstr *name, void **ctx,
> > @@ -605,7 +607,9 @@ static inline int security_sb_set_mnt_opts(struct super_block *sb,
> >  }
> >
> >  static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                                             struct super_block *newsb)
> > +                                             struct super_block *newsb,
> > +                                             unsigned long kern_flags,
> > +                                             unsigned long *set_kern_flags)
> >  {
> >         return 0;
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index 714433e..3013237 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -420,9 +420,12 @@ int security_sb_set_mnt_opts(struct super_block *sb,
> >  EXPORT_SYMBOL(security_sb_set_mnt_opts);
> >
> >  int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                               struct super_block *newsb)
> > +                               struct super_block *newsb,
> > +                               unsigned long kern_flags,
> > +                               unsigned long *set_kern_flags)
> >  {
> > -       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb);
> > +       return call_int_hook(sb_clone_mnt_opts, 0, oldsb, newsb,
> > +                               kern_flags, set_kern_flags);
> >  }
> >  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 9926adb..9cc042d 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -525,8 +525,16 @@ static int sb_finish_set_opts(struct super_block *sb)
> >         }
> >
> >         sbsec->flags |= SE_SBINITIALIZED;
> > +
> > +       /*
> > +        * Explicitly set or clear SBLABEL_MNT.  It's not sufficient to simply
> > +        * leave the flag untouched because sb_clone_mnt_opts might be handing
> > +        * us a superblock that needs the flag to be cleared.
> > +        */
> >         if (selinux_is_sblabel_mnt(sb))
> >                 sbsec->flags |= SBLABEL_MNT;
> > +       else
> > +               sbsec->flags &= ~SBLABEL_MNT;
> >
> >         /* Initialize the root inode. */
> >         rc = inode_doinit_with_dentry(root_inode, root);
> > @@ -959,8 +967,11 @@ static int selinux_cmp_sb_context(const struct super_block *oldsb,
> >  }
> >
> >  static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> > -                                       struct super_block *newsb)
> > +                                       struct super_block *newsb,
> > +                                       unsigned long kern_flags,
> > +                                       unsigned long *set_kern_flags)
> >  {
> > +       int rc = 0;
> >         const struct superblock_security_struct *oldsbsec = oldsb->s_security;
> >         struct superblock_security_struct *newsbsec = newsb->s_security;
> >
> > @@ -975,6 +986,13 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         if (!ss_initialized)
> >                 return 0;
> >
> > +       /*
> > +        * Specifying internal flags without providing a place to
> > +        * place the results is not allowed.
> > +        */
> > +       if (kern_flags && !set_kern_flags)
> > +               return -EINVAL;
> > +
> >         /* how can we clone if the old one wasn't set up?? */
> >         BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED));
> >
> > @@ -990,6 +1008,18 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         newsbsec->def_sid = oldsbsec->def_sid;
> >         newsbsec->behavior = oldsbsec->behavior;
> >
> > +       if (newsbsec->behavior == SECURITY_FS_USE_NATIVE &&
> > +               !(kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) {
> > +               rc = security_fs_use(newsb);
> > +               if (rc)
> > +                       goto out;
> > +       }
> > +
> > +       if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !set_context) {
> > +               newsbsec->behavior = SECURITY_FS_USE_NATIVE;
> > +               *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > +       }
> > +
> >         if (set_context) {
> >                 u32 sid = oldsbsec->mntpoint_sid;
> >
> > @@ -1009,8 +1039,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
> >         }
> >
> >         sb_finish_set_opts(newsb);
> > +out:
> >         mutex_unlock(&newsbsec->lock);
> > -       return 0;
> > +       return rc;
> >  }
> >
> >  static int selinux_parse_opts_str(char *options,
> > --
> > 2.9.3
> >
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com

Re: [PATCH v3] security/selinux: allow security_sb_clone_mnt_opts to enable/disable native labeling behavior

[Paul Moore]

On Mon, Jun 5, 2017 at 8:46 PM, J . Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Jun 05, 2017 at 05:21:55PM -0400, Paul Moore wrote:
>> On Mon, Jun 5, 2017 at 11:45 AM, Scott Mayhew <smayhew@redhat.com> wrote:
>> > When an NFSv4 client performs a mount operation, it first mounts the
>> > NFSv4 root and then does path walk to the exported path and performs a
>> > submount on that, cloning the security mount options from the root's
>> > superblock to the submount's superblock in the process.
>> >
>> > Unless the NFS server has an explicit fsid=0 export with the
>> > "security_label" option, the NFSv4 root superblock will not have
>> > SBLABEL_MNT set, and neither will the submount superblock after cloning
>> > the security mount options.  As a result, setxattr's of security labels
>> > over NFSv4.2 will fail.  In a similar fashion, NFSv4.2 mounts mounted
>> > with the context= mount option will not show the correct labels because
>> > the nfs_server->caps flags of the cloned superblock will still have
>> > NFS_CAP_SECURITY_LABEL set.
>> >
>> > Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
>> > behavior will ensure that the SBLABEL_MNT flag has the correct value
>> > when the client traverses from an exported path without the
>> > "security_label" option to one with the "security_label" option and
>> > vice versa.  Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
>> > set upon return from security_sb_clone_mnt_opts() and clearing
>> > NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
>> > be displayed for NFSv4.2 mounts mounted with the context= mount option.
>> >
>> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
>> > ---
>> >  fs/nfs/super.c            | 17 ++++++++++++++++-
>> >  include/linux/lsm_hooks.h |  4 +++-
>> >  include/linux/security.h  |  8 ++++++--
>> >  security/security.c       |  7 +++++--
>> >  security/selinux/hooks.c  | 35 +++++++++++++++++++++++++++++++++--
>> >  5 files changed, 63 insertions(+), 8 deletions(-)
>>
>> Thanks for sorting this out Scott and Stephen.
>>
>> NFS folks, any objections to this patch?  If not, I'd like to pull
>> this into the SELinux tree but I'd like to have an ACK from you before
>> I do.
>
> Looks OK to me, but I think it's Trond or Anna (added to cc) that you
> want the ACK from.

It's been about four days with not comments from the NFS folks so I'm
just going to go ahead and merge this into the selinux/next branch so
we can get some more widespread testing.  NFS folks, if you want to
object please do so a week or two before the next merge window opens,
otherwise I'm going to send this patch upstream.

Thanks Scott for working on this patch, and everyone else for their
comments, testing, and review.

-- 
paul moore
www.paul-moore.com

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> On Fri, 26 May 2017, Stephen Smalley wrote:
> 
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > When the client traverses from filesystem exported without the
> > > "security_label" option to one exported with the "security_label"
> > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > security_sb_set_mnt_opts() so that the new superblock has
> > > SBLABEL_MNT
> > > set in its security mount options.  Otherwise, attempts to set
> > > security
> > > labels via setxattr over NFSv4.2 will fail.
> > > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 2f3822a..d7a3b89 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > *mntroot,
> > >  			  struct nfs_mount_info *mount_info)
> > >  {
> > > +	int error;
> > > +	unsigned long kflags = 0, kflags_out = 0;
> > > +	struct security_mnt_opts opts;
> > > +
> > >  	/* clone any lsm security options from the parent to the
> > > new
> > > sb */
> > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > rpc_ops->dir_inode_ops)
> > > 
> > >  		return -ESTALE;
> > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > >sb,
> > > s);
> > > +	if (error)
> > > +		goto err;
> > > +
> > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > NFS_CAP_SECURITY_LABEL)) {
> > > +		memset(&opts, 0, sizeof(opts));
> > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > +
> > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > kflags,
> > > &kflags_out);
> > > +		if (error)
> > > +			goto err;
> > > +
> > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > +			NFS_SB(s)->caps &=
> > > ~NFS_CAP_SECURITY_LABEL;
> > > +	}
> > > +err:
> > > +	return error;
> > >  }
> > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > 
> > Could this clobber a context set via context= mount option?
> 
> Argh, yes I suppose it could.  In my first attempt to fix this, I
> added
> a security_sb_get_mnt_opts() hook to get the original mount options
> and
> then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> security_sb_set_mnt_opts().  When I saw that
> security_sb_set_mnt_opts()
> wouldn't allow me to change a superblock that had already been
> initialized, I got rid of the hook and added the check in patch 1...
> maybe a combination of the two is needed?
> 
> Testing it again now, I'm not sure the context= mount option is
> working
> correctly with the latest kernel.

Looks like you are correct,
https://github.com/SELinuxProject/selinux-kernel/issues/35

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[J . Bruce Fields]

On Tue, May 30, 2017 at 10:38:45AM -0400, Stephen Smalley wrote:
> On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> > On Fri, 26 May 2017, Stephen Smalley wrote:
> > 
> > > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > > When the client traverses from filesystem exported without the
> > > > "security_label" option to one exported with the "security_label"
> > > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > > security_sb_set_mnt_opts() so that the new superblock has
> > > > SBLABEL_MNT
> > > > set in its security mount options.  Otherwise, attempts to set
> > > > security
> > > > labels via setxattr over NFSv4.2 will fail.
> > > > 
> > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > > ---
> > > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > > index 2f3822a..d7a3b89 100644
> > > > --- a/fs/nfs/super.c
> > > > +++ b/fs/nfs/super.c
> > > > @@ -2544,10 +2544,31 @@ EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > > >  int nfs_clone_sb_security(struct super_block *s, struct dentry
> > > > *mntroot,
> > > >  			  struct nfs_mount_info *mount_info)
> > > >  {
> > > > +	int error;
> > > > +	unsigned long kflags = 0, kflags_out = 0;
> > > > +	struct security_mnt_opts opts;
> > > > +
> > > >  	/* clone any lsm security options from the parent to the
> > > > new
> > > > sb */
> > > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > > rpc_ops->dir_inode_ops)
> > > > 
> > > >  		return -ESTALE;
> > > > -	return security_sb_clone_mnt_opts(mount_info->cloned-
> > > > >sb,
> > > > s);
> > > > +	error = security_sb_clone_mnt_opts(mount_info->cloned-
> > > > >sb,
> > > > s);
> > > > +	if (error)
> > > > +		goto err;
> > > > +
> > > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > > NFS_CAP_SECURITY_LABEL)) {
> > > > +		memset(&opts, 0, sizeof(opts));
> > > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > > +
> > > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > > kflags,
> > > > &kflags_out);
> > > > +		if (error)
> > > > +			goto err;
> > > > +
> > > > +		if (!(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> > > > +			NFS_SB(s)->caps &=
> > > > ~NFS_CAP_SECURITY_LABEL;
> > > > +	}
> > > > +err:
> > > > +	return error;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > > 
> > > Could this clobber a context set via context= mount option?
> > 
> > Argh, yes I suppose it could.  In my first attempt to fix this, I
> > added
> > a security_sb_get_mnt_opts() hook to get the original mount options
> > and
> > then passed that along with the SECURITY_LSM_NATIVE_LABELS flag to
> > security_sb_set_mnt_opts().  When I saw that
> > security_sb_set_mnt_opts()
> > wouldn't allow me to change a superblock that had already been
> > initialized, I got rid of the hook and added the check in patch 1...
> > maybe a combination of the two is needed?
> > 
> > Testing it again now, I'm not sure the context= mount option is
> > working
> > correctly with the latest kernel.
> 
> Looks like you are correct,
> https://github.com/SELinuxProject/selinux-kernel/issues/35

Ugh.  So, to make sure I understand: the desired behavior is that in the
case the client mounts with a context= option, behavior is exactly as if
the client or server didn't support the new security labeling protocol.
That would make sense to me.

--b.

Re: [PATCH RFC 2/2] nfs: update labeling behavior on a superblock when submounting

[Stephen Smalley]

On Tue, 2017-05-30 at 15:40 -0400, J . Bruce Fields wrote:
> On Tue, May 30, 2017 at 10:38:45AM -0400, Stephen Smalley wrote:
> > On Fri, 2017-05-26 at 11:28 -0400, Scott Mayhew wrote:
> > > On Fri, 26 May 2017, Stephen Smalley wrote:
> > > 
> > > > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > > > When the client traverses from filesystem exported without
> > > > > the
> > > > > "security_label" option to one exported with the
> > > > > "security_label"
> > > > > option, it needs to pass SECURITY_LSM_NATIVE_LABELS to
> > > > > security_sb_set_mnt_opts() so that the new superblock has
> > > > > SBLABEL_MNT
> > > > > set in its security mount options.  Otherwise, attempts to
> > > > > set
> > > > > security
> > > > > labels via setxattr over NFSv4.2 will fail.
> > > > > 
> > > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > > > ---
> > > > >  fs/nfs/super.c | 23 ++++++++++++++++++++++-
> > > > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > > > index 2f3822a..d7a3b89 100644
> > > > > --- a/fs/nfs/super.c
> > > > > +++ b/fs/nfs/super.c
> > > > > @@ -2544,10 +2544,31 @@
> > > > > EXPORT_SYMBOL_GPL(nfs_set_sb_security);
> > > > >  int nfs_clone_sb_security(struct super_block *s, struct
> > > > > dentry
> > > > > *mntroot,
> > > > >  			  struct nfs_mount_info *mount_info)
> > > > >  {
> > > > > +	int error;
> > > > > +	unsigned long kflags = 0, kflags_out = 0;
> > > > > +	struct security_mnt_opts opts;
> > > > > +
> > > > >  	/* clone any lsm security options from the parent to
> > > > > the
> > > > > new
> > > > > sb */
> > > > >  	if (d_inode(mntroot)->i_op != NFS_SB(s)->nfs_client-
> > > > > > rpc_ops->dir_inode_ops)
> > > > > 
> > > > >  		return -ESTALE;
> > > > > -	return security_sb_clone_mnt_opts(mount_info-
> > > > > >cloned-
> > > > > > sb,
> > > > > 
> > > > > s);
> > > > > +	error = security_sb_clone_mnt_opts(mount_info-
> > > > > >cloned-
> > > > > > sb,
> > > > > 
> > > > > s);
> > > > > +	if (error)
> > > > > +		goto err;
> > > > > +
> > > > > +	if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> > > > > +		!(NFS_SB(mount_info->cloned->sb)->caps &
> > > > > NFS_CAP_SECURITY_LABEL)) {
> > > > > +		memset(&opts, 0, sizeof(opts));
> > > > > +		kflags |= SECURITY_LSM_NATIVE_LABELS;
> > > > > +
> > > > > +		error = security_sb_set_mnt_opts(s, &opts,
> > > > > kflags,
> > > > > &kflags_out);
> > > > > +		if (error)
> > > > > +			goto err;
> > > > > +
> > > > > +		if (!(kflags_out &
> > > > > SECURITY_LSM_NATIVE_LABELS))
> > > > > +			NFS_SB(s)->caps &=
> > > > > ~NFS_CAP_SECURITY_LABEL;
> > > > > +	}
> > > > > +err:
> > > > > +	return error;
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(nfs_clone_sb_security);
> > > > 
> > > > Could this clobber a context set via context= mount option?
> > > 
> > > Argh, yes I suppose it could.  In my first attempt to fix this, I
> > > added
> > > a security_sb_get_mnt_opts() hook to get the original mount
> > > options
> > > and
> > > then passed that along with the SECURITY_LSM_NATIVE_LABELS flag
> > > to
> > > security_sb_set_mnt_opts().  When I saw that
> > > security_sb_set_mnt_opts()
> > > wouldn't allow me to change a superblock that had already been
> > > initialized, I got rid of the hook and added the check in patch
> > > 1...
> > > maybe a combination of the two is needed?
> > > 
> > > Testing it again now, I'm not sure the context= mount option is
> > > working
> > > correctly with the latest kernel.
> > 
> > Looks like you are correct,
> > https://github.com/SELinuxProject/selinux-kernel/issues/35
> 
> Ugh.  So, to make sure I understand: the desired behavior is that in
> the
> case the client mounts with a context= option, behavior is exactly as
> if
> the client or server didn't support the new security labeling
> protocol.
> That would make sense to me.

Yes, that's correct.  And in theory that is what nfs_set_sb_security()
is trying to do by clearing NFS_CAP_SECURITY_LABEL if
SECURITY_LSM_NATIVE_LABELS was not set by the security hook.

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Stephen Smalley]

On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> The problem is related to how filesystems are mounted in NFSv4.

What kernel version and what is a reproducer for the problem?  I don't
seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

> 
> When an NFSv4 client performs a mount operation, it first mounts the
> NFSv4 root and then does path walk to the exported path and performs
> a
> submount on that, cloning the security mount options from the root's
> superblock to the submount's superblock in the process.
> 
> Unless the NFS server has an explicit fsid=0 export with the
> "security_label" option, the NFSv4 root superblock will not have
> SBLABEL_MNT set, and neither will the submount superblock after
> cloning
> the security mount options.  As a result, setxattr's of security
> labels
> over NFSv4.2 will fail.
> 
> NFS servers with a modern nfs-utils package will automatically create
> a
> pseudo fs to fill in the gaps (including the root itself) leading up
> to
> the actual export, so it is uncommon these days for an NFS server to
> have an explicit fsid=0 export.
> 
> Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> flag on an initialized superblock would ensure that SBLABEL_MNT is
> set
> when the client traverses from an exported path without the
> "security_label" option to one with the "security_label" option.
> 
> Scott Mayhew (2):
>   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
>     initialized superblock
>   nfs: update labeling behavior on a superblock when submounting
> 
>  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
>  security/selinux/hooks.c |  4 ++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[J . Bruce Fields]

On Fri, May 26, 2017 at 10:48:17AM -0400, Stephen Smalley wrote:
> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > The problem is related to how filesystems are mounted in NFSv4.
> 
> What kernel version and what is a reproducer for the problem?  I don't
> seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

Basically just mount an export with security_label set, mount over NFS,
"ls -Z" will (correctly) show you the server-side security labels, but
"chcon" will fail with ENOTSUP.

If that's not reproducing, maybe you chould show us "exports -v" and
"mount" output--it might not reproduce if you have an "fsid=0" export or
if you're mounting with a protocol version older than 4.2.

--b.

> 
> > 
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.
> > 
> > NFS servers with a modern nfs-utils package will automatically create
> > a
> > pseudo fs to fill in the gaps (including the root itself) leading up
> > to
> > the actual export, so it is uncommon these days for an NFS server to
> > have an explicit fsid=0 export.
> > 
> > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > set
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option.
> > 
> > Scott Mayhew (2):
> >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> >     initialized superblock
> >   nfs: update labeling behavior on a superblock when submounting
> > 
> >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> >  security/selinux/hooks.c |  4 ++--
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[J . Bruce Fields]

On Fri, May 26, 2017 at 11:17:22AM -0400, J . Bruce Fields wrote:
> On Fri, May 26, 2017 at 10:48:17AM -0400, Stephen Smalley wrote:
> > On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > > The problem is related to how filesystems are mounted in NFSv4.
> > 
> > What kernel version and what is a reproducer for the problem?  I don't
> > seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.
> 
> Basically just mount an export with security_label set, mount over NFS,
> "ls -Z" will (correctly) show you the server-side security labels, but
> "chcon" will fail with ENOTSUP.
> 
> If that's not reproducing, maybe you chould show us "exports -v" and
> "mount" output--it might not reproduce if you have an "fsid=0" export or
> if you're mounting with a protocol version older than 4.2.

Oh, and, sorry, kernel version: I think you just need something recent
enough to support "security_label", which went into 4.11.

--b.

> 
> --b.
> 
> > 
> > > 
> > > When an NFSv4 client performs a mount operation, it first mounts the
> > > NFSv4 root and then does path walk to the exported path and performs
> > > a
> > > submount on that, cloning the security mount options from the root's
> > > superblock to the submount's superblock in the process.
> > > 
> > > Unless the NFS server has an explicit fsid=0 export with the
> > > "security_label" option, the NFSv4 root superblock will not have
> > > SBLABEL_MNT set, and neither will the submount superblock after
> > > cloning
> > > the security mount options.  As a result, setxattr's of security
> > > labels
> > > over NFSv4.2 will fail.
> > > 
> > > NFS servers with a modern nfs-utils package will automatically create
> > > a
> > > pseudo fs to fill in the gaps (including the root itself) leading up
> > > to
> > > the actual export, so it is uncommon these days for an NFS server to
> > > have an explicit fsid=0 export.
> > > 
> > > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > > set
> > > when the client traverses from an exported path without the
> > > "security_label" option to one with the "security_label" option.
> > > 
> > > Scott Mayhew (2):
> > >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> > >     initialized superblock
> > >   nfs: update labeling behavior on a superblock when submounting
> > > 
> > >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> > >  security/selinux/hooks.c |  4 ++--
> > >  2 files changed, 24 insertions(+), 3 deletions(-)
> > > 

Re: [PATCH RFC 0/2] Fix setting of security labels over NFSv4.2

[Scott Mayhew]

On Fri, 26 May 2017, Stephen Smalley wrote:

> On Thu, 2017-05-25 at 17:07 -0400, Scott Mayhew wrote:
> > Red Hat QE reported that chcon fails over NFSv4.2 on recent kernels.
> > The problem is related to how filesystems are mounted in NFSv4.
> 
> What kernel version and what is a reproducer for the problem?  I don't
> seem to see it on e.g. Fedora 25 with 4.10, unless I misunderstand.

I'm testing with Fedora 25 but with the upstream kernel.  The Fedora
kernel doesn't have commit 32ddd944 ("nfsd: opt in to labeled nfs per
export") yet.

-Scott
> 
> > 
> > When an NFSv4 client performs a mount operation, it first mounts the
> > NFSv4 root and then does path walk to the exported path and performs
> > a
> > submount on that, cloning the security mount options from the root's
> > superblock to the submount's superblock in the process.
> > 
> > Unless the NFS server has an explicit fsid=0 export with the
> > "security_label" option, the NFSv4 root superblock will not have
> > SBLABEL_MNT set, and neither will the submount superblock after
> > cloning
> > the security mount options.  As a result, setxattr's of security
> > labels
> > over NFSv4.2 will fail.
> > 
> > NFS servers with a modern nfs-utils package will automatically create
> > a
> > pseudo fs to fill in the gaps (including the root itself) leading up
> > to
> > the actual export, so it is uncommon these days for an NFS server to
> > have an explicit fsid=0 export.
> > 
> > Allowing the NFSv4 client to override the SECURITY_LSM_NATIVE_LABELS
> > flag on an initialized superblock would ensure that SBLABEL_MNT is
> > set
> > when the client traverses from an exported path without the
> > "security_label" option to one with the "security_label" option.
> > 
> > Scott Mayhew (2):
> >   selinux: allow SECURITY_LSM_NATIVE_LABELS to be set on an already
> >     initialized superblock
> >   nfs: update labeling behavior on a superblock when submounting
> > 
> >  fs/nfs/super.c           | 23 ++++++++++++++++++++++-
> >  security/selinux/hooks.c |  4 ++--
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> >