[lustre-devel] [PATCH 05/20] lustre: sec: do not expose security.c to listxattr/getxattr

[James Simmons]

From: Sebastien Buisson <sbuisson at ddn.com>

security.c xattr, which contains encryption context, should not be
exposed by the xattr-related system calls such as listxattr() and
getxattr() because of its special semantics.
Update sanity-sec test_57 to test this.

WC-bug-id: https://jira.whamcloud.com/browse/LU-14677
Lustre-commit: efb66de719329ce4d ("LU-14677 sec: do not expose security.c to listxattr/getxattr")
Signed-off-by: Sebastien Buisson <sbuisson at ddn.com>
Reviewed-on: https://review.whamcloud.com/44101
Reviewed-by: Andreas Dilger <adilger at whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell at whamcloud.com>
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
 fs/lustre/llite/crypto.c         | 16 ++++++++++++++++
 fs/lustre/llite/llite_internal.h |  5 +++++
 fs/lustre/llite/xattr.c          | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/fs/lustre/llite/crypto.c b/fs/lustre/llite/crypto.c
index 5d99037..0fae9a5 100644
--- a/fs/lustre/llite/crypto.c
+++ b/fs/lustre/llite/crypto.c
@@ -32,10 +32,26 @@
 static int ll_get_context(struct inode *inode, void *ctx, size_t len)
 {
 	struct dentry *dentry = d_find_any_alias(inode);
+	struct lu_env *env;
+	u16 refcheck;
 	int rc;
 
+	env = cl_env_get(&refcheck);
+	if (IS_ERR(env))
+		return PTR_ERR(env);
+
+	/* Set lcc_getencctx=1 to allow this thread to read
+	 * LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr, as requested by llcrypt.
+	 */
+	ll_cl_add(inode, env, NULL, LCC_RW);
+	ll_env_info(env)->lti_io_ctx.lcc_getencctx = 1;
+
 	rc = __vfs_getxattr(dentry, inode, LL_XATTR_NAME_ENCRYPTION_CONTEXT,
 			    ctx, len);
+
+	ll_cl_remove(inode, env);
+	cl_env_put(env, &refcheck);
+
 	if (dentry)
 		dput(dentry);
 
diff --git a/fs/lustre/llite/llite_internal.h b/fs/lustre/llite/llite_internal.h
index cfeec14..e0fda00 100644
--- a/fs/lustre/llite/llite_internal.h
+++ b/fs/lustre/llite/llite_internal.h
@@ -1312,6 +1312,11 @@ struct ll_cl_context {
 	struct cl_io   *lcc_io;
 	struct cl_page *lcc_page;
 	enum lcc_type		 lcc_type;
+	/**
+	 * Get encryption context operation in progress,
+	 * allow getxattr of LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr
+	 */
+	unsigned int		 lcc_getencctx:1;
 };
 
 struct ll_thread_info {
diff --git a/fs/lustre/llite/xattr.c b/fs/lustre/llite/xattr.c
index 001c828..59a1400 100644
--- a/fs/lustre/llite/xattr.c
+++ b/fs/lustre/llite/xattr.c
@@ -366,6 +366,21 @@ int ll_xattr_list(struct inode *inode, const char *name, int type, void *buffer,
 	void *xdata;
 	int rc;
 
+	/* Getting LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr is only allowed
+	 * when it comes from ll_get_context(), ie when llcrypt needs to
+	 * know the encryption context.
+	 * Otherwise, any direct reading of this xattr returns -EPERM.
+	 */
+	if (type == XATTR_SECURITY_T &&
+	    !strcmp(name, LL_XATTR_NAME_ENCRYPTION_CONTEXT)) {
+		struct ll_cl_context *lcc = ll_cl_find(inode);
+
+		if (!lcc || !lcc->lcc_getencctx) {
+			rc = -EPERM;
+			goto out_xattr;
+		}
+	}
+
 	if (sbi->ll_xattr_cache_enabled && type != XATTR_ACL_ACCESS_T &&
 	    (type != XATTR_SECURITY_T || strcmp(name, "security.selinux"))) {
 		rc = ll_xattr_cache_get(inode, name, buffer, size, valid);
@@ -632,9 +647,24 @@ ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	rem = rc;
 
 	while (rem > 0) {
+		bool hide_xattr = false;
+
+		/* Listing xattrs should not expose
+		 * LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr, unless it comes
+		 * from llcrypt.
+		 */
+		if (get_xattr_type(xattr_name)->flags == XATTR_SECURITY_T &&
+		    !strcmp(xattr_name, LL_XATTR_NAME_ENCRYPTION_CONTEXT)) {
+			struct ll_cl_context *lcc = ll_cl_find(inode);
+
+			if (!lcc || !lcc->lcc_getencctx)
+				hide_xattr = true;
+		}
+
 		len = strnlen(xattr_name, rem - 1) + 1;
 		rem -= len;
-		if (!xattr_type_filter(sbi, get_xattr_type(xattr_name))) {
+		if (!xattr_type_filter(sbi, hide_xattr ? NULL :
+				       get_xattr_type(xattr_name))) {
 			/* Skip OK xattr type, leave it in buffer. */
 			xattr_name += len;
 			continue;
-- 
1.8.3.1