[PATCH RFC 0/4] IMA on NFS prototype

[PATCH RFC 0/4] IMA on NFS prototype

[Chuck Lever]

This series implements support for accessing and updating the
security.ima xattr on files that reside on an NFS mount. Since NFS
does not have CAP_SYS_ADMIN, only root is allowed to set this xattr
(on clients or on the server).

EVM is not supported in this prototype. NFS does not support several
of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
Linux file capabilities are not supported, which makes EVM more
difficult to support on NFS mounts.

Please see the individual patch descriptions: standards action is
still required to define the official FATTR4 flag that all NFSv4.2
implementations recognize as meaning "the security.ima xattr". This
prototype is not guaranteed to interoperate with future prototypes
or standards-compliant implementations of this feature. It is for
experimental purposes only.

I'm interested in comments on the implementation, test results, or a
discussion of whether this proposal creates undesirable security
exposures.

A topic branch with this work is here:

git://git.linux-nfs.org/projects/cel/cel-2.6.git

in the nfs-ima-prototype topic branch.

---

Chuck Lever (4):
      NFS: Define common IMA-related protocol elements
      NFS: Rename security xattr handler
      NFS: Prototype support for IMA on NFS (client)
      NFSD: Prototype support for IMA on NFS (server)


 fs/nfs/nfs4_fs.h          |    1 
 fs/nfs/nfs4proc.c         |  138 ++++++++++++++++++++++++++++++++---
 fs/nfs/nfs4xdr.c          |  175 +++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4proc.c        |   15 ++++
 fs/nfsd/nfs4xdr.c         |   54 ++++++++++++--
 fs/nfsd/nfsd.h            |   10 +++
 fs/nfsd/vfs.c             |   32 ++++++++
 fs/nfsd/vfs.h             |    3 +
 fs/nfsd/xdr4.h            |    3 +
 fs/xattr.c                |    3 +
 include/linux/nfs4.h      |    5 +
 include/linux/nfs_fs_sb.h |    1 
 include/linux/nfs_xdr.h   |   21 +++++
 13 files changed, 440 insertions(+), 21 deletions(-)

--
Chuck Lever

[PATCH RFC 1/4] NFS: Define common IMA-related protocol elements

[Chuck Lever]

Common protocol definitions used by the server and client
implementations. A separate patch also makes it easier to see what
code needs to be change when the flag name and value is eventually
standardized.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/nfs4.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 1b06f0b..ca3adb1 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -41,6 +41,7 @@ struct nfs4_acl {
 };
 
 #define NFS4_MAXLABELLEN	2048
+#define NFS4_MAXIMALEN		(4096)
 
 struct nfs4_label {
 	uint32_t	lfs;
@@ -451,6 +452,7 @@ enum change_attr_type4 {
 #define FATTR4_WORD2_CHANGE_ATTR_TYPE	(1UL << 15)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
 #define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
+#define FATTR4_WORD2_LINUX_IMA		(1UL << 18)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)

[PATCH RFC 2/4] NFS: Rename security xattr handler

[Chuck Lever]

Refactor: In a moment, the handler will no longer deal with only
NFSv4 security labels. Give it a more generic name.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4proc.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 557a5d6..df0ee42 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7128,20 +7128,19 @@ static bool nfs4_xattr_list_nfs4_acl(struct dentry *dentry)
 
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 
-static int nfs4_xattr_set_nfs4_label(const struct xattr_handler *handler,
-				     struct dentry *unused, struct inode *inode,
-				     const char *key, const void *buf,
-				     size_t buflen, int flags)
+static int nfs4_xattr_set_security(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))
 		return nfs4_set_security_label(inode, buf, buflen);
-
 	return -EOPNOTSUPP;
 }
 
-static int nfs4_xattr_get_nfs4_label(const struct xattr_handler *handler,
-				     struct dentry *unused, struct inode *inode,
-				     const char *key, void *buf, size_t buflen)
+static int nfs4_xattr_get_security(const struct xattr_handler *handler,
+				   struct dentry *unused, struct inode *inode,
+				   const char *key, void *buf, size_t buflen)
 {
 	if (security_ismaclabel(key))
 		return nfs4_get_security_label(inode, buf, buflen);
@@ -7161,10 +7160,10 @@ static int nfs4_xattr_get_nfs4_label(const struct xattr_handler *handler,
 	return len;
 }
 
-static const struct xattr_handler nfs4_xattr_nfs4_label_handler = {
+static const struct xattr_handler nfs4_xattr_security_handler = {
 	.prefix = XATTR_SECURITY_PREFIX,
-	.get	= nfs4_xattr_get_nfs4_label,
-	.set	= nfs4_xattr_set_nfs4_label,
+	.get	= nfs4_xattr_get_security,
+	.set	= nfs4_xattr_set_security,
 };
 
 #else
@@ -9824,7 +9823,7 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
 const struct xattr_handler *nfs4_xattr_handlers[] = {
 	&nfs4_xattr_nfs4_acl_handler,
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
-	&nfs4_xattr_nfs4_label_handler,
+	&nfs4_xattr_security_handler,
 #endif
 	NULL
 };

[PATCH RFC 3/4] NFS: Prototype support for IMA on NFS (client)

[Chuck Lever]

When NFSv4 Security Label support is enabled and kernel Integrity
and IMA support is enabled (via CONFIG), then build in code to
handle the "security.ima" xattr. The NFS client converts this to
a particular FATTR4 bit which can be handled on the wire via a
standard NFSv4 GETATTR or SETATTR operation.

The FATTR4 bit is made up; meaning we still have to go through a
standards process to allocate a bit that all NFS vendors agree on.
Thus there is no guarantee this prototype will interoperate with
others or with a future standards-based implementation.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4_fs.h          |    1 
 fs/nfs/nfs4proc.c         |  115 +++++++++++++++++++++++++++++-
 fs/nfs/nfs4xdr.c          |  175 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs4.h      |    3 +
 include/linux/nfs_fs_sb.h |    1 
 include/linux/nfs_xdr.h   |   21 +++++
 6 files changed, 314 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 06ac3d9a..795f2f5 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -424,6 +424,7 @@ extern int nfs4_detect_session_trunking(struct nfs_client *clp,
 extern const u32 nfs4_pathconf_bitmap[3];
 extern const u32 nfs4_fsinfo_bitmap[3];
 extern const u32 nfs4_fs_locations_bitmap[3];
+extern const u32 nfs4_ima_bitmap[3];
 
 void nfs40_shutdown_client(struct nfs_client *);
 void nfs41_shutdown_client(struct nfs_client *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index df0ee42..bbbd5b7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -205,6 +205,9 @@ static int nfs4_map_errors(int err)
 	| FATTR4_WORD1_MOUNTED_ON_FILEID,
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 	FATTR4_WORD2_SECURITY_LABEL
+#ifdef CONFIG_IMA
+	| FATTR4_WORD2_LINUX_IMA
+#endif
 #endif
 };
 
@@ -275,6 +278,10 @@ static int nfs4_map_errors(int err)
 	| FATTR4_WORD1_MOUNTED_ON_FILEID,
 };
 
+const u32 nfs4_ima_bitmap[3] = {
+	0, 0, FATTR4_WORD2_LINUX_IMA
+};
+
 static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
 		struct inode *inode)
 {
@@ -3575,7 +3582,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 
 #define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
 #define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_MODE_UMASK - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_LINUX_IMA - 1UL)
 
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
@@ -3621,7 +3628,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL);
+				NFS_CAP_SECURITY_LABEL|
+				NFS_CAP_IMA);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3648,6 +3656,12 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
+#ifdef CONFIG_IMA
+		if (res.attr_bitmask[2] & FATTR4_WORD2_LINUX_IMA) {
+			res.attr_bitmask[2] &= ~FATTR4_WORD2_LINUX_IMA;
+			server->caps |= NFS_CAP_IMA;
+		}
+#endif
 #endif
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
@@ -5723,6 +5737,87 @@ static int nfs4_do_set_security_label(struct inode *inode,
 out:
 	return status;
 }
+
+#ifdef CONFIG_IMA
+
+static int _nfs4_get_security_ima(struct inode *inode, void *buf, size_t buflen)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs4_getima_arg arg = {
+		.fh		= NFS_FH(inode),
+		.bitmask	= nfs4_ima_bitmap,
+	};
+	struct nfs4_getima_res res = {
+		.data		= buf,
+		.len		= buflen,
+		.server		= server,
+	};
+	struct rpc_message msg = {
+		.rpc_proc	= &nfs4_procedures[NFSPROC4_CLNT_GETIMA],
+		.rpc_argp	= &arg,
+		.rpc_resp	= &res,
+	};
+
+	return nfs4_call_sync(server->client, server, &msg,
+			      &arg.seq_args, &res.seq_res, 0);
+}
+
+static int nfs4_get_security_ima(struct inode *inode, void *buf, size_t buflen)
+{
+	struct nfs4_exception exception = { };
+	int err;
+
+	if (!nfs_server_capable(inode, NFS_CAP_IMA))
+		return -EOPNOTSUPP;
+	do {
+		err = _nfs4_get_security_ima(inode, buf, buflen);
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+	return err;
+}
+
+static int _nfs4_set_security_ima(struct inode *inode, const void *buf,
+				  size_t buflen)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs4_setima_arg arg = {
+		.fh		= NFS_FH(inode),
+		.data		= buf,
+		.len		= buflen,
+	};
+	struct nfs_setattrres res = {
+		.server		= server,
+	};
+	struct rpc_message msg = {
+		.rpc_proc	= &nfs4_procedures[NFSPROC4_CLNT_SETIMA],
+		.rpc_argp	= &arg,
+		.rpc_resp	= &res,
+	};
+
+	nfs4_stateid_copy(&arg.stateid, &zero_stateid);
+	return nfs4_call_sync(server->client, server, &msg,
+			      &arg.seq_args, &res.seq_res, 1);
+}
+
+static int nfs4_set_security_ima(struct inode *inode, const void *buf,
+				 size_t buflen)
+{
+	struct nfs4_exception exception = { };
+	int err;
+
+	if (!nfs_server_capable(inode, NFS_CAP_IMA))
+		return -EOPNOTSUPP;
+	do {
+		err = _nfs4_set_security_ima(inode, buf, buflen);
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+	return err;
+}
+
+#endif	/* CONFIG_IMA */
+
 #endif	/* CONFIG_NFS_V4_SECURITY_LABEL */
 
 
@@ -7128,11 +7223,23 @@ static bool nfs4_xattr_list_nfs4_acl(struct dentry *dentry)
 
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 
+#ifdef CONFIG_IMA
+/* XXX: security/ does not seem to provide this helper */
+static bool nfs4_security_isima(const char *name)
+{
+	return (strcmp(name, XATTR_IMA_SUFFIX) == 0);
+}
+#endif	/* CONFIG_IMA */
+
 static int nfs4_xattr_set_security(const struct xattr_handler *handler,
 				   struct dentry *unused, struct inode *inode,
 				   const char *key, const void *buf,
 				   size_t buflen, int flags)
 {
+#ifdef CONFIG_IMA
+	if (nfs4_security_isima(key))
+		return nfs4_set_security_ima(inode, buf, buflen);
+#endif	/* CONFIG_IMA */
 	if (security_ismaclabel(key))
 		return nfs4_set_security_label(inode, buf, buflen);
 	return -EOPNOTSUPP;
@@ -7142,6 +7249,10 @@ static int nfs4_xattr_get_security(const struct xattr_handler *handler,
 				   struct dentry *unused, struct inode *inode,
 				   const char *key, void *buf, size_t buflen)
 {
+#ifdef CONFIG_IMA
+	if (nfs4_security_isima(key))
+		return nfs4_get_security_ima(inode, buf, buflen);
+#endif	/* CONFIG_IMA */
 	if (security_ismaclabel(key))
 		return nfs4_get_security_label(inode, buf, buflen);
 	return -EOPNOTSUPP;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 2fc8f6f..0e26893 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -434,6 +434,14 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
 #define decode_layoutget_maxsz	0
 #endif /* CONFIG_NFS_V4_1 */
 
+#define encode_getima_maxsz	(encode_getattr_maxsz)
+/* XXX: not quite right: we don't need getfattr_maxsz here */
+#define decode_getima_maxsz	(decode_getattr_maxsz + \
+				 1 + XDR_QUADLEN(NFS4_MAXIMALEN))
+#define encode_setima_maxsz	(encode_setattr_maxsz + \
+				 1 + XDR_QUADLEN(NFS4_MAXIMALEN))
+#define decode_setima_maxsz	(decode_setattr_maxsz)
+
 #define NFS4_enc_compound_sz	(1024)  /* XXX: large enough? */
 #define NFS4_dec_compound_sz	(1024)  /* XXX: large enough? */
 #define NFS4_enc_read_sz	(compound_encode_hdr_maxsz + \
@@ -901,6 +909,22 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
 #define NFS4_dec_free_stateid_sz	(compound_decode_hdr_maxsz + \
 					 decode_sequence_maxsz + \
 					 decode_free_stateid_maxsz)
+#define NFS4_enc_getima_sz	(compound_encode_hdr_maxsz + \
+				 encode_sequence_maxsz + \
+				 encode_putfh_maxsz + \
+				 encode_getima_maxsz)
+#define NFS4_dec_getima_sz	(compound_decode_hdr_maxsz + \
+				 decode_sequence_maxsz + \
+				 decode_putfh_maxsz + \
+				 decode_getima_maxsz)
+#define NFS4_enc_setima_sz	(compound_encode_hdr_maxsz +\
+				 encode_sequence_maxsz + \
+				 encode_putfh_maxsz + \
+				 encode_setima_maxsz)
+#define NFS4_dec_setima_sz	(compound_decode_hdr_maxsz +\
+				 decode_sequence_maxsz + \
+				 decode_putfh_maxsz + \
+				 decode_setima_maxsz)
 
 const u32 nfs41_maxwrite_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
 				      compound_encode_hdr_maxsz +
@@ -1280,6 +1304,13 @@ static void encode_fs_locations(struct xdr_stream *xdr, const u32* bitmask, stru
 			ARRAY_SIZE(nfs4_fs_locations_bitmap), hdr);
 }
 
+static void encode_getima(struct xdr_stream *xdr, const u32 *bitmask,
+			  struct compound_hdr *hdr)
+{
+	encode_getattr(xdr, nfs4_ima_bitmap, bitmask,
+			ARRAY_SIZE(nfs4_ima_bitmap), hdr);
+}
+
 static void encode_getfh(struct xdr_stream *xdr, struct compound_hdr *hdr)
 {
 	encode_op_hdr(xdr, OP_GETFH, decode_getfh_maxsz, hdr);
@@ -1698,6 +1729,17 @@ static void encode_setattr(struct xdr_stream *xdr, const struct nfs_setattrargs
 			server->attr_bitmask);
 }
 
+static void encode_setima(struct xdr_stream *xdr,
+			  const struct nfs4_setima_arg *arg,
+			  struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_SETATTR, decode_setattr_maxsz, hdr);
+	encode_nfs4_stateid(xdr, &arg->stateid);
+	xdr_encode_bitmap4(xdr, nfs4_ima_bitmap, ARRAY_SIZE(nfs4_ima_bitmap));
+	xdr_stream_encode_u32(xdr, sizeof(__be32) + xdr_align_size(arg->len));
+	xdr_stream_encode_opaque(xdr, arg->data, arg->len);
+}
+
 static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclientid *setclientid, struct compound_hdr *hdr)
 {
 	__be32 *p;
@@ -3144,6 +3186,42 @@ static void nfs4_xdr_enc_free_stateid(struct rpc_rqst *req,
 }
 #endif /* CONFIG_NFS_V4_1 */
 
+/*
+ * Encode GETATTR(IMA) request
+ */
+static void nfs4_xdr_enc_getima(struct rpc_rqst *req, struct xdr_stream *xdr,
+				const void *data)
+{
+	const struct nfs4_getima_arg *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_getima(xdr, args->bitmask, &hdr);
+	encode_nops(&hdr);
+}
+
+/*
+ * Encode an SETATTR(IMA) request
+ */
+static void nfs4_xdr_enc_setima(struct rpc_rqst *req, struct xdr_stream *xdr,
+				const void *data)
+{
+	const struct nfs4_setima_arg *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_setima(xdr, args, &hdr);
+	encode_nops(&hdr);
+}
+
 static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
 {
 	dprintk("nfs: %s: prematurely hit end of receive buffer. "
@@ -4846,6 +4924,52 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
 	return decode_getfattr_generic(xdr, fattr, NULL, NULL, NULL, server);
 }
 
+static int decode_getima(struct xdr_stream *xdr, void *data, u32 len,
+			 const struct nfs_server *server)
+{
+	u32 attrlen, bitmap[3] = {0};
+	unsigned int savep;
+	int status;
+	u32 size;
+
+	status = decode_op_hdr(xdr, OP_GETATTR);
+	if (status < 0)
+		return status;
+	status = decode_attr_bitmap(xdr, bitmap);
+	if (status < 0)
+		return status;
+	status = decode_attr_length(xdr, &attrlen, &savep);
+	if (status < 0)
+		return status;
+
+	if (likely(bitmap[2] & FATTR4_WORD2_LINUX_IMA)) {
+		__be32 *p;
+
+		p = xdr_inline_decode(xdr, 4);
+		if (unlikely(!p))
+			return -EIO;
+		size = be32_to_cpup(p++);
+		p = xdr_inline_decode(xdr, size);
+		if (unlikely(!p))
+			return -EIO;
+
+		if (size > NFS4_MAXIMALEN)
+			return -E2BIG;
+		/* @len == 0 means "just return the size" */
+		if (len > 0) {
+			if (size > len)
+				return -ERANGE;
+			memcpy(data, p, len);
+		}
+	} else
+		return -ENODATA;
+
+	status = verify_attr_len(xdr, savep, attrlen);
+	if (status < 0)
+		return status;
+	return size;
+}
+
 /*
  * Decode potentially multiple layout types.
  */
@@ -7547,6 +7671,55 @@ static int nfs4_xdr_dec_free_stateid(struct rpc_rqst *rqstp,
 }
 #endif /* CONFIG_NFS_V4_1 */
 
+/*
+ * Decode GETATTR(IMA) response
+ */
+static int nfs4_xdr_dec_getima(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
+			       void *data)
+{
+	struct nfs4_getima_res *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_getima(xdr, res->data, res->len, res->server);
+out:
+	return status;
+}
+
+/*
+ * Decode SETATTR(IMA) response
+ */
+static int nfs4_xdr_dec_setima(struct rpc_rqst *rqstp,
+			       struct xdr_stream *xdr,
+			       void *data)
+{
+	struct nfs_setattrres *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_setattr(xdr);
+out:
+	return status;
+}
+
 /**
  * nfs4_decode_dirent - Decode a single NFSv4 directory entry stored in
  *                      the local page cache.
@@ -7791,6 +7964,8 @@ int nfs4_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
 	PROC42(COPY,		enc_copy,		dec_copy),
 	PROC42(OFFLOAD_CANCEL,	enc_offload_cancel,	dec_offload_cancel),
 	PROC(LOOKUPP,		enc_lookupp,		dec_lookupp),
+	PROC(GETIMA,		enc_getima,		dec_getima),
+	PROC(SETIMA,		enc_setima,		dec_setima),
 };
 
 static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index ca3adb1..4d5c771 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -540,6 +540,9 @@ enum {
 	NFSPROC4_CLNT_OFFLOAD_CANCEL,
 
 	NFSPROC4_CLNT_LOOKUPP,
+
+	NFSPROC4_CLNT_GETIMA,
+	NFSPROC4_CLNT_SETIMA,
 };
 
 /* nfs41 types */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 6aa8cc8..b92f954 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -261,5 +261,6 @@ struct nfs_server {
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
 #define NFS_CAP_OFFLOAD_CANCEL	(1U << 25)
+#define NFS_CAP_IMA		(1U << 26)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 441a93e..4ee30f0 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1010,6 +1010,27 @@ struct nfs4_getattr_res {
 	struct nfs4_label		*label;
 };
 
+struct nfs4_getima_arg {
+	struct nfs4_sequence_args	seq_args;
+	const struct nfs_fh		*fh;
+	const u32			*bitmask;
+};
+
+struct nfs4_getima_res {
+	struct nfs4_sequence_res	seq_res;
+	const struct nfs_server		*server;
+	void				*data;
+	u32				len;
+};
+
+struct nfs4_setima_arg {
+	struct nfs4_sequence_args	seq_args;
+	struct nfs_fh			*fh;
+	nfs4_stateid                    stateid;
+	const void			*data;
+	u32				len;
+};
+
 struct nfs4_link_arg {
 	struct nfs4_sequence_args 	seq_args;
 	const struct nfs_fh *		fh;

[PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[Chuck Lever]

When NFSv4 Security Label support is enabled and kernel Integrity
and IMA support is enabled (via CONFIG), then build in code to
handle the "security.ima" xattr. The NFS server converts incoming
GETATTR and SETATTR calls to acesses and updates of the xattr.

The FATTR4 bit is made up; meaning we still have to go through a
standards process to allocate a bit that all NFS vendors agree on.
Thus there is no guarantee this prototype will interoperate with
others or with a future standards-based implementation.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4proc.c |   15 ++++++++++++++
 fs/nfsd/nfs4xdr.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++------
 fs/nfsd/nfsd.h     |   10 ++++++++++
 fs/nfsd/vfs.c      |   32 +++++++++++++++++++++++++++++++
 fs/nfsd/vfs.h      |    3 +++
 fs/nfsd/xdr4.h     |    3 +++
 fs/xattr.c         |    3 ++-
 7 files changed, 113 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0cfd257..ad205f9 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -106,6 +106,10 @@
 	if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) &&
 			!(exp->ex_flags & NFSEXP_SECURITY_LABEL))
 		return nfserr_attrnotsupp;
+#ifndef CONFIG_IMA
+	if (bmval[2] & FATTR4_WORD2_LINUX_IMA)
+		return nfserr_attrnotsupp;
+#endif
 	if (writable && !bmval_is_subset(bmval, writable))
 		return nfserr_inval;
 	if (writable && (bmval[2] & FATTR4_WORD2_MODE_UMASK) &&
@@ -979,6 +983,11 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
 				&setattr->sa_label);
 	if (status)
 		goto out;
+	if (setattr->sa_ima.len)
+		status = nfsd4_set_ima_metadata(rqstp, &cstate->current_fh,
+						&setattr->sa_ima);
+	if (status)
+		goto out;
 	status = nfsd_setattr(rqstp, &cstate->current_fh, &setattr->sa_iattr,
 				0, (time_t)0);
 out:
@@ -2135,6 +2144,12 @@ static inline u32 nfsd4_getattr_rsize(struct svc_rqst *rqstp,
 		ret += NFS4_MAXLABELLEN + 12;
 		bmap2 &= ~FATTR4_WORD2_SECURITY_LABEL;
 	}
+#ifdef CONFIG_IMA
+	if (bmap2 & FATTR4_WORD2_LINUX_IMA) {
+		ret += NFS4_MAXIMALEN + 4;
+		bmap2 &= ~FATTR4_WORD2_LINUX_IMA;
+	}
+#endif
 	/*
 	 * Largest of remaining attributes are 16 bytes (e.g.,
 	 * supported_attributes)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 3de42a7..19e9f25 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -39,6 +39,7 @@
 #include <linux/statfs.h>
 #include <linux/utsname.h>
 #include <linux/pagemap.h>
+#include <linux/xattr.h>
 #include <linux/sunrpc/svcauth_gss.h>
 
 #include "idmap.h"
@@ -318,7 +319,8 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
 static __be32
 nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
 		   struct iattr *iattr, struct nfs4_acl **acl,
-		   struct xdr_netobj *label, int *umask)
+		   struct xdr_netobj *label, int *umask,
+		   struct xdr_netobj *ima)
 {
 	struct timespec ts;
 	int expected_len, len = 0;
@@ -455,7 +457,6 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
 			goto xdr_error;
 		}
 	}
-
 	label->len = 0;
 	if (IS_ENABLED(CONFIG_NFSD_V4_SECURITY_LABEL) &&
 	    bmval[2] & FATTR4_WORD2_SECURITY_LABEL) {
@@ -489,6 +490,23 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
 		*umask = dummy32 & S_IRWXUGO;
 		iattr->ia_valid |= ATTR_MODE;
 	}
+#if defined(CONFIG_NFSD_V4_SECURITY_LABEL) && defined(CONFIG_IMA)
+	ima->len = 0;
+	if (bmval[2] & FATTR4_WORD2_LINUX_IMA) {
+		READ_BUF(4);
+		len += 4;
+		dummy32 = be32_to_cpup(p++);
+		READ_BUF(dummy32);
+		if (dummy32 > NFS4_MAXIMALEN)
+			return nfserr_badlabel;
+		len += (XDR_QUADLEN(dummy32) << 2);
+		READMEM(buf, dummy32);
+		ima->len = dummy32;
+		ima->data = svcxdr_dupstr(argp, buf, dummy32);
+		if (!ima->data)
+			return nfserr_jukebox;
+	}
+#endif
 	if (len != expected_len)
 		goto xdr_error;
 
@@ -684,7 +702,7 @@ static __be32 nfsd4_decode_bind_conn_to_session(struct nfsd4_compoundargs *argp,
 
 	status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
 				    &create->cr_acl, &create->cr_label,
-				    &create->cr_umask);
+				    &create->cr_umask, &create->cr_ima);
 	if (status)
 		goto out;
 
@@ -936,7 +954,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 		case NFS4_CREATE_GUARDED:
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
 				&open->op_iattr, &open->op_acl, &open->op_label,
-				&open->op_umask);
+				&open->op_umask, &open->op_ima);
 			if (status)
 				goto out;
 			break;
@@ -951,7 +969,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 			COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
 				&open->op_iattr, &open->op_acl, &open->op_label,
-				&open->op_umask);
+				&open->op_umask, &open->op_ima);
 			if (status)
 				goto out;
 			break;
@@ -1188,7 +1206,8 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 	if (status)
 		return status;
 	return nfsd4_decode_fattr(argp, setattr->sa_bmval, &setattr->sa_iattr,
-				  &setattr->sa_acl, &setattr->sa_label, NULL);
+				  &setattr->sa_acl, &setattr->sa_label, NULL,
+				  &setattr->sa_ima);
 }
 
 static __be32
@@ -2430,6 +2449,7 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 		.dentry	= dentry,
 	};
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct xdr_netobj ima = { 0, NULL };
 
 	BUG_ON(bmval1 & NFSD_WRITEONLY_ATTRS_WORD1);
 	BUG_ON(!nfsd_attrs_supported(minorversion, bmval));
@@ -2489,6 +2509,16 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 				goto out_nfserr;
 		}
 	}
+	if (bmval2 & FATTR4_WORD2_LINUX_IMA) {
+		err = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA,
+					 (char **)&ima.data, 0,
+					 GFP_KERNEL);
+		if (err == -ENODATA)
+			bmval2 &= ~FATTR4_WORD2_LINUX_IMA;
+		else if (err < 0)
+			goto out_nfserr;
+		ima.len = err;
+	}
 #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
 
 	status = nfsd4_encode_bitmap(xdr, bmval0, bmval1, bmval2);
@@ -2510,6 +2540,9 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 			supp[0] &= ~FATTR4_WORD0_ACL;
 		if (!contextsupport)
 			supp[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
+#ifndef CONFIG_IMA
+		supp[2] &= ~FATTR4_WORD2_LINUX_IMA;
+#endif
 		if (!supp[2]) {
 			p = xdr_reserve_space(xdr, 12);
 			if (!p)
@@ -2913,6 +2946,14 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 			goto out;
 	}
 
+	if (bmval2 & FATTR4_WORD2_LINUX_IMA) {
+		p = xdr_reserve_space(xdr, sizeof(__be32) +
+				      xdr_align_size(ima.len));
+		if (!p)
+			goto out_resource;
+		xdr_encode_netobj(p, &ima);
+	}
+
 	attrlen = htonl(xdr->buf->len - attrlen_offset - 4);
 	write_bytes_to_xdr_buf(xdr->buf, attrlen_offset, &attrlen, 4);
 	status = nfs_ok;
@@ -2922,6 +2963,7 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 	if (context)
 		security_release_secctx(context, contextlen);
 #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
+	kfree(ima.data);
 	kfree(acl);
 	if (tempfh) {
 		fh_put(tempfh);
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 0668999..93be978 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -353,7 +353,12 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
 
 /* 4.2 */
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
+#ifdef CONFIG_IMA
+#define NFSD4_2_SECURITY_ATTRS \
+	(FATTR4_WORD2_SECURITY_LABEL	| FATTR4_WORD2_LINUX_IMA)
+#else
 #define NFSD4_2_SECURITY_ATTRS		FATTR4_WORD2_SECURITY_LABEL
+#endif
 #else
 #define NFSD4_2_SECURITY_ATTRS		0
 #endif
@@ -393,8 +398,13 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
 	(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP \
 	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
+#ifdef CONFIG_IMA
+#define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
+	(FATTR4_WORD2_SECURITY_LABEL | FATTR4_WORD2_LINUX_IMA)
+#else
 #define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
 	FATTR4_WORD2_SECURITY_LABEL
+#endif
 #else
 #define MAYBE_FATTR4_WORD2_SECURITY_LABEL 0
 #endif
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7dc98e1..159b0fc 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -543,12 +543,44 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	inode_unlock(d_inode(dentry));
 	return nfserrno(host_error);
 }
+
+#ifdef CONFIG_IMA
+__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			      struct xdr_netobj *ima)
+{
+	struct dentry *dentry;
+	int host_error;
+	__be32 error;
+
+	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, NFSD_MAY_SATTR);
+	if (error)
+		return error;
+	dentry = fhp->fh_dentry;
+
+	inode_lock(d_inode(dentry));
+	host_error = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
+					   ima->data, ima->len, 0);
+	inode_unlock(d_inode(dentry));
+	return nfserrno(host_error);
+}
+#else
+__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			      struct xdr_netobj *ima)
+{
+	return nfserr_notsupp;
+}
+#endif
 #else
 __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		struct xdr_netobj *label)
 {
 	return nfserr_notsupp;
 }
+__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			      struct xdr_netobj *ima)
+{
+	return nfserr_notsupp;
+}
 #endif
 
 __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index a7e1073..acfc2b0 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -55,6 +55,9 @@ __be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
 #ifdef CONFIG_NFSD_V4
 __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
 		    struct xdr_netobj *);
+__be32		nfsd4_set_ima_metadata(struct svc_rqst *rqstp,
+				       struct svc_fh *fhp,
+				       struct xdr_netobj *ima);
 __be32		nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
 				    struct file *, loff_t, loff_t, int);
 __be32		nfsd4_clone_file_range(struct file *, u64, struct file *,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index feeb6d4..2f3307f 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -123,6 +123,7 @@ struct nfsd4_create {
 	struct nfsd4_change_info  cr_cinfo; /* response */
 	struct nfs4_acl *cr_acl;
 	struct xdr_netobj cr_label;
+	struct xdr_netobj cr_ima;
 };
 #define cr_datalen	u.link.datalen
 #define cr_data		u.link.data
@@ -255,6 +256,7 @@ struct nfsd4_open {
 	struct nfs4_clnt_odstate *op_odstate; /* used during processing */
 	struct nfs4_acl *op_acl;
 	struct xdr_netobj op_label;
+	struct xdr_netobj op_ima;
 };
 
 struct nfsd4_open_confirm {
@@ -339,6 +341,7 @@ struct nfsd4_setattr {
 	struct iattr	sa_iattr;           /* request */
 	struct nfs4_acl *sa_acl;
 	struct xdr_netobj sa_label;
+	struct xdr_netobj sa_ima;
 };
 
 struct nfsd4_setclientid {
diff --git a/fs/xattr.c b/fs/xattr.c
index 0d6a6a4..fbbb021 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -202,7 +202,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
 
 	return error;
 }
-
+EXPORT_SYMBOL_GPL(__vfs_setxattr_noperm);
 
 int
 vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
@@ -295,6 +295,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
 	*xattr_value = value;
 	return error;
 }
+EXPORT_SYMBOL_GPL(vfs_getxattr_alloc);
 
 ssize_t
 __vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,

Re: [PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[J. Bruce Fields]

On Thu, Feb 14, 2019 at 03:43:26PM -0500, Chuck Lever wrote:
> When NFSv4 Security Label support is enabled and kernel Integrity
> and IMA support is enabled (via CONFIG), then build in code to
> handle the "security.ima" xattr. The NFS server converts incoming
> GETATTR and SETATTR calls to acesses and updates of the xattr.
> 
> The FATTR4 bit is made up; meaning we still have to go through a
> standards process to allocate a bit that all NFS vendors agree on.
> Thus there is no guarantee this prototype will interoperate with
> others or with a future standards-based implementation.

Why the dependence on CONFIG_NFSD_V4_SECURITY_LABEL?

(Also, I wonder if we actually need CONFIG_NFSD_V4_SECURITY_LABEL or if
we could just remove it, or replace it by CONFIG_SECURITY where
necessary.)

--b.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4proc.c |   15 ++++++++++++++
>  fs/nfsd/nfs4xdr.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++------
>  fs/nfsd/nfsd.h     |   10 ++++++++++
>  fs/nfsd/vfs.c      |   32 +++++++++++++++++++++++++++++++
>  fs/nfsd/vfs.h      |    3 +++
>  fs/nfsd/xdr4.h     |    3 +++
>  fs/xattr.c         |    3 ++-
>  7 files changed, 113 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 0cfd257..ad205f9 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -106,6 +106,10 @@
>  	if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) &&
>  			!(exp->ex_flags & NFSEXP_SECURITY_LABEL))
>  		return nfserr_attrnotsupp;
> +#ifndef CONFIG_IMA
> +	if (bmval[2] & FATTR4_WORD2_LINUX_IMA)
> +		return nfserr_attrnotsupp;
> +#endif
>  	if (writable && !bmval_is_subset(bmval, writable))
>  		return nfserr_inval;
>  	if (writable && (bmval[2] & FATTR4_WORD2_MODE_UMASK) &&
> @@ -979,6 +983,11 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
>  				&setattr->sa_label);
>  	if (status)
>  		goto out;
> +	if (setattr->sa_ima.len)
> +		status = nfsd4_set_ima_metadata(rqstp, &cstate->current_fh,
> +						&setattr->sa_ima);
> +	if (status)
> +		goto out;
>  	status = nfsd_setattr(rqstp, &cstate->current_fh, &setattr->sa_iattr,
>  				0, (time_t)0);
>  out:
> @@ -2135,6 +2144,12 @@ static inline u32 nfsd4_getattr_rsize(struct svc_rqst *rqstp,
>  		ret += NFS4_MAXLABELLEN + 12;
>  		bmap2 &= ~FATTR4_WORD2_SECURITY_LABEL;
>  	}
> +#ifdef CONFIG_IMA
> +	if (bmap2 & FATTR4_WORD2_LINUX_IMA) {
> +		ret += NFS4_MAXIMALEN + 4;
> +		bmap2 &= ~FATTR4_WORD2_LINUX_IMA;
> +	}
> +#endif
>  	/*
>  	 * Largest of remaining attributes are 16 bytes (e.g.,
>  	 * supported_attributes)
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 3de42a7..19e9f25 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -39,6 +39,7 @@
>  #include <linux/statfs.h>
>  #include <linux/utsname.h>
>  #include <linux/pagemap.h>
> +#include <linux/xattr.h>
>  #include <linux/sunrpc/svcauth_gss.h>
>  
>  #include "idmap.h"
> @@ -318,7 +319,8 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
>  static __be32
>  nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
>  		   struct iattr *iattr, struct nfs4_acl **acl,
> -		   struct xdr_netobj *label, int *umask)
> +		   struct xdr_netobj *label, int *umask,
> +		   struct xdr_netobj *ima)
>  {
>  	struct timespec ts;
>  	int expected_len, len = 0;
> @@ -455,7 +457,6 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
>  			goto xdr_error;
>  		}
>  	}
> -
>  	label->len = 0;
>  	if (IS_ENABLED(CONFIG_NFSD_V4_SECURITY_LABEL) &&
>  	    bmval[2] & FATTR4_WORD2_SECURITY_LABEL) {
> @@ -489,6 +490,23 @@ static char *savemem(struct nfsd4_compoundargs *argp, __be32 *p, int nbytes)
>  		*umask = dummy32 & S_IRWXUGO;
>  		iattr->ia_valid |= ATTR_MODE;
>  	}
> +#if defined(CONFIG_NFSD_V4_SECURITY_LABEL) && defined(CONFIG_IMA)
> +	ima->len = 0;
> +	if (bmval[2] & FATTR4_WORD2_LINUX_IMA) {
> +		READ_BUF(4);
> +		len += 4;
> +		dummy32 = be32_to_cpup(p++);
> +		READ_BUF(dummy32);
> +		if (dummy32 > NFS4_MAXIMALEN)
> +			return nfserr_badlabel;
> +		len += (XDR_QUADLEN(dummy32) << 2);
> +		READMEM(buf, dummy32);
> +		ima->len = dummy32;
> +		ima->data = svcxdr_dupstr(argp, buf, dummy32);
> +		if (!ima->data)
> +			return nfserr_jukebox;
> +	}
> +#endif
>  	if (len != expected_len)
>  		goto xdr_error;
>  
> @@ -684,7 +702,7 @@ static __be32 nfsd4_decode_bind_conn_to_session(struct nfsd4_compoundargs *argp,
>  
>  	status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
>  				    &create->cr_acl, &create->cr_label,
> -				    &create->cr_umask);
> +				    &create->cr_umask, &create->cr_ima);
>  	if (status)
>  		goto out;
>  
> @@ -936,7 +954,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
>  		case NFS4_CREATE_GUARDED:
>  			status = nfsd4_decode_fattr(argp, open->op_bmval,
>  				&open->op_iattr, &open->op_acl, &open->op_label,
> -				&open->op_umask);
> +				&open->op_umask, &open->op_ima);
>  			if (status)
>  				goto out;
>  			break;
> @@ -951,7 +969,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
>  			COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
>  			status = nfsd4_decode_fattr(argp, open->op_bmval,
>  				&open->op_iattr, &open->op_acl, &open->op_label,
> -				&open->op_umask);
> +				&open->op_umask, &open->op_ima);
>  			if (status)
>  				goto out;
>  			break;
> @@ -1188,7 +1206,8 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
>  	if (status)
>  		return status;
>  	return nfsd4_decode_fattr(argp, setattr->sa_bmval, &setattr->sa_iattr,
> -				  &setattr->sa_acl, &setattr->sa_label, NULL);
> +				  &setattr->sa_acl, &setattr->sa_label, NULL,
> +				  &setattr->sa_ima);
>  }
>  
>  static __be32
> @@ -2430,6 +2449,7 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
>  		.dentry	= dentry,
>  	};
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> +	struct xdr_netobj ima = { 0, NULL };
>  
>  	BUG_ON(bmval1 & NFSD_WRITEONLY_ATTRS_WORD1);
>  	BUG_ON(!nfsd_attrs_supported(minorversion, bmval));
> @@ -2489,6 +2509,16 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
>  				goto out_nfserr;
>  		}
>  	}
> +	if (bmval2 & FATTR4_WORD2_LINUX_IMA) {
> +		err = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA,
> +					 (char **)&ima.data, 0,
> +					 GFP_KERNEL);
> +		if (err == -ENODATA)
> +			bmval2 &= ~FATTR4_WORD2_LINUX_IMA;
> +		else if (err < 0)
> +			goto out_nfserr;
> +		ima.len = err;
> +	}
>  #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
>  
>  	status = nfsd4_encode_bitmap(xdr, bmval0, bmval1, bmval2);
> @@ -2510,6 +2540,9 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
>  			supp[0] &= ~FATTR4_WORD0_ACL;
>  		if (!contextsupport)
>  			supp[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
> +#ifndef CONFIG_IMA
> +		supp[2] &= ~FATTR4_WORD2_LINUX_IMA;
> +#endif
>  		if (!supp[2]) {
>  			p = xdr_reserve_space(xdr, 12);
>  			if (!p)
> @@ -2913,6 +2946,14 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
>  			goto out;
>  	}
>  
> +	if (bmval2 & FATTR4_WORD2_LINUX_IMA) {
> +		p = xdr_reserve_space(xdr, sizeof(__be32) +
> +				      xdr_align_size(ima.len));
> +		if (!p)
> +			goto out_resource;
> +		xdr_encode_netobj(p, &ima);
> +	}
> +
>  	attrlen = htonl(xdr->buf->len - attrlen_offset - 4);
>  	write_bytes_to_xdr_buf(xdr->buf, attrlen_offset, &attrlen, 4);
>  	status = nfs_ok;
> @@ -2922,6 +2963,7 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
>  	if (context)
>  		security_release_secctx(context, contextlen);
>  #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
> +	kfree(ima.data);
>  	kfree(acl);
>  	if (tempfh) {
>  		fh_put(tempfh);
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 0668999..93be978 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -353,7 +353,12 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
>  
>  /* 4.2 */
>  #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> +#ifdef CONFIG_IMA
> +#define NFSD4_2_SECURITY_ATTRS \
> +	(FATTR4_WORD2_SECURITY_LABEL	| FATTR4_WORD2_LINUX_IMA)
> +#else
>  #define NFSD4_2_SECURITY_ATTRS		FATTR4_WORD2_SECURITY_LABEL
> +#endif
>  #else
>  #define NFSD4_2_SECURITY_ATTRS		0
>  #endif
> @@ -393,8 +398,13 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
>  	(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP \
>  	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)
>  #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> +#ifdef CONFIG_IMA
> +#define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
> +	(FATTR4_WORD2_SECURITY_LABEL | FATTR4_WORD2_LINUX_IMA)
> +#else
>  #define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>  	FATTR4_WORD2_SECURITY_LABEL
> +#endif
>  #else
>  #define MAYBE_FATTR4_WORD2_SECURITY_LABEL 0
>  #endif
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7dc98e1..159b0fc 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -543,12 +543,44 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	inode_unlock(d_inode(dentry));
>  	return nfserrno(host_error);
>  }
> +
> +#ifdef CONFIG_IMA
> +__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			      struct xdr_netobj *ima)
> +{
> +	struct dentry *dentry;
> +	int host_error;
> +	__be32 error;
> +
> +	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, NFSD_MAY_SATTR);
> +	if (error)
> +		return error;
> +	dentry = fhp->fh_dentry;
> +
> +	inode_lock(d_inode(dentry));
> +	host_error = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
> +					   ima->data, ima->len, 0);
> +	inode_unlock(d_inode(dentry));
> +	return nfserrno(host_error);
> +}
> +#else
> +__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			      struct xdr_netobj *ima)
> +{
> +	return nfserr_notsupp;
> +}
> +#endif
>  #else
>  __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		struct xdr_netobj *label)
>  {
>  	return nfserr_notsupp;
>  }
> +__be32 nfsd4_set_ima_metadata(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			      struct xdr_netobj *ima)
> +{
> +	return nfserr_notsupp;
> +}
>  #endif
>  
>  __be32 nfsd4_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index a7e1073..acfc2b0 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -55,6 +55,9 @@ __be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
>  #ifdef CONFIG_NFSD_V4
>  __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
>  		    struct xdr_netobj *);
> +__be32		nfsd4_set_ima_metadata(struct svc_rqst *rqstp,
> +				       struct svc_fh *fhp,
> +				       struct xdr_netobj *ima);
>  __be32		nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
>  				    struct file *, loff_t, loff_t, int);
>  __be32		nfsd4_clone_file_range(struct file *, u64, struct file *,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index feeb6d4..2f3307f 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -123,6 +123,7 @@ struct nfsd4_create {
>  	struct nfsd4_change_info  cr_cinfo; /* response */
>  	struct nfs4_acl *cr_acl;
>  	struct xdr_netobj cr_label;
> +	struct xdr_netobj cr_ima;
>  };
>  #define cr_datalen	u.link.datalen
>  #define cr_data		u.link.data
> @@ -255,6 +256,7 @@ struct nfsd4_open {
>  	struct nfs4_clnt_odstate *op_odstate; /* used during processing */
>  	struct nfs4_acl *op_acl;
>  	struct xdr_netobj op_label;
> +	struct xdr_netobj op_ima;
>  };
>  
>  struct nfsd4_open_confirm {
> @@ -339,6 +341,7 @@ struct nfsd4_setattr {
>  	struct iattr	sa_iattr;           /* request */
>  	struct nfs4_acl *sa_acl;
>  	struct xdr_netobj sa_label;
> +	struct xdr_netobj sa_ima;
>  };
>  
>  struct nfsd4_setclientid {
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 0d6a6a4..fbbb021 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -202,7 +202,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
>  
>  	return error;
>  }
> -
> +EXPORT_SYMBOL_GPL(__vfs_setxattr_noperm);
>  
>  int
>  vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
> @@ -295,6 +295,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
>  	*xattr_value = value;
>  	return error;
>  }
> +EXPORT_SYMBOL_GPL(vfs_getxattr_alloc);
>  
>  ssize_t
>  __vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,

Re: [PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[Chuck Lever]

> On Feb 18, 2019, at 2:32 PM, bfields@fieldses.org wrote:
> 
> On Thu, Feb 14, 2019 at 03:43:26PM -0500, Chuck Lever wrote:
>> When NFSv4 Security Label support is enabled and kernel Integrity
>> and IMA support is enabled (via CONFIG), then build in code to
>> handle the "security.ima" xattr. The NFS server converts incoming
>> GETATTR and SETATTR calls to acesses and updates of the xattr.
>> 
>> The FATTR4 bit is made up; meaning we still have to go through a
>> standards process to allocate a bit that all NFS vendors agree on.
>> Thus there is no guarantee this prototype will interoperate with
>> others or with a future standards-based implementation.
> 
> Why the dependence on CONFIG_NFSD_V4_SECURITY_LABEL?

Hrm, well there is some mechanism on the client side that IMA
needs that is behind CONFIG_NFS_V4_SECURITY_LABEL. I guess I
didn't think about not doing the same thing on the server. It
may just be an artifact of an earlier version of this code.


> (Also, I wonder if we actually need CONFIG_NFSD_V4_SECURITY_LABEL or if
> we could just remove it, or replace it by CONFIG_SECURITY where
> necessary.)

On the server, there is already a (run-time) export option to
enable and disable security labels. Is there a reason a
distribution would want to disable client or server support
for security labels at build time?

--
Chuck Lever

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Mimi Zohar]

Hi Chuck,

> EVM is not supported in this prototype. NFS does not support several
> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
> Linux file capabilities are not supported, which makes EVM more
> difficult to support on NFS mounts.

There's no requirement for all of these xattrs to exist.  If an xattr
does exist, then it is included in the security.evm hmac/signature.

Mimi

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Chuck Lever]

> On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> Hi Chuck,
> 
>> EVM is not supported in this prototype. NFS does not support several
>> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
>> Linux file capabilities are not supported, which makes EVM more
>> difficult to support on NFS mounts.
> 
> There's no requirement for all of these xattrs to exist.  If an xattr
> does exist, then it is included in the security.evm hmac/signature.

Understood. The issue is that if they exist on a file residing on an NFS server,
such xattrs would not be visible to clients. My understanding is that then EVM
verification would fail on such files on NFS clients.

We could possibly make EVM work in limited scenarios until such time that
the NFS protocol can make those xattrs available to NFS clients. I hope that
having only security.ima is useful at least for experimenting and maybe more.

However, if folks think having security.evm also is needed, that is straight-
forward... just saying that there are currently other limits in NFS that make a
full EVM implementation problematic.

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Mimi Zohar]

On Tue, 2019-02-19 at 22:51 -0500, Chuck Lever wrote:
> > On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > Hi Chuck,
> > 
> >> EVM is not supported in this prototype. NFS does not support several
> >> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
> >> Linux file capabilities are not supported, which makes EVM more
> >> difficult to support on NFS mounts.
> > 
> > There's no requirement for all of these xattrs to exist.  If an xattr
> > does exist, then it is included in the security.evm hmac/signature.
> 
> Understood. The issue is that if they exist on a file residing on an NFS server,
> such xattrs would not be visible to clients. My understanding is that then EVM
> verification would fail on such files on NFS clients.
> 
> We could possibly make EVM work in limited scenarios until such time that
> the NFS protocol can make those xattrs available to NFS clients. I hope that
> having only security.ima is useful at least for experimenting and maybe more.
> 
> However, if folks think having security.evm also is needed, that is straight-
> forward... just saying that there are currently other limits in NFS that make a
> full EVM implementation problematic.

Thank you for the explanation.  Yes, I think there is a benefit of
having a file signature, without EVM.

Mimi

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Chuck Lever]

> On Feb 20, 2019, at 7:26 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Tue, 2019-02-19 at 22:51 -0500, Chuck Lever wrote:
>>> On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> Hi Chuck,
>>> 
>>>> EVM is not supported in this prototype. NFS does not support several
>>>> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
>>>> Linux file capabilities are not supported, which makes EVM more
>>>> difficult to support on NFS mounts.
>>> 
>>> There's no requirement for all of these xattrs to exist.  If an xattr
>>> does exist, then it is included in the security.evm hmac/signature.
>> 
>> Understood. The issue is that if they exist on a file residing on an NFS server,
>> such xattrs would not be visible to clients. My understanding is that then EVM
>> verification would fail on such files on NFS clients.
>> 
>> We could possibly make EVM work in limited scenarios until such time that
>> the NFS protocol can make those xattrs available to NFS clients. I hope that
>> having only security.ima is useful at least for experimenting and maybe more.
>> 
>> However, if folks think having security.evm also is needed, that is straight-
>> forward... just saying that there are currently other limits in NFS that make a
>> full EVM implementation problematic.
> 
> Thank you for the explanation.  Yes, I think there is a benefit of
> having a file signature, without EVM.

It's been pointed out to me that a malicious actor inserted between
an NFS server and an NFS client can concurrently substitute the IMA
signature and a file's content with that of another file on the same
NFS share.

This could be used to substitute /etc/group for /etc/passwd, for
example. Both files are unchanged and have verifiable IMA signatures.
The /etc/group file contains a passwd-like entry for root in it, but
without a password field. That would allow the actor to gain root
access on the NFS client.

NFS can mitigate this substitution by using Kerberos 5 integrity to
protect wire traffic from tampering. However, a malicious NFS server
could also perform this substitution, and krb5i would not be able to
detect it.

I'm wondering if there's a mechanism within IMA's toolset to detect
such a substitution on an NFS client.

--
Chuck Lever

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Mimi Zohar]

On Thu, 2019-02-21 at 09:49 -0500, Chuck Lever wrote:
> 
> > On Feb 20, 2019, at 7:26 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Tue, 2019-02-19 at 22:51 -0500, Chuck Lever wrote:
> >>> On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>> Hi Chuck,
> >>> 
> >>>> EVM is not supported in this prototype. NFS does not support several
> >>>> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
> >>>> Linux file capabilities are not supported, which makes EVM more
> >>>> difficult to support on NFS mounts.
> >>> 
> >>> There's no requirement for all of these xattrs to exist.  If an xattr
> >>> does exist, then it is included in the security.evm hmac/signature.
> >> 
> >> Understood. The issue is that if they exist on a file residing on an NFS server,
> >> such xattrs would not be visible to clients. My understanding is that then EVM
> >> verification would fail on such files on NFS clients.
> >> 
> >> We could possibly make EVM work in limited scenarios until such time that
> >> the NFS protocol can make those xattrs available to NFS clients. I hope that
> >> having only security.ima is useful at least for experimenting and maybe more.
> >> 
> >> However, if folks think having security.evm also is needed, that is straight-
> >> forward... just saying that there are currently other limits in NFS that make a
> >> full EVM implementation problematic.
> > 
> > Thank you for the explanation.  Yes, I think there is a benefit of
> > having a file signature, without EVM.
> 
> It's been pointed out to me that a malicious actor inserted between
> an NFS server and an NFS client can concurrently substitute the IMA
> signature and a file's content with that of another file on the same
> NFS share.
> 
> This could be used to substitute /etc/group for /etc/passwd, for
> example. Both files are unchanged and have verifiable IMA signatures.
> The /etc/group file contains a passwd-like entry for root in it, but
> without a password field. That would allow the actor to gain root
> access on the NFS client.
> 
> NFS can mitigate this substitution by using Kerberos 5 integrity to
> protect wire traffic from tampering. However, a malicious NFS server
> could also perform this substitution, and krb5i would not be able to
> detect it.
> 
> I'm wondering if there's a mechanism within IMA's toolset to detect
> such a substitution on an NFS client.

This problem isn't limited to NFS, but is a general problem.  The file
name is part of the directory information, which would need to be
protected all the way up to root. (Dmitry's directory patches protects
one level of the directory tree.)

At last years LSF/MM, Allison Henderson gave a talk titled "XFS parent
pointers" - https://lwn.net/Articles/753480/.  After Allison's talk,
instead of protecting the entire filesystem directory hierarchy, Boaz
Harrosh suggested including the XFS parent pointers' pathname, stored
as an xattr, in the set of EVM protected xattrs.

Mimi

Re: [PATCH RFC 0/4] IMA on NFS prototype

[Chuck Lever]

> On Feb 21, 2019, at 10:51 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2019-02-21 at 09:49 -0500, Chuck Lever wrote:
>> 
>>> On Feb 20, 2019, at 7:26 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Tue, 2019-02-19 at 22:51 -0500, Chuck Lever wrote:
>>>>> On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> 
>>>>> Hi Chuck,
>>>>> 
>>>>>> EVM is not supported in this prototype. NFS does not support several
>>>>>> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
>>>>>> Linux file capabilities are not supported, which makes EVM more
>>>>>> difficult to support on NFS mounts.
>>>>> 
>>>>> There's no requirement for all of these xattrs to exist.  If an xattr
>>>>> does exist, then it is included in the security.evm hmac/signature.
>>>> 
>>>> Understood. The issue is that if they exist on a file residing on an NFS server,
>>>> such xattrs would not be visible to clients. My understanding is that then EVM
>>>> verification would fail on such files on NFS clients.
>>>> 
>>>> We could possibly make EVM work in limited scenarios until such time that
>>>> the NFS protocol can make those xattrs available to NFS clients. I hope that
>>>> having only security.ima is useful at least for experimenting and maybe more.
>>>> 
>>>> However, if folks think having security.evm also is needed, that is straight-
>>>> forward... just saying that there are currently other limits in NFS that make a
>>>> full EVM implementation problematic.
>>> 
>>> Thank you for the explanation.  Yes, I think there is a benefit of
>>> having a file signature, without EVM.
>> 
>> It's been pointed out to me that a malicious actor inserted between
>> an NFS server and an NFS client can concurrently substitute the IMA
>> signature and a file's content with that of another file on the same
>> NFS share.
>> 
>> This could be used to substitute /etc/group for /etc/passwd, for
>> example. Both files are unchanged and have verifiable IMA signatures.
>> The /etc/group file contains a passwd-like entry for root in it, but
>> without a password field. That would allow the actor to gain root
>> access on the NFS client.
>> 
>> NFS can mitigate this substitution by using Kerberos 5 integrity to
>> protect wire traffic from tampering. However, a malicious NFS server
>> could also perform this substitution, and krb5i would not be able to
>> detect it.
>> 
>> I'm wondering if there's a mechanism within IMA's toolset to detect
>> such a substitution on an NFS client.
> 
> This problem isn't limited to NFS, but is a general problem.  The file
> name is part of the directory information, which would need to be
> protected all the way up to root. (Dmitry's directory patches protects
> one level of the directory tree.)

OK, glad to know NFS is not alone!

We would need to guarantee that the file's absolute pathname as seen
by NFS clients is the same as its pathname as seen locally on the NFS
server. This wasn't true for NFSv3, but is usually true for NFSv4,
which exposes a pseudofilesystem root -- on many NFSv4 servers this
looks just like the real local filesystem missing any files that are
not shared. (NFS folks, feel free to chime in and tell me I'm wrong).


> At last years LSF/MM, Allison Henderson gave a talk titled "XFS parent
> pointers" - https://lwn.net/Articles/753480/.  After Allison's talk,
> instead of protecting the entire filesystem directory hierarchy, Boaz
> Harrosh suggested including the XFS parent pointers' pathname, stored
> as an xattr, in the set of EVM protected xattrs.

What is to be done about hard linked files?


--
Chuck Lever

Re: [PATCH RFC 0/4] IMA on NFS prototype

[J. Bruce Fields]

On Thu, Feb 21, 2019 at 10:58:49AM -0500, Chuck Lever wrote:
> 
> 
> > On Feb 21, 2019, at 10:51 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2019-02-21 at 09:49 -0500, Chuck Lever wrote:
> >> 
> >>> On Feb 20, 2019, at 7:26 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>> On Tue, 2019-02-19 at 22:51 -0500, Chuck Lever wrote:
> >>>>> On Feb 19, 2019, at 7:36 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>>> 
> >>>>> Hi Chuck,
> >>>>> 
> >>>>>> EVM is not supported in this prototype. NFS does not support several
> >>>>>> of the xattrs that are protected by EVM: SMACK64, Posix ACLs, and
> >>>>>> Linux file capabilities are not supported, which makes EVM more
> >>>>>> difficult to support on NFS mounts.
> >>>>> 
> >>>>> There's no requirement for all of these xattrs to exist.  If an xattr
> >>>>> does exist, then it is included in the security.evm hmac/signature.
> >>>> 
> >>>> Understood. The issue is that if they exist on a file residing on an NFS server,
> >>>> such xattrs would not be visible to clients. My understanding is that then EVM
> >>>> verification would fail on such files on NFS clients.
> >>>> 
> >>>> We could possibly make EVM work in limited scenarios until such time that
> >>>> the NFS protocol can make those xattrs available to NFS clients. I hope that
> >>>> having only security.ima is useful at least for experimenting and maybe more.
> >>>> 
> >>>> However, if folks think having security.evm also is needed, that is straight-
> >>>> forward... just saying that there are currently other limits in NFS that make a
> >>>> full EVM implementation problematic.
> >>> 
> >>> Thank you for the explanation.  Yes, I think there is a benefit of
> >>> having a file signature, without EVM.
> >> 
> >> It's been pointed out to me that a malicious actor inserted between
> >> an NFS server and an NFS client can concurrently substitute the IMA
> >> signature and a file's content with that of another file on the same
> >> NFS share.
> >> 
> >> This could be used to substitute /etc/group for /etc/passwd, for
> >> example. Both files are unchanged and have verifiable IMA signatures.
> >> The /etc/group file contains a passwd-like entry for root in it, but
> >> without a password field. That would allow the actor to gain root
> >> access on the NFS client.
> >> 
> >> NFS can mitigate this substitution by using Kerberos 5 integrity to
> >> protect wire traffic from tampering. However, a malicious NFS server
> >> could also perform this substitution, and krb5i would not be able to
> >> detect it.
> >> 
> >> I'm wondering if there's a mechanism within IMA's toolset to detect
> >> such a substitution on an NFS client.
> > 
> > This problem isn't limited to NFS, but is a general problem.  The file
> > name is part of the directory information, which would need to be
> > protected all the way up to root. (Dmitry's directory patches protects
> > one level of the directory tree.)
> 
> OK, glad to know NFS is not alone!
> 
> We would need to guarantee that the file's absolute pathname as seen
> by NFS clients is the same as its pathname as seen locally on the NFS
> server. This wasn't true for NFSv3, but is usually true for NFSv4,
> which exposes a pseudofilesystem root -- on many NFSv4 servers this
> looks just like the real local filesystem missing any files that are
> not shared. (NFS folks, feel free to chime in and tell me I'm wrong).

That's very implementation-specific.  Better to say something like "the
pathname seen by the NFS client is the same as that in namespace the
server exports".

--b.

Re: [PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[Bruce Fields]

On Mon, Feb 18, 2019 at 02:41:24PM -0500, Chuck Lever wrote:
> 
> 
> > On Feb 18, 2019, at 2:32 PM, bfields@fieldses.org wrote:
> > 
> > On Thu, Feb 14, 2019 at 03:43:26PM -0500, Chuck Lever wrote:
> >> When NFSv4 Security Label support is enabled and kernel Integrity
> >> and IMA support is enabled (via CONFIG), then build in code to
> >> handle the "security.ima" xattr. The NFS server converts incoming
> >> GETATTR and SETATTR calls to acesses and updates of the xattr.
> >> 
> >> The FATTR4 bit is made up; meaning we still have to go through a
> >> standards process to allocate a bit that all NFS vendors agree on.
> >> Thus there is no guarantee this prototype will interoperate with
> >> others or with a future standards-based implementation.
> > 
> > Why the dependence on CONFIG_NFSD_V4_SECURITY_LABEL?
> 
> Hrm, well there is some mechanism on the client side that IMA
> needs that is behind CONFIG_NFS_V4_SECURITY_LABEL. I guess I
> didn't think about not doing the same thing on the server. It
> may just be an artifact of an earlier version of this code.
> 
> 
> > (Also, I wonder if we actually need CONFIG_NFSD_V4_SECURITY_LABEL or if
> > we could just remove it, or replace it by CONFIG_SECURITY where
> > necessary.)
> 
> On the server, there is already a (run-time) export option to
> enable and disable security labels. Is there a reason a
> distribution would want to disable client or server support
> for security labels at build time?

Distributions tend to want kernels that can do anything, with run time
controls that are adequate to handle any use cases.

So given that we need adequate run-time configuration, why might someone
also want the ability to disable at build time?  Some reasons I can
think of:

	- they need a really small kernel.
	- the feature is too hard to support, or they think it
	  introduces security risks, so they don't want their users
	  turning it on at all.

I could see any of those being reasons for someone not to want NFSD_V4
or SECURITY at all, but is there likely to be a big need to configure in
both of those things but configure out NFSD_V4_SECURITY_LABEL?  That
seems unnecessarily fine grained.

--b.

Re: [PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[Chuck Lever]

> On Mar 1, 2019, at 10:04 AM, Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Mon, Feb 18, 2019 at 02:41:24PM -0500, Chuck Lever wrote:
>> 
>> 
>>> On Feb 18, 2019, at 2:32 PM, bfields@fieldses.org wrote:
>>> 
>>> On Thu, Feb 14, 2019 at 03:43:26PM -0500, Chuck Lever wrote:
>>>> When NFSv4 Security Label support is enabled and kernel Integrity
>>>> and IMA support is enabled (via CONFIG), then build in code to
>>>> handle the "security.ima" xattr. The NFS server converts incoming
>>>> GETATTR and SETATTR calls to acesses and updates of the xattr.
>>>> 
>>>> The FATTR4 bit is made up; meaning we still have to go through a
>>>> standards process to allocate a bit that all NFS vendors agree on.
>>>> Thus there is no guarantee this prototype will interoperate with
>>>> others or with a future standards-based implementation.
>>> 
>>> Why the dependence on CONFIG_NFSD_V4_SECURITY_LABEL?
>> 
>> Hrm, well there is some mechanism on the client side that IMA
>> needs that is behind CONFIG_NFS_V4_SECURITY_LABEL. I guess I
>> didn't think about not doing the same thing on the server. It
>> may just be an artifact of an earlier version of this code.
>> 
>> 
>>> (Also, I wonder if we actually need CONFIG_NFSD_V4_SECURITY_LABEL or if
>>> we could just remove it, or replace it by CONFIG_SECURITY where
>>> necessary.)
>> 
>> On the server, there is already a (run-time) export option to
>> enable and disable security labels. Is there a reason a
>> distribution would want to disable client or server support
>> for security labels at build time?
> 
> Distributions tend to want kernels that can do anything, with run time
> controls that are adequate to handle any use cases.
> 
> So given that we need adequate run-time configuration, why might someone
> also want the ability to disable at build time?  Some reasons I can
> think of:
> 
> 	- they need a really small kernel.
> 	- the feature is too hard to support, or they think it
> 	  introduces security risks, so they don't want their users
> 	  turning it on at all.
> 
> I could see any of those being reasons for someone not to want NFSD_V4
> or SECURITY at all, but is there likely to be a big need to configure in
> both of those things but configure out NFSD_V4_SECURITY_LABEL?  That
> seems unnecessarily fine grained.

I'm not clear, then. Are you proposing to control support for IMA labels
with the "security_labels" export option?


--
Chuck Lever

Re: [PATCH RFC 4/4] NFSD: Prototype support for IMA on NFS (server)

[Bruce Fields]

On Fri, Mar 01, 2019 at 11:01:14AM -0500, Chuck Lever wrote:
> 
> 
> > On Mar 1, 2019, at 10:04 AM, Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > On Mon, Feb 18, 2019 at 02:41:24PM -0500, Chuck Lever wrote:
> >> 
> >> 
> >>> On Feb 18, 2019, at 2:32 PM, bfields@fieldses.org wrote:
> >>> 
> >>> On Thu, Feb 14, 2019 at 03:43:26PM -0500, Chuck Lever wrote:
> >>>> When NFSv4 Security Label support is enabled and kernel Integrity
> >>>> and IMA support is enabled (via CONFIG), then build in code to
> >>>> handle the "security.ima" xattr. The NFS server converts incoming
> >>>> GETATTR and SETATTR calls to acesses and updates of the xattr.
> >>>> 
> >>>> The FATTR4 bit is made up; meaning we still have to go through a
> >>>> standards process to allocate a bit that all NFS vendors agree on.
> >>>> Thus there is no guarantee this prototype will interoperate with
> >>>> others or with a future standards-based implementation.
> >>> 
> >>> Why the dependence on CONFIG_NFSD_V4_SECURITY_LABEL?
> >> 
> >> Hrm, well there is some mechanism on the client side that IMA
> >> needs that is behind CONFIG_NFS_V4_SECURITY_LABEL. I guess I
> >> didn't think about not doing the same thing on the server. It
> >> may just be an artifact of an earlier version of this code.
> >> 
> >> 
> >>> (Also, I wonder if we actually need CONFIG_NFSD_V4_SECURITY_LABEL or if
> >>> we could just remove it, or replace it by CONFIG_SECURITY where
> >>> necessary.)
> >> 
> >> On the server, there is already a (run-time) export option to
> >> enable and disable security labels. Is there a reason a
> >> distribution would want to disable client or server support
> >> for security labels at build time?
> > 
> > Distributions tend to want kernels that can do anything, with run time
> > controls that are adequate to handle any use cases.
> > 
> > So given that we need adequate run-time configuration, why might someone
> > also want the ability to disable at build time?  Some reasons I can
> > think of:
> > 
> > 	- they need a really small kernel.
> > 	- the feature is too hard to support, or they think it
> > 	  introduces security risks, so they don't want their users
> > 	  turning it on at all.
> > 
> > I could see any of those being reasons for someone not to want NFSD_V4
> > or SECURITY at all, but is there likely to be a big need to configure in
> > both of those things but configure out NFSD_V4_SECURITY_LABEL?  That
> > seems unnecessarily fine grained.
> 
> I'm not clear, then. Are you proposing to control support for IMA labels
> with the "security_labels" export option?

Just security labels.  I'd think it'd make sense to support IMA labels
whenever IMA and NFSD_V4 are both turned on.

--b.