From: Chuck Lever When a server handshake completes successfully, tlshd might provide a set of TLS session tags. SUNRPC can save these within the svc_xprt; NFSD can later use them to authorize or reject operations that target NFS exports that have a similar set of tags associated with them. A second handshake on the same transport would destroy the saved tags while other workers read them, so svcauth_tls_accept() now refuses AUTH_TLS on a transport that already carries a TLS session, and svc_tcp_handshake() rechecks the session flag under XPT_BUSY to close the race with a handshake that completes concurrently. Signed-off-by: Chuck Lever --- include/linux/sunrpc/svc_xprt.h | 2 ++ net/sunrpc/svc_xprt.c | 11 ++++++++--- net/sunrpc/svcauth_unix.c | 12 ++++++++++++ net/sunrpc/svcsock.c | 33 ++++++++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h index da2a2531e110..15f678d00876 100644 --- a/include/linux/sunrpc/svc_xprt.h +++ b/include/linux/sunrpc/svc_xprt.h @@ -9,6 +9,7 @@ #define SUNRPC_SVC_XPRT_H #include +#include struct module; @@ -79,6 +80,7 @@ struct svc_xprt { const struct cred *xpt_cred; struct rpc_xprt *xpt_bc_xprt; /* NFSv4.1 backchannel */ struct rpc_xprt_switch *xpt_bc_xps; /* NFSv4.1 backchannel */ + struct tagset xpt_handshake_tags; /* TLS session tags */ }; /* flag bits for xpt_flags */ diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 63d1002e63e7..1638fc09db8b 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -168,6 +168,8 @@ static void svc_xprt_free(struct kref *kref) struct svc_xprt *xprt = container_of(kref, struct svc_xprt, xpt_ref); struct module *owner = xprt->xpt_class->xcl_owner; + + tagset_destroy(&xprt->xpt_handshake_tags); if (test_bit(XPT_CACHE_AUTH, &xprt->xpt_flags)) svcauth_unix_info_release(xprt); put_cred(xprt->xpt_cred); @@ -188,9 +190,12 @@ void svc_xprt_put(struct svc_xprt *xprt) } EXPORT_SYMBOL_GPL(svc_xprt_put); -/* - * Called by transport drivers to initialize the transport independent - * portion of the transport instance. +/** + * svc_xprt_init - initialize transport-independent fields of an xprt + * @net: Network namespace + * @xcl: Transport class + * @xprt: Transport to be initialized + * @serv: RPC service */ void svc_xprt_init(struct net *net, struct svc_xprt_class *xcl, struct svc_xprt *xprt, struct svc_serv *serv) diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c index 64a2658faddb..7a779e773107 100644 --- a/net/sunrpc/svcauth_unix.c +++ b/net/sunrpc/svcauth_unix.c @@ -1129,6 +1129,18 @@ svcauth_tls_accept(struct svc_rqst *rqstp) return SVC_DENIED; } + /* + * AUTH_TLS initiates a handshake. Refuse it on a transport + * that already has a TLS session: a second handshake would + * destroy xpt_handshake_tags. This test can pass before a + * concurrent handshake completes; svc_tcp_handshake() + * rechecks under XPT_BUSY before destroying the tags. + */ + if (test_bit(XPT_TLS_SESSION, &xprt->xpt_flags)) { + rqstp->rq_auth_stat = rpc_autherr_badcred; + return SVC_DENIED; + } + /* Signal that mapping to nobody uid/gid is required */ cred->cr_uid = INVALID_UID; cred->cr_gid = INVALID_GID; diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index b4ad84910687..cc06ed3075db 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -470,7 +470,18 @@ static void svc_tcp_handshake_done(void *data, int status, key_serial_t peerid, if (!status) { if (peerid != TLS_NO_PEERID) set_bit(XPT_PEER_AUTH, &xprt->xpt_flags); - set_bit(XPT_TLS_SESSION, &xprt->xpt_flags); + /* + * Leaving XPT_TLS_SESSION clear on copy failure makes + * svc_tcp_handshake() close the connection. The tags + * cannot be recovered later on this transport because + * a second handshake is refused once a session is + * established; a reconnect retries both the handshake + * and the copy. + */ + if (tagset_copy(&xprt->xpt_handshake_tags, tags, GFP_KERNEL)) + set_bit(XPT_TLS_SESSION, &xprt->xpt_flags); + else + pr_warn_ratelimited("svc: failed to copy TLS session tags\n"); } clear_bit(XPT_HANDSHAKE, &xprt->xpt_flags); complete_all(&svsk->sk_handshake_done); @@ -481,6 +492,9 @@ static void svc_tcp_handshake_done(void *data, int status, key_serial_t peerid, * svc_tcp_handshake - Perform a transport-layer security handshake * @xprt: connected transport endpoint * + * If the transport already has a TLS session, the handshake request + * is declined: a fresh handshake would destroy the saved session + * tags. */ static void svc_tcp_handshake(struct svc_xprt *xprt) { @@ -493,8 +507,25 @@ static void svc_tcp_handshake(struct svc_xprt *xprt) }; int ret; + /* + * The XPT_TLS_SESSION test in svcauth_tls_accept() is not + * race-free: a worker can pass it before a concurrent + * handshake completes and raise XPT_HANDSHAKE afterwards. + * XPT_BUSY serializes handshake starts, so this test cannot + * go stale: a set bit here means an established session + * whose tags other workers may be reading. Decline to start + * a handshake that would destroy them. + */ + if (test_bit(XPT_TLS_SESSION, &xprt->xpt_flags)) { + clear_bit(XPT_HANDSHAKE, &xprt->xpt_flags); + set_bit(XPT_DATA, &xprt->xpt_flags); + svc_xprt_enqueue(xprt); + return; + } + trace_svc_tls_upcall(xprt); + tagset_destroy(&xprt->xpt_handshake_tags); clear_bit(XPT_TLS_SESSION, &xprt->xpt_flags); init_completion(&svsk->sk_handshake_done); -- 2.54.0