[PATCH 0/5] nfs4.1: Initial SP4_MACH_CRED implementation

[PATCH 0/5] nfs4.1: Initial SP4_MACH_CRED implementation

[Weston Andros Adamson]

These patches are the initial client-side SP4_MACH_CRED implementation.

Patch 1 is a repost with a few cleanups

Patch 2 adds the sp4 handler that replaces the cred on an rpc message (and
picks the right rpc_clnt).  Note that for now, we always use the machine cred
if supported - we could only use it if the user cred expires, but this is
simple and spec compliant.

Patch 3-5 implement SP4_MACH_CRED "features":
  - cleanup - CLOSE and LOCKU use machine cred
  - secinfo - SECINFO and SECINFO_NO_NAME use machine cred
  - stateid - TEST_STATEID and FREE_STATEID use machine cred

There are three more features that I've been working on, but need to be tested
before I post:
  - commit - COMMIT uses machine cred
  - write - WRITE uses machine cred, forces stable writes unless commit feature
            is also enabled
  - recovery - OPEN and LOCK can use machine cred - only in recovery.

The commit and write cases should be easy to test against a hacked nfsd, but
I need to think more about how to test the recovery stuff...

-dros

[PATCH 1/5] nfs4.1: Minimal SP4_MACH_CRED implementation

[Weston Andros Adamson]

This is a minimal client side implementation of SP4_MACH_CRED.  It will
attempt to negotiate SP4_MACH_CRED iff the EXCHANGE_ID is using
krb5i or krb5p auth.  SP4_MACH_CRED will be used if the server supports the
minimal operations:

 BIND_CONN_TO_SESSION
 EXCHANGE_ID
 CREATE_SESSION
 DESTROY_SESSION
 DESTROY_CLIENTID

This implementation only includes the EXCHANGE_ID negotiation code because
the client will already use the machine cred for these operations.

This patch lays the groundwork for adding more SP4_MACH_CRED features -
instead of trying to use SP4_MACH_CRED in every procedure (by looking up by
"main" operation or any op contained within), we will make sure that
appropriate sets of operations are supported in the initial
negotiation and then only use these negotiated features. The hope is that this
will limit the scope of testing.

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c         | 120 +++++++++++++++++++++++++++++++++++++++++++---
 fs/nfs/nfs4xdr.c          |  73 ++++++++++++++++++++++++----
 include/linux/nfs_fs_sb.h |   4 ++
 include/linux/nfs_xdr.h   |  19 ++++++++
 4 files changed, 199 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0e64ccc..faa3731 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5915,16 +5915,68 @@ out:
 }
 
 /*
- * nfs4_proc_exchange_id()
+ * Minimum set of SP4_MACH_CRED operations from RFC 5661
+ */
+static const uint32_t _nfs4_spo_must_enforce[NFS4_OP_MAP_NUM_WORDS] = {
+	[1] = 1 << (OP_BIND_CONN_TO_SESSION - 32) |
+	      1 << (OP_EXCHANGE_ID - 32) |
+	      1 << (OP_CREATE_SESSION - 32) |
+	      1 << (OP_DESTROY_SESSION - 32) |
+	      1 << (OP_DESTROY_CLIENTID - 32)
+};
+
+/*
+ * Select the state protection mode for client `clp' given the server results
+ * from exchange_id in `sp'.
  *
- * Returns zero, a negative errno, or a negative NFS4ERR status code.
+ * Returns 0 on success, negative errno otherwise.
+ */
+static int _nfs4_sp4_select_mode(struct nfs_client *clp,
+				 struct nfs41_state_protection *sp)
+{
+	unsigned int i;
+
+	if (sp->how == SP4_MACH_CRED) {
+		/* Print state protect result */
+		dfprintk(MOUNT, "Server SP4_MACH_CRED support:\n");
+		for (i = 0; i <= LAST_NFS4_OP; i++) {
+			if (test_bit(i, sp->enforce.u.longs))
+				dfprintk(MOUNT, "  enforce op %d\n", i);
+			if (test_bit(i, sp->allow.u.longs))
+				dfprintk(MOUNT, "  allow op %d\n", i);
+		}
+
+		/*
+		 * Minimal mode - state operations are allowed to use machine
+		 * credential.  Note this already happens by default, so the
+		 * client doesn't have to do anything more than the negotiation.
+		 *
+		 * NOTE: we don't care if EXCHANGE_ID is in the list -
+		 *       we're already using the machine cred for exchange_id
+		 *       and will never use a different cred.
+		 */
+		if (test_bit(OP_BIND_CONN_TO_SESSION, sp->enforce.u.longs) &&
+		    test_bit(OP_CREATE_SESSION, sp->enforce.u.longs) &&
+		    test_bit(OP_DESTROY_SESSION, sp->enforce.u.longs) &&
+		    test_bit(OP_DESTROY_CLIENTID, sp->enforce.u.longs)) {
+			dfprintk(MOUNT, "sp4_mach_cred: enabled\n");
+			set_bit(NFS_SP4_MACH_CRED_MINIMAL, &clp->cl_sp4_flags);
+		} else {
+			dfprintk(MOUNT, "sp4_mach_cred: disabled\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * _nfs4_proc_exchange_id()
  *
- * Since the clientid has expired, all compounds using sessions
- * associated with the stale clientid will be returning
- * NFS4ERR_BADSESSION in the sequence operation, and will therefore
- * be in some phase of session reset.
+ * Wrapper for EXCHANGE_ID operation.
  */
-int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
+static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
+	u32 sp4_how)
 {
 	nfs4_verifier verifier;
 	struct nfs41_exchange_id_args args = {
@@ -5971,10 +6023,35 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 		goto out_server_scope;
 	}
 
+	switch (sp4_how) {
+	case SP4_NONE:
+		args.state_protect.how = SP4_NONE;
+		break;
+
+	case SP4_MACH_CRED:
+		args.state_protect.how = SP4_MACH_CRED;
+
+		/* request minumum set of operations */
+		memcpy(args.state_protect.enforce.u.words,
+		       &_nfs4_spo_must_enforce,
+		       sizeof(uint32_t) * NFS4_OP_MAP_NUM_WORDS);
+
+		break;
+
+	default:
+		/* unsupported! */
+		WARN_ON_ONCE(1);
+		status = -EINVAL;
+		goto out_server_scope;
+	}
+
 	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
 	if (status == 0)
 		status = nfs4_check_cl_exchange_flags(res.flags);
 
+	if (status == 0)
+		status = _nfs4_sp4_select_mode(clp, &res.state_protect);
+
 	if (status == 0) {
 		clp->cl_clientid = res.clientid;
 		clp->cl_exchange_flags = (res.flags & ~EXCHGID4_FLAG_CONFIRMED_R);
@@ -6021,6 +6098,35 @@ out:
 	return status;
 }
 
+/*
+ * nfs4_proc_exchange_id()
+ *
+ * Returns zero, a negative errno, or a negative NFS4ERR status code.
+ *
+ * Since the clientid has expired, all compounds using sessions
+ * associated with the stale clientid will be returning
+ * NFS4ERR_BADSESSION in the sequence operation, and will therefore
+ * be in some phase of session reset.
+ *
+ * Will attempt to negotiate SP4_MACH_CRED if krb5i / krb5p auth is used.
+ */
+int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
+{
+	rpc_authflavor_t authflavor = clp->cl_rpcclient->cl_auth->au_flavor;
+	int status;
+
+	/* try SP4_MACH_CRED if krb5i/p	*/
+	if (authflavor == RPC_AUTH_GSS_KRB5I ||
+	    authflavor == RPC_AUTH_GSS_KRB5P) {
+		status = _nfs4_proc_exchange_id(clp, cred, SP4_MACH_CRED);
+		if (!status)
+			return 0;
+	}
+
+	/* try SP4_NONE */
+	return _nfs4_proc_exchange_id(clp, cred, SP4_NONE);
+}
+
 static int _nfs4_proc_destroy_clientid(struct nfs_client *clp,
 		struct rpc_cred *cred)
 {
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 1a4a3bd..0a645f7 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -294,7 +294,9 @@ static int nfs4_stat_to_errno(int);
 				XDR_QUADLEN(NFS4_EXCHANGE_ID_LEN) + \
 				1 /* flags */ + \
 				1 /* spa_how */ + \
-				0 /* SP4_NONE (for now) */ + \
+				/* max is SP4_MACH_CRED (for now) */ + \
+				1 + NFS4_OP_MAP_NUM_WORDS + \
+				1 + NFS4_OP_MAP_NUM_WORDS + \
 				1 /* implementation id array of size 1 */ + \
 				1 /* nii_domain */ + \
 				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
@@ -306,7 +308,9 @@ static int nfs4_stat_to_errno(int);
 				1 /* eir_sequenceid */ + \
 				1 /* eir_flags */ + \
 				1 /* spr_how */ + \
-				0 /* SP4_NONE (for now) */ + \
+				  /* max is SP4_MACH_CRED (for now) */ + \
+				1 + NFS4_OP_MAP_NUM_WORDS + \
+				1 + NFS4_OP_MAP_NUM_WORDS + \
 				2 /* eir_server_owner.so_minor_id */ + \
 				/* eir_server_owner.so_major_id<> */ \
 				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + 1 + \
@@ -1726,6 +1730,14 @@ static void encode_bind_conn_to_session(struct xdr_stream *xdr,
 	*p = 0;	/* use_conn_in_rdma_mode = False */
 }
 
+static void encode_op_map(struct xdr_stream *xdr, struct nfs4_op_map *op_map)
+{
+	unsigned int i;
+	encode_uint32(xdr, NFS4_OP_MAP_NUM_WORDS);
+	for (i = 0; i < NFS4_OP_MAP_NUM_WORDS; i++)
+		encode_uint32(xdr, op_map->u.words[i]);
+}
+
 static void encode_exchange_id(struct xdr_stream *xdr,
 			       struct nfs41_exchange_id_args *args,
 			       struct compound_hdr *hdr)
@@ -1739,9 +1751,19 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 
 	encode_string(xdr, args->id_len, args->id);
 
-	p = reserve_space(xdr, 12);
-	*p++ = cpu_to_be32(args->flags);
-	*p++ = cpu_to_be32(0);	/* zero length state_protect4_a */
+	encode_uint32(xdr, args->flags);
+	encode_uint32(xdr, args->state_protect.how);
+
+	switch (args->state_protect.how) {
+	case SP4_NONE:
+		break;
+	case SP4_MACH_CRED:
+		encode_op_map(xdr, &args->state_protect.enforce);
+		encode_op_map(xdr, &args->state_protect.allow);
+		break;
+	default:
+		BUG();
+	}
 
 	if (send_implementation_id &&
 	    sizeof(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN) > 1 &&
@@ -1752,7 +1774,7 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 			       utsname()->version, utsname()->machine);
 
 	if (len > 0) {
-		*p = cpu_to_be32(1);	/* implementation id array length=1 */
+		encode_uint32(xdr, 1);	/* implementation id array length=1 */
 
 		encode_string(xdr,
 			sizeof(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1,
@@ -1763,7 +1785,7 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 		p = xdr_encode_hyper(p, 0);
 		*p = cpu_to_be32(0);
 	} else
-		*p = cpu_to_be32(0);	/* implementation id array length=0 */
+		encode_uint32(xdr, 0);	/* implementation id array length=0 */
 }
 
 static void encode_create_session(struct xdr_stream *xdr,
@@ -5375,6 +5397,23 @@ static int decode_secinfo_no_name(struct xdr_stream *xdr, struct nfs4_secinfo_re
 	return decode_secinfo_common(xdr, res);
 }
 
+static int decode_op_map(struct xdr_stream *xdr, struct nfs4_op_map *op_map)
+{
+	__be32 *p;
+	uint32_t bitmap_words;
+	unsigned int i;
+
+	p = xdr_inline_decode(xdr, 4);
+	bitmap_words = be32_to_cpup(p++);
+	if (bitmap_words > NFS4_OP_MAP_NUM_WORDS)
+		return -EIO;
+	p = xdr_inline_decode(xdr, 4 * bitmap_words);
+	for (i = 0; i < bitmap_words; i++)
+		op_map->u.words[i] = be32_to_cpup(p++);
+
+	return 0;
+}
+
 static int decode_exchange_id(struct xdr_stream *xdr,
 			      struct nfs41_exchange_id_res *res)
 {
@@ -5398,10 +5437,24 @@ static int decode_exchange_id(struct xdr_stream *xdr,
 	res->seqid = be32_to_cpup(p++);
 	res->flags = be32_to_cpup(p++);
 
-	/* We ask for SP4_NONE */
-	dummy = be32_to_cpup(p);
-	if (dummy != SP4_NONE)
+	res->state_protect.how = be32_to_cpup(p);
+	switch (res->state_protect.how) {
+	case SP4_NONE:
+		break;
+	case SP4_MACH_CRED:
+		status = decode_op_map(xdr, &res->state_protect.enforce);
+		if (status)
+			return status;
+		status = decode_op_map(xdr, &res->state_protect.allow);
+		if (status)
+			return status;
+		break;
+	default:
+		/* completely unexpected since we only ever ask for SP_NONE or
+		 * SP4_MACH_CRED */
+		WARN_ON_ONCE(1);
 		return -EIO;
+	}
 
 	/* server_owner4.so_minor_id */
 	p = xdr_inline_decode(xdr, 8);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index d221243..a0af429 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -87,6 +87,10 @@ struct nfs_client {
 	struct nfs41_server_owner *cl_serverowner;
 	struct nfs41_server_scope *cl_serverscope;
 	struct nfs41_impl_id	*cl_implid;
+	/* nfs 4.1+ state protection modes: */
+	unsigned long		cl_sp4_flags;
+#define NFS_SP4_MACH_CRED_MINIMAL  1	/* Minimal sp4_mach_cred - state ops
+					 * must use machine cred */
 #endif /* CONFIG_NFS_V4 */
 
 #ifdef CONFIG_NFS_FSCACHE
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 8651574..18681d9 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1107,6 +1107,23 @@ struct pnfs_ds_commit_info {
 	struct pnfs_commit_bucket *buckets;
 };
 
+#define NFS4_OP_MAP_NUM_LONGS \
+	DIV_ROUND_UP(LAST_NFS4_OP, 8 * sizeof(unsigned long))
+#define NFS4_OP_MAP_NUM_WORDS \
+	(NFS4_OP_MAP_NUM_LONGS * sizeof(unsigned long) / sizeof(u32))
+struct nfs4_op_map {
+	union {
+		unsigned long longs[NFS4_OP_MAP_NUM_LONGS];
+		u32 words[NFS4_OP_MAP_NUM_WORDS];
+	} u;
+};
+
+struct nfs41_state_protection {
+	u32 how;
+	struct nfs4_op_map enforce;
+	struct nfs4_op_map allow;
+};
+
 #define NFS4_EXCHANGE_ID_LEN	(48)
 struct nfs41_exchange_id_args {
 	struct nfs_client		*client;
@@ -1114,6 +1131,7 @@ struct nfs41_exchange_id_args {
 	unsigned int 			id_len;
 	char 				id[NFS4_EXCHANGE_ID_LEN];
 	u32				flags;
+	struct nfs41_state_protection	state_protect;
 };
 
 struct nfs41_server_owner {
@@ -1146,6 +1164,7 @@ struct nfs41_exchange_id_res {
 	struct nfs41_server_owner	*server_owner;
 	struct nfs41_server_scope	*server_scope;
 	struct nfs41_impl_id		*impl_id;
+	struct nfs41_state_protection	state_protect;
 };
 
 struct nfs41_create_session_args {
-- 
1.7.12.4 (Apple Git-37)

[PATCH 2/5] nfs4.1: add state protection handler

[Weston Andros Adamson]

Add nfs4_state_protect - the function responsible for switching to the machine
credential and the correct rpcclnt when SP4_MACH_CRED is in use.

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4_fs.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index ee81e35..3cf422f 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -273,6 +273,36 @@ is_ds_client(struct nfs_client *clp)
 {
 	return clp->cl_exchange_flags & EXCHGID4_FLAG_USE_PNFS_DS;
 }
+
+/*
+ * Function responsible for determining if an rpc_message should use the
+ * machine cred under SP4_MACH_CRED and if so switching the credential and
+ * authflavor (using the nfs_client's rpc_clnt which will be krb5i/p).
+ */
+static inline void
+nfs4_state_protect(struct nfs_client *clp, unsigned long sp4_mode,
+		   struct rpc_clnt **clntp, struct rpc_message *msg)
+{
+	struct rpc_cred *newcred = NULL;
+	rpc_authflavor_t flavor;
+
+	if (test_bit(sp4_mode, &clp->cl_sp4_flags)) {
+		dfprintk(PROC, "NFS: using machine cred for operation %s\n",
+			msg->rpc_proc->p_name);
+		spin_lock(&clp->cl_lock);
+		if (clp->cl_machine_cred != NULL)
+			newcred = get_rpccred(clp->cl_machine_cred);
+		spin_unlock(&clp->cl_lock);
+		if (msg->rpc_cred)
+			put_rpccred(msg->rpc_cred);
+		msg->rpc_cred = newcred;
+
+		flavor = clp->cl_rpcclient->cl_auth->au_flavor;
+		WARN_ON(flavor != RPC_AUTH_GSS_KRB5I &&
+			flavor != RPC_AUTH_GSS_KRB5P);
+		*clntp = clp->cl_rpcclient;
+	}
+}
 #else /* CONFIG_NFS_v4_1 */
 static inline struct nfs4_session *nfs4_get_session(const struct nfs_server *server)
 {
@@ -298,6 +328,12 @@ is_ds_client(struct nfs_client *clp)
 {
 	return false;
 }
+
+static inline void
+nfs4_state_protect(struct nfs_client *clp, unsigned long sp4_flags,
+		   struct rpc_clnt **clntp, struct rpc_message *msg)
+{
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 extern const struct nfs4_minor_version_ops *nfs_v4_minor_ops[];
-- 
1.7.12.4 (Apple Git-37)

[PATCH 3/5] nfs4.1: Add SP4_MACH_CRED cleanup support

[Weston Andros Adamson]

CLOSE and LOCKU use the machine cred

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c         | 28 +++++++++++++++++++++++++++-
 include/linux/nfs_fs_sb.h |  1 +
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index faa3731..63b6e17 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2511,6 +2511,9 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait)
 	};
 	int status = -ENOMEM;
 
+	nfs4_state_protect(server->nfs_client, NFS_SP4_MACH_CRED_CLEANUP,
+		&task_setup_data.rpc_client, &msg);
+
 	calldata = kzalloc(sizeof(*calldata), gfp_mask);
 	if (calldata == NULL)
 		goto out;
@@ -5093,6 +5096,9 @@ static struct rpc_task *nfs4_do_unlck(struct file_lock *fl,
 		.flags = RPC_TASK_ASYNC,
 	};
 
+	nfs4_state_protect(NFS_SERVER(lsp->ls_state->inode)->nfs_client,
+		NFS_SP4_MACH_CRED_CLEANUP, &task_setup_data.rpc_client, &msg);
+
 	/* Ensure this is an unlock - when canceling a lock, the
 	 * canceled lock is passed in, and it won't be an unlock.
 	 */
@@ -5926,6 +5932,15 @@ static const uint32_t _nfs4_spo_must_enforce[NFS4_OP_MAP_NUM_WORDS] = {
 };
 
 /*
+ * Operations we'd like to see to enable certain features
+ */
+static const uint32_t _nfs4_spo_must_allow[NFS4_OP_MAP_NUM_WORDS] = {
+	[0] = 1 << (OP_CLOSE) |
+	      1 << (OP_LOCKU)
+};
+
+
+/*
  * Select the state protection mode for client `clp' given the server results
  * from exchange_id in `sp'.
  *
@@ -5965,6 +5980,12 @@ static int _nfs4_sp4_select_mode(struct nfs_client *clp,
 			dfprintk(MOUNT, "sp4_mach_cred: disabled\n");
 			return -EINVAL;
 		}
+
+		if (test_bit(OP_CLOSE, sp->allow.u.longs) &&
+		    test_bit(OP_LOCKU, sp->allow.u.longs)) {
+			dfprintk(MOUNT, "  cleanup mode enabled\n");
+			set_bit(NFS_SP4_MACH_CRED_CLEANUP, &clp->cl_sp4_flags);
+		}
 	}
 
 	return 0;
@@ -6031,11 +6052,16 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	case SP4_MACH_CRED:
 		args.state_protect.how = SP4_MACH_CRED;
 
-		/* request minumum set of operations */
+		/* request enforcement of minumum set of operations */
 		memcpy(args.state_protect.enforce.u.words,
 		       &_nfs4_spo_must_enforce,
 		       sizeof(uint32_t) * NFS4_OP_MAP_NUM_WORDS);
 
+		/* ask for any other ops we may need for SP4 features */
+		memcpy(args.state_protect.allow.u.words,
+		       &_nfs4_spo_must_allow,
+		       sizeof(uint32_t) * NFS4_OP_MAP_NUM_WORDS);
+
 		break;
 
 	default:
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index a0af429..7da24e0 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -91,6 +91,7 @@ struct nfs_client {
 	unsigned long		cl_sp4_flags;
 #define NFS_SP4_MACH_CRED_MINIMAL  1	/* Minimal sp4_mach_cred - state ops
 					 * must use machine cred */
+#define NFS_SP4_MACH_CRED_CLEANUP  2	/* CLOSE and LOCKU */
 #endif /* CONFIG_NFS_V4 */
 
 #ifdef CONFIG_NFS_FSCACHE
-- 
1.7.12.4 (Apple Git-37)

[PATCH 4/5] nfs4.1: Add SP4_MACH_CRED secinfo support

[Weston Andros Adamson]

SECINFO and SECINFO_NONAME use the machine cred

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c         | 17 +++++++++++++++--
 include/linux/nfs_fs_sb.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 63b6e17..d4aa158 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5815,9 +5815,14 @@ static int _nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, struct
 		.rpc_argp = &args,
 		.rpc_resp = &res,
 	};
+	struct rpc_clnt *rpc_client = NFS_SERVER(dir)->client;
+
+	nfs4_state_protect(NFS_SERVER(dir)->nfs_client,
+		NFS_SP4_MACH_CRED_SECINFO, &rpc_client, &msg);
 
 	dprintk("NFS call  secinfo %s\n", name->name);
-	status = nfs4_call_sync(NFS_SERVER(dir)->client, NFS_SERVER(dir), &msg, &args.seq_args, &res.seq_res, 0);
+	status = nfs4_call_sync(rpc_client, NFS_SERVER(dir), &msg,
+				&args.seq_args, &res.seq_res, 0);
 	dprintk("NFS reply  secinfo: %d\n", status);
 	return status;
 }
@@ -5936,7 +5941,9 @@ static const uint32_t _nfs4_spo_must_enforce[NFS4_OP_MAP_NUM_WORDS] = {
  */
 static const uint32_t _nfs4_spo_must_allow[NFS4_OP_MAP_NUM_WORDS] = {
 	[0] = 1 << (OP_CLOSE) |
-	      1 << (OP_LOCKU)
+	      1 << (OP_LOCKU),
+	[1] = 1 << (OP_SECINFO - 32) |
+	      1 << (OP_SECINFO_NO_NAME - 32)
 };
 
 
@@ -5986,6 +5993,12 @@ static int _nfs4_sp4_select_mode(struct nfs_client *clp,
 			dfprintk(MOUNT, "  cleanup mode enabled\n");
 			set_bit(NFS_SP4_MACH_CRED_CLEANUP, &clp->cl_sp4_flags);
 		}
+
+		if (test_bit(OP_SECINFO, sp->allow.u.longs) &&
+		    test_bit(OP_SECINFO_NO_NAME, sp->allow.u.longs)) {
+			dfprintk(MOUNT, "  secinfo mode enabled\n");
+			set_bit(NFS_SP4_MACH_CRED_SECINFO, &clp->cl_sp4_flags);
+		}
 	}
 
 	return 0;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 7da24e0..02fa36b 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -92,6 +92,7 @@ struct nfs_client {
 #define NFS_SP4_MACH_CRED_MINIMAL  1	/* Minimal sp4_mach_cred - state ops
 					 * must use machine cred */
 #define NFS_SP4_MACH_CRED_CLEANUP  2	/* CLOSE and LOCKU */
+#define NFS_SP4_MACH_CRED_SECINFO  3	/* SECINFO and SECINFO_NONAME */
 #endif /* CONFIG_NFS_V4 */
 
 #ifdef CONFIG_NFS_FSCACHE
-- 
1.7.12.4 (Apple Git-37)

[PATCH 5/5] nfs4.1: Add SP4_MACH_CRED stateid support

[Weston Andros Adamson]

TEST_STATEID and FREE_STATEID use the machine cred

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c         | 19 +++++++++++++++++--
 include/linux/nfs_fs_sb.h |  1 +
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d4aa158..7dee833 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5943,7 +5943,9 @@ static const uint32_t _nfs4_spo_must_allow[NFS4_OP_MAP_NUM_WORDS] = {
 	[0] = 1 << (OP_CLOSE) |
 	      1 << (OP_LOCKU),
 	[1] = 1 << (OP_SECINFO - 32) |
-	      1 << (OP_SECINFO_NO_NAME - 32)
+	      1 << (OP_SECINFO_NO_NAME - 32) |
+	      1 << (OP_TEST_STATEID - 32) |
+	      1 << (OP_FREE_STATEID - 32)
 };
 
 
@@ -5999,6 +6001,12 @@ static int _nfs4_sp4_select_mode(struct nfs_client *clp,
 			dfprintk(MOUNT, "  secinfo mode enabled\n");
 			set_bit(NFS_SP4_MACH_CRED_SECINFO, &clp->cl_sp4_flags);
 		}
+
+		if (test_bit(OP_TEST_STATEID, sp->allow.u.longs) &&
+		    test_bit(OP_FREE_STATEID, sp->allow.u.longs)) {
+			dfprintk(MOUNT, "  stateid mode enabled\n");
+			set_bit(NFS_SP4_MACH_CRED_STATEID, &clp->cl_sp4_flags);
+		}
 	}
 
 	return 0;
@@ -7325,11 +7333,15 @@ static int _nfs41_test_stateid(struct nfs_server *server,
 		.rpc_resp = &res,
 		.rpc_cred = cred,
 	};
+	struct rpc_clnt *rpc_client = server->client;
+
+	nfs4_state_protect(server->nfs_client, NFS_SP4_MACH_CRED_STATEID,
+		&rpc_client, &msg);
 
 	dprintk("NFS call  test_stateid %p\n", stateid);
 	nfs41_init_sequence(&args.seq_args, &res.seq_res, 0);
 	nfs4_set_sequence_privileged(&args.seq_args);
-	status = nfs4_call_sync_sequence(server->client, server, &msg,
+	status = nfs4_call_sync_sequence(rpc_client, server, &msg,
 			&args.seq_args, &res.seq_res);
 	if (status != NFS_OK) {
 		dprintk("NFS reply test_stateid: failed, %d\n", status);
@@ -7421,6 +7433,9 @@ static struct rpc_task *_nfs41_free_stateid(struct nfs_server *server,
 	};
 	struct nfs_free_stateid_data *data;
 
+	nfs4_state_protect(server->nfs_client, NFS_SP4_MACH_CRED_STATEID,
+		&task_setup.rpc_client, &msg);
+
 	dprintk("NFS call  free_stateid %p\n", stateid);
 	data = kmalloc(sizeof(*data), GFP_NOFS);
 	if (!data)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 02fa36b..8ef3371 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -93,6 +93,7 @@ struct nfs_client {
 					 * must use machine cred */
 #define NFS_SP4_MACH_CRED_CLEANUP  2	/* CLOSE and LOCKU */
 #define NFS_SP4_MACH_CRED_SECINFO  3	/* SECINFO and SECINFO_NONAME */
+#define NFS_SP4_MACH_CRED_STATEID  4	/* TEST_STATEID and FREE_STATEID */
 #endif /* CONFIG_NFS_V4 */
 
 #ifdef CONFIG_NFS_FSCACHE
-- 
1.7.12.4 (Apple Git-37)

Re: [PATCH 0/5] nfs4.1: Initial SP4_MACH_CRED implementation

[Adamson, Dros]

I've decided to change a few things since posting this, so unless someone is in the middle of reviewing, please wait for the new patchset.

Thanks!

-dros

On Aug 6, 2013, at 5:08 PM, Weston Andros Adamson <dros@netapp.com> wrote:

> These patches are the initial client-side SP4_MACH_CRED implementation.
> 
> Patch 1 is a repost with a few cleanups
> 
> Patch 2 adds the sp4 handler that replaces the cred on an rpc message (and
> picks the right rpc_clnt).  Note that for now, we always use the machine cred
> if supported - we could only use it if the user cred expires, but this is
> simple and spec compliant.
> 
> Patch 3-5 implement SP4_MACH_CRED "features":
>  - cleanup - CLOSE and LOCKU use machine cred
>  - secinfo - SECINFO and SECINFO_NO_NAME use machine cred
>  - stateid - TEST_STATEID and FREE_STATEID use machine cred
> 
> There are three more features that I've been working on, but need to be tested
> before I post:
>  - commit - COMMIT uses machine cred
>  - write - WRITE uses machine cred, forces stable writes unless commit feature
>            is also enabled
>  - recovery - OPEN and LOCK can use machine cred - only in recovery.
> 
> The commit and write cases should be easy to test against a hacked nfsd, but
> I need to think more about how to test the recovery stuff...
> 
> -dros
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] nfs4.1: Initial SP4_MACH_CRED implementation

[J. Bruce Fields]

On Tue, Aug 06, 2013 at 05:08:26PM -0400, Weston Andros Adamson wrote:
> These patches are the initial client-side SP4_MACH_CRED implementation.
> 
> Patch 1 is a repost with a few cleanups
> 
> Patch 2 adds the sp4 handler that replaces the cred on an rpc message (and
> picks the right rpc_clnt).  Note that for now, we always use the machine cred
> if supported - we could only use it if the user cred expires, but this is
> simple and spec compliant.

So anything on the spo_may_enforce_list will *always* use the machine
cred?

I wonder....  This is within the letter of the spec, but the spec also
does also give the motivation.  ("The purpose of spo_must_allow is to
allow clients to solve the following conundrum....")

I'd worry that server implementors will also stick to the letter of the
spec and allow this, but also design under the assumption that this is a
rare case.  (E.g., allow the access but trigger some extra auditing??)

I'd probably try it this way first too, as you say, it's simpler, but I
wonder whether it's really what we want to client doing all the time.

> Patch 3-5 implement SP4_MACH_CRED "features":
>   - cleanup - CLOSE and LOCKU use machine cred
>   - secinfo - SECINFO and SECINFO_NO_NAME use machine cred
>   - stateid - TEST_STATEID and FREE_STATEID use machine cred
> 
> There are three more features that I've been working on, but need to be tested
> before I post:
>   - commit - COMMIT uses machine cred
>   - write - WRITE uses machine cred, forces stable writes unless commit feature
>             is also enabled
>   - recovery - OPEN and LOCK can use machine cred - only in recovery.
> 
> The commit and write cases should be easy to test against a hacked nfsd, but
> I need to think more about how to test the recovery stuff...

Can't you just hack nfsd to allow those ops and then do a server
restart, or try Bryan's fault-injection interface if you want to test
some more exotic recovery scenario?

--b.

Re: [PATCH 0/5] nfs4.1: Initial SP4_MACH_CRED implementation

[Adamson, Dros]

On Aug 8, 2013, at 11:56 AM, J. Bruce Fields <bfields@fieldses.org> wrote:

> On Tue, Aug 06, 2013 at 05:08:26PM -0400, Weston Andros Adamson wrote:
>> These patches are the initial client-side SP4_MACH_CRED implementation.
>> 
>> Patch 1 is a repost with a few cleanups
>> 
>> Patch 2 adds the sp4 handler that replaces the cred on an rpc message (and
>> picks the right rpc_clnt).  Note that for now, we always use the machine cred
>> if supported - we could only use it if the user cred expires, but this is
>> simple and spec compliant.
> 
> So anything on the spo_may_enforce_list will *always* use the machine
> cred?
> 
> I wonder....  This is within the letter of the spec, but the spec also
> does also give the motivation.  ("The purpose of spo_must_allow is to
> allow clients to solve the following conundrum....")
> 
> I'd worry that server implementors will also stick to the letter of the
> spec and allow this, but also design under the assumption that this is a
> rare case.  (E.g., allow the access but trigger some extra auditing??)
> 
> I'd probably try it this way first too, as you say, it's simpler, but I
> wonder whether it's really what we want to client doing all the time.

OK, I agree.  I'll add that to my current patchset.

> 
>> Patch 3-5 implement SP4_MACH_CRED "features":
>>  - cleanup - CLOSE and LOCKU use machine cred
>>  - secinfo - SECINFO and SECINFO_NO_NAME use machine cred
>>  - stateid - TEST_STATEID and FREE_STATEID use machine cred
>> 
>> There are three more features that I've been working on, but need to be tested
>> before I post:
>>  - commit - COMMIT uses machine cred
>>  - write - WRITE uses machine cred, forces stable writes unless commit feature
>>            is also enabled
>>  - recovery - OPEN and LOCK can use machine cred - only in recovery.
>> 
>> The commit and write cases should be easy to test against a hacked nfsd, but
>> I need to think more about how to test the recovery stuff...
> 
> Can't you just hack nfsd to allow those ops and then do a server
> restart, or try Bryan's fault-injection interface if you want to test
> some more exotic recovery scenario?
> 

Yes - I'm working on testing the client side code now.  Great idea about Bryan's fault injection framework! If I need some extra hooks I'll add them and send patches. 
Also, I've completed testing WRITE and COMMIT against a hacked nfsd and will include this patch in the repost.

-dros

> --b.