[PATCH RFC 0/4] Deal with lost delegations and EKEYEXPIRED

[PATCH RFC 0/4] Deal with lost delegations and EKEYEXPIRED

[Andrew Elble]

I've finally discovered that the majority of our lost delegation problems
come from EKEYEXPIRED. This seems to work fine in our environment, but
I am unsure of the ramifications of this in a broader context, so it's
time to get other folks to look at it.

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

 fs/nfs/nfs4proc.c         | 20 +++++++++++++++++
 fs/nfsd/export.c          |  4 ++++
 fs/nfsd/nfs4proc.c        | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4state.c       | 35 ++++++++++++++++++++++++------
 fs/nfsd/nfs4xdr.c         | 51 ++++++++++++++++++++-----------------------
 fs/nfsd/nfsd.h            |  1 +
 fs/nfsd/state.h           |  1 +
 fs/nfsd/xdr4.h            |  5 +++++
 include/linux/nfs4.h      | 11 ++++++++++
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   | 11 ----------
 11 files changed, 149 insertions(+), 46 deletions(-)

-- 
2.6.3

[PATCH RFC 1/4] 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 e7e78537aea2..95ead47154ec 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -592,4 +592,15 @@ enum data_content4 {
 	NFS4_CONTENT_HOLE		= 1,
 };
 
+#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 570d630f98ae..7f11c487ab0f 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 RFC 2/4] nfs: machine credential support for additional operations

[Andrew Elble]

Allow LAYOUTRETURN and DELEGRETURN to use machine credentials if the
server supports it. Add request for OPEN_DOWNGRADE as the close path
also uses that.

Signed-off-by: Andrew Elble <aweits@rit.edu>
---
 fs/nfs/nfs4proc.c         | 20 ++++++++++++++++++++
 include/linux/nfs_fs_sb.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 765a03559363..f7f45792676d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5384,6 +5384,11 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co
 	if (data == NULL)
 		return -ENOMEM;
 	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
+
+	nfs4_state_protect(server->nfs_client,
+			NFS_SP4_MACH_CRED_CLEANUP,
+			&task_setup_data.rpc_client, &msg);
+
 	data->args.fhandle = &data->fh;
 	data->args.stateid = &data->stateid;
 	data->args.bitmask = server->cache_consistency_bitmask;
@@ -6862,10 +6867,13 @@ static const struct nfs41_state_protection nfs4_sp4_mach_cred_request = {
 	},
 	.allow.u.words = {
 		[0] = 1 << (OP_CLOSE) |
+		      1 << (OP_OPEN_DOWNGRADE) |
 		      1 << (OP_LOCKU) |
+		      1 << (OP_DELEGRETURN) |
 		      1 << (OP_COMMIT),
 		[1] = 1 << (OP_SECINFO - 32) |
 		      1 << (OP_SECINFO_NO_NAME - 32) |
+		      1 << (OP_LAYOUTRETURN - 32) |
 		      1 << (OP_TEST_STATEID - 32) |
 		      1 << (OP_FREE_STATEID - 32) |
 		      1 << (OP_WRITE - 32)
@@ -6930,11 +6938,19 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
 		}
 
 		if (test_bit(OP_CLOSE, sp->allow.u.longs) &&
+		    test_bit(OP_OPEN_DOWNGRADE, sp->allow.u.longs) &&
+		    test_bit(OP_DELEGRETURN, 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);
 		}
 
+		if (test_bit(OP_LAYOUTRETURN, sp->allow.u.longs)) {
+			dfprintk(MOUNT, "  pnfs cleanup mode enabled\n");
+			set_bit(NFS_SP4_MACH_CRED_PNFS_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");
@@ -8086,6 +8102,10 @@ int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp, bool sync)
 	};
 	int status = 0;
 
+	nfs4_state_protect(NFS_SERVER(lrp->args.inode)->nfs_client,
+			NFS_SP4_MACH_CRED_PNFS_CLEANUP,
+			&task_setup_data.rpc_client, &msg);
+
 	dprintk("--> %s\n", __func__);
 	if (!sync) {
 		lrp->inode = nfs_igrab_and_active(lrp->args.inode);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 2469ab0bb3a1..7fcc13c8cf1f 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -102,6 +102,7 @@ struct nfs_client {
 #define NFS_SP4_MACH_CRED_STATEID  4	/* TEST_STATEID and FREE_STATEID */
 #define NFS_SP4_MACH_CRED_WRITE    5	/* WRITE */
 #define NFS_SP4_MACH_CRED_COMMIT   6	/* COMMIT */
+#define NFS_SP4_MACH_CRED_PNFS_CLEANUP  7 /* LAYOUTRETURN */
 #endif /* CONFIG_NFS_V4 */
 
 	/* Our own IP address, as a null-terminated string.
-- 
2.6.3

Re: [PATCH RFC 2/4] nfs: machine credential support for additional operations

[Trond Myklebust]

On Wed, Dec 2, 2015 at 6:39 AM, Andrew Elble <aweits@rit.edu> wrote:
> Allow LAYOUTRETURN and DELEGRETURN to use machine credentials if the
> server supports it. Add request for OPEN_DOWNGRADE as the close path
> also uses that.
>
> Signed-off-by: Andrew Elble <aweits@rit.edu>
> ---
>  fs/nfs/nfs4proc.c         | 20 ++++++++++++++++++++
>  include/linux/nfs_fs_sb.h |  1 +
>  2 files changed, 21 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 765a03559363..f7f45792676d 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5384,6 +5384,11 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co
>         if (data == NULL)
>                 return -ENOMEM;
>         nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
> +
> +       nfs4_state_protect(server->nfs_client,
> +                       NFS_SP4_MACH_CRED_CLEANUP,
> +                       &task_setup_data.rpc_client, &msg);
> +
>         data->args.fhandle = &data->fh;
>         data->args.stateid = &data->stateid;
>         data->args.bitmask = server->cache_consistency_bitmask;
> @@ -6862,10 +6867,13 @@ static const struct nfs41_state_protection nfs4_sp4_mach_cred_request = {
>         },
>         .allow.u.words = {
>                 [0] = 1 << (OP_CLOSE) |
> +                     1 << (OP_OPEN_DOWNGRADE) |
>                       1 << (OP_LOCKU) |
> +                     1 << (OP_DELEGRETURN) |
>                       1 << (OP_COMMIT),
>                 [1] = 1 << (OP_SECINFO - 32) |
>                       1 << (OP_SECINFO_NO_NAME - 32) |
> +                     1 << (OP_LAYOUTRETURN - 32) |
>                       1 << (OP_TEST_STATEID - 32) |
>                       1 << (OP_FREE_STATEID - 32) |
>                       1 << (OP_WRITE - 32)
> @@ -6930,11 +6938,19 @@ static int nfs4_sp4_select_mode(struct nfs_client *clp,
>                 }
>
>                 if (test_bit(OP_CLOSE, sp->allow.u.longs) &&
> +                   test_bit(OP_OPEN_DOWNGRADE, sp->allow.u.longs) &&
> +                   test_bit(OP_DELEGRETURN, 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);
>                 }
>
> +               if (test_bit(OP_LAYOUTRETURN, sp->allow.u.longs)) {
> +                       dfprintk(MOUNT, "  pnfs cleanup mode enabled\n");
> +                       set_bit(NFS_SP4_MACH_CRED_PNFS_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");
> @@ -8086,6 +8102,10 @@ int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp, bool sync)
>         };
>         int status = 0;
>
> +       nfs4_state_protect(NFS_SERVER(lrp->args.inode)->nfs_client,
> +                       NFS_SP4_MACH_CRED_PNFS_CLEANUP,
> +                       &task_setup_data.rpc_client, &msg);
> +
>         dprintk("--> %s\n", __func__);
>         if (!sync) {
>                 lrp->inode = nfs_igrab_and_active(lrp->args.inode);
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 2469ab0bb3a1..7fcc13c8cf1f 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -102,6 +102,7 @@ struct nfs_client {
>  #define NFS_SP4_MACH_CRED_STATEID  4   /* TEST_STATEID and FREE_STATEID */
>  #define NFS_SP4_MACH_CRED_WRITE    5   /* WRITE */
>  #define NFS_SP4_MACH_CRED_COMMIT   6   /* COMMIT */
> +#define NFS_SP4_MACH_CRED_PNFS_CLEANUP  7 /* LAYOUTRETURN */
>  #endif /* CONFIG_NFS_V4 */
>
>         /* Our own IP address, as a null-terminated string.

This patch looks fine, but can we please break it out of the series?
There doesn't appear to be any dependency between this and the other
patches, so it would be easier if I could just take it directly.

Re: [PATCH RFC 2/4] nfs: machine credential support for additional operations

[Andrew W Elble]

Trond Myklebust <trond.myklebust@primarydata.com> writes:

> On Wed, Dec 2, 2015 at 6:39 AM, Andrew Elble <aweits@rit.edu> wrote:
>> Allow LAYOUTRETURN and DELEGRETURN to use machine credentials if the
>> server supports it. Add request for OPEN_DOWNGRADE as the close path
>> also uses that.
>
> This patch looks fine, but can we please break it out of the series?
> There doesn't appear to be any dependency between this and the other
> patches, so it would be easier if I could just take it directly.

I'm fine with that - I have to (at least) do v2 on the rest, do you want
me to repost it separately?

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

[PATCH RFC 3/4] 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 6b800b5b8fed..65efc900e97e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1959,7 +1959,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;
 
@@ -2393,7 +2393,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;
 			}
@@ -2640,7 +2640,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);
@@ -2656,7 +2656,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);
@@ -2766,7 +2766,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)
@@ -2813,7 +2813,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)
@@ -3052,7 +3052,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 ce7362c88b48..25c9c79460f9 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -644,6 +644,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 RFC 4/4] nfsd: implement machine credential support for some operations

[Andrew Elble]

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,
TEST/FREE STATEID, and LAYOUTRETURN.
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    |  4 ++++
 fs/nfsd/nfs4proc.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4state.c | 21 ++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   | 51 ++++++++++++++++++++++---------------------------
 fs/nfsd/nfsd.h      |  1 +
 fs/nfsd/state.h     |  1 +
 fs/nfsd/xdr4.h      |  3 +++
 7 files changed, 108 insertions(+), 28 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index b4d84b579f20..0395e3e8fc3e 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -954,6 +954,10 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 		    rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)
 			return 0;
 	}
+
+	if (nfsd4_spo_must_allow(rqstp))
+		return 0;
+
 	return nfserr_wrongsec;
 }
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a9f096c7e99f..270ad2aa1446 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2285,6 +2285,61 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 };
 
+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 nfsd4_operation *thisd;
+	struct nfs4_op_map *allow = &cstate->clp->cl_spo_must_allow;
+	u32 opiter;
+
+	if (!cstate->minorversion)
+		return false;
+
+	thisd = OPDESC(this);
+
+	if (!(thisd->op_flags & OP_IS_PUTFH_LIKE)) {
+		if (cstate->spo_must_allowed == true)
+			/*
+			 * a prior putfh + op has set
+			 * spo_must_allow conditions
+			 */
+			return true;
+		/* evaluate op against spo_must_allow with no prior putfh */
+		if (test_bit(this->opnum, allow->u.longs) &&
+			cstate->clp->cl_mach_cred &&
+			nfsd4_mach_creds_match(cstate->clp, rqstp))
+			return true;
+		else
+			return false;
+	}
+	/*
+	 * this->opnum has PUTFH ramifications
+	 * scan forward to next putfh or end of compound op
+	 */
+	opiter = resp->opcnt;
+	while (opiter < argp->opcnt) {
+		this = &argp->ops[opiter++];
+		thisd = OPDESC(this);
+		if (thisd->op_flags & OP_IS_PUTFH_LIKE)
+			break;
+		if (test_bit(this->opnum, allow->u.longs) &&
+			cstate->clp->cl_mach_cred &&
+			nfsd4_mach_creds_match(cstate->clp, rqstp)) {
+			/*
+			 * the op covered by the fh is a
+			 * spo_must_allow operation
+			 */
+			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 65efc900e97e..34d235b84c42 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2367,6 +2367,25 @@ 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] &= (
+#ifdef CONFIG_NFSD_PNFS
+					1 << (OP_LAYOUTRETURN - 32) |
+#endif
+					1 << (OP_TEST_STATEID - 32) |
+					1 << (OP_FREE_STATEID - 32));
 		if (!svc_rqst_integrity_protected(rqstp))
 			return nfserr_inval;
 	case SP4_NONE:
@@ -2443,6 +2462,8 @@ out_new:
 	}
 	new->cl_minorversion = cstate->minorversion;
 	new->cl_mach_cred = (exid->spa_how == SP4_MACH_CRED);
+	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 51c9e9ca39a4..e2043aa95e27 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1297,16 +1297,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 */
@@ -3841,14 +3839,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)
@@ -3859,6 +3849,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)
@@ -3887,18 +3878,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);
@@ -3925,6 +3918,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..d20824002d90 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) { }
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 77fdf4de91ba..2b59c74f098c 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 25c9c79460f9..c88aca9c42d7 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 RFC 0/4] Deal with lost delegations and EKEYEXPIRED

[Weston Andros Adamson]

These patches look reasonable to me. I agree with Trond that you should
separate the client and server patches.

One part I’m not sure about is in nfsd4_spo_must_allow dealing with putfh like
ops vs not putfh-like ops. I’ll have to check the spec and take a deeper look at
that when I get some time, but maybe a brief explanation in a comment would
help?

To be honest, I've always been hazy on where in the spec the ramifications of
SP4_MACH_CRED only covering part of a compound is discussed… I’ll take a
look soon.

-dros



> On Dec 2, 2015, at 9:39 AM, Andrew Elble <aweits@rit.edu> wrote:
> 
> I've finally discovered that the majority of our lost delegation problems
> come from EKEYEXPIRED. This seems to work fine in our environment, but
> I am unsure of the ramifications of this in a broader context, so it's
> time to get other folks to look at it.
> 
> Andrew Elble (4):
>  nfs/nfsd: Move useful bitfield ops to a commonly accessible place
>  nfs: machine credential support for additional operations
>  nfsd: allow mach_creds_match to be used more broadly
>  nfsd: implement machine credential support for some operations
> 
> fs/nfs/nfs4proc.c         | 20 +++++++++++++++++
> fs/nfsd/export.c          |  4 ++++
> fs/nfsd/nfs4proc.c        | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4state.c       | 35 ++++++++++++++++++++++++------
> fs/nfsd/nfs4xdr.c         | 51 ++++++++++++++++++++-----------------------
> fs/nfsd/nfsd.h            |  1 +
> fs/nfsd/state.h           |  1 +
> fs/nfsd/xdr4.h            |  5 +++++
> include/linux/nfs4.h      | 11 ++++++++++
> include/linux/nfs_fs_sb.h |  1 +
> include/linux/nfs_xdr.h   | 11 ----------
> 11 files changed, 149 insertions(+), 46 deletions(-)
> 
> -- 
> 2.6.3
> 
> --
> 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 RFC 0/4] Deal with lost delegations and EKEYEXPIRED

[Andrew W Elble]

> One part I’m not sure about is in nfsd4_spo_must_allow dealing with putfh like
> ops vs not putfh-like ops. I’ll have to check the spec and take a deeper look at
> that when I get some time, but maybe a brief explanation in a comment would
> help?

Will put on the list for v2.

RFC5561 2.6.3.1.1.7:
   The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC to any operation
   other than a put filehandle operation, LOOKUP, LOOKUPP, and OPEN (by
   component name).

RFC5561 15.4:
   | NFS4ERR_WRONGSEC                  | LINK, LOOKUP, LOOKUPP, OPEN,  |
   |                                   | PUTFH, PUTPUBFH, PUTROOTFH,   |
   |                                   | RENAME, RESTOREFH

See need_wrongsec_check() (called from nfsd4_proc_compound()).

The implementation problem is that fh_verify() also calls check_nfsd_access(),
so these contortions are to avoid sending NFS4ERR_WRONGSEC on things we
shouldn't be, while still granting the spo_must_allow operation to succeed.
(based on [somthing-putfh-like]->...->[something-spo_must_allow-like]...->[end-or-putfh])

> To be honest, I've always been hazy on where in the spec the ramifications of
> SP4_MACH_CRED only covering part of a compound is discussed… I’ll take a
> look soon.

I think you mean 2.6.3.1? But it doesn't reference SP4_MACH_CRED
specifically, only 'acceptable security tuple'.

This may also be a little bit heavyweight for how the server is set up
currently. (i.e. simply changing the export (sec=krb5) to
(sec=krb5,krb5i) will eliminate the need for nfsd4_spo_must_allow()).

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