linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] selinux: make dentry_init_security() return security module name
@ 2018-06-26  8:43 Yan, Zheng
  2018-06-26  8:43 ` [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx Yan, Zheng
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Yan, Zheng @ 2018-06-26  8:43 UTC (permalink / raw)
  To: linux-security-module

This is preparation for CephFS security label. CephFS's implementation uses
dentry_init_security() to get security context before inode is created,
then sends open/mkdir/mknod request to MDS, together with security xattr
"security.<security module name>"

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/nfs/nfs4proc.c         | 3 ++-
 include/linux/lsm_hooks.h | 4 ++--
 include/linux/security.h  | 9 +++++----
 security/security.c       | 7 ++++---
 security/selinux/hooks.c  | 8 ++++++--
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6dd146885da9..d18a5fb7aec3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
 		return NULL;
 
 	err = security_dentry_init_security(dentry, sattr->ia_mode,
-				&dentry->d_name, (void **)&label->label, &label->len);
+				&dentry->d_name,  NULL,
+				(void **)&label->label, &label->len);
 	if (err == 0)
 		return label;
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8f1131c8dd54..e176c2032bdc 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1476,8 +1476,8 @@ union security_list_options {
 					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,
-					u32 *ctxlen);
+					const struct qstr *name, const char **label,
+					void **ctx, u32 *ctxlen);
 	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
 					struct qstr *name,
 					const struct cred *old,
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..df2d73998c64 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
 				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,
-					u32 *ctxlen);
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen);
 int security_dentry_create_files_as(struct dentry *dentry, int mode,
 					struct qstr *name,
 					const struct cred *old,
@@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
 static inline int security_dentry_init_security(struct dentry *dentry,
 						 int mode,
 						 const struct qstr *name,
-						 void **ctx,
-						 u32 *ctxlen)
+						 const char **label,
+						 void **ctx, u32 *ctxlen)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/security.c b/security/security.c
index 68f46d849abe..69818d46aa28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
 }
 
 int security_dentry_init_security(struct dentry *dentry, int mode,
-					const struct qstr *name, void **ctx,
-					u32 *ctxlen)
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen)
 {
 	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
-				name, ctx, ctxlen);
+				name, label, ctx, ctxlen);
 }
 EXPORT_SYMBOL(security_dentry_init_security);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2b5ee5fbd652..eca3879d9357 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
 }
 
 static int selinux_dentry_init_security(struct dentry *dentry, int mode,
-					const struct qstr *name, void **ctx,
-					u32 *ctxlen)
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen)
 {
 	u32 newsid;
 	int rc;
@@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
 	if (rc)
 		return rc;
 
+	if (label)
+		*label = XATTR_SELINUX_SUFFIX;
+
 	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
 				       ctxlen);
 }
-- 
2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx
  2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
@ 2018-06-26  8:43 ` Yan, Zheng
  2018-09-06 15:14   ` Jeff Layton
  2018-06-26  8:43 ` [PATCH 3/3] ceph: xattr security label support Yan, Zheng
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Yan, Zheng @ 2018-06-26  8:43 UTC (permalink / raw)
  To: linux-security-module

this is preparation for security label support

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/acl.c   | 22 +++++++---------------
 fs/ceph/dir.c   | 28 ++++++++++++++--------------
 fs/ceph/file.c  | 18 +++++++++---------
 fs/ceph/super.h | 29 +++++++++++++++--------------
 fs/ceph/xattr.c | 10 ++++++++++
 5 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 3351ea14390b..f13ba4250f00 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -172,7 +172,7 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 }
 
 int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
-		       struct ceph_acls_info *info)
+		       struct ceph_acl_sec_ctx *as_ctx)
 {
 	struct posix_acl *acl, *default_acl;
 	size_t val_size1 = 0, val_size2 = 0;
@@ -248,9 +248,9 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 
 	kfree(tmp_buf);
 
-	info->acl = acl;
-	info->default_acl = default_acl;
-	info->pagelist = pagelist;
+	as_ctx->acl = acl;
+	as_ctx->default_acl = default_acl;
+	as_ctx->pagelist = pagelist;
 	return 0;
 
 out_err:
@@ -262,18 +262,10 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 	return err;
 }
 
-void ceph_init_inode_acls(struct inode* inode, struct ceph_acls_info *info)
+void ceph_init_inode_acls(struct inode* inode, struct ceph_acl_sec_ctx *as_ctx)
 {
 	if (!inode)
 		return;
-	ceph_set_cached_acl(inode, ACL_TYPE_ACCESS, info->acl);
-	ceph_set_cached_acl(inode, ACL_TYPE_DEFAULT, info->default_acl);
-}
-
-void ceph_release_acls_info(struct ceph_acls_info *info)
-{
-	posix_acl_release(info->acl);
-	posix_acl_release(info->default_acl);
-	if (info->pagelist)
-		ceph_pagelist_release(info->pagelist);
+	ceph_set_cached_acl(inode, ACL_TYPE_ACCESS, as_ctx->acl);
+	ceph_set_cached_acl(inode, ACL_TYPE_DEFAULT, as_ctx->default_acl);
 }
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 036ac0f3a393..f451ad5a37ab 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -821,7 +821,7 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
-	struct ceph_acls_info acls = {};
+	struct ceph_acl_sec_ctx as_ctx = {};
 	int err;
 
 	if (ceph_snap(dir) != CEPH_NOSNAP)
@@ -830,7 +830,7 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	if (ceph_quota_is_max_files_exceeded(dir))
 		return -EDQUOT;
 
-	err = ceph_pre_init_acls(dir, &mode, &acls);
+	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 	if (err < 0)
 		return err;
 
@@ -849,9 +849,9 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	req->r_args.mknod.rdev = cpu_to_le32(rdev);
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
 	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
-	if (acls.pagelist) {
-		req->r_pagelist = acls.pagelist;
-		acls.pagelist = NULL;
+	if (as_ctx.pagelist) {
+		req->r_pagelist = as_ctx.pagelist;
+		as_ctx.pagelist = NULL;
 	}
 	err = ceph_mdsc_do_request(mdsc, dir, req);
 	if (!err && !req->r_reply_info.head->is_dentry)
@@ -859,10 +859,10 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	ceph_mdsc_put_request(req);
 out:
 	if (!err)
-		ceph_init_inode_acls(d_inode(dentry), &acls);
+		ceph_init_inode_acls(d_inode(dentry), &as_ctx);
 	else
 		d_drop(dentry);
-	ceph_release_acls_info(&acls);
+	ceph_release_acl_sec_ctx(&as_ctx);
 	return err;
 }
 
@@ -919,7 +919,7 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
-	struct ceph_acls_info acls = {};
+	struct ceph_acl_sec_ctx as_ctx = {};
 	int err = -EROFS;
 	int op;
 
@@ -942,7 +942,7 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	}
 
 	mode |= S_IFDIR;
-	err = ceph_pre_init_acls(dir, &mode, &acls);
+	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 	if (err < 0)
 		goto out;
 
@@ -959,9 +959,9 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	req->r_args.mkdir.mode = cpu_to_le32(mode);
 	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
 	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
-	if (acls.pagelist) {
-		req->r_pagelist = acls.pagelist;
-		acls.pagelist = NULL;
+	if (as_ctx.pagelist) {
+		req->r_pagelist = as_ctx.pagelist;
+		as_ctx.pagelist = NULL;
 	}
 	err = ceph_mdsc_do_request(mdsc, dir, req);
 	if (!err &&
@@ -971,10 +971,10 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	ceph_mdsc_put_request(req);
 out:
 	if (!err)
-		ceph_init_inode_acls(d_inode(dentry), &acls);
+		ceph_init_inode_acls(d_inode(dentry), &as_ctx);
 	else
 		d_drop(dentry);
-	ceph_release_acls_info(&acls);
+	ceph_release_acl_sec_ctx(&as_ctx);
 	return err;
 }
 
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index ad0bed99b1d5..701506ec5768 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -436,7 +436,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
 	struct dentry *dn;
-	struct ceph_acls_info acls = {};
+	struct ceph_acl_sec_ctx as_ctx = {};
 	int mask;
 	int err;
 
@@ -450,7 +450,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (flags & O_CREAT) {
 		if (ceph_quota_is_max_files_exceeded(dir))
 			return -EDQUOT;
-		err = ceph_pre_init_acls(dir, &mode, &acls);
+		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 		if (err < 0)
 			return err;
 	}
@@ -459,16 +459,16 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	req = prepare_open_request(dir->i_sb, flags, mode);
 	if (IS_ERR(req)) {
 		err = PTR_ERR(req);
-		goto out_acl;
+		goto out_ctx;
 	}
 	req->r_dentry = dget(dentry);
 	req->r_num_caps = 2;
 	if (flags & O_CREAT) {
 		req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
 		req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
-		if (acls.pagelist) {
-			req->r_pagelist = acls.pagelist;
-			acls.pagelist = NULL;
+		if (as_ctx.pagelist) {
+			req->r_pagelist = as_ctx.pagelist;
+			as_ctx.pagelist = NULL;
 		}
 	}
 
@@ -506,7 +506,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	} else {
 		dout("atomic_open finish_open on dn %p\n", dn);
 		if (req->r_op == CEPH_MDS_OP_CREATE && req->r_reply_info.has_create_ino) {
-			ceph_init_inode_acls(d_inode(dentry), &acls);
+			ceph_init_inode_acls(d_inode(dentry), &as_ctx);
 			*opened |= FILE_CREATED;
 		}
 		err = finish_open(file, dentry, ceph_open, opened);
@@ -515,8 +515,8 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (!req->r_err && req->r_target_inode)
 		ceph_put_fmode(ceph_inode(req->r_target_inode), req->r_fmode);
 	ceph_mdsc_put_request(req);
-out_acl:
-	ceph_release_acls_info(&acls);
+out_ctx:
+	ceph_release_acl_sec_ctx(&as_ctx);
 	dout("atomic_open result=%d\n", err);
 	return err;
 }
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 5df5262b24b0..83561421afda 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -906,6 +906,14 @@ extern void __init ceph_xattr_init(void);
 extern void ceph_xattr_exit(void);
 extern const struct xattr_handler *ceph_xattr_handlers[];
 
+struct ceph_acl_sec_ctx {
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	void *default_acl;
+	void *acl;
+#endif
+	struct ceph_pagelist *pagelist;
+};
+
 #ifdef CONFIG_SECURITY
 extern bool ceph_security_xattr_deadlock(struct inode *in);
 extern bool ceph_security_xattr_wanted(struct inode *in);
@@ -920,21 +928,17 @@ static inline bool ceph_security_xattr_wanted(struct inode *in)
 }
 #endif
 
-/* acl.c */
-struct ceph_acls_info {
-	void *default_acl;
-	void *acl;
-	struct ceph_pagelist *pagelist;
-};
+void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx);
 
+/* acl.c */
 #ifdef CONFIG_CEPH_FS_POSIX_ACL
 
 struct posix_acl *ceph_get_acl(struct inode *, int);
 int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type);
 int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
-		       struct ceph_acls_info *info);
-void ceph_init_inode_acls(struct inode *inode, struct ceph_acls_info *info);
-void ceph_release_acls_info(struct ceph_acls_info *info);
+		       struct ceph_acl_sec_ctx *as_ctx);
+void ceph_init_inode_acls(struct inode *inode,
+			  struct ceph_acl_sec_ctx *as_ctx);
 
 static inline void ceph_forget_all_cached_acls(struct inode *inode)
 {
@@ -947,15 +951,12 @@ static inline void ceph_forget_all_cached_acls(struct inode *inode)
 #define ceph_set_acl NULL
 
 static inline int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
-				     struct ceph_acls_info *info)
+				     struct ceph_acl_sec_ctx *as_ctx)
 {
 	return 0;
 }
 static inline void ceph_init_inode_acls(struct inode *inode,
-					struct ceph_acls_info *info)
-{
-}
-static inline void ceph_release_acls_info(struct ceph_acls_info *info)
+					struct ceph_acl_sec_ctx *as_ctx)
 {
 }
 static inline int ceph_acl_chmod(struct dentry *dentry, struct inode *inode)
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 5bc8edb4c2a6..ef0e968d56a1 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1190,3 +1190,13 @@ bool ceph_security_xattr_deadlock(struct inode *in)
 	return ret;
 }
 #endif
+
+void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
+{
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	posix_acl_release(as_ctx->acl);
+	posix_acl_release(as_ctx->default_acl);
+#endif
+	if (as_ctx->pagelist)
+		ceph_pagelist_release(as_ctx->pagelist);
+}
-- 
2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] ceph: xattr security label support
  2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
  2018-06-26  8:43 ` [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx Yan, Zheng
@ 2018-06-26  8:43 ` Yan, Zheng
  2018-09-06 15:50   ` Jeff Layton
  2018-06-26 13:28 ` [PATCH 1/3] selinux: make dentry_init_security() return security module name Stephen Smalley
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Yan, Zheng @ 2018-06-26  8:43 UTC (permalink / raw)
  To: linux-security-module

When creating new file/directory, uses dentry_init_security() to prepare
security context for the new inode, then sends openc/mkdir request to MDS,
together with security xattr "security.<security module name>"

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/Kconfig |   5 ++
 fs/ceph/caps.c  |   1 +
 fs/ceph/dir.c   |  12 +++++
 fs/ceph/file.c  |   3 ++
 fs/ceph/inode.c |   1 +
 fs/ceph/super.h |  19 +++++++
 fs/ceph/xattr.c | 140 ++++++++++++++++++++++++++++++++++++++++++------
 7 files changed, 164 insertions(+), 17 deletions(-)

diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index 52095f473464..e1a4100c99eb 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -35,3 +35,8 @@ config CEPH_FS_POSIX_ACL
 	  groups beyond the owner/group/world scheme.
 
 	  If you don't know what Access Control Lists are, say N
+
+config CEPH_FS_SECURITY_LABEL
+	bool
+	depends on CEPH_FS && SECURITY
+	default y
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 990258cbd836..ec49a3858288 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3144,6 +3144,7 @@ static void handle_cap_grant(struct inode *inode,
 			ci->i_xattrs.blob = ceph_buffer_get(xattr_buf);
 			ci->i_xattrs.version = version;
 			ceph_forget_all_cached_acls(inode);
+			ceph_security_invalidate_secctx(inode);
 		}
 	}
 
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index f451ad5a37ab..18ece4be4493 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -833,6 +833,9 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 	if (err < 0)
 		return err;
+	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
+	if (err < 0)
+	       goto out;
 
 	dout("mknod in dir %p dentry %p mode 0%ho rdev %d\n",
 	     dir, dentry, mode, rdev);
@@ -878,6 +881,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
+	struct ceph_acl_sec_ctx as_ctx = {};
 	int err;
 
 	if (ceph_snap(dir) != CEPH_NOSNAP)
@@ -886,6 +890,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	if (ceph_quota_is_max_files_exceeded(dir))
 		return -EDQUOT;
 
+	err = ceph_security_init_secctx(dentry, S_IFLNK | S_IRWXUGO, &as_ctx);
+	if (err < 0)
+	       goto out;
+
 	dout("symlink in dir %p dentry %p to '%s'\n", dir, dentry, dest);
 	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SYMLINK, USE_AUTH_MDS);
 	if (IS_ERR(req)) {
@@ -911,6 +919,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 out:
 	if (err)
 		d_drop(dentry);
+	ceph_release_acl_sec_ctx(&as_ctx);
 	return err;
 }
 
@@ -945,6 +954,9 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 	if (err < 0)
 		goto out;
+	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
+	if (err < 0)
+	       goto out;
 
 	req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
 	if (IS_ERR(req)) {
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 701506ec5768..0e835ca720cb 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -453,6 +453,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
 		if (err < 0)
 			return err;
+		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
+		if (err < 0)
+			goto out_ctx;
 	}
 
 	/* do the open */
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 1fe3a02336b0..db0079fd5c06 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -896,6 +896,7 @@ static int fill_inode(struct inode *inode, struct page *locked_page,
 			       iinfo->xattr_data, iinfo->xattr_len);
 		ci->i_xattrs.version = le64_to_cpu(info->xattr_version);
 		ceph_forget_all_cached_acls(inode);
+		ceph_security_invalidate_secctx(inode);
 		xattr_blob = NULL;
 	}
 
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 83561421afda..60151b8cd5c7 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -910,6 +910,10 @@ struct ceph_acl_sec_ctx {
 #ifdef CONFIG_CEPH_FS_POSIX_ACL
 	void *default_acl;
 	void *acl;
+#endif
+#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
+	void *sec_ctx;
+	u32 sec_ctxlen;
 #endif
 	struct ceph_pagelist *pagelist;
 };
@@ -928,6 +932,21 @@ static inline bool ceph_security_xattr_wanted(struct inode *in)
 }
 #endif
 
+#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
+extern int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
+				     struct ceph_acl_sec_ctx *ctx);
+extern void ceph_security_invalidate_secctx(struct inode *inode);
+#else
+static inline int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
+					    struct ceph_acl_sec_ctx *ctx)
+{
+	return 0;
+}
+static inline void ceph_security_invalidate_secctx(struct inode *inode)
+{
+}
+#endif
+
 void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx);
 
 /* acl.c */
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index ef0e968d56a1..3e87208cbde8 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -8,6 +8,7 @@
 #include <linux/ceph/decode.h>
 
 #include <linux/xattr.h>
+#include <linux/security.h>
 #include <linux/posix_acl_xattr.h>
 #include <linux/slab.h>
 
@@ -17,26 +18,9 @@
 static int __remove_xattr(struct ceph_inode_info *ci,
 			  struct ceph_inode_xattr *xattr);
 
-static const struct xattr_handler ceph_other_xattr_handler;
-
-/*
- * List of handlers for synthetic system.* attributes. Other
- * attributes are handled directly.
- */
-const struct xattr_handler *ceph_xattr_handlers[] = {
-#ifdef CONFIG_CEPH_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
-	&ceph_other_xattr_handler,
-	NULL,
-};
-
 static bool ceph_is_valid_xattr(const char *name)
 {
 	return !strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) ||
-	       !strncmp(name, XATTR_SECURITY_PREFIX,
-			XATTR_SECURITY_PREFIX_LEN) ||
 	       !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) ||
 	       !strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
 }
@@ -1189,6 +1173,109 @@ bool ceph_security_xattr_deadlock(struct inode *in)
 	spin_unlock(&ci->i_ceph_lock);
 	return ret;
 }
+
+#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
+int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
+			   struct ceph_acl_sec_ctx *as_ctx)
+{
+	struct ceph_pagelist *pagelist = as_ctx->pagelist;
+	const char *label;
+	size_t label_len;
+	int err;
+
+	err = security_dentry_init_security(dentry, mode, &dentry->d_name,
+					    &label, &as_ctx->sec_ctx,
+					    &as_ctx->sec_ctxlen);
+	if (err < 0) {
+		err = 0; /* do nothing */
+		goto out;
+	}
+
+	err = -ENOMEM;
+	if (!pagelist) {
+		pagelist = kmalloc(sizeof(struct ceph_pagelist), GFP_KERNEL);
+		if (!pagelist)
+			goto out;
+		ceph_pagelist_init(pagelist);
+
+		err = ceph_pagelist_reserve(pagelist, PAGE_SIZE);
+		if (err)
+			goto out;
+		ceph_pagelist_encode_32(pagelist, 1);
+	}
+
+	label_len = strlen(label);
+	err = ceph_pagelist_reserve(pagelist, XATTR_SECURITY_PREFIX_LEN +
+				    label_len + as_ctx->sec_ctxlen + 8);
+	if (err)
+		goto out;
+
+	if (as_ctx->pagelist) {
+		/* update count of KV pairs */
+		BUG_ON(pagelist->length <= sizeof(__le32));
+		if (list_is_singular(&pagelist->head)) {
+			le32_add_cpu((__le32*)pagelist->mapped_tail, 1);
+		} else {
+			struct page *page = list_first_entry(&pagelist->head,
+							     struct page, lru);
+			void *addr = kmap_atomic(page);
+			le32_add_cpu((__le32*)addr, 1);
+			kunmap_atomic(addr);
+		}
+	} else {
+		as_ctx->pagelist = pagelist;
+	}
+
+	ceph_pagelist_encode_32(pagelist,
+				XATTR_SECURITY_PREFIX_LEN + label_len);
+	ceph_pagelist_append(pagelist, XATTR_SECURITY_PREFIX,
+			     XATTR_SECURITY_PREFIX_LEN);
+	ceph_pagelist_append(pagelist, label, label_len);
+
+	ceph_pagelist_encode_32(pagelist, as_ctx->sec_ctxlen);
+	ceph_pagelist_append(pagelist, as_ctx->sec_ctx, as_ctx->sec_ctxlen);
+
+	err = 0;
+out:
+	if (pagelist && !as_ctx->pagelist)
+		ceph_pagelist_release(pagelist);
+	return err;
+}
+
+void ceph_security_invalidate_secctx(struct inode *inode)
+{
+	security_inode_invalidate_secctx(inode);
+}
+
+static int ceph_xattr_set_security_label(const struct xattr_handler *handler,
+				    struct dentry *unused, struct inode *inode,
+				    const char *key, const void *buf,
+				    size_t buflen, int flags)
+{
+	if (security_ismaclabel(key)) {
+		const char *name = xattr_full_name(handler, key);
+		return __ceph_setxattr(inode, name, buf, buflen, flags);
+	}
+	return  -EOPNOTSUPP;
+}
+
+static int ceph_xattr_get_security_label(const struct xattr_handler *handler,
+				    struct dentry *unused, struct inode *inode,
+				    const char *key, void *buf, size_t buflen)
+{
+        if (security_ismaclabel(key)) {
+		const char *name = xattr_full_name(handler, key);
+		return __ceph_getxattr(inode, name, buf, buflen);
+	}
+	return  -EOPNOTSUPP;
+}
+
+static const struct xattr_handler ceph_security_label_handler = {
+        .prefix = XATTR_SECURITY_PREFIX,
+        .get    = ceph_xattr_get_security_label,
+        .set    = ceph_xattr_set_security_label,
+};
+#endif
 #endif
 
 void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
@@ -1196,7 +1283,26 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
 #ifdef CONFIG_CEPH_FS_POSIX_ACL
 	posix_acl_release(as_ctx->acl);
 	posix_acl_release(as_ctx->default_acl);
+#endif
+#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
+	security_release_secctx(as_ctx->sec_ctx, as_ctx->sec_ctxlen);
 #endif
 	if (as_ctx->pagelist)
 		ceph_pagelist_release(as_ctx->pagelist);
 }
+
+/*
+ * List of handlers for synthetic system.* attributes. Other
+ * attributes are handled directly.
+ */
+const struct xattr_handler *ceph_xattr_handlers[] = {
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	&posix_acl_access_xattr_handler,
+	&posix_acl_default_xattr_handler,
+#endif
+#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
+	&ceph_security_label_handler,
+#endif
+	&ceph_other_xattr_handler,
+	NULL,
+};
-- 
2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
  2018-06-26  8:43 ` [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx Yan, Zheng
  2018-06-26  8:43 ` [PATCH 3/3] ceph: xattr security label support Yan, Zheng
@ 2018-06-26 13:28 ` Stephen Smalley
  2018-06-26 15:32   ` Yan, Zheng
  2018-06-26 16:21 ` Casey Schaufler
  2018-09-06 15:08 ` Jeff Layton
  4 siblings, 1 reply; 18+ messages in thread
From: Stephen Smalley @ 2018-06-26 13:28 UTC (permalink / raw)
  To: linux-security-module

On 06/26/2018 04:43 AM, Yan, Zheng wrote:
> This is preparation for CephFS security label. CephFS's implementation uses
> dentry_init_security() to get security context before inode is created,
> then sends open/mkdir/mknod request to MDS, together with security xattr
> "security.<security module name>"

Can you describe how your approach compares to the NFSv4 labeling support, and why it requires
exporting this information from this hook when NFSv4 did not?

> 
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/nfs/nfs4proc.c         | 3 ++-
>  include/linux/lsm_hooks.h | 4 ++--
>  include/linux/security.h  | 9 +++++----
>  security/security.c       | 7 ++++---
>  security/selinux/hooks.c  | 8 ++++++--
>  5 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6dd146885da9..d18a5fb7aec3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>  		return NULL;
>  
>  	err = security_dentry_init_security(dentry, sattr->ia_mode,
> -				&dentry->d_name, (void **)&label->label, &label->len);
> +				&dentry->d_name,  NULL,
> +				(void **)&label->label, &label->len);
>  	if (err == 0)
>  		return label;
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8f1131c8dd54..e176c2032bdc 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1476,8 +1476,8 @@ union security_list_options {
>  					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,
> -					u32 *ctxlen);
> +					const struct qstr *name, const char **label,

Seems like "label" could be confusing given that it means something else in the NFSv4 code,
and what is actually being provided here is the xattr name suffix.

> +					void **ctx, u32 *ctxlen);
>  	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 63030c85ee19..df2d73998c64 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>  				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,
> -					u32 *ctxlen);
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen);
>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>  static inline int security_dentry_init_security(struct dentry *dentry,
>  						 int mode,
>  						 const struct qstr *name,
> -						 void **ctx,
> -						 u32 *ctxlen)
> +						 const char **label,
> +						 void **ctx, u32 *ctxlen)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/security/security.c b/security/security.c
> index 68f46d849abe..69818d46aa28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>  }
>  
>  int security_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> -				name, ctx, ctxlen);
> +				name, label, ctx, ctxlen);
>  }
>  EXPORT_SYMBOL(security_dentry_init_security);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..eca3879d9357 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>  }
>  
>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	u32 newsid;
>  	int rc;
> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>  	if (rc)
>  		return rc;
>  
> +	if (label)
> +		*label = XATTR_SELINUX_SUFFIX;
> +
>  	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>  				       ctxlen);
>  }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-06-26 13:28 ` [PATCH 1/3] selinux: make dentry_init_security() return security module name Stephen Smalley
@ 2018-06-26 15:32   ` Yan, Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Yan, Zheng @ 2018-06-26 15:32 UTC (permalink / raw)
  To: linux-security-module



> On Jun 26, 2018, at 21:28, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> 
> On 06/26/2018 04:43 AM, Yan, Zheng wrote:
>> This is preparation for CephFS security label. CephFS's implementation uses
>> dentry_init_security() to get security context before inode is created,
>> then sends open/mkdir/mknod request to MDS, together with security xattr
>> "security.<security module name>"
> 
> Can you describe how your approach compares to the NFSv4 labeling support, and why it requires
> exporting this information from this hook when NFSv4 did not?

NFS client only support single security label, it passes unnamed security context to NFSD. NFSD
stores the security context by calling security_inode_setsecctx().  For selinux, the security context
is stored in xattr ?security.selinux?. 

CephFS does not exports other filesystems. So It's irrelevant to CephFS which security module is
enabled on the host that runs ceph-mds (counterpart of NFSD) 

Regards
Yan, Zheng

>> 
>> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
>> ---
>> fs/nfs/nfs4proc.c         | 3 ++-
>> include/linux/lsm_hooks.h | 4 ++--
>> include/linux/security.h  | 9 +++++----
>> security/security.c       | 7 ++++---
>> security/selinux/hooks.c  | 8 ++++++--
>> 5 files changed, 19 insertions(+), 12 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 6dd146885da9..d18a5fb7aec3 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>> 		return NULL;
>> 
>> 	err = security_dentry_init_security(dentry, sattr->ia_mode,
>> -				&dentry->d_name, (void **)&label->label, &label->len);
>> +				&dentry->d_name,  NULL,
>> +				(void **)&label->label, &label->len);
>> 	if (err == 0)
>> 		return label;
>> 
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 8f1131c8dd54..e176c2032bdc 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1476,8 +1476,8 @@ union security_list_options {
>> 					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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name, const char **label,
> 
> Seems like "label" could be confusing given that it means something else in the NFSv4 code,
> and what is actually being provided here is the xattr name suffix.
> 
>> +					void **ctx, u32 *ctxlen);
>> 	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>> 					struct qstr *name,
>> 					const struct cred *old,
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 63030c85ee19..df2d73998c64 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>> 				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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen);
>> int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> 					struct qstr *name,
>> 					const struct cred *old,
>> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>> static inline int security_dentry_init_security(struct dentry *dentry,
>> 						 int mode,
>> 						 const struct qstr *name,
>> -						 void **ctx,
>> -						 u32 *ctxlen)
>> +						 const char **label,
>> +						 void **ctx, u32 *ctxlen)
>> {
>> 	return -EOPNOTSUPP;
>> }
>> diff --git a/security/security.c b/security/security.c
>> index 68f46d849abe..69818d46aa28 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>> }
>> 
>> int security_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>> {
>> 	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
>> -				name, ctx, ctxlen);
>> +				name, label, ctx, ctxlen);
>> }
>> EXPORT_SYMBOL(security_dentry_init_security);
>> 
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2b5ee5fbd652..eca3879d9357 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>> }
>> 
>> static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>> {
>> 	u32 newsid;
>> 	int rc;
>> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>> 	if (rc)
>> 		return rc;
>> 
>> +	if (label)
>> +		*label = XATTR_SELINUX_SUFFIX;
>> +
>> 	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>> 				       ctxlen);
>> }
>> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
                   ` (2 preceding siblings ...)
  2018-06-26 13:28 ` [PATCH 1/3] selinux: make dentry_init_security() return security module name Stephen Smalley
@ 2018-06-26 16:21 ` Casey Schaufler
  2018-06-27  1:46   ` Yan, Zheng
  2018-09-06 15:08 ` Jeff Layton
  4 siblings, 1 reply; 18+ messages in thread
From: Casey Schaufler @ 2018-06-26 16:21 UTC (permalink / raw)
  To: linux-security-module

On 6/26/2018 1:43 AM, Yan, Zheng wrote:
> This is preparation for CephFS security label. CephFS's implementation uses
> dentry_init_security() to get security context before inode is created,
> then sends open/mkdir/mknod request to MDS, together with security xattr
> "security.<security module name>"

There is no requirement that a security module use
"security.<security module name>" to name it's attributes.
Smack, for example, uses "security.SMACK64". Further, Smack
uses multiple attributes; "security.SMACK64TRANSMUTE",
"security.SMACK64MMAP", "security.SMACK64EXEC" as well as
"security.SMACK64".

Also, when (if) we get full module stacking in place you
will need to handle the possibility that there may be more
than one security module providing security xattrs.

>
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/nfs/nfs4proc.c         | 3 ++-
>  include/linux/lsm_hooks.h | 4 ++--
>  include/linux/security.h  | 9 +++++----
>  security/security.c       | 7 ++++---
>  security/selinux/hooks.c  | 8 ++++++--
>  5 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6dd146885da9..d18a5fb7aec3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>  		return NULL;
>  
>  	err = security_dentry_init_security(dentry, sattr->ia_mode,
> -				&dentry->d_name, (void **)&label->label, &label->len);
> +				&dentry->d_name,  NULL,
> +				(void **)&label->label, &label->len);
>  	if (err == 0)
>  		return label;
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8f1131c8dd54..e176c2032bdc 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1476,8 +1476,8 @@ union security_list_options {
>  					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,
> -					u32 *ctxlen);
> +					const struct qstr *name, const char **label,
> +					void **ctx, u32 *ctxlen);
>  	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 63030c85ee19..df2d73998c64 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>  				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,
> -					u32 *ctxlen);
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen);
>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>  static inline int security_dentry_init_security(struct dentry *dentry,
>  						 int mode,
>  						 const struct qstr *name,
> -						 void **ctx,
> -						 u32 *ctxlen)
> +						 const char **label,
> +						 void **ctx, u32 *ctxlen)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/security/security.c b/security/security.c
> index 68f46d849abe..69818d46aa28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>  }
>  
>  int security_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> -				name, ctx, ctxlen);
> +				name, label, ctx, ctxlen);
>  }
>  EXPORT_SYMBOL(security_dentry_init_security);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..eca3879d9357 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>  }
>  
>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	u32 newsid;
>  	int rc;
> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>  	if (rc)
>  		return rc;
>  
> +	if (label)
> +		*label = XATTR_SELINUX_SUFFIX;
> +
>  	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>  				       ctxlen);
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-06-26 16:21 ` Casey Schaufler
@ 2018-06-27  1:46   ` Yan, Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Yan, Zheng @ 2018-06-27  1:46 UTC (permalink / raw)
  To: linux-security-module



> On Jun 27, 2018, at 00:21, Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> On 6/26/2018 1:43 AM, Yan, Zheng wrote:
>> This is preparation for CephFS security label. CephFS's implementation uses
>> dentry_init_security() to get security context before inode is created,
>> then sends open/mkdir/mknod request to MDS, together with security xattr
>> "security.<security module name>"
> 
> There is no requirement that a security module use
> "security.<security module name>" to name it's attributes.
> Smack, for example, uses "security.SMACK64?.

This does not conflict with this patch. If smack implements dentry_init_security(), if can return whatever it likes.

> Further, Smack uses multiple attributes; "security.SMACK64TRANSMUTE",
> "security.SMACK64MMAP", "security.SMACK64EXEC" as well as
> "security.SMACK64?.
> 
> Also, when (if) we get full module stacking in place you
> will need to handle the possibility that there may be more
> than one security module providing security xattrs.
> 

We can introduce another type of ?dentry_init_security?, which return a xattr array. 

All cephfs wants is a function to get security xattrs before inode is created. It does not matter if the function return one xattr or multiple xattrs.

Regards
Yan, Zheng




>> 
>> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
>> ---
>> fs/nfs/nfs4proc.c         | 3 ++-
>> include/linux/lsm_hooks.h | 4 ++--
>> include/linux/security.h  | 9 +++++----
>> security/security.c       | 7 ++++---
>> security/selinux/hooks.c  | 8 ++++++--
>> 5 files changed, 19 insertions(+), 12 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 6dd146885da9..d18a5fb7aec3 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>> 		return NULL;
>> 
>> 	err = security_dentry_init_security(dentry, sattr->ia_mode,
>> -				&dentry->d_name, (void **)&label->label, &label->len);
>> +				&dentry->d_name,  NULL,
>> +				(void **)&label->label, &label->len);
>> 	if (err == 0)
>> 		return label;
>> 
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 8f1131c8dd54..e176c2032bdc 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1476,8 +1476,8 @@ union security_list_options {
>> 					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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name, const char **label,
>> +					void **ctx, u32 *ctxlen);
>> 	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>> 					struct qstr *name,
>> 					const struct cred *old,
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 63030c85ee19..df2d73998c64 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>> 				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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen);
>> int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> 					struct qstr *name,
>> 					const struct cred *old,
>> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>> static inline int security_dentry_init_security(struct dentry *dentry,
>> 						 int mode,
>> 						 const struct qstr *name,
>> -						 void **ctx,
>> -						 u32 *ctxlen)
>> +						 const char **label,
>> +						 void **ctx, u32 *ctxlen)
>> {
>> 	return -EOPNOTSUPP;
>> }
>> diff --git a/security/security.c b/security/security.c
>> index 68f46d849abe..69818d46aa28 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>> }
>> 
>> int security_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>> {
>> 	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
>> -				name, ctx, ctxlen);
>> +				name, label, ctx, ctxlen);
>> }
>> EXPORT_SYMBOL(security_dentry_init_security);
>> 
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2b5ee5fbd652..eca3879d9357 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>> }
>> 
>> static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>> {
>> 	u32 newsid;
>> 	int rc;
>> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>> 	if (rc)
>> 		return rc;
>> 
>> +	if (label)
>> +		*label = XATTR_SELINUX_SUFFIX;
>> +
>> 	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>> 				       ctxlen);
>> }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
                   ` (3 preceding siblings ...)
  2018-06-26 16:21 ` Casey Schaufler
@ 2018-09-06 15:08 ` Jeff Layton
  2018-09-06 15:39   ` Casey Schaufler
  4 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2018-09-06 15:08 UTC (permalink / raw)
  To: linux-security-module

On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> This is preparation for CephFS security label. CephFS's implementation uses
> dentry_init_security() to get security context before inode is created,
> then sends open/mkdir/mknod request to MDS, together with security xattr
> "security.<security module name>"
> 
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/nfs/nfs4proc.c         | 3 ++-
>  include/linux/lsm_hooks.h | 4 ++--
>  include/linux/security.h  | 9 +++++----
>  security/security.c       | 7 ++++---
>  security/selinux/hooks.c  | 8 ++++++--
>  5 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6dd146885da9..d18a5fb7aec3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>  		return NULL;
>  
>  	err = security_dentry_init_security(dentry, sattr->ia_mode,
> -				&dentry->d_name, (void **)&label->label, &label->len);
> +				&dentry->d_name,  NULL,
> +				(void **)&label->label, &label->len);
>  	if (err == 0)
>  		return label;
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8f1131c8dd54..e176c2032bdc 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1476,8 +1476,8 @@ union security_list_options {
>  					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,
> -					u32 *ctxlen);
> +					const struct qstr *name, const char **label,
> +					void **ctx, u32 *ctxlen);
>  	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 63030c85ee19..df2d73998c64 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>  				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,
> -					u32 *ctxlen);
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen);
>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
>  					struct qstr *name,
>  					const struct cred *old,
> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>  static inline int security_dentry_init_security(struct dentry *dentry,
>  						 int mode,
>  						 const struct qstr *name,
> -						 void **ctx,
> -						 u32 *ctxlen)
> +						 const char **label,
> +						 void **ctx, u32 *ctxlen)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/security/security.c b/security/security.c
> index 68f46d849abe..69818d46aa28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>  }
>  
>  int security_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> -				name, ctx, ctxlen);
> +				name, label, ctx, ctxlen);
>  }
>  EXPORT_SYMBOL(security_dentry_init_security);
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..eca3879d9357 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>  }
>  
>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> -					const struct qstr *name, void **ctx,
> -					u32 *ctxlen)
> +					const struct qstr *name,
> +					const char **label,
> +					void **ctx, u32 *ctxlen)
>  {
>  	u32 newsid;
>  	int rc;
> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>  	if (rc)
>  		return rc;
>  
> +	if (label)
> +		*label = XATTR_SELINUX_SUFFIX;
> +

nit: I'd probably call this "name" since that's what it's called in
selinux_inode_init_security. "label" has a different connotation in this
context.

>  	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>  				       ctxlen);
>  }

Patch looks reasonable to me though.

Acked-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx
  2018-06-26  8:43 ` [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx Yan, Zheng
@ 2018-09-06 15:14   ` Jeff Layton
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2018-09-06 15:14 UTC (permalink / raw)
  To: linux-security-module

On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> this is preparation for security label support
> 
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/ceph/acl.c   | 22 +++++++---------------
>  fs/ceph/dir.c   | 28 ++++++++++++++--------------
>  fs/ceph/file.c  | 18 +++++++++---------
>  fs/ceph/super.h | 29 +++++++++++++++--------------
>  fs/ceph/xattr.c | 10 ++++++++++
>  5 files changed, 55 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> index 3351ea14390b..f13ba4250f00 100644
> --- a/fs/ceph/acl.c
> +++ b/fs/ceph/acl.c
> @@ -172,7 +172,7 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  }
>  
>  int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
> -		       struct ceph_acls_info *info)
> +		       struct ceph_acl_sec_ctx *as_ctx)
>  {
>  	struct posix_acl *acl, *default_acl;
>  	size_t val_size1 = 0, val_size2 = 0;
> @@ -248,9 +248,9 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
>  
>  	kfree(tmp_buf);
>  
> -	info->acl = acl;
> -	info->default_acl = default_acl;
> -	info->pagelist = pagelist;
> +	as_ctx->acl = acl;
> +	as_ctx->default_acl = default_acl;
> +	as_ctx->pagelist = pagelist;
>  	return 0;
>  
>  out_err:
> @@ -262,18 +262,10 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
>  	return err;
>  }
>  
> -void ceph_init_inode_acls(struct inode* inode, struct ceph_acls_info *info)
> +void ceph_init_inode_acls(struct inode* inode, struct ceph_acl_sec_ctx *as_ctx)
>  {
>  	if (!inode)
>  		return;
> -	ceph_set_cached_acl(inode, ACL_TYPE_ACCESS, info->acl);
> -	ceph_set_cached_acl(inode, ACL_TYPE_DEFAULT, info->default_acl);
> -}
> -
> -void ceph_release_acls_info(struct ceph_acls_info *info)
> -{
> -	posix_acl_release(info->acl);
> -	posix_acl_release(info->default_acl);
> -	if (info->pagelist)
> -		ceph_pagelist_release(info->pagelist);
> +	ceph_set_cached_acl(inode, ACL_TYPE_ACCESS, as_ctx->acl);
> +	ceph_set_cached_acl(inode, ACL_TYPE_DEFAULT, as_ctx->default_acl);
>  }
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 036ac0f3a393..f451ad5a37ab 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -821,7 +821,7 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
>  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
>  	struct ceph_mds_request *req;
> -	struct ceph_acls_info acls = {};
> +	struct ceph_acl_sec_ctx as_ctx = {};
>  	int err;
>  
>  	if (ceph_snap(dir) != CEPH_NOSNAP)
> @@ -830,7 +830,7 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
>  	if (ceph_quota_is_max_files_exceeded(dir))
>  		return -EDQUOT;
>  
> -	err = ceph_pre_init_acls(dir, &mode, &acls);
> +	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  	if (err < 0)
>  		return err;
>  
> @@ -849,9 +849,9 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
>  	req->r_args.mknod.rdev = cpu_to_le32(rdev);
>  	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
>  	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
> -	if (acls.pagelist) {
> -		req->r_pagelist = acls.pagelist;
> -		acls.pagelist = NULL;
> +	if (as_ctx.pagelist) {
> +		req->r_pagelist = as_ctx.pagelist;
> +		as_ctx.pagelist = NULL;
>  	}
>  	err = ceph_mdsc_do_request(mdsc, dir, req);
>  	if (!err && !req->r_reply_info.head->is_dentry)
> @@ -859,10 +859,10 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
>  	ceph_mdsc_put_request(req);
>  out:
>  	if (!err)
> -		ceph_init_inode_acls(d_inode(dentry), &acls);
> +		ceph_init_inode_acls(d_inode(dentry), &as_ctx);
>  	else
>  		d_drop(dentry);
> -	ceph_release_acls_info(&acls);
> +	ceph_release_acl_sec_ctx(&as_ctx);
>  	return err;
>  }
>  
> @@ -919,7 +919,7 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
>  	struct ceph_mds_request *req;
> -	struct ceph_acls_info acls = {};
> +	struct ceph_acl_sec_ctx as_ctx = {};
>  	int err = -EROFS;
>  	int op;
>  
> @@ -942,7 +942,7 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  	}
>  
>  	mode |= S_IFDIR;
> -	err = ceph_pre_init_acls(dir, &mode, &acls);
> +	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  	if (err < 0)
>  		goto out;
>  
> @@ -959,9 +959,9 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  	req->r_args.mkdir.mode = cpu_to_le32(mode);
>  	req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
>  	req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
> -	if (acls.pagelist) {
> -		req->r_pagelist = acls.pagelist;
> -		acls.pagelist = NULL;
> +	if (as_ctx.pagelist) {
> +		req->r_pagelist = as_ctx.pagelist;
> +		as_ctx.pagelist = NULL;
>  	}
>  	err = ceph_mdsc_do_request(mdsc, dir, req);
>  	if (!err &&
> @@ -971,10 +971,10 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  	ceph_mdsc_put_request(req);
>  out:
>  	if (!err)
> -		ceph_init_inode_acls(d_inode(dentry), &acls);
> +		ceph_init_inode_acls(d_inode(dentry), &as_ctx);
>  	else
>  		d_drop(dentry);
> -	ceph_release_acls_info(&acls);
> +	ceph_release_acl_sec_ctx(&as_ctx);
>  	return err;
>  }
>  
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index ad0bed99b1d5..701506ec5768 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -436,7 +436,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
>  	struct ceph_mds_request *req;
>  	struct dentry *dn;
> -	struct ceph_acls_info acls = {};
> +	struct ceph_acl_sec_ctx as_ctx = {};
>  	int mask;
>  	int err;
>  
> @@ -450,7 +450,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	if (flags & O_CREAT) {
>  		if (ceph_quota_is_max_files_exceeded(dir))
>  			return -EDQUOT;
> -		err = ceph_pre_init_acls(dir, &mode, &acls);
> +		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  		if (err < 0)
>  			return err;
>  	}
> @@ -459,16 +459,16 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	req = prepare_open_request(dir->i_sb, flags, mode);
>  	if (IS_ERR(req)) {
>  		err = PTR_ERR(req);
> -		goto out_acl;
> +		goto out_ctx;
>  	}
>  	req->r_dentry = dget(dentry);
>  	req->r_num_caps = 2;
>  	if (flags & O_CREAT) {
>  		req->r_dentry_drop = CEPH_CAP_FILE_SHARED | CEPH_CAP_AUTH_EXCL;
>  		req->r_dentry_unless = CEPH_CAP_FILE_EXCL;
> -		if (acls.pagelist) {
> -			req->r_pagelist = acls.pagelist;
> -			acls.pagelist = NULL;
> +		if (as_ctx.pagelist) {
> +			req->r_pagelist = as_ctx.pagelist;
> +			as_ctx.pagelist = NULL;
>  		}
>  	}
>  
> @@ -506,7 +506,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	} else {
>  		dout("atomic_open finish_open on dn %p\n", dn);
>  		if (req->r_op == CEPH_MDS_OP_CREATE && req->r_reply_info.has_create_ino) {
> -			ceph_init_inode_acls(d_inode(dentry), &acls);
> +			ceph_init_inode_acls(d_inode(dentry), &as_ctx);
>  			*opened |= FILE_CREATED;
>  		}
>  		err = finish_open(file, dentry, ceph_open, opened);
> @@ -515,8 +515,8 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	if (!req->r_err && req->r_target_inode)
>  		ceph_put_fmode(ceph_inode(req->r_target_inode), req->r_fmode);
>  	ceph_mdsc_put_request(req);
> -out_acl:
> -	ceph_release_acls_info(&acls);
> +out_ctx:
> +	ceph_release_acl_sec_ctx(&as_ctx);
>  	dout("atomic_open result=%d\n", err);
>  	return err;
>  }
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 5df5262b24b0..83561421afda 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -906,6 +906,14 @@ extern void __init ceph_xattr_init(void);
>  extern void ceph_xattr_exit(void);
>  extern const struct xattr_handler *ceph_xattr_handlers[];
>  
> +struct ceph_acl_sec_ctx {
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	void *default_acl;
> +	void *acl;
> +#endif
> +	struct ceph_pagelist *pagelist;
> +};
> +
>  #ifdef CONFIG_SECURITY
>  extern bool ceph_security_xattr_deadlock(struct inode *in);
>  extern bool ceph_security_xattr_wanted(struct inode *in);
> @@ -920,21 +928,17 @@ static inline bool ceph_security_xattr_wanted(struct inode *in)
>  }
>  #endif
>  
> -/* acl.c */
> -struct ceph_acls_info {
> -	void *default_acl;
> -	void *acl;
> -	struct ceph_pagelist *pagelist;
> -};
> +void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx);
>  
> +/* acl.c */
>  #ifdef CONFIG_CEPH_FS_POSIX_ACL
>  
>  struct posix_acl *ceph_get_acl(struct inode *, int);
>  int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
> -		       struct ceph_acls_info *info);
> -void ceph_init_inode_acls(struct inode *inode, struct ceph_acls_info *info);
> -void ceph_release_acls_info(struct ceph_acls_info *info);
> +		       struct ceph_acl_sec_ctx *as_ctx);
> +void ceph_init_inode_acls(struct inode *inode,
> +			  struct ceph_acl_sec_ctx *as_ctx);
>  
>  static inline void ceph_forget_all_cached_acls(struct inode *inode)
>  {
> @@ -947,15 +951,12 @@ static inline void ceph_forget_all_cached_acls(struct inode *inode)
>  #define ceph_set_acl NULL
>  
>  static inline int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
> -				     struct ceph_acls_info *info)
> +				     struct ceph_acl_sec_ctx *as_ctx)
>  {
>  	return 0;
>  }
>  static inline void ceph_init_inode_acls(struct inode *inode,
> -					struct ceph_acls_info *info)
> -{
> -}
> -static inline void ceph_release_acls_info(struct ceph_acls_info *info)
> +					struct ceph_acl_sec_ctx *as_ctx)
>  {
>  }
>  static inline int ceph_acl_chmod(struct dentry *dentry, struct inode *inode)
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 5bc8edb4c2a6..ef0e968d56a1 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -1190,3 +1190,13 @@ bool ceph_security_xattr_deadlock(struct inode *in)
>  	return ret;
>  }
>  #endif
> +
> +void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
> +{
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	posix_acl_release(as_ctx->acl);
> +	posix_acl_release(as_ctx->default_acl);
> +#endif
> +	if (as_ctx->pagelist)
> +		ceph_pagelist_release(as_ctx->pagelist);
> +}

Straightforward enough.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-06 15:08 ` Jeff Layton
@ 2018-09-06 15:39   ` Casey Schaufler
  2018-09-07  8:31     ` Yan, Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Casey Schaufler @ 2018-09-06 15:39 UTC (permalink / raw)
  To: linux-security-module

On 9/6/2018 8:08 AM, Jeff Layton wrote:
> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
>> This is preparation for CephFS security label. CephFS's implementation uses
>> dentry_init_security() to get security context before inode is created,
>> then sends open/mkdir/mknod request to MDS, together with security xattr
>> "security.<security module name>"

Please excuse my late entry into this review.

First, *Do not prefix general LSM interface work with "selinux:"*
You should only use that prefix when you're dealing with the
internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
If you are changing security.h or lsm_hooks.h you are almost
certainly changing LSM interfaces.

Because you prefixed this work with "selinux:" I pretty much
ignored it. Please don't do that again.

Why aren't you using security_inode_getsecctx, like kernfs and nfs?


>>
>> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
>> ---
>>  fs/nfs/nfs4proc.c         | 3 ++-
>>  include/linux/lsm_hooks.h | 4 ++--
>>  include/linux/security.h  | 9 +++++----
>>  security/security.c       | 7 ++++---
>>  security/selinux/hooks.c  | 8 ++++++--
>>  5 files changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 6dd146885da9..d18a5fb7aec3 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>>  		return NULL;
>>  
>>  	err = security_dentry_init_security(dentry, sattr->ia_mode,
>> -				&dentry->d_name, (void **)&label->label, &label->len);
>> +				&dentry->d_name,  NULL,
>> +				(void **)&label->label, &label->len);
>>  	if (err == 0)
>>  		return label;
>>  
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 8f1131c8dd54..e176c2032bdc 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1476,8 +1476,8 @@ union security_list_options {
>>  					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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name, const char **label,
>> +					void **ctx, u32 *ctxlen);
>>  	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>>  					struct qstr *name,
>>  					const struct cred *old,
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 63030c85ee19..df2d73998c64 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>>  				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,
>> -					u32 *ctxlen);
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen);
>>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
>>  					struct qstr *name,
>>  					const struct cred *old,
>> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>>  static inline int security_dentry_init_security(struct dentry *dentry,
>>  						 int mode,
>>  						 const struct qstr *name,
>> -						 void **ctx,
>> -						 u32 *ctxlen)
>> +						 const char **label,
>> +						 void **ctx, u32 *ctxlen)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> diff --git a/security/security.c b/security/security.c
>> index 68f46d849abe..69818d46aa28 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>>  }
>>  
>>  int security_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>>  {
>>  	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
>> -				name, ctx, ctxlen);
>> +				name, label, ctx, ctxlen);
>>  }
>>  EXPORT_SYMBOL(security_dentry_init_security);
>>  
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2b5ee5fbd652..eca3879d9357 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>>  }
>>  
>>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>> -					const struct qstr *name, void **ctx,
>> -					u32 *ctxlen)
>> +					const struct qstr *name,
>> +					const char **label,
>> +					void **ctx, u32 *ctxlen)
>>  {
>>  	u32 newsid;
>>  	int rc;
>> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>>  	if (rc)
>>  		return rc;
>>  
>> +	if (label)
>> +		*label = XATTR_SELINUX_SUFFIX;
>> +
> nit: I'd probably call this "name" since that's what it's called in
> selinux_inode_init_security. "label" has a different connotation in this
> context.
>
>>  	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>>  				       ctxlen);
>>  }
> Patch looks reasonable to me though.
>
> Acked-by: Jeff Layton <jlayton@kernel.org>
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/3] ceph: xattr security label support
  2018-06-26  8:43 ` [PATCH 3/3] ceph: xattr security label support Yan, Zheng
@ 2018-09-06 15:50   ` Jeff Layton
  2018-09-06 16:05     ` Jeff Layton
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2018-09-06 15:50 UTC (permalink / raw)
  To: linux-security-module

On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> When creating new file/directory, uses dentry_init_security() to prepare
> security context for the new inode, then sends openc/mkdir request to MDS,
> together with security xattr "security.<security module name>"
> 
> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/ceph/Kconfig |   5 ++
>  fs/ceph/caps.c  |   1 +
>  fs/ceph/dir.c   |  12 +++++
>  fs/ceph/file.c  |   3 ++
>  fs/ceph/inode.c |   1 +
>  fs/ceph/super.h |  19 +++++++
>  fs/ceph/xattr.c | 140 ++++++++++++++++++++++++++++++++++++++++++------
>  7 files changed, 164 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
> index 52095f473464..e1a4100c99eb 100644
> --- a/fs/ceph/Kconfig
> +++ b/fs/ceph/Kconfig
> @@ -35,3 +35,8 @@ config CEPH_FS_POSIX_ACL
>  	  groups beyond the owner/group/world scheme.
>  
>  	  If you don't know what Access Control Lists are, say N
> +
> +config CEPH_FS_SECURITY_LABEL
> +	bool
> +	depends on CEPH_FS && SECURITY
> +	default y


Some help text would be nice here, unless you intend to keep this option
hidden?

> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 990258cbd836..ec49a3858288 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3144,6 +3144,7 @@ static void handle_cap_grant(struct inode *inode,
>  			ci->i_xattrs.blob = ceph_buffer_get(xattr_buf);
>  			ci->i_xattrs.version = version;
>  			ceph_forget_all_cached_acls(inode);
> +			ceph_security_invalidate_secctx(inode);
>  		}
>  	}
>  
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index f451ad5a37ab..18ece4be4493 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -833,6 +833,9 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
>  	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  	if (err < 0)
>  		return err;
> +	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> +	if (err < 0)
> +	       goto out;
>  
>  	dout("mknod in dir %p dentry %p mode 0%ho rdev %d\n",
>  	     dir, dentry, mode, rdev);
> @@ -878,6 +881,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
>  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
>  	struct ceph_mds_request *req;
> +	struct ceph_acl_sec_ctx as_ctx = {};
>  	int err;
>  
>  	if (ceph_snap(dir) != CEPH_NOSNAP)
> @@ -886,6 +890,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
>  	if (ceph_quota_is_max_files_exceeded(dir))
>  		return -EDQUOT;
>  
> +	err = ceph_security_init_secctx(dentry, S_IFLNK | S_IRWXUGO, &as_ctx);
> +	if (err < 0)
> +	       goto out;
> +
>  	dout("symlink in dir %p dentry %p to '%s'\n", dir, dentry, dest);
>  	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SYMLINK, USE_AUTH_MDS);
>  	if (IS_ERR(req)) {
> @@ -911,6 +919,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
>  out:
>  	if (err)
>  		d_drop(dentry);
> +	ceph_release_acl_sec_ctx(&as_ctx);
>  	return err;
>  }
>  
> @@ -945,6 +954,9 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  	if (err < 0)
>  		goto out;
> +	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> +	if (err < 0)
> +	       goto out;
>  
>  	req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
>  	if (IS_ERR(req)) {
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 701506ec5768..0e835ca720cb 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -453,6 +453,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
>  		if (err < 0)
>  			return err;
> +		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> +		if (err < 0)
> +			goto out_ctx;
>  	}
>  
>  	/* do the open */
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 1fe3a02336b0..db0079fd5c06 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -896,6 +896,7 @@ static int fill_inode(struct inode *inode, struct page *locked_page,
>  			       iinfo->xattr_data, iinfo->xattr_len);
>  		ci->i_xattrs.version = le64_to_cpu(info->xattr_version);
>  		ceph_forget_all_cached_acls(inode);
> +		ceph_security_invalidate_secctx(inode);
>  		xattr_blob = NULL;
>  	}
>  
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 83561421afda..60151b8cd5c7 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -910,6 +910,10 @@ struct ceph_acl_sec_ctx {
>  #ifdef CONFIG_CEPH_FS_POSIX_ACL
>  	void *default_acl;
>  	void *acl;
> +#endif
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> +	void *sec_ctx;
> +	u32 sec_ctxlen;
>  #endif
>  	struct ceph_pagelist *pagelist;
>  };
> @@ -928,6 +932,21 @@ static inline bool ceph_security_xattr_wanted(struct inode *in)
>  }
>  #endif
>  
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> +extern int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> +				     struct ceph_acl_sec_ctx *ctx);
> +extern void ceph_security_invalidate_secctx(struct inode *inode);
> +#else
> +static inline int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> +					    struct ceph_acl_sec_ctx *ctx)
> +{
> +	return 0;
> +}
> +static inline void ceph_security_invalidate_secctx(struct inode *inode)
> +{
> +}
> +#endif
> +
>  void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx);
>  
>  /* acl.c */
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index ef0e968d56a1..3e87208cbde8 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -8,6 +8,7 @@
>  #include <linux/ceph/decode.h>
>  
>  #include <linux/xattr.h>
> +#include <linux/security.h>
>  #include <linux/posix_acl_xattr.h>
>  #include <linux/slab.h>
>  
> @@ -17,26 +18,9 @@
>  static int __remove_xattr(struct ceph_inode_info *ci,
>  			  struct ceph_inode_xattr *xattr);
>  
> -static const struct xattr_handler ceph_other_xattr_handler;
> -
> -/*
> - * List of handlers for synthetic system.* attributes. Other
> - * attributes are handled directly.
> - */
> -const struct xattr_handler *ceph_xattr_handlers[] = {
> -#ifdef CONFIG_CEPH_FS_POSIX_ACL
> -	&posix_acl_access_xattr_handler,
> -	&posix_acl_default_xattr_handler,
> -#endif
> -	&ceph_other_xattr_handler,
> -	NULL,
> -};
> -
>  static bool ceph_is_valid_xattr(const char *name)
>  {
>  	return !strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) ||
> -	       !strncmp(name, XATTR_SECURITY_PREFIX,
> -			XATTR_SECURITY_PREFIX_LEN) ||
>  	       !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) ||
>  	       !strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
>  }
> @@ -1189,6 +1173,109 @@ bool ceph_security_xattr_deadlock(struct inode *in)
>  	spin_unlock(&ci->i_ceph_lock);
>  	return ret;
>  }
> +
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> +int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> +			   struct ceph_acl_sec_ctx *as_ctx)
> +{
> +	struct ceph_pagelist *pagelist = as_ctx->pagelist;
> +	const char *label;
> +	size_t label_len;
> +	int err;
> +
> +	err = security_dentry_init_security(dentry, mode, &dentry->d_name,
> +					    &label, &as_ctx->sec_ctx,
> +					    &as_ctx->sec_ctxlen);
> +	if (err < 0) {
> +		err = 0; /* do nothing */
> +		goto out;
> +	}
> +
> +	err = -ENOMEM;
> +	if (!pagelist) {
> +		pagelist = kmalloc(sizeof(struct ceph_pagelist), GFP_KERNEL);
> +		if (!pagelist)
> +			goto out;
> +		ceph_pagelist_init(pagelist);
> +
> +		err = ceph_pagelist_reserve(pagelist, PAGE_SIZE);
> +		if (err)
> +			goto out;
> +		ceph_pagelist_encode_32(pagelist, 1);
> +	}
> +
> +	label_len = strlen(label);
> +	err = ceph_pagelist_reserve(pagelist, XATTR_SECURITY_PREFIX_LEN +
> +				    label_len + as_ctx->sec_ctxlen + 8);
> +	if (err)
> +		goto out;
> +
> +	if (as_ctx->pagelist) {
> +		/* update count of KV pairs */
> +		BUG_ON(pagelist->length <= sizeof(__le32));
> +		if (list_is_singular(&pagelist->head)) {
> +			le32_add_cpu((__le32*)pagelist->mapped_tail, 1);
> +		} else {
> +			struct page *page = list_first_entry(&pagelist->head,
> +							     struct page, lru);
> +			void *addr = kmap_atomic(page);
> +			le32_add_cpu((__le32*)addr, 1);
> +			kunmap_atomic(addr);
> +		}
> +	} else {
> +		as_ctx->pagelist = pagelist;
> +	}
> +
> +	ceph_pagelist_encode_32(pagelist,
> +				XATTR_SECURITY_PREFIX_LEN + label_len);
> +	ceph_pagelist_append(pagelist, XATTR_SECURITY_PREFIX,
> +			     XATTR_SECURITY_PREFIX_LEN);
> +	ceph_pagelist_append(pagelist, label, label_len);
> +
> +	ceph_pagelist_encode_32(pagelist, as_ctx->sec_ctxlen);
> +	ceph_pagelist_append(pagelist, as_ctx->sec_ctx, as_ctx->sec_ctxlen);
> +
> +	err = 0;
> +out:
> +	if (pagelist && !as_ctx->pagelist)
> +		ceph_pagelist_release(pagelist);
> +	return err;
> +}
> +
> +void ceph_security_invalidate_secctx(struct inode *inode)
> +{
> +	security_inode_invalidate_secctx(inode);
> +}
> +
> +static int ceph_xattr_set_security_label(const struct xattr_handler *handler,
> +				    struct dentry *unused, struct inode *inode,
> +				    const char *key, const void *buf,
> +				    size_t buflen, int flags)
> +{
> +	if (security_ismaclabel(key)) {
> +		const char *name = xattr_full_name(handler, key);
> +		return __ceph_setxattr(inode, name, buf, buflen, flags);
> +	}
> +	return  -EOPNOTSUPP;
> +}
> +
> +static int ceph_xattr_get_security_label(const struct xattr_handler *handler,
> +				    struct dentry *unused, struct inode *inode,
> +				    const char *key, void *buf, size_t buflen)
> +{
> +        if (security_ismaclabel(key)) {

nit: there may be some whitespace damage above.

> +		const char *name = xattr_full_name(handler, key);
> +		return __ceph_getxattr(inode, name, buf, buflen);
> +	}
> +	return  -EOPNOTSUPP;
> +}


What I'm a little unclear on in all of this is what happens when you
have an existing cephfs with no security labels, and one client boots to
a kernel with this support. This is unlike the situation with most local
filesystems, as we can't really expect to do a full relabel when
enabling this on an existing fs.

This function just returns -EOPNOTSUPP, but inode_doinit_with_dentry
seems like it might expect -ENODATA in this situation. Is this the
correct way to handle this?


> +static const struct xattr_handler ceph_security_label_handler = {
> +        .prefix = XATTR_SECURITY_PREFIX,
> +        .get    = ceph_xattr_get_security_label,
> +        .set    = ceph_xattr_set_security_label,
> +};
> +#endif
>  #endif
>  
>  void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
> @@ -1196,7 +1283,26 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
>  #ifdef CONFIG_CEPH_FS_POSIX_ACL
>  	posix_acl_release(as_ctx->acl);
>  	posix_acl_release(as_ctx->default_acl);
> +#endif
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> +	security_release_secctx(as_ctx->sec_ctx, as_ctx->sec_ctxlen);
>  #endif
>  	if (as_ctx->pagelist)
>  		ceph_pagelist_release(as_ctx->pagelist);
>  }
> +
> +/*
> + * List of handlers for synthetic system.* attributes. Other
> + * attributes are handled directly.
> + */
> +const struct xattr_handler *ceph_xattr_handlers[] = {
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
> +#endif
> +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> +	&ceph_security_label_handler,
> +#endif
> +	&ceph_other_xattr_handler,
> +	NULL,
> +};



-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/3] ceph: xattr security label support
  2018-09-06 15:50   ` Jeff Layton
@ 2018-09-06 16:05     ` Jeff Layton
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2018-09-06 16:05 UTC (permalink / raw)
  To: linux-security-module

On Thu, 2018-09-06 at 11:50 -0400, Jeff Layton wrote:
> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> > When creating new file/directory, uses dentry_init_security() to prepare
> > security context for the new inode, then sends openc/mkdir request to MDS,
> > together with security xattr "security.<security module name>"
> > 
> > Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> > ---
> >  fs/ceph/Kconfig |   5 ++
> >  fs/ceph/caps.c  |   1 +
> >  fs/ceph/dir.c   |  12 +++++
> >  fs/ceph/file.c  |   3 ++
> >  fs/ceph/inode.c |   1 +
> >  fs/ceph/super.h |  19 +++++++
> >  fs/ceph/xattr.c | 140 ++++++++++++++++++++++++++++++++++++++++++------
> >  7 files changed, 164 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
> > index 52095f473464..e1a4100c99eb 100644
> > --- a/fs/ceph/Kconfig
> > +++ b/fs/ceph/Kconfig
> > @@ -35,3 +35,8 @@ config CEPH_FS_POSIX_ACL
> >  	  groups beyond the owner/group/world scheme.
> >  
> >  	  If you don't know what Access Control Lists are, say N
> > +
> > +config CEPH_FS_SECURITY_LABEL
> > +	bool
> > +	depends on CEPH_FS && SECURITY
> > +	default y
> 
> 
> Some help text would be nice here, unless you intend to keep this option
> hidden?
> 
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index 990258cbd836..ec49a3858288 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -3144,6 +3144,7 @@ static void handle_cap_grant(struct inode *inode,
> >  			ci->i_xattrs.blob = ceph_buffer_get(xattr_buf);
> >  			ci->i_xattrs.version = version;
> >  			ceph_forget_all_cached_acls(inode);
> > +			ceph_security_invalidate_secctx(inode);
> >  		}
> >  	}
> >  
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index f451ad5a37ab..18ece4be4493 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -833,6 +833,9 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
> >  	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
> >  	if (err < 0)
> >  		return err;
> > +	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> > +	if (err < 0)
> > +	       goto out;
> >  
> >  	dout("mknod in dir %p dentry %p mode 0%ho rdev %d\n",
> >  	     dir, dentry, mode, rdev);
> > @@ -878,6 +881,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
> >  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
> >  	struct ceph_mds_client *mdsc = fsc->mdsc;
> >  	struct ceph_mds_request *req;
> > +	struct ceph_acl_sec_ctx as_ctx = {};
> >  	int err;
> >  
> >  	if (ceph_snap(dir) != CEPH_NOSNAP)
> > @@ -886,6 +890,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
> >  	if (ceph_quota_is_max_files_exceeded(dir))
> >  		return -EDQUOT;
> >  
> > +	err = ceph_security_init_secctx(dentry, S_IFLNK | S_IRWXUGO, &as_ctx);
> > +	if (err < 0)
> > +	       goto out;
> > +
> >  	dout("symlink in dir %p dentry %p to '%s'\n", dir, dentry, dest);
> >  	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SYMLINK, USE_AUTH_MDS);
> >  	if (IS_ERR(req)) {
> > @@ -911,6 +919,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
> >  out:
> >  	if (err)
> >  		d_drop(dentry);
> > +	ceph_release_acl_sec_ctx(&as_ctx);
> >  	return err;
> >  }
> >  
> > @@ -945,6 +954,9 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> >  	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
> >  	if (err < 0)
> >  		goto out;
> > +	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> > +	if (err < 0)
> > +	       goto out;
> >  
> >  	req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
> >  	if (IS_ERR(req)) {
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index 701506ec5768..0e835ca720cb 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -453,6 +453,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >  		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
> >  		if (err < 0)
> >  			return err;
> > +		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> > +		if (err < 0)
> > +			goto out_ctx;
> >  	}
> >  
> >  	/* do the open */
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 1fe3a02336b0..db0079fd5c06 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -896,6 +896,7 @@ static int fill_inode(struct inode *inode, struct page *locked_page,
> >  			       iinfo->xattr_data, iinfo->xattr_len);
> >  		ci->i_xattrs.version = le64_to_cpu(info->xattr_version);
> >  		ceph_forget_all_cached_acls(inode);
> > +		ceph_security_invalidate_secctx(inode);
> >  		xattr_blob = NULL;
> >  	}
> >  
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index 83561421afda..60151b8cd5c7 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -910,6 +910,10 @@ struct ceph_acl_sec_ctx {
> >  #ifdef CONFIG_CEPH_FS_POSIX_ACL
> >  	void *default_acl;
> >  	void *acl;
> > +#endif
> > +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> > +	void *sec_ctx;
> > +	u32 sec_ctxlen;
> >  #endif
> >  	struct ceph_pagelist *pagelist;
> >  };
> > @@ -928,6 +932,21 @@ static inline bool ceph_security_xattr_wanted(struct inode *in)
> >  }
> >  #endif
> >  
> > +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> > +extern int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> > +				     struct ceph_acl_sec_ctx *ctx);
> > +extern void ceph_security_invalidate_secctx(struct inode *inode);
> > +#else
> > +static inline int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> > +					    struct ceph_acl_sec_ctx *ctx)
> > +{
> > +	return 0;
> > +}
> > +static inline void ceph_security_invalidate_secctx(struct inode *inode)
> > +{
> > +}
> > +#endif
> > +
> >  void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx);
> >  
> >  /* acl.c */
> > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> > index ef0e968d56a1..3e87208cbde8 100644
> > --- a/fs/ceph/xattr.c
> > +++ b/fs/ceph/xattr.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/ceph/decode.h>
> >  
> >  #include <linux/xattr.h>
> > +#include <linux/security.h>
> >  #include <linux/posix_acl_xattr.h>
> >  #include <linux/slab.h>
> >  
> > @@ -17,26 +18,9 @@
> >  static int __remove_xattr(struct ceph_inode_info *ci,
> >  			  struct ceph_inode_xattr *xattr);
> >  
> > -static const struct xattr_handler ceph_other_xattr_handler;
> > -
> > -/*
> > - * List of handlers for synthetic system.* attributes. Other
> > - * attributes are handled directly.
> > - */
> > -const struct xattr_handler *ceph_xattr_handlers[] = {
> > -#ifdef CONFIG_CEPH_FS_POSIX_ACL
> > -	&posix_acl_access_xattr_handler,
> > -	&posix_acl_default_xattr_handler,
> > -#endif
> > -	&ceph_other_xattr_handler,
> > -	NULL,
> > -};
> > -
> >  static bool ceph_is_valid_xattr(const char *name)
> >  {
> >  	return !strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) ||
> > -	       !strncmp(name, XATTR_SECURITY_PREFIX,
> > -			XATTR_SECURITY_PREFIX_LEN) ||
> >  	       !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) ||
> >  	       !strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
> >  }
> > @@ -1189,6 +1173,109 @@ bool ceph_security_xattr_deadlock(struct inode *in)
> >  	spin_unlock(&ci->i_ceph_lock);
> >  	return ret;
> >  }
> > +
> > +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> > +int ceph_security_init_secctx(struct dentry *dentry, umode_t mode,
> > +			   struct ceph_acl_sec_ctx *as_ctx)
> > +{
> > +	struct ceph_pagelist *pagelist = as_ctx->pagelist;
> > +	const char *label;
> > +	size_t label_len;
> > +	int err;
> > +
> > +	err = security_dentry_init_security(dentry, mode, &dentry->d_name,
> > +					    &label, &as_ctx->sec_ctx,
> > +					    &as_ctx->sec_ctxlen);
> > +	if (err < 0) {
> > +		err = 0; /* do nothing */
> > +		goto out;
> > +	}
> > +
> > +	err = -ENOMEM;
> > +	if (!pagelist) {
> > +		pagelist = kmalloc(sizeof(struct ceph_pagelist), GFP_KERNEL);
> > +		if (!pagelist)
> > +			goto out;
> > +		ceph_pagelist_init(pagelist);
> > +
> > +		err = ceph_pagelist_reserve(pagelist, PAGE_SIZE);
> > +		if (err)
> > +			goto out;
> > +		ceph_pagelist_encode_32(pagelist, 1);
> > +	}
> > +
> > +	label_len = strlen(label);
> > +	err = ceph_pagelist_reserve(pagelist, XATTR_SECURITY_PREFIX_LEN +
> > +				    label_len + as_ctx->sec_ctxlen + 8);
> > +	if (err)
> > +		goto out;
> > +
> > +	if (as_ctx->pagelist) {
> > +		/* update count of KV pairs */
> > +		BUG_ON(pagelist->length <= sizeof(__le32));
> > +		if (list_is_singular(&pagelist->head)) {
> > +			le32_add_cpu((__le32*)pagelist->mapped_tail, 1);
> > +		} else {
> > +			struct page *page = list_first_entry(&pagelist->head,
> > +							     struct page, lru);
> > +			void *addr = kmap_atomic(page);
> > +			le32_add_cpu((__le32*)addr, 1);
> > +			kunmap_atomic(addr);
> > +		}
> > +	} else {
> > +		as_ctx->pagelist = pagelist;
> > +	}
> > +
> > +	ceph_pagelist_encode_32(pagelist,
> > +				XATTR_SECURITY_PREFIX_LEN + label_len);
> > +	ceph_pagelist_append(pagelist, XATTR_SECURITY_PREFIX,
> > +			     XATTR_SECURITY_PREFIX_LEN);
> > +	ceph_pagelist_append(pagelist, label, label_len);
> > +
> > +	ceph_pagelist_encode_32(pagelist, as_ctx->sec_ctxlen);
> > +	ceph_pagelist_append(pagelist, as_ctx->sec_ctx, as_ctx->sec_ctxlen);
> > +
> > +	err = 0;
> > +out:
> > +	if (pagelist && !as_ctx->pagelist)
> > +		ceph_pagelist_release(pagelist);
> > +	return err;
> > +}
> > +
> > +void ceph_security_invalidate_secctx(struct inode *inode)
> > +{
> > +	security_inode_invalidate_secctx(inode);
> > +}
> > +
> > +static int ceph_xattr_set_security_label(const struct xattr_handler *handler,
> > +				    struct dentry *unused, struct inode *inode,
> > +				    const char *key, const void *buf,
> > +				    size_t buflen, int flags)
> > +{
> > +	if (security_ismaclabel(key)) {
> > +		const char *name = xattr_full_name(handler, key);
> > +		return __ceph_setxattr(inode, name, buf, buflen, flags);
> > +	}
> > +	return  -EOPNOTSUPP;
> > +}
> > +
> > +static int ceph_xattr_get_security_label(const struct xattr_handler *handler,
> > +				    struct dentry *unused, struct inode *inode,
> > +				    const char *key, void *buf, size_t buflen)
> > +{
> > +        if (security_ismaclabel(key)) {
> 
> nit: there may be some whitespace damage above.
> 
> > +		const char *name = xattr_full_name(handler, key);
> > +		return __ceph_getxattr(inode, name, buf, buflen);
> > +	}
> > +	return  -EOPNOTSUPP;
> > +}
> 
> 
> What I'm a little unclear on in all of this is what happens when you
> have an existing cephfs with no security labels, and one client boots to
> a kernel with this support. This is unlike the situation with most local
> filesystems, as we can't really expect to do a full relabel when
> enabling this on an existing fs.
> 
> This function just returns -EOPNOTSUPP, but inode_doinit_with_dentry
> seems like it might expect -ENODATA in this situation. Is this the
> correct way to handle this?
> 

Ahh, nevermind -- I missed that the above -EOPNOTSUPP is for when we
don't recognize the xattr key name. It looks like we should get back
ENODATA in this situation, which should result in the inode getting the
default file label for the sb.

> 
> > +static const struct xattr_handler ceph_security_label_handler = {
> > +        .prefix = XATTR_SECURITY_PREFIX,
> > +        .get    = ceph_xattr_get_security_label,
> > +        .set    = ceph_xattr_set_security_label,
> > +};
> > +#endif
> >  #endif
> >  
> >  void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
> > @@ -1196,7 +1283,26 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
> >  #ifdef CONFIG_CEPH_FS_POSIX_ACL
> >  	posix_acl_release(as_ctx->acl);
> >  	posix_acl_release(as_ctx->default_acl);
> > +#endif
> > +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> > +	security_release_secctx(as_ctx->sec_ctx, as_ctx->sec_ctxlen);
> >  #endif
> >  	if (as_ctx->pagelist)
> >  		ceph_pagelist_release(as_ctx->pagelist);
> >  }
> > +
> > +/*
> > + * List of handlers for synthetic system.* attributes. Other
> > + * attributes are handled directly.
> > + */
> > +const struct xattr_handler *ceph_xattr_handlers[] = {
> > +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> > +	&posix_acl_access_xattr_handler,
> > +	&posix_acl_default_xattr_handler,
> > +#endif
> > +#ifdef CONFIG_CEPH_FS_SECURITY_LABEL
> > +	&ceph_security_label_handler,
> > +#endif
> > +	&ceph_other_xattr_handler,
> > +	NULL,
> > +};
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-06 15:39   ` Casey Schaufler
@ 2018-09-07  8:31     ` Yan, Zheng
  2018-09-07 20:31       ` Casey Schaufler
  0 siblings, 1 reply; 18+ messages in thread
From: Yan, Zheng @ 2018-09-07  8:31 UTC (permalink / raw)
  To: linux-security-module

On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/6/2018 8:08 AM, Jeff Layton wrote:
> > On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> >> This is preparation for CephFS security label. CephFS's implementation uses
> >> dentry_init_security() to get security context before inode is created,
> >> then sends open/mkdir/mknod request to MDS, together with security xattr
> >> "security.<security module name>"
>
> Please excuse my late entry into this review.
>
> First, *Do not prefix general LSM interface work with "selinux:"*
> You should only use that prefix when you're dealing with the
> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
> If you are changing security.h or lsm_hooks.h you are almost
> certainly changing LSM interfaces.
>
> Because you prefixed this work with "selinux:" I pretty much
> ignored it. Please don't do that again.
>
> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
>

Sorry, I don't understand why you mention security_inode_getsecctx().
The selinux_dentry_init_security is called for new inode, I don't how
security_inode_getsecctx() can help.

Regards
Yan, Zheng



>
> >>
> >> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> >> ---
> >>  fs/nfs/nfs4proc.c         | 3 ++-
> >>  include/linux/lsm_hooks.h | 4 ++--
> >>  include/linux/security.h  | 9 +++++----
> >>  security/security.c       | 7 ++++---
> >>  security/selinux/hooks.c  | 8 ++++++--
> >>  5 files changed, 19 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index 6dd146885da9..d18a5fb7aec3 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> >>              return NULL;
> >>
> >>      err = security_dentry_init_security(dentry, sattr->ia_mode,
> >> -                            &dentry->d_name, (void **)&label->label, &label->len);
> >> +                            &dentry->d_name,  NULL,
> >> +                            (void **)&label->label, &label->len);
> >>      if (err == 0)
> >>              return label;
> >>
> >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >> index 8f1131c8dd54..e176c2032bdc 100644
> >> --- a/include/linux/lsm_hooks.h
> >> +++ b/include/linux/lsm_hooks.h
> >> @@ -1476,8 +1476,8 @@ union security_list_options {
> >>                                      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,
> >> -                                    u32 *ctxlen);
> >> +                                    const struct qstr *name, const char **label,
> >> +                                    void **ctx, u32 *ctxlen);
> >>      int (*dentry_create_files_as)(struct dentry *dentry, int mode,
> >>                                      struct qstr *name,
> >>                                      const struct cred *old,
> >> diff --git a/include/linux/security.h b/include/linux/security.h
> >> index 63030c85ee19..df2d73998c64 100644
> >> --- a/include/linux/security.h
> >> +++ b/include/linux/security.h
> >> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> >>                              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,
> >> -                                    u32 *ctxlen);
> >> +                                    const struct qstr *name,
> >> +                                    const char **label,
> >> +                                    void **ctx, u32 *ctxlen);
> >>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
> >>                                      struct qstr *name,
> >>                                      const struct cred *old,
> >> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
> >>  static inline int security_dentry_init_security(struct dentry *dentry,
> >>                                               int mode,
> >>                                               const struct qstr *name,
> >> -                                             void **ctx,
> >> -                                             u32 *ctxlen)
> >> +                                             const char **label,
> >> +                                             void **ctx, u32 *ctxlen)
> >>  {
> >>      return -EOPNOTSUPP;
> >>  }
> >> diff --git a/security/security.c b/security/security.c
> >> index 68f46d849abe..69818d46aa28 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
> >>  }
> >>
> >>  int security_dentry_init_security(struct dentry *dentry, int mode,
> >> -                                    const struct qstr *name, void **ctx,
> >> -                                    u32 *ctxlen)
> >> +                                    const struct qstr *name,
> >> +                                    const char **label,
> >> +                                    void **ctx, u32 *ctxlen)
> >>  {
> >>      return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> >> -                            name, ctx, ctxlen);
> >> +                            name, label, ctx, ctxlen);
> >>  }
> >>  EXPORT_SYMBOL(security_dentry_init_security);
> >>
> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >> index 2b5ee5fbd652..eca3879d9357 100644
> >> --- a/security/selinux/hooks.c
> >> +++ b/security/selinux/hooks.c
> >> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
> >>  }
> >>
> >>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> >> -                                    const struct qstr *name, void **ctx,
> >> -                                    u32 *ctxlen)
> >> +                                    const struct qstr *name,
> >> +                                    const char **label,
> >> +                                    void **ctx, u32 *ctxlen)
> >>  {
> >>      u32 newsid;
> >>      int rc;
> >> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> >>      if (rc)
> >>              return rc;
> >>
> >> +    if (label)
> >> +            *label = XATTR_SELINUX_SUFFIX;
> >> +
> > nit: I'd probably call this "name" since that's what it's called in
> > selinux_inode_init_security. "label" has a different connotation in this
> > context.
> >
> >>      return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
> >>                                     ctxlen);
> >>  }
> > Patch looks reasonable to me though.
> >
> > Acked-by: Jeff Layton <jlayton@kernel.org>
> >
> >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-07  8:31     ` Yan, Zheng
@ 2018-09-07 20:31       ` Casey Schaufler
  2018-09-10  3:06         ` Yan, Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Casey Schaufler @ 2018-09-07 20:31 UTC (permalink / raw)
  To: linux-security-module

On 9/7/2018 1:31 AM, Yan, Zheng wrote:
> On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 9/6/2018 8:08 AM, Jeff Layton wrote:
>>> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
>>>> This is preparation for CephFS security label. CephFS's implementation uses
>>>> dentry_init_security() to get security context before inode is created,
>>>> then sends open/mkdir/mknod request to MDS, together with security xattr
>>>> "security.<security module name>"
>> Please excuse my late entry into this review.
>>
>> First, *Do not prefix general LSM interface work with "selinux:"*
>> You should only use that prefix when you're dealing with the
>> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
>> If you are changing security.h or lsm_hooks.h you are almost
>> certainly changing LSM interfaces.
>>
>> Because you prefixed this work with "selinux:" I pretty much
>> ignored it. Please don't do that again.
>>
>> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
>>
> Sorry, I don't understand why you mention security_inode_getsecctx().
> The selinux_dentry_init_security is called for new inode, I don't how
> security_inode_getsecctx() can help.

I didn't do a great job describing my issue. Let me try again.

There is no way you should need to know the name of the security
module (e.g. SELinux, Smack) in the CephFS code. Nor should you be
assuming that there is only one security attribute, nor that it is
named "security.<security module name>".  Sure, that's true for
SELinux, but SELinux isn't the only security module in town.

Other filesystems don't need anything like this. Why do you?

I will leave the implications of having multiple security modules
with security attributes aside because support for that is a work
in progress. Leave it to say that other filesystem will have no
problem with it at all.

>
> Regards
> Yan, Zheng
>
>
>
>>>> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
>>>> ---
>>>>  fs/nfs/nfs4proc.c         | 3 ++-
>>>>  include/linux/lsm_hooks.h | 4 ++--
>>>>  include/linux/security.h  | 9 +++++----
>>>>  security/security.c       | 7 ++++---
>>>>  security/selinux/hooks.c  | 8 ++++++--
>>>>  5 files changed, 19 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>> index 6dd146885da9..d18a5fb7aec3 100644
>>>> --- a/fs/nfs/nfs4proc.c
>>>> +++ b/fs/nfs/nfs4proc.c
>>>> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
>>>>              return NULL;
>>>>
>>>>      err = security_dentry_init_security(dentry, sattr->ia_mode,
>>>> -                            &dentry->d_name, (void **)&label->label, &label->len);
>>>> +                            &dentry->d_name,  NULL,
>>>> +                            (void **)&label->label, &label->len);
>>>>      if (err == 0)
>>>>              return label;
>>>>
>>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>>> index 8f1131c8dd54..e176c2032bdc 100644
>>>> --- a/include/linux/lsm_hooks.h
>>>> +++ b/include/linux/lsm_hooks.h
>>>> @@ -1476,8 +1476,8 @@ union security_list_options {
>>>>                                      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,
>>>> -                                    u32 *ctxlen);
>>>> +                                    const struct qstr *name, const char **label,
>>>> +                                    void **ctx, u32 *ctxlen);
>>>>      int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>>>>                                      struct qstr *name,
>>>>                                      const struct cred *old,
>>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>>> index 63030c85ee19..df2d73998c64 100644
>>>> --- a/include/linux/security.h
>>>> +++ b/include/linux/security.h
>>>> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>>>>                              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,
>>>> -                                    u32 *ctxlen);
>>>> +                                    const struct qstr *name,
>>>> +                                    const char **label,
>>>> +                                    void **ctx, u32 *ctxlen);
>>>>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
>>>>                                      struct qstr *name,
>>>>                                      const struct cred *old,
>>>> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
>>>>  static inline int security_dentry_init_security(struct dentry *dentry,
>>>>                                               int mode,
>>>>                                               const struct qstr *name,
>>>> -                                             void **ctx,
>>>> -                                             u32 *ctxlen)
>>>> +                                             const char **label,
>>>> +                                             void **ctx, u32 *ctxlen)
>>>>  {
>>>>      return -EOPNOTSUPP;
>>>>  }
>>>> diff --git a/security/security.c b/security/security.c
>>>> index 68f46d849abe..69818d46aa28 100644
>>>> --- a/security/security.c
>>>> +++ b/security/security.c
>>>> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
>>>>  }
>>>>
>>>>  int security_dentry_init_security(struct dentry *dentry, int mode,
>>>> -                                    const struct qstr *name, void **ctx,
>>>> -                                    u32 *ctxlen)
>>>> +                                    const struct qstr *name,
>>>> +                                    const char **label,
>>>> +                                    void **ctx, u32 *ctxlen)
>>>>  {
>>>>      return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
>>>> -                            name, ctx, ctxlen);
>>>> +                            name, label, ctx, ctxlen);
>>>>  }
>>>>  EXPORT_SYMBOL(security_dentry_init_security);
>>>>
>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>> index 2b5ee5fbd652..eca3879d9357 100644
>>>> --- a/security/selinux/hooks.c
>>>> +++ b/security/selinux/hooks.c
>>>> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
>>>>  }
>>>>
>>>>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>>>> -                                    const struct qstr *name, void **ctx,
>>>> -                                    u32 *ctxlen)
>>>> +                                    const struct qstr *name,
>>>> +                                    const char **label,
>>>> +                                    void **ctx, u32 *ctxlen)
>>>>  {
>>>>      u32 newsid;
>>>>      int rc;
>>>> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
>>>>      if (rc)
>>>>              return rc;
>>>>
>>>> +    if (label)
>>>> +            *label = XATTR_SELINUX_SUFFIX;
>>>> +
>>> nit: I'd probably call this "name" since that's what it's called in
>>> selinux_inode_init_security. "label" has a different connotation in this
>>> context.
>>>
>>>>      return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
>>>>                                     ctxlen);
>>>>  }
>>> Patch looks reasonable to me though.
>>>
>>> Acked-by: Jeff Layton <jlayton@kernel.org>
>>>
>>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-07 20:31       ` Casey Schaufler
@ 2018-09-10  3:06         ` Yan, Zheng
  2018-09-11 17:23           ` Casey Schaufler
  0 siblings, 1 reply; 18+ messages in thread
From: Yan, Zheng @ 2018-09-10  3:06 UTC (permalink / raw)
  To: linux-security-module

On Sat, Sep 8, 2018 at 4:31 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/7/2018 1:31 AM, Yan, Zheng wrote:
> > On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 9/6/2018 8:08 AM, Jeff Layton wrote:
> >>> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> >>>> This is preparation for CephFS security label. CephFS's implementation uses
> >>>> dentry_init_security() to get security context before inode is created,
> >>>> then sends open/mkdir/mknod request to MDS, together with security xattr
> >>>> "security.<security module name>"
> >> Please excuse my late entry into this review.
> >>
> >> First, *Do not prefix general LSM interface work with "selinux:"*
> >> You should only use that prefix when you're dealing with the
> >> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
> >> If you are changing security.h or lsm_hooks.h you are almost
> >> certainly changing LSM interfaces.
> >>
> >> Because you prefixed this work with "selinux:" I pretty much
> >> ignored it. Please don't do that again.
> >>
> >> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
> >>
> > Sorry, I don't understand why you mention security_inode_getsecctx().
> > The selinux_dentry_init_security is called for new inode, I don't how
> > security_inode_getsecctx() can help.
>
> I didn't do a great job describing my issue. Let me try again.
>
> There is no way you should need to know the name of the security
> module (e.g. SELinux, Smack) in the CephFS code. Nor should you be
> assuming that there is only one security attribute, nor that it is
> named "security.<security module name>".  Sure, that's true for
> SELinux, but SELinux isn't the only security module in town.
>
> Other filesystems don't need anything like this. Why do you?
>

What I want is a way to get security context before inode is created.
The closest interface is security_dentry_init_security(). Currently
the only NFS uses security_dentry_init_security() and only selinux has
dentry_init_security callback. NFS store selinux context in its
security label (NFS inode can only have one security label, security
label can only store one security context). SO current NFS security
label implementation can not support multiple security modules.

I don't want to limit cephfs to single security module. So I make
cephfs store security context in security.<security module name>
xattr. When multiple security modules work is done in the future,
cephfs can easily supports it. (introduce a new
security_dentry_init_security() which return security contexts for all
enabled security modules)

Regards
Yan, Zheng

> I will leave the implications of having multiple security modules
> with security attributes aside because support for that is a work
> in progress. Leave it to say that other filesystem will have no
> problem with it at all.
>
> >
> > Regards
> > Yan, Zheng
> >
> >
> >
> >>>> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> >>>> ---
> >>>>  fs/nfs/nfs4proc.c         | 3 ++-
> >>>>  include/linux/lsm_hooks.h | 4 ++--
> >>>>  include/linux/security.h  | 9 +++++----
> >>>>  security/security.c       | 7 ++++---
> >>>>  security/selinux/hooks.c  | 8 ++++++--
> >>>>  5 files changed, 19 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >>>> index 6dd146885da9..d18a5fb7aec3 100644
> >>>> --- a/fs/nfs/nfs4proc.c
> >>>> +++ b/fs/nfs/nfs4proc.c
> >>>> @@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> >>>>              return NULL;
> >>>>
> >>>>      err = security_dentry_init_security(dentry, sattr->ia_mode,
> >>>> -                            &dentry->d_name, (void **)&label->label, &label->len);
> >>>> +                            &dentry->d_name,  NULL,
> >>>> +                            (void **)&label->label, &label->len);
> >>>>      if (err == 0)
> >>>>              return label;
> >>>>
> >>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >>>> index 8f1131c8dd54..e176c2032bdc 100644
> >>>> --- a/include/linux/lsm_hooks.h
> >>>> +++ b/include/linux/lsm_hooks.h
> >>>> @@ -1476,8 +1476,8 @@ union security_list_options {
> >>>>                                      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,
> >>>> -                                    u32 *ctxlen);
> >>>> +                                    const struct qstr *name, const char **label,
> >>>> +                                    void **ctx, u32 *ctxlen);
> >>>>      int (*dentry_create_files_as)(struct dentry *dentry, int mode,
> >>>>                                      struct qstr *name,
> >>>>                                      const struct cred *old,
> >>>> diff --git a/include/linux/security.h b/include/linux/security.h
> >>>> index 63030c85ee19..df2d73998c64 100644
> >>>> --- a/include/linux/security.h
> >>>> +++ b/include/linux/security.h
> >>>> @@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> >>>>                              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,
> >>>> -                                    u32 *ctxlen);
> >>>> +                                    const struct qstr *name,
> >>>> +                                    const char **label,
> >>>> +                                    void **ctx, u32 *ctxlen);
> >>>>  int security_dentry_create_files_as(struct dentry *dentry, int mode,
> >>>>                                      struct qstr *name,
> >>>>                                      const struct cred *old,
> >>>> @@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
> >>>>  static inline int security_dentry_init_security(struct dentry *dentry,
> >>>>                                               int mode,
> >>>>                                               const struct qstr *name,
> >>>> -                                             void **ctx,
> >>>> -                                             u32 *ctxlen)
> >>>> +                                             const char **label,
> >>>> +                                             void **ctx, u32 *ctxlen)
> >>>>  {
> >>>>      return -EOPNOTSUPP;
> >>>>  }
> >>>> diff --git a/security/security.c b/security/security.c
> >>>> index 68f46d849abe..69818d46aa28 100644
> >>>> --- a/security/security.c
> >>>> +++ b/security/security.c
> >>>> @@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
> >>>>  }
> >>>>
> >>>>  int security_dentry_init_security(struct dentry *dentry, int mode,
> >>>> -                                    const struct qstr *name, void **ctx,
> >>>> -                                    u32 *ctxlen)
> >>>> +                                    const struct qstr *name,
> >>>> +                                    const char **label,
> >>>> +                                    void **ctx, u32 *ctxlen)
> >>>>  {
> >>>>      return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> >>>> -                            name, ctx, ctxlen);
> >>>> +                            name, label, ctx, ctxlen);
> >>>>  }
> >>>>  EXPORT_SYMBOL(security_dentry_init_security);
> >>>>
> >>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >>>> index 2b5ee5fbd652..eca3879d9357 100644
> >>>> --- a/security/selinux/hooks.c
> >>>> +++ b/security/selinux/hooks.c
> >>>> @@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
> >>>>  }
> >>>>
> >>>>  static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> >>>> -                                    const struct qstr *name, void **ctx,
> >>>> -                                    u32 *ctxlen)
> >>>> +                                    const struct qstr *name,
> >>>> +                                    const char **label,
> >>>> +                                    void **ctx, u32 *ctxlen)
> >>>>  {
> >>>>      u32 newsid;
> >>>>      int rc;
> >>>> @@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> >>>>      if (rc)
> >>>>              return rc;
> >>>>
> >>>> +    if (label)
> >>>> +            *label = XATTR_SELINUX_SUFFIX;
> >>>> +
> >>> nit: I'd probably call this "name" since that's what it's called in
> >>> selinux_inode_init_security. "label" has a different connotation in this
> >>> context.
> >>>
> >>>>      return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
> >>>>                                     ctxlen);
> >>>>  }
> >>> Patch looks reasonable to me though.
> >>>
> >>> Acked-by: Jeff Layton <jlayton@kernel.org>
> >>>
> >>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-10  3:06         ` Yan, Zheng
@ 2018-09-11 17:23           ` Casey Schaufler
  2018-09-12  1:14             ` Yan, Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Casey Schaufler @ 2018-09-11 17:23 UTC (permalink / raw)
  To: linux-security-module

On 9/9/2018 8:06 PM, Yan, Zheng wrote:
> On Sat, Sep 8, 2018 at 4:31 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 9/7/2018 1:31 AM, Yan, Zheng wrote:
>>> On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> On 9/6/2018 8:08 AM, Jeff Layton wrote:
>>>>> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
>>>>>> This is preparation for CephFS security label. CephFS's implementation uses
>>>>>> dentry_init_security() to get security context before inode is created,
>>>>>> then sends open/mkdir/mknod request to MDS, together with security xattr
>>>>>> "security.<security module name>"
>>>> Please excuse my late entry into this review.
>>>>
>>>> First, *Do not prefix general LSM interface work with "selinux:"*
>>>> You should only use that prefix when you're dealing with the
>>>> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
>>>> If you are changing security.h or lsm_hooks.h you are almost
>>>> certainly changing LSM interfaces.
>>>>
>>>> Because you prefixed this work with "selinux:" I pretty much
>>>> ignored it. Please don't do that again.
>>>>
>>>> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
>>>>
>>> Sorry, I don't understand why you mention security_inode_getsecctx().
>>> The selinux_dentry_init_security is called for new inode, I don't how
>>> security_inode_getsecctx() can help.
>> I didn't do a great job describing my issue. Let me try again.
>>
>> There is no way you should need to know the name of the security
>> module (e.g. SELinux, Smack) in the CephFS code. Nor should you be
>> assuming that there is only one security attribute, nor that it is
>> named "security.<security module name>".  Sure, that's true for
>> SELinux, but SELinux isn't the only security module in town.
>>
>> Other filesystems don't need anything like this. Why do you?
>>
> What I want is a way to get security context before inode is created.

What are you doing differently from the other filesystems?
Why doesn't the security_d_instantiate mechanism work for you?

> The closest interface is security_dentry_init_security(). Currently
> the only NFS uses security_dentry_init_security() and only selinux has
> dentry_init_security callback. NFS store selinux context in its
> security label (NFS inode can only have one security label, security
> label can only store one security context). SO current NFS security
> label implementation can not support multiple security modules.

That's correct. The labeled NFS is a very specific mechanism. It
does not do anything except Mandatory Access Control labeling. It
cannot be used for any other attributes. I am not a fan of the
labeled NFS implementation, and was vocal about it at the time.

> I don't want to limit cephfs to single security module. So I make
> cephfs store security context in security.<security module name>
> xattr. 

Please look at the mechanism used in kernfs.

> When multiple security modules work is done in the future,
> cephfs can easily supports it. (introduce a new
> security_dentry_init_security() which return security contexts for all
> enabled security modules)

That is easier said than done. I've proposed several mechanisms,
none of which are especially satisfactory. The best mechanism I see
is the one used in kernfs, which is why I suggest you use that.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
  2018-09-11 17:23           ` Casey Schaufler
@ 2018-09-12  1:14             ` Yan, Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Yan, Zheng @ 2018-09-12  1:14 UTC (permalink / raw)
  To: linux-security-module

On Wed, Sep 12, 2018 at 1:23 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/9/2018 8:06 PM, Yan, Zheng wrote:
> > On Sat, Sep 8, 2018 at 4:31 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 9/7/2018 1:31 AM, Yan, Zheng wrote:
> >>> On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>> On 9/6/2018 8:08 AM, Jeff Layton wrote:
> >>>>> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> >>>>>> This is preparation for CephFS security label. CephFS's implementation uses
> >>>>>> dentry_init_security() to get security context before inode is created,
> >>>>>> then sends open/mkdir/mknod request to MDS, together with security xattr
> >>>>>> "security.<security module name>"
> >>>> Please excuse my late entry into this review.
> >>>>
> >>>> First, *Do not prefix general LSM interface work with "selinux:"*
> >>>> You should only use that prefix when you're dealing with the
> >>>> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
> >>>> If you are changing security.h or lsm_hooks.h you are almost
> >>>> certainly changing LSM interfaces.
> >>>>
> >>>> Because you prefixed this work with "selinux:" I pretty much
> >>>> ignored it. Please don't do that again.
> >>>>
> >>>> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
> >>>>
> >>> Sorry, I don't understand why you mention security_inode_getsecctx().
> >>> The selinux_dentry_init_security is called for new inode, I don't how
> >>> security_inode_getsecctx() can help.
> >> I didn't do a great job describing my issue. Let me try again.
> >>
> >> There is no way you should need to know the name of the security
> >> module (e.g. SELinux, Smack) in the CephFS code. Nor should you be
> >> assuming that there is only one security attribute, nor that it is
> >> named "security.<security module name>".  Sure, that's true for
> >> SELinux, but SELinux isn't the only security module in town.
> >>
> >> Other filesystems don't need anything like this. Why do you?
> >>
> > What I want is a way to get security context before inode is created.
>
> What are you doing differently from the other filesystems?
> Why doesn't the security_d_instantiate mechanism work for you?
>

Because I want to minimize overhead of setting security xattrs for new
inode. When creating new file/dir, cephfs client sends one request to
MDS. The request can carry xattrs. MDS can create new file/dir and
setting xattrs at the same time. If cephfs follows other filesystems'
code, cephfs client needs to send to two request to MDS for each new
file/dir, one for create, one for setting security xattrs.

> > The closest interface is security_dentry_init_security(). Currently
> > the only NFS uses security_dentry_init_security() and only selinux has
> > dentry_init_security callback. NFS store selinux context in its
> > security label (NFS inode can only have one security label, security
> > label can only store one security context). SO current NFS security
> > label implementation can not support multiple security modules.
>
> That's correct. The labeled NFS is a very specific mechanism. It
> does not do anything except Mandatory Access Control labeling. It
> cannot be used for any other attributes. I am not a fan of the
> labeled NFS implementation, and was vocal about it at the time.
>
> > I don't want to limit cephfs to single security module. So I make
> > cephfs store security context in security.<security module name>
> > xattr.
>
> Please look at the mechanism used in kernfs.
>

kernfs' mechanism is simple. It just provides
kernfs_trusted_xattr_handler.  It does not initialize security context
for new file/dir. I don't see how to adapt its mechanism for cephfs.

Regards
Yan, Zheng

> > When multiple security modules work is done in the future,
> > cephfs can easily supports it. (introduce a new
> > security_dentry_init_security() which return security contexts for all
> > enabled security modules)
>
> That is easier said than done. I've proposed several mechanisms,
> none of which are especially satisfactory. The best mechanism I see
> is the one used in kernfs, which is why I suggest you use that.
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] selinux: make dentry_init_security() return security module name
@ 2018-06-26  8:04 Yan, Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Yan, Zheng @ 2018-06-26  8:04 UTC (permalink / raw)
  To: linux-security-module

This is preparation for CephFS security label. CephFS's implementation uses
dentry_init_security() to get security context before inode is created,
then sends open/mkdir/mknod request to MDS, together with security xattr
"security.<security module name>"

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/nfs/nfs4proc.c         | 3 ++-
 include/linux/lsm_hooks.h | 4 ++--
 include/linux/security.h  | 9 +++++----
 security/security.c       | 7 ++++---
 security/selinux/hooks.c  | 8 ++++++--
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6dd146885da9..d18a5fb7aec3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -122,7 +122,8 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
 		return NULL;
 
 	err = security_dentry_init_security(dentry, sattr->ia_mode,
-				&dentry->d_name, (void **)&label->label, &label->len);
+				&dentry->d_name,  NULL,
+				(void **)&label->label, &label->len);
 	if (err == 0)
 		return label;
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8f1131c8dd54..e176c2032bdc 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1476,8 +1476,8 @@ union security_list_options {
 					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,
-					u32 *ctxlen);
+					const struct qstr *name, const char **label,
+					void **ctx, u32 *ctxlen);
 	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
 					struct qstr *name,
 					const struct cred *old,
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..df2d73998c64 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -246,8 +246,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
 				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,
-					u32 *ctxlen);
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen);
 int security_dentry_create_files_as(struct dentry *dentry, int mode,
 					struct qstr *name,
 					const struct cred *old,
@@ -609,8 +610,8 @@ static inline void security_inode_free(struct inode *inode)
 static inline int security_dentry_init_security(struct dentry *dentry,
 						 int mode,
 						 const struct qstr *name,
-						 void **ctx,
-						 u32 *ctxlen)
+						 const char **label,
+						 void **ctx, u32 *ctxlen)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/security.c b/security/security.c
index 68f46d849abe..69818d46aa28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -450,11 +450,12 @@ void security_inode_free(struct inode *inode)
 }
 
 int security_dentry_init_security(struct dentry *dentry, int mode,
-					const struct qstr *name, void **ctx,
-					u32 *ctxlen)
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen)
 {
 	return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
-				name, ctx, ctxlen);
+				name, label, ctx, ctxlen);
 }
 EXPORT_SYMBOL(security_dentry_init_security);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2b5ee5fbd652..eca3879d9357 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2985,8 +2985,9 @@ static void selinux_inode_free_security(struct inode *inode)
 }
 
 static int selinux_dentry_init_security(struct dentry *dentry, int mode,
-					const struct qstr *name, void **ctx,
-					u32 *ctxlen)
+					const struct qstr *name,
+					const char **label,
+					void **ctx, u32 *ctxlen)
 {
 	u32 newsid;
 	int rc;
@@ -2998,6 +2999,9 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
 	if (rc)
 		return rc;
 
+	if (label)
+		*label = XATTR_SELINUX_SUFFIX;
+
 	return security_sid_to_context(&selinux_state, newsid, (char **)ctx,
 				       ctxlen);
 }
-- 
2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-09-12  1:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-26  8:43 [PATCH 1/3] selinux: make dentry_init_security() return security module name Yan, Zheng
2018-06-26  8:43 ` [PATCH 2/3] ceph: rename struct ceph_acls_info to ceph_acl_sec_ctx Yan, Zheng
2018-09-06 15:14   ` Jeff Layton
2018-06-26  8:43 ` [PATCH 3/3] ceph: xattr security label support Yan, Zheng
2018-09-06 15:50   ` Jeff Layton
2018-09-06 16:05     ` Jeff Layton
2018-06-26 13:28 ` [PATCH 1/3] selinux: make dentry_init_security() return security module name Stephen Smalley
2018-06-26 15:32   ` Yan, Zheng
2018-06-26 16:21 ` Casey Schaufler
2018-06-27  1:46   ` Yan, Zheng
2018-09-06 15:08 ` Jeff Layton
2018-09-06 15:39   ` Casey Schaufler
2018-09-07  8:31     ` Yan, Zheng
2018-09-07 20:31       ` Casey Schaufler
2018-09-10  3:06         ` Yan, Zheng
2018-09-11 17:23           ` Casey Schaufler
2018-09-12  1:14             ` Yan, Zheng
  -- strict thread matches above, loose matches on Subject: below --
2018-06-26  8:04 Yan, Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).