Return-Path: Received: from fieldses.org ([173.255.197.46]:56234 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726200AbeIDUMB (ORCPT ); Tue, 4 Sep 2018 16:12:01 -0400 Date: Tue, 4 Sep 2018 11:46:20 -0400 To: Trond Myklebust Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH 03/27] SUNRPC: The transmitted message must lie in the RPCSEC window of validity Message-ID: <20180904154620.GC17478@fieldses.org> References: <20180903152936.24325-1-trond.myklebust@hammerspace.com> <20180903152936.24325-2-trond.myklebust@hammerspace.com> <20180903152936.24325-3-trond.myklebust@hammerspace.com> <20180903152936.24325-4-trond.myklebust@hammerspace.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20180903152936.24325-4-trond.myklebust@hammerspace.com> From: bfields@fieldses.org (J. Bruce Fields) Sender: linux-nfs-owner@vger.kernel.org List-ID: On Mon, Sep 03, 2018 at 11:29:12AM -0400, Trond Myklebust wrote: > If a message has been encoded using RPCSEC_GSS, the server is > maintaining a window of sequence numbers that it considers valid. > The client should normally be tracking that window, and needs to > verify that the sequence number used by the message being transmitted > still lies inside the window of validity. > > So far, we've been able to assume this condition would be realised > automatically, since the server has been encoding the message only Do you mean "the client has been encoding..."? --b. > after taking the socket lock. Once we change that condition, we > will need the explicit check. > > Signed-off-by: Trond Myklebust > --- > include/linux/sunrpc/auth.h | 2 ++ > include/linux/sunrpc/auth_gss.h | 1 + > net/sunrpc/auth.c | 10 ++++++++ > net/sunrpc/auth_gss/auth_gss.c | 41 +++++++++++++++++++++++++++++++++ > net/sunrpc/clnt.c | 3 +++ > net/sunrpc/xprt.c | 7 ++++++ > 6 files changed, 64 insertions(+) > > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h > index 58a6765c1c5e..2c97a3933ef9 100644 > --- a/include/linux/sunrpc/auth.h > +++ b/include/linux/sunrpc/auth.h > @@ -157,6 +157,7 @@ struct rpc_credops { > int (*crkey_timeout)(struct rpc_cred *); > bool (*crkey_to_expire)(struct rpc_cred *); > char * (*crstringify_acceptor)(struct rpc_cred *); > + bool (*crneed_reencode)(struct rpc_task *); > }; > > extern const struct rpc_authops authunix_ops; > @@ -192,6 +193,7 @@ __be32 * rpcauth_marshcred(struct rpc_task *, __be32 *); > __be32 * rpcauth_checkverf(struct rpc_task *, __be32 *); > int rpcauth_wrap_req(struct rpc_task *task, kxdreproc_t encode, void *rqstp, __be32 *data, void *obj); > int rpcauth_unwrap_resp(struct rpc_task *task, kxdrdproc_t decode, void *rqstp, __be32 *data, void *obj); > +bool rpcauth_xmit_need_reencode(struct rpc_task *task); > int rpcauth_refreshcred(struct rpc_task *); > void rpcauth_invalcred(struct rpc_task *); > int rpcauth_uptodatecred(struct rpc_task *); > diff --git a/include/linux/sunrpc/auth_gss.h b/include/linux/sunrpc/auth_gss.h > index 0c9eac351aab..30427b729070 100644 > --- a/include/linux/sunrpc/auth_gss.h > +++ b/include/linux/sunrpc/auth_gss.h > @@ -70,6 +70,7 @@ struct gss_cl_ctx { > refcount_t count; > enum rpc_gss_proc gc_proc; > u32 gc_seq; > + u32 gc_seq_xmit; > spinlock_t gc_seq_lock; > struct gss_ctx *gc_gss_ctx; > struct xdr_netobj gc_wire_ctx; > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c > index 305ecea92170..59df5cdba0ac 100644 > --- a/net/sunrpc/auth.c > +++ b/net/sunrpc/auth.c > @@ -817,6 +817,16 @@ rpcauth_unwrap_resp(struct rpc_task *task, kxdrdproc_t decode, void *rqstp, > return rpcauth_unwrap_req_decode(decode, rqstp, data, obj); > } > > +bool > +rpcauth_xmit_need_reencode(struct rpc_task *task) > +{ > + struct rpc_cred *cred = task->tk_rqstp->rq_cred; > + > + if (!cred || !cred->cr_ops->crneed_reencode) > + return false; > + return cred->cr_ops->crneed_reencode(task); > +} > + > int > rpcauth_refreshcred(struct rpc_task *task) > { > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c > index 21c0aa0a0d1d..c898a7c75e84 100644 > --- a/net/sunrpc/auth_gss/auth_gss.c > +++ b/net/sunrpc/auth_gss/auth_gss.c > @@ -1984,6 +1984,46 @@ gss_unwrap_req_decode(kxdrdproc_t decode, struct rpc_rqst *rqstp, > return decode(rqstp, &xdr, obj); > } > > +static bool > +gss_seq_is_newer(u32 new, u32 old) > +{ > + return (s32)(new - old) > 0; > +} > + > +static bool > +gss_xmit_need_reencode(struct rpc_task *task) > +{ > + struct rpc_rqst *req = task->tk_rqstp; > + struct rpc_cred *cred = req->rq_cred; > + struct gss_cl_ctx *ctx = gss_cred_get_ctx(cred); > + u32 win, seq_xmit; > + bool ret = true; > + > + if (!ctx) > + return true; > + > + if (gss_seq_is_newer(req->rq_seqno, READ_ONCE(ctx->gc_seq))) > + goto out; > + > + seq_xmit = READ_ONCE(ctx->gc_seq_xmit); > + while (gss_seq_is_newer(req->rq_seqno, seq_xmit)) { > + u32 tmp = seq_xmit; > + > + seq_xmit = cmpxchg(&ctx->gc_seq_xmit, tmp, req->rq_seqno); > + if (seq_xmit == tmp) { > + ret = false; > + goto out; > + } > + } > + > + win = ctx->gc_win; > + if (win > 0) > + ret = !gss_seq_is_newer(req->rq_seqno, seq_xmit - win); > +out: > + gss_put_ctx(ctx); > + return ret; > +} > + > static int > gss_unwrap_resp(struct rpc_task *task, > kxdrdproc_t decode, void *rqstp, __be32 *p, void *obj) > @@ -2052,6 +2092,7 @@ static const struct rpc_credops gss_credops = { > .crunwrap_resp = gss_unwrap_resp, > .crkey_timeout = gss_key_timeout, > .crstringify_acceptor = gss_stringify_acceptor, > + .crneed_reencode = gss_xmit_need_reencode, > }; > > static const struct rpc_credops gss_nullops = { > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c > index 4f1ec8013332..d41b5ac1d4e8 100644 > --- a/net/sunrpc/clnt.c > +++ b/net/sunrpc/clnt.c > @@ -2184,6 +2184,9 @@ call_status(struct rpc_task *task) > /* shutdown or soft timeout */ > rpc_exit(task, status); > break; > + case -EBADMSG: > + task->tk_action = call_transmit; > + break; > default: > if (clnt->cl_chatty) > printk("%s: RPC call returned error %d\n", > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > index 6aa09edc9567..3973e10ea2bd 100644 > --- a/net/sunrpc/xprt.c > +++ b/net/sunrpc/xprt.c > @@ -1014,6 +1014,13 @@ void xprt_transmit(struct rpc_task *task) > dprintk("RPC: %5u xprt_transmit(%u)\n", task->tk_pid, req->rq_slen); > > if (!req->rq_reply_bytes_recvd) { > + > + /* Verify that our message lies in the RPCSEC_GSS window */ > + if (!req->rq_bytes_sent && rpcauth_xmit_need_reencode(task)) { > + task->tk_status = -EBADMSG; > + return; > + } > + > if (list_empty(&req->rq_list) && rpc_reply_expected(task)) { > /* > * Add to the list only if we're expecting a reply > -- > 2.17.1