[PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[Andrew Elble]

The majority of our lost delegation problems come from the inability
to return the delegations when the credentials that obtained them
have expired (conundrum mentioned in RFC 5661 18.35.3)

ObMemoryJog:

http://www.spinics.net/lists/linux-nfs/msg56230.html

v3: removal of OP_IS_PUTFH_LIKE checks
    added clarification of addressing the conundrum

Andrew Elble (3):
  nfs/nfsd: Move useful bitfield ops to a commonly accessible place
  nfsd: allow mach_creds_match to be used more broadly
  nfsd: implement machine credential support for some operations

 fs/nfsd/export.c        | 10 ++++++++++
 fs/nfsd/nfs4proc.c      | 39 +++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4state.c     | 32 ++++++++++++++++++++++++-------
 fs/nfsd/nfs4xdr.c       | 51 ++++++++++++++++++++++---------------------------
 fs/nfsd/nfsd.h          |  5 +++++
 fs/nfsd/state.h         |  1 +
 fs/nfsd/xdr4.h          |  5 +++++
 include/linux/nfs4.h    | 11 +++++++++++
 include/linux/nfs_xdr.h | 11 -----------
 9 files changed, 119 insertions(+), 46 deletions(-)

-- 
2.6.3

[PATCH v3 1/3] nfs/nfsd: Move useful bitfield ops to a commonly accessible place

[Andrew Elble]

So these may be used in nfsd as well

Signed-off-by: Andrew Elble <aweits@rit.edu>
---
 include/linux/nfs4.h    | 11 +++++++++++
 include/linux/nfs_xdr.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index bfed6b367350..c6564ada9beb 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -643,4 +643,15 @@ enum pnfs_update_layout_reason {
 	PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET,
 };
 
+#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;
+};
+
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index c304a11b5b1a..e66abc2d1f88 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1185,17 +1185,6 @@ 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;
-- 
2.6.3

[PATCH v3 2/3] nfsd: allow mach_creds_match to be used more broadly

[Andrew Elble]

Rename mach_creds_match() to nfsd4_mach_creds_match() and un-staticify

Signed-off-by: Andrew Elble <aweits@rit.edu>
---
 fs/nfsd/nfs4state.c | 14 +++++++-------
 fs/nfsd/xdr4.h      |  2 ++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f5f82e145018..51db26811a14 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1972,7 +1972,7 @@ static bool svc_rqst_integrity_protected(struct svc_rqst *rqstp)
 	       service == RPC_GSS_SVC_PRIVACY;
 }
 
-static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
+bool nfsd4_mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
 {
 	struct svc_cred *cr = &rqstp->rq_cred;
 
@@ -2424,7 +2424,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 				status = nfserr_inval;
 				goto out;
 			}
-			if (!mach_creds_match(conf, rqstp)) {
+			if (!nfsd4_mach_creds_match(conf, rqstp)) {
 				status = nfserr_wrong_cred;
 				goto out;
 			}
@@ -2676,7 +2676,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 
 	if (conf) {
 		status = nfserr_wrong_cred;
-		if (!mach_creds_match(conf, rqstp))
+		if (!nfsd4_mach_creds_match(conf, rqstp))
 			goto out_free_conn;
 		cs_slot = &conf->cl_cs_slot;
 		status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
@@ -2692,7 +2692,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 			goto out_free_conn;
 		}
 		status = nfserr_wrong_cred;
-		if (!mach_creds_match(unconf, rqstp))
+		if (!nfsd4_mach_creds_match(unconf, rqstp))
 			goto out_free_conn;
 		cs_slot = &unconf->cl_cs_slot;
 		status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
@@ -2801,7 +2801,7 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
 	if (!session)
 		goto out_no_session;
 	status = nfserr_wrong_cred;
-	if (!mach_creds_match(session->se_client, rqstp))
+	if (!nfsd4_mach_creds_match(session->se_client, rqstp))
 		goto out;
 	status = nfsd4_map_bcts_dir(&bcts->dir);
 	if (status)
@@ -2848,7 +2848,7 @@ nfsd4_destroy_session(struct svc_rqst *r,
 	if (!ses)
 		goto out_client_lock;
 	status = nfserr_wrong_cred;
-	if (!mach_creds_match(ses->se_client, r))
+	if (!nfsd4_mach_creds_match(ses->se_client, r))
 		goto out_put_session;
 	status = mark_session_dead_locked(ses, 1 + ref_held_by_me);
 	if (status)
@@ -3087,7 +3087,7 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
 		status = nfserr_stale_clientid;
 		goto out;
 	}
-	if (!mach_creds_match(clp, rqstp)) {
+	if (!nfsd4_mach_creds_match(clp, rqstp)) {
 		clp = NULL;
 		status = nfserr_wrong_cred;
 		goto out;
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index d9554813e58a..74342a7c208a 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -654,6 +654,8 @@ set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp)
 
 }
 
+
+bool nfsd4_mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp);
 int nfs4svc_encode_voidres(struct svc_rqst *, __be32 *, void *);
 int nfs4svc_decode_compoundargs(struct svc_rqst *, __be32 *,
 		struct nfsd4_compoundargs *);
-- 
2.6.3

[PATCH v3 3/3] nfsd: implement machine credential support for some operations

[Andrew Elble]

This addresses the conundrum referenced in RFC5661 18.35.3,
and will allow clients to return state to the server using the
machine credentials.

The biggest part of the problem is that we need to allow the client
to send a compound op with integrity/privacy on mounts that don't
have it enabled.

Add server support for properly decoding and using spo_must_enforce
and spo_must_allow bits. Add support for machine credentials to be
used for CLOSE, OPEN_DOWNGRADE, LOCKU, DELEGRETURN,
and TEST/FREE STATEID.
Implement a check so as to not throw WRONGSEC errors when these
operations are used if integrity/privacy isn't turned on.

Signed-off-by: Andrew Elble <aweits@rit.edu>
---
 fs/nfsd/export.c    | 10 ++++++++++
 fs/nfsd/nfs4proc.c  | 39 +++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4state.c | 18 ++++++++++++++++++
 fs/nfsd/nfs4xdr.c   | 51 +++++++++++++++++++++++----------------------------
 fs/nfsd/nfsd.h      |  5 +++++
 fs/nfsd/state.h     |  1 +
 fs/nfsd/xdr4.h      |  3 +++
 7 files changed, 99 insertions(+), 28 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index b4d84b579f20..79de2f38dd63 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -954,6 +954,16 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 		    rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)
 			return 0;
 	}
+
+	/* If the compound op contains a spo_must_allowed op,
+	 * it will be sent with integrity/protection which
+	 * will have to be expressly allowed on mounts that
+	 * don't support it
+	 */
+
+	if (nfsd4_spo_must_allow(rqstp))
+		return 0;
+
 	return nfserr_wrongsec;
 }
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index de1ff1d98bb1..b1159b3e9816 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2335,6 +2335,45 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 };
 
+/**
+ * nfsd4_spo_must_allow - Determine if the compound op contains an
+ * operation that is allowed to be sent with machine credentials
+ *
+ * @rqstp: a pointer to the struct svc_rqst
+ *
+ * Checks to see if the compound contains a spo_must_allow op
+ * and confirms that it was sent with the proper machine creds.
+ */
+
+bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
+{
+	struct nfsd4_compoundres *resp = rqstp->rq_resp;
+	struct nfsd4_compoundargs *argp = rqstp->rq_argp;
+	struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
+	struct nfsd4_compound_state *cstate = &resp->cstate;
+	struct nfs4_op_map *allow = &cstate->clp->cl_spo_must_allow;
+	u32 opiter;
+
+	if (!cstate->minorversion)
+		return false;
+
+	if (cstate->spo_must_allowed == true)
+		return true;
+
+	opiter = resp->opcnt;
+	while (opiter < argp->opcnt) {
+		this = &argp->ops[opiter++];
+		if (test_bit(this->opnum, allow->u.longs) &&
+			cstate->clp->cl_mach_cred &&
+			nfsd4_mach_creds_match(cstate->clp, rqstp)) {
+			cstate->spo_must_allowed = true;
+			return true;
+		}
+	}
+	cstate->spo_must_allowed = false;
+	return false;
+}
+
 int nfsd4_max_reply(struct svc_rqst *rqstp, struct nfsd4_op *op)
 {
 	struct nfsd4_operation *opdesc;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 51db26811a14..313873de1eb7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2388,6 +2388,22 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 
 	switch (exid->spa_how) {
 	case SP4_MACH_CRED:
+		exid->spo_must_enforce[0] = 0;
+		exid->spo_must_enforce[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));
+
+		exid->spo_must_allow[0] &= (1 << (OP_CLOSE) |
+					1 << (OP_OPEN_DOWNGRADE) |
+					1 << (OP_LOCKU) |
+					1 << (OP_DELEGRETURN));
+
+		exid->spo_must_allow[1] &= (
+					1 << (OP_TEST_STATEID - 32) |
+					1 << (OP_FREE_STATEID - 32));
 		if (!svc_rqst_integrity_protected(rqstp)) {
 			status = nfserr_inval;
 			goto out_nolock;
@@ -2473,6 +2489,8 @@ out_new:
 			goto out;
 	}
 	new->cl_minorversion = cstate->minorversion;
+	new->cl_spo_must_allow.u.words[0] = exid->spo_must_allow[0];
+	new->cl_spo_must_allow.u.words[1] = exid->spo_must_allow[1];
 
 	gen_clid(new, nn);
 	add_to_unconfirmed(new);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9df898ba648f..84ef94794496 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1299,16 +1299,14 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
 		break;
 	case SP4_MACH_CRED:
 		/* spo_must_enforce */
-		READ_BUF(4);
-		dummy = be32_to_cpup(p++);
-		READ_BUF(dummy * 4);
-		p += dummy;
-
+		status = nfsd4_decode_bitmap(argp,
+					exid->spo_must_enforce);
+		if (status)
+			goto out;
 		/* spo_must_allow */
-		READ_BUF(4);
-		dummy = be32_to_cpup(p++);
-		READ_BUF(dummy * 4);
-		p += dummy;
+		status = nfsd4_decode_bitmap(argp, exid->spo_must_allow);
+		if (status)
+			goto out;
 		break;
 	case SP4_SSV:
 		/* ssp_ops */
@@ -3867,14 +3865,6 @@ nfsd4_encode_write(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_w
 	return nfserr;
 }
 
-static const u32 nfs4_minimal_spo_must_enforce[2] = {
-	[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)
-};
-
 static __be32
 nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 			 struct nfsd4_exchange_id *exid)
@@ -3885,6 +3875,7 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 	char *server_scope;
 	int major_id_sz;
 	int server_scope_sz;
+	int status = 0;
 	uint64_t minor_id = 0;
 
 	if (nfserr)
@@ -3913,18 +3904,20 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 	case SP4_NONE:
 		break;
 	case SP4_MACH_CRED:
-		/* spo_must_enforce, spo_must_allow */
-		p = xdr_reserve_space(xdr, 16);
-		if (!p)
-			return nfserr_resource;
-
 		/* spo_must_enforce bitmap: */
-		*p++ = cpu_to_be32(2);
-		*p++ = cpu_to_be32(nfs4_minimal_spo_must_enforce[0]);
-		*p++ = cpu_to_be32(nfs4_minimal_spo_must_enforce[1]);
-		/* empty spo_must_allow bitmap: */
-		*p++ = cpu_to_be32(0);
-
+		status = nfsd4_encode_bitmap(xdr,
+					exid->spo_must_enforce[0],
+					exid->spo_must_enforce[1],
+					exid->spo_must_enforce[2]);
+		if (status)
+			goto out;
+		/* spo_must_allow bitmap: */
+		status = nfsd4_encode_bitmap(xdr,
+					exid->spo_must_allow[0],
+					exid->spo_must_allow[1],
+					exid->spo_must_allow[2]);
+		if (status)
+			goto out;
 		break;
 	default:
 		WARN_ON_ONCE(1);
@@ -3951,6 +3944,8 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 	/* Implementation id */
 	*p++ = cpu_to_be32(0);	/* zero length nfs_impl_id4 array */
 	return 0;
+out:
+	return status;
 }
 
 static __be32
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index cf980523898b..9446849888d5 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -124,6 +124,7 @@ void nfs4_state_shutdown_net(struct net *net);
 void nfs4_reset_lease(time_t leasetime);
 int nfs4_reset_recoverydir(char *recdir);
 char * nfs4_recoverydir(void);
+bool nfsd4_spo_must_allow(struct svc_rqst *rqstp);
 #else
 static inline int nfsd4_init_slabs(void) { return 0; }
 static inline void nfsd4_free_slabs(void) { }
@@ -134,6 +135,10 @@ static inline void nfs4_state_shutdown_net(struct net *net) { }
 static inline void nfs4_reset_lease(time_t leasetime) { }
 static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
 static inline char * nfs4_recoverydir(void) {return NULL; }
+static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
+{
+	return false;
+}
 #endif
 
 /*
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 986e51e5ceac..7049a8a559c5 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -345,6 +345,7 @@ struct nfs4_client {
 	u32			cl_exchange_flags;
 	/* number of rpc's in progress over an associated session: */
 	atomic_t		cl_refcount;
+	struct nfs4_op_map      cl_spo_must_allow;
 
 	/* for nfs41 callbacks */
 	/* We currently support a single back channel with a single slot */
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 74342a7c208a..beea0c5edc51 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -59,6 +59,7 @@ struct nfsd4_compound_state {
 	struct nfsd4_session	*session;
 	struct nfsd4_slot	*slot;
 	int			data_offset;
+	bool                    spo_must_allowed;
 	size_t			iovlen;
 	u32			minorversion;
 	__be32			status;
@@ -403,6 +404,8 @@ struct nfsd4_exchange_id {
 	clientid_t	clientid;
 	u32		seqid;
 	int		spa_how;
+	u32             spo_must_enforce[3];
+	u32             spo_must_allow[3];
 };
 
 struct nfsd4_sequence {
-- 
2.6.3

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[J. Bruce Fields]

Looks good, thanks.

On Wed, Jun 15, 2016 at 12:52:06PM -0400, Andrew Elble wrote:
> The majority of our lost delegation problems come from the inability
> to return the delegations when the credentials that obtained them
> have expired (conundrum mentioned in RFC 5661 18.35.3)

I take it you've tested these and verified delegations aren't getting
lost?  Are there client changes needed as well, or is the client already
doing the right thing?

--b.

> 
> ObMemoryJog:
> 
> http://www.spinics.net/lists/linux-nfs/msg56230.html
> 
> v3: removal of OP_IS_PUTFH_LIKE checks
>     added clarification of addressing the conundrum
> 
> Andrew Elble (3):
>   nfs/nfsd: Move useful bitfield ops to a commonly accessible place
>   nfsd: allow mach_creds_match to be used more broadly
>   nfsd: implement machine credential support for some operations
> 
>  fs/nfsd/export.c        | 10 ++++++++++
>  fs/nfsd/nfs4proc.c      | 39 +++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4state.c     | 32 ++++++++++++++++++++++++-------
>  fs/nfsd/nfs4xdr.c       | 51 ++++++++++++++++++++++---------------------------
>  fs/nfsd/nfsd.h          |  5 +++++
>  fs/nfsd/state.h         |  1 +
>  fs/nfsd/xdr4.h          |  5 +++++
>  include/linux/nfs4.h    | 11 +++++++++++
>  include/linux/nfs_xdr.h | 11 -----------
>  9 files changed, 119 insertions(+), 46 deletions(-)
> 
> -- 
> 2.6.3

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[J. Bruce Fields]

On Wed, Jun 15, 2016 at 03:21:16PM -0400, J. Bruce Fields wrote:
> Looks good, thanks.
> 
> On Wed, Jun 15, 2016 at 12:52:06PM -0400, Andrew Elble wrote:
> > The majority of our lost delegation problems come from the inability
> > to return the delegations when the credentials that obtained them
> > have expired (conundrum mentioned in RFC 5661 18.35.3)
> 
> I take it you've tested these and verified delegations aren't getting
> lost?  Are there client changes needed as well, or is the client already
> doing the right thing?

(Also: remind me what they symptoms ("lost delegation problems")
actually were from the point of view of users?  Does the client end up
resending failing DELEGRETURNS and failing to make progress?)

--b.

> 
> --b.
> 
> > 
> > ObMemoryJog:
> > 
> > http://www.spinics.net/lists/linux-nfs/msg56230.html
> > 
> > v3: removal of OP_IS_PUTFH_LIKE checks
> >     added clarification of addressing the conundrum
> > 
> > Andrew Elble (3):
> >   nfs/nfsd: Move useful bitfield ops to a commonly accessible place
> >   nfsd: allow mach_creds_match to be used more broadly
> >   nfsd: implement machine credential support for some operations
> > 
> >  fs/nfsd/export.c        | 10 ++++++++++
> >  fs/nfsd/nfs4proc.c      | 39 +++++++++++++++++++++++++++++++++++++
> >  fs/nfsd/nfs4state.c     | 32 ++++++++++++++++++++++++-------
> >  fs/nfsd/nfs4xdr.c       | 51 ++++++++++++++++++++++---------------------------
> >  fs/nfsd/nfsd.h          |  5 +++++
> >  fs/nfsd/state.h         |  1 +
> >  fs/nfsd/xdr4.h          |  5 +++++
> >  include/linux/nfs4.h    | 11 +++++++++++
> >  include/linux/nfs_xdr.h | 11 -----------
> >  9 files changed, 119 insertions(+), 46 deletions(-)
> > 
> > -- 
> > 2.6.3

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[Andrew W Elble]

> (Also: remind me what they symptoms ("lost delegation problems")
> actually were from the point of view of users?  Does the client end up
> resending failing DELEGRETURNS and failing to make progress?)

Client endlessly loops in state recovery with no path to recovery.
I'm guessing Jason is seeing some of this as well.

Thanks,

Andy

-- 
Andrew W. Elble
aweits@discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[J. Bruce Fields]

On Wed, Jun 15, 2016 at 04:13:58PM -0400, Andrew W Elble wrote:
> 
> > (Also: remind me what they symptoms ("lost delegation problems")
> > actually were from the point of view of users?  Does the client end up
> > resending failing DELEGRETURNS and failing to make progress?)
> 
> Client endlessly loops in state recovery with no path to recovery.
> I'm guessing Jason is seeing some of this as well.

OK.  Just added a note at the end of the changelog saying "Without this,
Linux clients with credentials that expired while holding
delegations were getting stuck in an endless loop."--in hopes that might
help somebody recognize their problem.

--b.

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[Andrew W Elble]

> I take it you've tested these and verified delegations aren't getting
> lost?

These patches have been running in our environment (based on 4.1.22)
for quite a bit of time now with no lost delegations.

> Are there client changes needed as well, or is the client already
> doing the right thing?

Change for client was taken as commit 99ade3c71b1e40e7174d6527709399a87f3d05e0

Thanks,

Andy

-- 
Andrew W. Elble
aweits@discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[J. Bruce Fields]

On Wed, Jun 15, 2016 at 03:36:48PM -0400, Andrew W Elble wrote:
> 
> > I take it you've tested these and verified delegations aren't getting
> > lost?
> 
> These patches have been running in our environment (based on 4.1.22)
> for quite a bit of time now with no lost delegations.
> 
> > Are there client changes needed as well, or is the client already
> > doing the right thing?
> 
> Change for client was taken as commit 99ade3c71b1e40e7174d6527709399a87f3d05e0

Got it, thanks.--b.

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[Jeff Layton]

On Wed, 2016-06-15 at 12:52 -0400, Andrew Elble wrote:
> The majority of our lost delegation problems come from the inability
> to return the delegations when the credentials that obtained them
> have expired (conundrum mentioned in RFC 5661 18.35.3)
> 
> ObMemoryJog:
> 
> http://www.spinics.net/lists/linux-nfs/msg56230.html
> 
> v3: removal of OP_IS_PUTFH_LIKE checks
>     added clarification of addressing the conundrum
> 
> Andrew Elble (3):
>   nfs/nfsd: Move useful bitfield ops to a commonly accessible place
>   nfsd: allow mach_creds_match to be used more broadly
>   nfsd: implement machine credential support for some operations
> 
>  fs/nfsd/export.c        | 10 ++++++++++
>  fs/nfsd/nfs4proc.c      | 39 +++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4state.c     | 32 ++++++++++++++++++++++++-------
>  fs/nfsd/nfs4xdr.c       | 51 ++++++++++++++++++++++-----------------
> ----------
>  fs/nfsd/nfsd.h          |  5 +++++
>  fs/nfsd/state.h         |  1 +
>  fs/nfsd/xdr4.h          |  5 +++++
>  include/linux/nfs4.h    | 11 +++++++++++
>  include/linux/nfs_xdr.h | 11 -----------
>  9 files changed, 119 insertions(+), 46 deletions(-)
> 

All look good to me.

Reviewed-by: Jeff Layton <jlayton@redhat.com>

Re: [PATCH v3 0/3] Deal with lost delegations and EKEYEXPIRED

[J. Bruce Fields]

On Thu, Jun 16, 2016 at 07:49:07AM -0400, Jeff Layton wrote:
> On Wed, 2016-06-15 at 12:52 -0400, Andrew Elble wrote:
> > The majority of our lost delegation problems come from the inability
> > to return the delegations when the credentials that obtained them
> > have expired (conundrum mentioned in RFC 5661 18.35.3)
> > 
> > ObMemoryJog:
> > 
> > http://www.spinics.net/lists/linux-nfs/msg56230.html
> > 
> > v3: removal of OP_IS_PUTFH_LIKE checks
> >     added clarification of addressing the conundrum
> > 
> > Andrew Elble (3):
> >   nfs/nfsd: Move useful bitfield ops to a commonly accessible place
> >   nfsd: allow mach_creds_match to be used more broadly
> >   nfsd: implement machine credential support for some operations
> > 
> >  fs/nfsd/export.c        | 10 ++++++++++
> >  fs/nfsd/nfs4proc.c      | 39 +++++++++++++++++++++++++++++++++++++
> >  fs/nfsd/nfs4state.c     | 32 ++++++++++++++++++++++++-------
> >  fs/nfsd/nfs4xdr.c       | 51 ++++++++++++++++++++++-----------------
> > ----------
> >  fs/nfsd/nfsd.h          |  5 +++++
> >  fs/nfsd/state.h         |  1 +
> >  fs/nfsd/xdr4.h          |  5 +++++
> >  include/linux/nfs4.h    | 11 +++++++++++
> >  include/linux/nfs_xdr.h | 11 -----------
> >  9 files changed, 119 insertions(+), 46 deletions(-)
> > 
> 
> All look good to me.
> 
> Reviewed-by: Jeff Layton <jlayton@redhat.com>

Thanks!--b.