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 
> ---
>  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,
> + _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 

[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 
---
 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,
+   _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