Now that TLS handshake support is available in the kernel, let's
have a look at what is needed to support NFS in-transit confiden-
tiality in the Linux NFS client.
These apply to v6.4-rc2 (actually, net-next to be precise), but
previously they've been tested at multiple NFS bake-a-thon events.
---
Chuck Lever (12):
NFS: Improvements for fs_context-related tracepoints
SUNRPC: Plumb an API for setting transport layer security
SUNRPC: Trace the rpc_create_args
SUNRPC: Refactor rpc_call_null_helper()
SUNRPC: Add RPC client support for the RPC_AUTH_TLS auth flavor
SUNRPC: Ignore data_ready callbacks during TLS handshakes
SUNRPC: Capture CMSG metadata on client-side receive
SUNRPC: Add a connect worker function for TLS
SUNRPC: Add RPC-with-TLS support to xprtsock.c
SUNRPC: Add RPC-with-TLS tracepoints
NFS: Have struct nfs_client carry a TLS policy field
NFS: Add an "xprtsec=" NFS mount option
fs/nfs/client.c | 7 +
fs/nfs/fs_context.c | 55 +++++
fs/nfs/internal.h | 2 +
fs/nfs/nfs3client.c | 1 +
fs/nfs/nfs4client.c | 18 +-
fs/nfs/super.c | 12 ++
include/linux/nfs_fs_sb.h | 3 +-
include/linux/sunrpc/auth.h | 1 +
include/linux/sunrpc/clnt.h | 2 +
include/linux/sunrpc/xprt.h | 17 ++
include/linux/sunrpc/xprtsock.h | 3 +
include/trace/events/sunrpc.h | 96 ++++++++-
net/sunrpc/Makefile | 2 +-
net/sunrpc/auth.c | 2 +-
net/sunrpc/auth_tls.c | 120 +++++++++++
net/sunrpc/clnt.c | 22 +-
net/sunrpc/xprtsock.c | 343 +++++++++++++++++++++++++++++++-
17 files changed, 677 insertions(+), 29 deletions(-)
create mode 100644 net/sunrpc/auth_tls.c
--
Chuck Lever
From: Chuck Lever <[email protected]>
Add an initial set of policies along with fields for upper layers to
pass the requested policy down to the transport layer.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/clnt.h | 2 ++
include/linux/sunrpc/xprt.h | 17 +++++++++++++++++
net/sunrpc/clnt.c | 4 ++++
3 files changed, 23 insertions(+)
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 770ef2cb5775..063692cd2a60 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -58,6 +58,7 @@ struct rpc_clnt {
cl_noretranstimeo: 1,/* No retransmit timeouts */
cl_autobind : 1,/* use getport() */
cl_chatty : 1;/* be verbose */
+ struct xprtsec_parms cl_xprtsec; /* transport security policy */
struct rpc_rtt * cl_rtt; /* RTO estimator data */
const struct rpc_timeout *cl_timeout; /* Timeout strategy */
@@ -139,6 +140,7 @@ struct rpc_create_args {
struct svc_xprt *bc_xprt; /* NFSv4.1 backchannel */
const struct cred *cred;
unsigned int max_connect;
+ struct xprtsec_parms xprtsec;
};
struct rpc_add_xprt_test {
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index b9f59aabee53..9e7f12c240c5 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -129,6 +129,21 @@ struct rpc_rqst {
#define rq_svec rq_snd_buf.head
#define rq_slen rq_snd_buf.len
+/* RPC transport layer security policies */
+enum xprtsec_policies {
+ RPC_XPRTSEC_NONE = 0,
+ RPC_XPRTSEC_TLS_ANON,
+ RPC_XPRTSEC_TLS_X509,
+};
+
+struct xprtsec_parms {
+ enum xprtsec_policies policy;
+
+ /* authentication material */
+ key_serial_t cert_serial;
+ key_serial_t privkey_serial;
+};
+
struct rpc_xprt_ops {
void (*set_buffer_size)(struct rpc_xprt *xprt, size_t sndsize, size_t rcvsize);
int (*reserve_xprt)(struct rpc_xprt *xprt, struct rpc_task *task);
@@ -229,6 +244,7 @@ struct rpc_xprt {
*/
unsigned long bind_timeout,
reestablish_timeout;
+ struct xprtsec_parms xprtsec;
unsigned int connect_cookie; /* A cookie that gets bumped
every time the transport
is reconnected */
@@ -333,6 +349,7 @@ struct xprt_create {
struct svc_xprt *bc_xprt; /* NFSv4.1 backchannel */
struct rpc_xprt_switch *bc_xps;
unsigned int flags;
+ struct xprtsec_parms xprtsec;
};
struct xprt_class {
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index d2ee56634308..a18074f8edf2 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -385,6 +385,7 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args,
if (!clnt)
goto out_err;
clnt->cl_parent = parent ? : clnt;
+ clnt->cl_xprtsec = args->xprtsec;
err = rpc_alloc_clid(clnt);
if (err)
@@ -532,6 +533,7 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args)
.addrlen = args->addrsize,
.servername = args->servername,
.bc_xprt = args->bc_xprt,
+ .xprtsec = args->xprtsec,
};
char servername[48];
struct rpc_clnt *clnt;
@@ -727,6 +729,7 @@ int rpc_switch_client_transport(struct rpc_clnt *clnt,
struct rpc_clnt *parent;
int err;
+ args->xprtsec = clnt->cl_xprtsec;
xprt = xprt_create_transport(args);
if (IS_ERR(xprt))
return PTR_ERR(xprt);
@@ -3046,6 +3049,7 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
if (!xprtargs->ident)
xprtargs->ident = ident;
+ xprtargs->xprtsec = clnt->cl_xprtsec;
xprt = xprt_create_transport(xprtargs);
if (IS_ERR(xprt)) {
ret = PTR_ERR(xprt);
From: Chuck Lever <[email protected]>
Add some missing observability to the fs_context tracepoints
added by commit 33ce83ef0bb0 ("NFS: Replace fs_context-related
dprintk() call sites with tracepoints").
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfs/fs_context.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 9bcd53d5c7d4..5626d358ee2e 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -791,16 +791,19 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
ctx->mount_server.addrlen = len;
break;
case Opt_nconnect:
+ trace_nfs_mount_assign(param->key, param->string);
if (result.uint_32 < 1 || result.uint_32 > NFS_MAX_CONNECTIONS)
goto out_of_bounds;
ctx->nfs_server.nconnect = result.uint_32;
break;
case Opt_max_connect:
+ trace_nfs_mount_assign(param->key, param->string);
if (result.uint_32 < 1 || result.uint_32 > NFS_MAX_TRANSPORTS)
goto out_of_bounds;
ctx->nfs_server.max_connect = result.uint_32;
break;
case Opt_lookupcache:
+ trace_nfs_mount_assign(param->key, param->string);
switch (result.uint_32) {
case Opt_lookupcache_all:
ctx->flags &= ~(NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE);
@@ -817,6 +820,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
}
break;
case Opt_local_lock:
+ trace_nfs_mount_assign(param->key, param->string);
switch (result.uint_32) {
case Opt_local_lock_all:
ctx->flags |= (NFS_MOUNT_LOCAL_FLOCK |
@@ -837,6 +841,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
}
break;
case Opt_write:
+ trace_nfs_mount_assign(param->key, param->string);
switch (result.uint_32) {
case Opt_write_lazy:
ctx->flags &=
From: Chuck Lever <[email protected]>
Pass the upper layer's rpc_create_args to the rpc_clnt_new()
tracepoint so additional parts of the upper layer's request can be
recorded.
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
include/trace/events/sunrpc.h | 52 +++++++++++++++++++++++++++++++++--------
net/sunrpc/clnt.c | 2 +-
2 files changed, 43 insertions(+), 11 deletions(-)
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 31bc7025cb44..34784f29a63d 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -139,36 +139,68 @@ DEFINE_RPC_CLNT_EVENT(release);
DEFINE_RPC_CLNT_EVENT(replace_xprt);
DEFINE_RPC_CLNT_EVENT(replace_xprt_err);
+TRACE_DEFINE_ENUM(RPC_XPRTSEC_NONE);
+TRACE_DEFINE_ENUM(RPC_XPRTSEC_TLS_X509);
+
+#define rpc_show_xprtsec_policy(policy) \
+ __print_symbolic(policy, \
+ { RPC_XPRTSEC_NONE, "none" }, \
+ { RPC_XPRTSEC_TLS_ANON, "tls-anon" }, \
+ { RPC_XPRTSEC_TLS_X509, "tls-x509" })
+
+#define rpc_show_create_flags(flags) \
+ __print_flags(flags, "|", \
+ { RPC_CLNT_CREATE_HARDRTRY, "HARDRTRY" }, \
+ { RPC_CLNT_CREATE_AUTOBIND, "AUTOBIND" }, \
+ { RPC_CLNT_CREATE_NONPRIVPORT, "NONPRIVPORT" }, \
+ { RPC_CLNT_CREATE_NOPING, "NOPING" }, \
+ { RPC_CLNT_CREATE_DISCRTRY, "DISCRTRY" }, \
+ { RPC_CLNT_CREATE_QUIET, "QUIET" }, \
+ { RPC_CLNT_CREATE_INFINITE_SLOTS, \
+ "INFINITE_SLOTS" }, \
+ { RPC_CLNT_CREATE_NO_IDLE_TIMEOUT, \
+ "NO_IDLE_TIMEOUT" }, \
+ { RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT, \
+ "NO_RETRANS_TIMEOUT" }, \
+ { RPC_CLNT_CREATE_SOFTERR, "SOFTERR" }, \
+ { RPC_CLNT_CREATE_REUSEPORT, "REUSEPORT" })
+
TRACE_EVENT(rpc_clnt_new,
TP_PROTO(
const struct rpc_clnt *clnt,
const struct rpc_xprt *xprt,
- const char *program,
- const char *server
+ const struct rpc_create_args *args
),
- TP_ARGS(clnt, xprt, program, server),
+ TP_ARGS(clnt, xprt, args),
TP_STRUCT__entry(
__field(unsigned int, client_id)
+ __field(unsigned long, xprtsec)
+ __field(unsigned long, flags)
+ __string(program, clnt->cl_program->name)
+ __string(server, xprt->servername)
__string(addr, xprt->address_strings[RPC_DISPLAY_ADDR])
__string(port, xprt->address_strings[RPC_DISPLAY_PORT])
- __string(program, program)
- __string(server, server)
),
TP_fast_assign(
__entry->client_id = clnt->cl_clid;
+ __entry->xprtsec = args->xprtsec.policy;
+ __entry->flags = args->flags;
+ __assign_str(program, clnt->cl_program->name);
+ __assign_str(server, xprt->servername);
__assign_str(addr, xprt->address_strings[RPC_DISPLAY_ADDR]);
__assign_str(port, xprt->address_strings[RPC_DISPLAY_PORT]);
- __assign_str(program, program);
- __assign_str(server, server);
),
- TP_printk("client=" SUNRPC_TRACE_CLID_SPECIFIER
- " peer=[%s]:%s program=%s server=%s",
+ TP_printk("client=" SUNRPC_TRACE_CLID_SPECIFIER " peer=[%s]:%s"
+ " program=%s server=%s xprtsec=%s flags=%s",
__entry->client_id, __get_str(addr), __get_str(port),
- __get_str(program), __get_str(server))
+ __get_str(program), __get_str(server),
+ rpc_show_xprtsec_policy(__entry->xprtsec),
+ rpc_show_create_flags(__entry->flags)
+ )
);
TRACE_EVENT(rpc_clnt_new_err,
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index a18074f8edf2..4cdb539b5854 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -435,7 +435,7 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args,
if (parent)
refcount_inc(&parent->cl_count);
- trace_rpc_clnt_new(clnt, xprt, program->name, args->servername);
+ trace_rpc_clnt_new(clnt, xprt, args);
return clnt;
out_no_path:
From: Chuck Lever <[email protected]>
I'm about to add a use case that does not want RPC_TASK_NULLCREDS.
Refactor rpc_call_null_helper() so that callers provide NULLCREDS
when they need it.
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/clnt.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 4cdb539b5854..2dca0ae489ec 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2811,8 +2811,7 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
.rpc_op_cred = cred,
.callback_ops = ops ?: &rpc_null_ops,
.callback_data = data,
- .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
- RPC_TASK_NULLCREDS,
+ .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
};
return rpc_run_task(&task_setup_data);
@@ -2820,7 +2819,8 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int flags)
{
- return rpc_call_null_helper(clnt, NULL, cred, flags, NULL, NULL);
+ return rpc_call_null_helper(clnt, NULL, cred, flags | RPC_TASK_NULLCREDS,
+ NULL, NULL);
}
EXPORT_SYMBOL_GPL(rpc_call_null);
@@ -2920,12 +2920,13 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
goto success;
}
- task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
- &rpc_cb_add_xprt_call_ops, data);
+ task = rpc_call_null_helper(clnt, xprt, NULL,
+ RPC_TASK_ASYNC | RPC_TASK_NULLCREDS,
+ &rpc_cb_add_xprt_call_ops, data);
if (IS_ERR(task))
return PTR_ERR(task);
-
data->xps->xps_nunique_destaddr_xprts++;
+
rpc_put_task(task);
success:
return 1;
@@ -2940,7 +2941,8 @@ static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
int status = -EADDRINUSE;
/* Test the connection */
- task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
+ task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_NULLCREDS,
+ NULL, NULL);
if (IS_ERR(task))
return PTR_ERR(task);
From: Chuck Lever <[email protected]>
kTLS sockets use CMSG to report decryption errors and the need
for session re-keying.
For RPC-with-TLS, an "application data" message contains a ULP
payload, and that is passed along to the RPC client. An "alert"
message triggers connection reset. Everything else is discarded.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtsock.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 37f608c2c0a0..6f2fc863b47e 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -47,6 +47,8 @@
#include <net/checksum.h>
#include <net/udp.h>
#include <net/tcp.h>
+#include <net/tls.h>
+
#include <linux/bvec.h>
#include <linux/highmem.h>
#include <linux/uio.h>
@@ -342,13 +344,56 @@ xs_alloc_sparse_pages(struct xdr_buf *buf, size_t want, gfp_t gfp)
return want;
}
+static int
+xs_sock_process_cmsg(struct socket *sock, struct msghdr *msg,
+ struct cmsghdr *cmsg, int ret)
+{
+ if (cmsg->cmsg_level == SOL_TLS &&
+ cmsg->cmsg_type == TLS_GET_RECORD_TYPE) {
+ u8 content_type = *((u8 *)CMSG_DATA(cmsg));
+
+ switch (content_type) {
+ case TLS_RECORD_TYPE_DATA:
+ /* TLS sets EOR at the end of each application data
+ * record, even though there might be more frames
+ * waiting to be decrypted.
+ */
+ msg->msg_flags &= ~MSG_EOR;
+ break;
+ case TLS_RECORD_TYPE_ALERT:
+ ret = -ENOTCONN;
+ break;
+ default:
+ ret = -EAGAIN;
+ }
+ }
+ return ret;
+}
+
+static int
+xs_sock_recv_cmsg(struct socket *sock, struct msghdr *msg, int flags)
+{
+ union {
+ struct cmsghdr cmsg;
+ u8 buf[CMSG_SPACE(sizeof(u8))];
+ } u;
+ int ret;
+
+ msg->msg_control = &u;
+ msg->msg_controllen = sizeof(u);
+ ret = sock_recvmsg(sock, msg, flags);
+ if (msg->msg_controllen != sizeof(u))
+ ret = xs_sock_process_cmsg(sock, msg, &u.cmsg, ret);
+ return ret;
+}
+
static ssize_t
xs_sock_recvmsg(struct socket *sock, struct msghdr *msg, int flags, size_t seek)
{
ssize_t ret;
if (seek != 0)
iov_iter_advance(&msg->msg_iter, seek);
- ret = sock_recvmsg(sock, msg, flags);
+ ret = xs_sock_recv_cmsg(sock, msg, flags);
return ret > 0 ? ret + seek : ret;
}
@@ -374,7 +419,7 @@ xs_read_discard(struct socket *sock, struct msghdr *msg, int flags,
size_t count)
{
iov_iter_discard(&msg->msg_iter, ITER_DEST, count);
- return sock_recvmsg(sock, msg, flags);
+ return xs_sock_recv_cmsg(sock, msg, flags);
}
#if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
From: Chuck Lever <[email protected]>
The new authentication flavor is used only to discover peer support
for RPC-over-TLS.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/auth.h | 1
net/sunrpc/Makefile | 2 -
net/sunrpc/auth.c | 2 -
net/sunrpc/auth_tls.c | 120 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 123 insertions(+), 2 deletions(-)
create mode 100644 net/sunrpc/auth_tls.c
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 3e6ce288a7fc..1f13d923f439 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -144,6 +144,7 @@ struct rpc_credops {
extern const struct rpc_authops authunix_ops;
extern const struct rpc_authops authnull_ops;
+extern const struct rpc_authops authtls_ops;
int __init rpc_init_authunix(void);
int __init rpcauth_init_module(void);
diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile
index 1c8de397d6ad..f89c10fe7e6a 100644
--- a/net/sunrpc/Makefile
+++ b/net/sunrpc/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SUNRPC_GSS) += auth_gss/
obj-$(CONFIG_SUNRPC_XPRT_RDMA) += xprtrdma/
sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \
- auth.o auth_null.o auth_unix.o \
+ auth.o auth_null.o auth_tls.o auth_unix.o \
svc.o svcsock.o svcauth.o svcauth_unix.o \
addr.o rpcb_clnt.o timer.o xdr.o \
sunrpc_syms.o cache.o rpc_pipe.o sysfs.o \
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index fb75a883503f..2f16f9d17966 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -32,7 +32,7 @@ static unsigned int auth_hashbits = RPC_CREDCACHE_DEFAULT_HASHBITS;
static const struct rpc_authops __rcu *auth_flavors[RPC_AUTH_MAXFLAVOR] = {
[RPC_AUTH_NULL] = (const struct rpc_authops __force __rcu *)&authnull_ops,
[RPC_AUTH_UNIX] = (const struct rpc_authops __force __rcu *)&authunix_ops,
- NULL, /* others can be loadable modules */
+ [RPC_AUTH_TLS] = (const struct rpc_authops __force __rcu *)&authtls_ops,
};
static LIST_HEAD(cred_unused);
diff --git a/net/sunrpc/auth_tls.c b/net/sunrpc/auth_tls.c
new file mode 100644
index 000000000000..cf4185f188e9
--- /dev/null
+++ b/net/sunrpc/auth_tls.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, 2022 Oracle. All rights reserved.
+ *
+ * The AUTH_TLS credential is used only to probe a remote peer
+ * for RPC-over-TLS support.
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/sunrpc/clnt.h>
+
+static const char *starttls_token = "STARTTLS";
+static const size_t starttls_len = 8;
+
+static struct rpc_auth tls_auth;
+static struct rpc_cred tls_cred;
+
+static struct rpc_auth *tls_create(const struct rpc_auth_create_args *args,
+ struct rpc_clnt *clnt)
+{
+ refcount_inc(&tls_auth.au_count);
+ return &tls_auth;
+}
+
+static void tls_destroy(struct rpc_auth *auth)
+{
+}
+
+static struct rpc_cred *tls_lookup_cred(struct rpc_auth *auth,
+ struct auth_cred *acred, int flags)
+{
+ return get_rpccred(&tls_cred);
+}
+
+static void tls_destroy_cred(struct rpc_cred *cred)
+{
+}
+
+static int tls_match(struct auth_cred *acred, struct rpc_cred *cred, int taskflags)
+{
+ return 1;
+}
+
+static int tls_marshal(struct rpc_task *task, struct xdr_stream *xdr)
+{
+ __be32 *p;
+
+ p = xdr_reserve_space(xdr, 4 * XDR_UNIT);
+ if (!p)
+ return -EMSGSIZE;
+ /* Credential */
+ *p++ = rpc_auth_tls;
+ *p++ = xdr_zero;
+ /* Verifier */
+ *p++ = rpc_auth_null;
+ *p = xdr_zero;
+ return 0;
+}
+
+static int tls_refresh(struct rpc_task *task)
+{
+ set_bit(RPCAUTH_CRED_UPTODATE, &task->tk_rqstp->rq_cred->cr_flags);
+ return 0;
+}
+
+static int tls_validate(struct rpc_task *task, struct xdr_stream *xdr)
+{
+ __be32 *p;
+ void *str;
+
+ p = xdr_inline_decode(xdr, XDR_UNIT);
+ if (!p)
+ return -EIO;
+ if (*p != rpc_auth_null)
+ return -EIO;
+ if (xdr_stream_decode_opaque_inline(xdr, &str, starttls_len) != starttls_len)
+ return -EIO;
+ if (memcmp(str, starttls_token, starttls_len))
+ return -EIO;
+ return 0;
+}
+
+const struct rpc_authops authtls_ops = {
+ .owner = THIS_MODULE,
+ .au_flavor = RPC_AUTH_TLS,
+ .au_name = "NULL",
+ .create = tls_create,
+ .destroy = tls_destroy,
+ .lookup_cred = tls_lookup_cred,
+};
+
+static struct rpc_auth tls_auth = {
+ .au_cslack = NUL_CALLSLACK,
+ .au_rslack = NUL_REPLYSLACK,
+ .au_verfsize = NUL_REPLYSLACK,
+ .au_ralign = NUL_REPLYSLACK,
+ .au_ops = &authtls_ops,
+ .au_flavor = RPC_AUTH_TLS,
+ .au_count = REFCOUNT_INIT(1),
+};
+
+static const struct rpc_credops tls_credops = {
+ .cr_name = "AUTH_TLS",
+ .crdestroy = tls_destroy_cred,
+ .crmatch = tls_match,
+ .crmarshal = tls_marshal,
+ .crwrap_req = rpcauth_wrap_req_encode,
+ .crrefresh = tls_refresh,
+ .crvalidate = tls_validate,
+ .crunwrap_resp = rpcauth_unwrap_resp_decode,
+};
+
+static struct rpc_cred tls_cred = {
+ .cr_lru = LIST_HEAD_INIT(tls_cred.cr_lru),
+ .cr_auth = &tls_auth,
+ .cr_ops = &tls_credops,
+ .cr_count = REFCOUNT_INIT(2),
+ .cr_flags = 1UL << RPCAUTH_CRED_UPTODATE,
+};
From: Chuck Lever <[email protected]>
Introduce a connect worker function that will handle the AUTH_TLS
probe and TLS handshake, once a TCP connection is established.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/xprtsock.h | 1 +
net/sunrpc/xprtsock.c | 70 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index daef030f4848..574a6a5391ba 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -60,6 +60,7 @@ struct sock_xprt {
struct sockaddr_storage srcaddr;
unsigned short srcport;
int xprt_err;
+ struct rpc_clnt *clnt;
/*
* UDP socket buffer size parameters
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 6f2fc863b47e..7ea5984a52a3 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2411,6 +2411,62 @@ static void xs_tcp_setup_socket(struct work_struct *work)
current_restore_flags(pflags, PF_MEMALLOC);
}
+/**
+ * xs_tls_connect - establish a TLS session on a socket
+ * @work: queued work item
+ *
+ */
+static void xs_tls_connect(struct work_struct *work)
+{
+ struct sock_xprt *transport =
+ container_of(work, struct sock_xprt, connect_worker.work);
+ struct rpc_clnt *clnt;
+
+ clnt = transport->clnt;
+ transport->clnt = NULL;
+ if (IS_ERR(clnt))
+ goto out_unlock;
+
+ xs_tcp_setup_socket(work);
+
+ rpc_shutdown_client(clnt);
+
+out_unlock:
+ return;
+}
+
+static void xs_set_transport_clnt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+{
+ struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
+ struct rpc_create_args args = {
+ .net = xprt->xprt_net,
+ .protocol = xprt->prot,
+ .address = (struct sockaddr *)&xprt->addr,
+ .addrsize = xprt->addrlen,
+ .timeout = clnt->cl_timeout,
+ .servername = xprt->servername,
+ .nodename = clnt->cl_nodename,
+ .program = clnt->cl_program,
+ .prognumber = clnt->cl_prog,
+ .version = clnt->cl_vers,
+ .authflavor = RPC_AUTH_TLS,
+ .cred = clnt->cl_cred,
+ .xprtsec = {
+ .policy = RPC_XPRTSEC_NONE,
+ },
+ .flags = RPC_CLNT_CREATE_NOPING,
+ };
+
+ switch (xprt->xprtsec.policy) {
+ case RPC_XPRTSEC_TLS_ANON:
+ case RPC_XPRTSEC_TLS_X509:
+ transport->clnt = rpc_create(&args);
+ break;
+ default:
+ transport->clnt = ERR_PTR(-ENOTCONN);
+ }
+}
+
/**
* xs_connect - connect a socket to a remote endpoint
* @xprt: pointer to transport structure
@@ -2442,6 +2498,8 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
} else
dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
+ xs_set_transport_clnt(task->tk_client, xprt);
+
queue_delayed_work(xprtiod_workqueue,
&transport->connect_worker,
delay);
@@ -3057,7 +3115,17 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
INIT_WORK(&transport->recv_worker, xs_stream_data_receive_workfn);
INIT_WORK(&transport->error_worker, xs_error_handle);
- INIT_DELAYED_WORK(&transport->connect_worker, xs_tcp_setup_socket);
+
+ xprt->xprtsec = args->xprtsec;
+ switch (args->xprtsec.policy) {
+ case RPC_XPRTSEC_NONE:
+ INIT_DELAYED_WORK(&transport->connect_worker, xs_tcp_setup_socket);
+ break;
+ case RPC_XPRTSEC_TLS_ANON:
+ case RPC_XPRTSEC_TLS_X509:
+ INIT_DELAYED_WORK(&transport->connect_worker, xs_tls_connect);
+ break;
+ }
switch (addr->sa_family) {
case AF_INET:
From: Chuck Lever <[email protected]>
The RPC header parser doesn't recognize TLS handshake traffic, so it
will close the connection prematurely with an error. To avoid that,
shunt the transport's data_ready callback when there is a TLS
handshake in progress.
The XPRT_SOCK_IGNORE_RECV flag will be toggled by code added in a
subsequent patch.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/xprtsock.h | 1 +
net/sunrpc/xprtsock.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 38284f25eddf..daef030f4848 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -90,5 +90,6 @@ struct sock_xprt {
#define XPRT_SOCK_WAKE_DISCONNECT (7)
#define XPRT_SOCK_CONNECT_SENT (8)
#define XPRT_SOCK_NOSPACE (9)
+#define XPRT_SOCK_IGNORE_RECV (10)
#endif /* _LINUX_SUNRPC_XPRTSOCK_H */
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 5f9030b81c9e..37f608c2c0a0 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -695,6 +695,8 @@ static void xs_poll_check_readable(struct sock_xprt *transport)
{
clear_bit(XPRT_SOCK_DATA_READY, &transport->sock_state);
+ if (test_bit(XPRT_SOCK_IGNORE_RECV, &transport->sock_state))
+ return;
if (!xs_poll_socket_readable(transport))
return;
if (!test_and_set_bit(XPRT_SOCK_DATA_READY, &transport->sock_state))
@@ -1380,6 +1382,10 @@ static void xs_data_ready(struct sock *sk)
trace_xs_data_ready(xprt);
transport->old_data_ready(sk);
+
+ if (test_bit(XPRT_SOCK_IGNORE_RECV, &transport->sock_state))
+ return;
+
/* Any data means we had a useful conversation, so
* then we don't need to delay the next reconnect
*/
From: Chuck Lever <[email protected]>
After some discussion, we decided that controlling transport layer
security policy should be separate from the setting for the user
authentication flavor. To accomplish this, add a new NFS mount
option to select a transport layer security policy for RPC
operations associated with the mount point.
xprtsec=none - Transport layer security is forced off.
xprtsec=tls - Establish an encryption-only TLS session. If
the initial handshake fails, the mount fails.
If TLS is not available on a reconnect, drop
the connection and try again.
xprtsec=mtls - Both sides authenticate and an encrypted
session is created. If the initial handshake
fails, the mount fails. If TLS is not available
on a reconnect, drop the connection and try
again.
To support client peer authentication (mtls), the handshake daemon
will have configurable default authentication material (certificate
or pre-shared key). In the future, mount options can be added that
can provide this material on a per-mount basis.
Updates to mount.nfs (to support xprtsec=auto) and nfs(5) will be
sent under separate cover.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfs/client.c | 5 ++---
fs/nfs/fs_context.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/internal.h | 1 +
fs/nfs/nfs4client.c | 6 ++----
fs/nfs/super.c | 12 ++++++++++++
5 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 9bfdade0f6e6..c3a984b1879d 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -515,6 +515,7 @@ int nfs_create_rpc_client(struct nfs_client *clp,
.version = clp->rpc_ops->version,
.authflavor = flavor,
.cred = cl_init->cred,
+ .xprtsec = cl_init->xprtsec,
};
if (test_bit(NFS_CS_DISCRTRY, &clp->cl_flags))
@@ -680,9 +681,7 @@ static int nfs_init_server(struct nfs_server *server,
.cred = server->cred,
.nconnect = ctx->nfs_server.nconnect,
.init_flags = (1UL << NFS_CS_REUSEPORT),
- .xprtsec = {
- .policy = RPC_XPRTSEC_NONE,
- },
+ .xprtsec = ctx->xprtsec,
};
struct nfs_client *clp;
int error;
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 5626d358ee2e..e49e3d18ef88 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -18,6 +18,9 @@
#include <linux/nfs_fs.h>
#include <linux/nfs_mount.h>
#include <linux/nfs4_mount.h>
+
+#include <net/handshake.h>
+
#include "nfs.h"
#include "internal.h"
@@ -88,6 +91,7 @@ enum nfs_param {
Opt_vers,
Opt_wsize,
Opt_write,
+ Opt_xprtsec,
};
enum {
@@ -194,6 +198,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
fsparam_string("vers", Opt_vers),
fsparam_enum ("write", Opt_write, nfs_param_enums_write),
fsparam_u32 ("wsize", Opt_wsize),
+ fsparam_string("xprtsec", Opt_xprtsec),
{}
};
@@ -267,6 +272,20 @@ static const struct constant_table nfs_secflavor_tokens[] = {
{}
};
+enum {
+ Opt_xprtsec_none,
+ Opt_xprtsec_tls,
+ Opt_xprtsec_mtls,
+ nr__Opt_xprtsec
+};
+
+static const struct constant_table nfs_xprtsec_policies[] = {
+ { "none", Opt_xprtsec_none },
+ { "tls", Opt_xprtsec_tls },
+ { "mtls", Opt_xprtsec_mtls },
+ {}
+};
+
/*
* Sanity-check a server address provided by the mount command.
*
@@ -430,6 +449,29 @@ static int nfs_parse_security_flavors(struct fs_context *fc,
return 0;
}
+static int nfs_parse_xprtsec_policy(struct fs_context *fc,
+ struct fs_parameter *param)
+{
+ struct nfs_fs_context *ctx = nfs_fc2context(fc);
+
+ trace_nfs_mount_assign(param->key, param->string);
+
+ switch (lookup_constant(nfs_xprtsec_policies, param->string, -1)) {
+ case Opt_xprtsec_none:
+ ctx->xprtsec.policy = RPC_XPRTSEC_NONE;
+ break;
+ case Opt_xprtsec_tls:
+ ctx->xprtsec.policy = RPC_XPRTSEC_TLS_ANON;
+ break;
+ case Opt_xprtsec_mtls:
+ ctx->xprtsec.policy = RPC_XPRTSEC_TLS_X509;
+ break;
+ default:
+ return nfs_invalf(fc, "NFS: Unrecognized transport security policy");
+ }
+ return 0;
+}
+
static int nfs_parse_version_string(struct fs_context *fc,
const char *string)
{
@@ -696,6 +738,11 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
if (ret < 0)
return ret;
break;
+ case Opt_xprtsec:
+ ret = nfs_parse_xprtsec_policy(fc, param);
+ if (ret < 0)
+ return ret;
+ break;
case Opt_proto:
if (!param->string)
@@ -1574,6 +1621,9 @@ static int nfs_init_fs_context(struct fs_context *fc)
ctx->selected_flavor = RPC_AUTH_MAXFLAVOR;
ctx->minorversion = 0;
ctx->need_mount = true;
+ ctx->xprtsec.policy = RPC_XPRTSEC_NONE;
+ ctx->xprtsec.cert_serial = TLS_NO_CERT;
+ ctx->xprtsec.privkey_serial = TLS_NO_PRIVKEY;
fc->s_iflags |= SB_I_STABLE_WRITES;
}
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 5c986c0d3cce..0019c7578f9d 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -102,6 +102,7 @@ struct nfs_fs_context {
unsigned int bsize;
struct nfs_auth_info auth_info;
rpc_authflavor_t selected_flavor;
+ struct xprtsec_parms xprtsec;
char *client_address;
unsigned int version;
unsigned int minorversion;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 75ed8354576b..bfc68d4e8d32 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -1130,9 +1130,6 @@ static int nfs4_server_common_setup(struct nfs_server *server,
static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
{
struct nfs_fs_context *ctx = nfs_fc2context(fc);
- struct xprtsec_parms xprtsec = {
- .policy = RPC_XPRTSEC_NONE,
- };
struct rpc_timeout timeparms;
int error;
@@ -1164,7 +1161,7 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
ctx->nfs_server.nconnect,
ctx->nfs_server.max_connect,
fc->net_ns,
- &xprtsec);
+ &ctx->xprtsec);
if (error < 0)
return error;
@@ -1323,6 +1320,7 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
.dstaddr = (struct sockaddr *)sap,
.addrlen = salen,
.servername = hostname,
+ /* cel: bleh. We might need to pass TLS parameters here */
};
char buf[INET6_ADDRSTRLEN + 1];
struct sockaddr_storage address;
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 30e53e93049e..059b0beabc1b 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -59,6 +59,8 @@
#include <linux/uaccess.h>
#include <linux/nfs_ssc.h>
+#include <uapi/linux/tls.h>
+
#include "nfs4_fs.h"
#include "callback.h"
#include "delegation.h"
@@ -491,6 +493,16 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
seq_printf(m, ",timeo=%lu", 10U * nfss->client->cl_timeout->to_initval / HZ);
seq_printf(m, ",retrans=%u", nfss->client->cl_timeout->to_retries);
seq_printf(m, ",sec=%s", nfs_pseudoflavour_to_name(nfss->client->cl_auth->au_flavor));
+ switch (clp->cl_xprtsec.policy) {
+ case RPC_XPRTSEC_TLS_ANON:
+ seq_puts(m, ",xprtsec=tls");
+ break;
+ case RPC_XPRTSEC_TLS_X509:
+ seq_puts(m, ",xprtsec=mtls");
+ break;
+ default:
+ break;
+ }
if (version != 4)
nfs_show_mountd_options(m, nfss, showdefaults);
From: Chuck Lever <[email protected]>
The new field is used to match struct nfs_clients that have the same
TLS policy setting.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfs/client.c | 8 ++++++++
fs/nfs/internal.h | 1 +
fs/nfs/nfs3client.c | 1 +
fs/nfs/nfs4client.c | 20 +++++++++++++++-----
include/linux/nfs_fs_sb.h | 3 ++-
5 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index f50e025ae406..9bfdade0f6e6 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -184,6 +184,7 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
clp->cl_net = get_net(cl_init->net);
clp->cl_principal = "*";
+ clp->cl_xprtsec = cl_init->xprtsec;
return clp;
error_cleanup:
@@ -326,6 +327,10 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
sap))
continue;
+ /* Match the xprt security policy */
+ if (clp->cl_xprtsec.policy != data->xprtsec.policy)
+ continue;
+
refcount_inc(&clp->cl_count);
return clp;
}
@@ -675,6 +680,9 @@ static int nfs_init_server(struct nfs_server *server,
.cred = server->cred,
.nconnect = ctx->nfs_server.nconnect,
.init_flags = (1UL << NFS_CS_REUSEPORT),
+ .xprtsec = {
+ .policy = RPC_XPRTSEC_NONE,
+ },
};
struct nfs_client *clp;
int error;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 3cc027d3bd58..5c986c0d3cce 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -81,6 +81,7 @@ struct nfs_client_initdata {
struct net *net;
const struct rpc_timeout *timeparms;
const struct cred *cred;
+ struct xprtsec_parms xprtsec;
};
/*
diff --git a/fs/nfs/nfs3client.c b/fs/nfs/nfs3client.c
index 669cda757a5c..8fa187a9c46d 100644
--- a/fs/nfs/nfs3client.c
+++ b/fs/nfs/nfs3client.c
@@ -93,6 +93,7 @@ struct nfs_client *nfs3_set_ds_client(struct nfs_server *mds_srv,
.net = mds_clp->cl_net,
.timeparms = &ds_timeout,
.cred = mds_srv->cred,
+ .xprtsec = mds_clp->cl_xprtsec,
};
struct nfs_client *clp;
char buf[INET6_ADDRSTRLEN + 1];
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index d3051b051a56..75ed8354576b 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -896,7 +896,8 @@ static int nfs4_set_client(struct nfs_server *server,
int proto, const struct rpc_timeout *timeparms,
u32 minorversion, unsigned int nconnect,
unsigned int max_connect,
- struct net *net)
+ struct net *net,
+ struct xprtsec_parms *xprtsec)
{
struct nfs_client_initdata cl_init = {
.hostname = hostname,
@@ -909,6 +910,7 @@ static int nfs4_set_client(struct nfs_server *server,
.net = net,
.timeparms = timeparms,
.cred = server->cred,
+ .xprtsec = *xprtsec,
};
struct nfs_client *clp;
@@ -978,6 +980,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
.net = mds_clp->cl_net,
.timeparms = &ds_timeout,
.cred = mds_srv->cred,
+ .xprtsec = mds_srv->nfs_client->cl_xprtsec,
};
char buf[INET6_ADDRSTRLEN + 1];
@@ -1127,6 +1130,9 @@ static int nfs4_server_common_setup(struct nfs_server *server,
static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
{
struct nfs_fs_context *ctx = nfs_fc2context(fc);
+ struct xprtsec_parms xprtsec = {
+ .policy = RPC_XPRTSEC_NONE,
+ };
struct rpc_timeout timeparms;
int error;
@@ -1157,7 +1163,8 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
ctx->minorversion,
ctx->nfs_server.nconnect,
ctx->nfs_server.max_connect,
- fc->net_ns);
+ fc->net_ns,
+ &xprtsec);
if (error < 0)
return error;
@@ -1247,7 +1254,8 @@ struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
parent_client->cl_mvops->minor_version,
parent_client->cl_nconnect,
parent_client->cl_max_connect,
- parent_client->cl_net);
+ parent_client->cl_net,
+ &parent_client->cl_xprtsec);
if (!error)
goto init_server;
#endif /* IS_ENABLED(CONFIG_SUNRPC_XPRT_RDMA) */
@@ -1263,7 +1271,8 @@ struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
parent_client->cl_mvops->minor_version,
parent_client->cl_nconnect,
parent_client->cl_max_connect,
- parent_client->cl_net);
+ parent_client->cl_net,
+ &parent_client->cl_xprtsec);
if (error < 0)
goto error;
@@ -1336,7 +1345,8 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
error = nfs4_set_client(server, hostname, sap, salen, buf,
clp->cl_proto, clnt->cl_timeout,
clp->cl_minorversion,
- clp->cl_nconnect, clp->cl_max_connect, net);
+ clp->cl_nconnect, clp->cl_max_connect,
+ net, &clp->cl_xprtsec);
clear_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
if (error != 0) {
nfs_server_insert_lists(server);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index ea2f7e6b1b0b..fa5a592de798 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -63,7 +63,8 @@ struct nfs_client {
u32 cl_minorversion;/* NFSv4 minorversion */
unsigned int cl_nconnect; /* Number of connections */
unsigned int cl_max_connect; /* max number of xprts allowed */
- const char * cl_principal; /* used for machine cred */
+ const char * cl_principal; /* used for machine cred */
+ struct xprtsec_parms cl_xprtsec; /* xprt security policy */
#if IS_ENABLED(CONFIG_NFS_V4)
struct list_head cl_ds_clients; /* auth flavor data servers */
From: Chuck Lever <[email protected]>
RFC 9289 makes auditing TLS handshakes mandatory-to-implement.
Signed-off-by: Chuck Lever <[email protected]>
---
include/trace/events/sunrpc.h | 44 +++++++++++++++++++++++++++++++++++++++++
net/sunrpc/xprtsock.c | 5 ++++-
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 34784f29a63d..7cd4bbd6904c 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1525,6 +1525,50 @@ TRACE_EVENT(rpcb_unregister,
)
);
+/**
+ ** RPC-over-TLS tracepoints
+ **/
+
+DECLARE_EVENT_CLASS(rpc_tls_class,
+ TP_PROTO(
+ const struct rpc_clnt *clnt,
+ const struct rpc_xprt *xprt
+ ),
+
+ TP_ARGS(clnt, xprt),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, requested_policy)
+ __field(u32, version)
+ __string(servername, xprt->servername)
+ __string(progname, clnt->cl_program->name)
+ ),
+
+ TP_fast_assign(
+ __entry->requested_policy = clnt->cl_xprtsec.policy;
+ __entry->version = clnt->cl_vers;
+ __assign_str(servername, xprt->servername);
+ __assign_str(progname, clnt->cl_program->name)
+ ),
+
+ TP_printk("server=%s %sv%u requested_policy=%s",
+ __get_str(servername), __get_str(progname), __entry->version,
+ rpc_show_xprtsec_policy(__entry->requested_policy)
+ )
+);
+
+#define DEFINE_RPC_TLS_EVENT(name) \
+ DEFINE_EVENT(rpc_tls_class, rpc_tls_##name, \
+ TP_PROTO( \
+ const struct rpc_clnt *clnt, \
+ const struct rpc_xprt *xprt \
+ ), \
+ TP_ARGS(clnt, xprt))
+
+DEFINE_RPC_TLS_EVENT(unavailable);
+DEFINE_RPC_TLS_EVENT(not_started);
+
+
/* Record an xdr_buf containing a fully-formed RPC message */
DECLARE_EVENT_CLASS(svc_xdr_msg_class,
TP_PROTO(
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 686dd313f89f..7ade414aa1cb 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2630,6 +2630,7 @@ static void xs_tls_connect(struct work_struct *work)
/* This implicitly sends an RPC_AUTH_TLS probe */
lower_clnt = rpc_create(&args);
if (IS_ERR(lower_clnt)) {
+ trace_rpc_tls_unavailable(upper_clnt, upper_xprt);
clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
xprt_clear_connecting(upper_xprt);
xprt_wake_pending_tasks(upper_xprt, PTR_ERR(lower_clnt));
@@ -2645,8 +2646,10 @@ static void xs_tls_connect(struct work_struct *work)
lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
rcu_read_unlock();
status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
- if (status)
+ if (status) {
+ trace_rpc_tls_not_started(upper_clnt, upper_xprt);
goto out_close;
+ }
status = xs_tls_finish_connecting(lower_xprt, upper_transport);
if (status)
From: Chuck Lever <[email protected]>
Use the new TLS handshake API to enable the SunRPC client code
to request a TLS handshake. This implements support for RFC 9289.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/xprtsock.h | 1
net/sunrpc/xprtsock.c | 289 ++++++++++++++++++++++++++++++++++-----
2 files changed, 253 insertions(+), 37 deletions(-)
diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 574a6a5391ba..700a1e6c047c 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -57,6 +57,7 @@ struct sock_xprt {
struct work_struct error_worker;
struct work_struct recv_worker;
struct mutex recv_mutex;
+ struct completion handshake_done;
struct sockaddr_storage srcaddr;
unsigned short srcport;
int xprt_err;
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 7ea5984a52a3..686dd313f89f 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -48,6 +48,7 @@
#include <net/udp.h>
#include <net/tcp.h>
#include <net/tls.h>
+#include <net/handshake.h>
#include <linux/bvec.h>
#include <linux/highmem.h>
@@ -189,6 +190,11 @@ static struct ctl_table xs_tunables_table[] = {
*/
#define XS_IDLE_DISC_TO (5U * 60 * HZ)
+/*
+ * TLS handshake timeout.
+ */
+#define XS_TLS_HANDSHAKE_TO (10U * HZ)
+
#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
# undef RPC_DEBUG_DATA
# define RPCDBG_FACILITY RPCDBG_TRANS
@@ -1238,6 +1244,10 @@ static void xs_reset_transport(struct sock_xprt *transport)
if (atomic_read(&transport->xprt.swapper))
sk_clear_memalloc(sk);
+ /* XXX: Maybe also send a TLS Closure alert? */
+
+ tls_handshake_cancel(sk);
+
kernel_sock_shutdown(sock, SHUT_RDWR);
mutex_lock(&transport->recv_mutex);
@@ -2411,60 +2421,266 @@ static void xs_tcp_setup_socket(struct work_struct *work)
current_restore_flags(pflags, PF_MEMALLOC);
}
+/*
+ * Transfer the connected socket to @upper_transport, then mark that
+ * xprt CONNECTED.
+ */
+static int xs_tls_finish_connecting(struct rpc_xprt *lower_xprt,
+ struct sock_xprt *upper_transport)
+{
+ struct sock_xprt *lower_transport =
+ container_of(lower_xprt, struct sock_xprt, xprt);
+ struct rpc_xprt *upper_xprt = &upper_transport->xprt;
+
+ if (!upper_transport->inet) {
+ struct socket *sock = lower_transport->sock;
+ struct sock *sk = sock->sk;
+
+ /* Avoid temporary address, they are bad for long-lived
+ * connections such as NFS mounts.
+ * RFC4941, section 3.6 suggests that:
+ * Individual applications, which have specific
+ * knowledge about the normal duration of connections,
+ * MAY override this as appropriate.
+ */
+ if (xs_addr(upper_xprt)->sa_family == PF_INET6) {
+ ip6_sock_set_addr_preferences(sk,
+ IPV6_PREFER_SRC_PUBLIC);
+ }
+
+ xs_tcp_set_socket_timeouts(upper_xprt, sock);
+ tcp_sock_set_nodelay(sk);
+
+ lock_sock(sk);
+
+ /*
+ * @sk is already connected, so it now has the RPC callbacks.
+ * Reach into @lower_transport to save the original ones.
+ */
+ upper_transport->old_data_ready = lower_transport->old_data_ready;
+ upper_transport->old_state_change = lower_transport->old_state_change;
+ upper_transport->old_write_space = lower_transport->old_write_space;
+ upper_transport->old_error_report = lower_transport->old_error_report;
+ sk->sk_user_data = upper_xprt;
+
+ /* socket options */
+ sock_reset_flag(sk, SOCK_LINGER);
+
+ xprt_clear_connected(upper_xprt);
+
+ upper_transport->sock = sock;
+ upper_transport->inet = sk;
+ upper_transport->file = lower_transport->file;
+
+ release_sock(sk);
+
+ /* Reset lower_transport before shutting down its clnt */
+ mutex_lock(&lower_transport->recv_mutex);
+ lower_transport->inet = NULL;
+ lower_transport->sock = NULL;
+ lower_transport->file = NULL;
+
+ xprt_clear_connected(lower_xprt);
+ xs_sock_reset_connection_flags(lower_xprt);
+ xs_stream_reset_connect(lower_transport);
+ mutex_unlock(&lower_transport->recv_mutex);
+ }
+
+ if (!xprt_bound(upper_xprt))
+ return -ENOTCONN;
+
+ xs_set_memalloc(upper_xprt);
+
+ if (!xprt_test_and_set_connected(upper_xprt)) {
+ upper_xprt->connect_cookie++;
+ clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
+ xprt_clear_connecting(upper_xprt);
+
+ upper_xprt->stat.connect_count++;
+ upper_xprt->stat.connect_time += (long)jiffies -
+ upper_xprt->stat.connect_start;
+ xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
+ }
+ return 0;
+}
+
/**
- * xs_tls_connect - establish a TLS session on a socket
- * @work: queued work item
+ * xs_tls_handshake_done - TLS handshake completion handler
+ * @data: address of xprt to wake
+ * @status: status of handshake
+ * @peerid: serial number of key containing the remote's identity
*
*/
-static void xs_tls_connect(struct work_struct *work)
+static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid)
{
- struct sock_xprt *transport =
- container_of(work, struct sock_xprt, connect_worker.work);
- struct rpc_clnt *clnt;
+ struct rpc_xprt *lower_xprt = data;
+ struct sock_xprt *lower_transport =
+ container_of(lower_xprt, struct sock_xprt, xprt);
- clnt = transport->clnt;
- transport->clnt = NULL;
- if (IS_ERR(clnt))
- goto out_unlock;
+ lower_transport->xprt_err = status ? -EACCES : 0;
+ complete(&lower_transport->handshake_done);
+ xprt_put(lower_xprt);
+}
- xs_tcp_setup_socket(work);
+static int xs_tls_handshake_sync(struct rpc_xprt *lower_xprt, struct xprtsec_parms *xprtsec)
+{
+ struct sock_xprt *lower_transport =
+ container_of(lower_xprt, struct sock_xprt, xprt);
+ struct tls_handshake_args args = {
+ .ta_sock = lower_transport->sock,
+ .ta_done = xs_tls_handshake_done,
+ .ta_data = xprt_get(lower_xprt),
+ .ta_peername = lower_xprt->servername,
+ };
+ struct sock *sk = lower_transport->inet;
+ int rc;
- rpc_shutdown_client(clnt);
+ init_completion(&lower_transport->handshake_done);
+ set_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
-out_unlock:
- return;
+ lower_transport->xprt_err = -ETIMEDOUT;
+ switch (xprtsec->policy) {
+ case RPC_XPRTSEC_TLS_ANON:
+ rc = tls_client_hello_anon(&args, GFP_KERNEL);
+ if (rc)
+ goto out_put_xprt;
+ break;
+ case RPC_XPRTSEC_TLS_X509:
+ args.ta_my_cert = xprtsec->cert_serial;
+ args.ta_my_privkey = xprtsec->privkey_serial;
+ rc = tls_client_hello_x509(&args, GFP_KERNEL);
+ if (rc)
+ goto out_put_xprt;
+ break;
+ default:
+ rc = -EACCES;
+ goto out_put_xprt;
+ }
+
+ rc = wait_for_completion_interruptible_timeout(&lower_transport->handshake_done,
+ XS_TLS_HANDSHAKE_TO);
+ if (rc <= 0) {
+ if (!tls_handshake_cancel(sk)) {
+ if (rc == 0)
+ rc = -ETIMEDOUT;
+ goto out_put_xprt;
+ }
+ }
+
+ rc = lower_transport->xprt_err;
+
+out:
+ xs_stream_reset_connect(lower_transport);
+ clear_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
+ return rc;
+
+out_put_xprt:
+ xprt_put(lower_xprt);
+ goto out;
}
-static void xs_set_transport_clnt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+/**
+ * xs_tls_connect - establish a TLS session on a socket
+ * @work: queued work item
+ *
+ * For RPC-with-TLS, there is a two-stage connection process.
+ *
+ * The "upper-layer xprt" is visible to the RPC consumer. Once it has
+ * been marked connected, the consumer knows that a TCP connection and
+ * a TLS session have been established.
+ *
+ * A "lower-layer xprt", created in this function, handles the mechanics
+ * of connecting the TCP socket, performing the RPC_AUTH_TLS probe, and
+ * then driving the TLS handshake. Once all that is complete, the upper
+ * layer xprt is marked connected.
+ */
+static void xs_tls_connect(struct work_struct *work)
{
- struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
+ struct sock_xprt *upper_transport =
+ container_of(work, struct sock_xprt, connect_worker.work);
+ struct rpc_clnt *upper_clnt = upper_transport->clnt;
+ struct rpc_xprt *upper_xprt = &upper_transport->xprt;
struct rpc_create_args args = {
- .net = xprt->xprt_net,
- .protocol = xprt->prot,
- .address = (struct sockaddr *)&xprt->addr,
- .addrsize = xprt->addrlen,
- .timeout = clnt->cl_timeout,
- .servername = xprt->servername,
- .nodename = clnt->cl_nodename,
- .program = clnt->cl_program,
- .prognumber = clnt->cl_prog,
- .version = clnt->cl_vers,
+ .net = upper_xprt->xprt_net,
+ .protocol = upper_xprt->prot,
+ .address = (struct sockaddr *)&upper_xprt->addr,
+ .addrsize = upper_xprt->addrlen,
+ .timeout = upper_clnt->cl_timeout,
+ .servername = upper_xprt->servername,
+ .nodename = upper_clnt->cl_nodename,
+ .program = upper_clnt->cl_program,
+ .prognumber = upper_clnt->cl_prog,
+ .version = upper_clnt->cl_vers,
.authflavor = RPC_AUTH_TLS,
- .cred = clnt->cl_cred,
+ .cred = upper_clnt->cl_cred,
.xprtsec = {
.policy = RPC_XPRTSEC_NONE,
},
- .flags = RPC_CLNT_CREATE_NOPING,
};
+ unsigned int pflags = current->flags;
+ struct rpc_clnt *lower_clnt;
+ struct rpc_xprt *lower_xprt;
+ int status;
- switch (xprt->xprtsec.policy) {
- case RPC_XPRTSEC_TLS_ANON:
- case RPC_XPRTSEC_TLS_X509:
- transport->clnt = rpc_create(&args);
- break;
- default:
- transport->clnt = ERR_PTR(-ENOTCONN);
+ if (atomic_read(&upper_xprt->swapper))
+ current->flags |= PF_MEMALLOC;
+
+ xs_stream_start_connect(upper_transport);
+
+ /* This implicitly sends an RPC_AUTH_TLS probe */
+ lower_clnt = rpc_create(&args);
+ if (IS_ERR(lower_clnt)) {
+ clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
+ xprt_clear_connecting(upper_xprt);
+ xprt_wake_pending_tasks(upper_xprt, PTR_ERR(lower_clnt));
+ smp_mb__before_atomic();
+ xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
+ goto out_unlock;
}
+
+ /* RPC_AUTH_TLS probe was successful. Try a TLS handshake on
+ * the lower xprt.
+ */
+ rcu_read_lock();
+ lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
+ rcu_read_unlock();
+ status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
+ if (status)
+ goto out_close;
+
+ status = xs_tls_finish_connecting(lower_xprt, upper_transport);
+ if (status)
+ goto out_close;
+
+ trace_rpc_socket_connect(upper_xprt, upper_transport->sock, 0);
+ if (!xprt_test_and_set_connected(upper_xprt)) {
+ upper_xprt->connect_cookie++;
+ clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
+ xprt_clear_connecting(upper_xprt);
+
+ upper_xprt->stat.connect_count++;
+ upper_xprt->stat.connect_time += (long)jiffies -
+ upper_xprt->stat.connect_start;
+ xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
+ }
+ rpc_shutdown_client(lower_clnt);
+
+out_unlock:
+ current_restore_flags(pflags, PF_MEMALLOC);
+ upper_transport->clnt = NULL;
+ xprt_unlock_connect(upper_xprt, upper_transport);
+ return;
+
+out_close:
+ rpc_shutdown_client(lower_clnt);
+
+ /* xprt_force_disconnect() wakes tasks with a fixed tk_status code.
+ * Wake them first here to ensure they get our tk_status code.
+ */
+ xprt_wake_pending_tasks(upper_xprt, status);
+ xs_tcp_force_close(upper_xprt);
+ xprt_clear_connecting(upper_xprt);
+ goto out_unlock;
}
/**
@@ -2498,8 +2714,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
} else
dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
- xs_set_transport_clnt(task->tk_client, xprt);
-
+ transport->clnt = task->tk_client;
queue_delayed_work(xprtiod_workqueue,
&transport->connect_worker,
delay);
On Tue, 2023-05-16 at 15:38 -0400, Chuck Lever wrote:
> Now that TLS handshake support is available in the kernel, let's
> have a look at what is needed to support NFS in-transit confiden-
> tiality in the Linux NFS client.
>
> These apply to v6.4-rc2 (actually, net-next to be precise), but
> previously they've been tested at multiple NFS bake-a-thon events.
>
> ---
>
> Chuck Lever (12):
> NFS: Improvements for fs_context-related tracepoints
> SUNRPC: Plumb an API for setting transport layer security
> SUNRPC: Trace the rpc_create_args
> SUNRPC: Refactor rpc_call_null_helper()
> SUNRPC: Add RPC client support for the RPC_AUTH_TLS auth flavor
> SUNRPC: Ignore data_ready callbacks during TLS handshakes
> SUNRPC: Capture CMSG metadata on client-side receive
> SUNRPC: Add a connect worker function for TLS
> SUNRPC: Add RPC-with-TLS support to xprtsock.c
> SUNRPC: Add RPC-with-TLS tracepoints
> NFS: Have struct nfs_client carry a TLS policy field
> NFS: Add an "xprtsec=" NFS mount option
>
>
> fs/nfs/client.c | 7 +
> fs/nfs/fs_context.c | 55 +++++
> fs/nfs/internal.h | 2 +
> fs/nfs/nfs3client.c | 1 +
> fs/nfs/nfs4client.c | 18 +-
> fs/nfs/super.c | 12 ++
> include/linux/nfs_fs_sb.h | 3 +-
> include/linux/sunrpc/auth.h | 1 +
> include/linux/sunrpc/clnt.h | 2 +
> include/linux/sunrpc/xprt.h | 17 ++
> include/linux/sunrpc/xprtsock.h | 3 +
> include/trace/events/sunrpc.h | 96 ++++++++-
> net/sunrpc/Makefile | 2 +-
> net/sunrpc/auth.c | 2 +-
> net/sunrpc/auth_tls.c | 120 +++++++++++
> net/sunrpc/clnt.c | 22 +-
> net/sunrpc/xprtsock.c | 343 +++++++++++++++++++++++++++++++-
> 17 files changed, 677 insertions(+), 29 deletions(-)
> create mode 100644 net/sunrpc/auth_tls.c
>
These all look reasonable to me. For any that don't already have it, you
can add:
Reviewed-by: Jeff Layton <[email protected]>
I'd really like to see these in linux-next soon, so that there is a
prayer of them making v6.5.
Thanks,
--
Jeff Layton <[email protected]>
Hi Chuck,
On Tue, May 16, 2023 at 3:52 PM Chuck Lever <[email protected]> wrote:
>
> From: Chuck Lever <[email protected]>
>
> Use the new TLS handshake API to enable the SunRPC client code
> to request a TLS handshake. This implements support for RFC 9289.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> include/linux/sunrpc/xprtsock.h | 1
> net/sunrpc/xprtsock.c | 289 ++++++++++++++++++++++++++++++++++-----
> 2 files changed, 253 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
> index 574a6a5391ba..700a1e6c047c 100644
> --- a/include/linux/sunrpc/xprtsock.h
> +++ b/include/linux/sunrpc/xprtsock.h
> @@ -57,6 +57,7 @@ struct sock_xprt {
> struct work_struct error_worker;
> struct work_struct recv_worker;
> struct mutex recv_mutex;
> + struct completion handshake_done;
> struct sockaddr_storage srcaddr;
> unsigned short srcport;
> int xprt_err;
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 7ea5984a52a3..686dd313f89f 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -48,6 +48,7 @@
> #include <net/udp.h>
> #include <net/tcp.h>
> #include <net/tls.h>
> +#include <net/handshake.h>
>
> #include <linux/bvec.h>
> #include <linux/highmem.h>
> @@ -189,6 +190,11 @@ static struct ctl_table xs_tunables_table[] = {
> */
> #define XS_IDLE_DISC_TO (5U * 60 * HZ)
>
> +/*
> + * TLS handshake timeout.
> + */
> +#define XS_TLS_HANDSHAKE_TO (10U * HZ)
> +
> #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> # undef RPC_DEBUG_DATA
> # define RPCDBG_FACILITY RPCDBG_TRANS
> @@ -1238,6 +1244,10 @@ static void xs_reset_transport(struct sock_xprt *transport)
> if (atomic_read(&transport->xprt.swapper))
> sk_clear_memalloc(sk);
>
> + /* XXX: Maybe also send a TLS Closure alert? */
> +
> + tls_handshake_cancel(sk);
> +
> kernel_sock_shutdown(sock, SHUT_RDWR);
>
> mutex_lock(&transport->recv_mutex);
> @@ -2411,60 +2421,266 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> current_restore_flags(pflags, PF_MEMALLOC);
> }
>
> +/*
> + * Transfer the connected socket to @upper_transport, then mark that
> + * xprt CONNECTED.
> + */
> +static int xs_tls_finish_connecting(struct rpc_xprt *lower_xprt,
> + struct sock_xprt *upper_transport)
> +{
> + struct sock_xprt *lower_transport =
> + container_of(lower_xprt, struct sock_xprt, xprt);
> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
> +
> + if (!upper_transport->inet) {
> + struct socket *sock = lower_transport->sock;
> + struct sock *sk = sock->sk;
> +
> + /* Avoid temporary address, they are bad for long-lived
> + * connections such as NFS mounts.
> + * RFC4941, section 3.6 suggests that:
> + * Individual applications, which have specific
> + * knowledge about the normal duration of connections,
> + * MAY override this as appropriate.
> + */
> + if (xs_addr(upper_xprt)->sa_family == PF_INET6) {
> + ip6_sock_set_addr_preferences(sk,
> + IPV6_PREFER_SRC_PUBLIC);
> + }
> +
> + xs_tcp_set_socket_timeouts(upper_xprt, sock);
> + tcp_sock_set_nodelay(sk);
> +
> + lock_sock(sk);
> +
> + /*
> + * @sk is already connected, so it now has the RPC callbacks.
> + * Reach into @lower_transport to save the original ones.
> + */
> + upper_transport->old_data_ready = lower_transport->old_data_ready;
> + upper_transport->old_state_change = lower_transport->old_state_change;
> + upper_transport->old_write_space = lower_transport->old_write_space;
> + upper_transport->old_error_report = lower_transport->old_error_report;
> + sk->sk_user_data = upper_xprt;
> +
> + /* socket options */
> + sock_reset_flag(sk, SOCK_LINGER);
> +
> + xprt_clear_connected(upper_xprt);
> +
> + upper_transport->sock = sock;
> + upper_transport->inet = sk;
> + upper_transport->file = lower_transport->file;
> +
> + release_sock(sk);
> +
> + /* Reset lower_transport before shutting down its clnt */
> + mutex_lock(&lower_transport->recv_mutex);
> + lower_transport->inet = NULL;
> + lower_transport->sock = NULL;
> + lower_transport->file = NULL;
> +
> + xprt_clear_connected(lower_xprt);
> + xs_sock_reset_connection_flags(lower_xprt);
> + xs_stream_reset_connect(lower_transport);
> + mutex_unlock(&lower_transport->recv_mutex);
> + }
> +
> + if (!xprt_bound(upper_xprt))
> + return -ENOTCONN;
> +
> + xs_set_memalloc(upper_xprt);
> +
> + if (!xprt_test_and_set_connected(upper_xprt)) {
> + upper_xprt->connect_cookie++;
> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> + xprt_clear_connecting(upper_xprt);
> +
> + upper_xprt->stat.connect_count++;
> + upper_xprt->stat.connect_time += (long)jiffies -
> + upper_xprt->stat.connect_start;
> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> + }
> + return 0;
> +}
> +
> /**
> - * xs_tls_connect - establish a TLS session on a socket
> - * @work: queued work item
> + * xs_tls_handshake_done - TLS handshake completion handler
> + * @data: address of xprt to wake
> + * @status: status of handshake
> + * @peerid: serial number of key containing the remote's identity
> *
> */
> -static void xs_tls_connect(struct work_struct *work)
> +static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid)
> {
> - struct sock_xprt *transport =
> - container_of(work, struct sock_xprt, connect_worker.work);
> - struct rpc_clnt *clnt;
> + struct rpc_xprt *lower_xprt = data;
> + struct sock_xprt *lower_transport =
> + container_of(lower_xprt, struct sock_xprt, xprt);
>
> - clnt = transport->clnt;
> - transport->clnt = NULL;
> - if (IS_ERR(clnt))
> - goto out_unlock;
> + lower_transport->xprt_err = status ? -EACCES : 0;
> + complete(&lower_transport->handshake_done);
> + xprt_put(lower_xprt);
> +}
>
> - xs_tcp_setup_socket(work);
> +static int xs_tls_handshake_sync(struct rpc_xprt *lower_xprt, struct xprtsec_parms *xprtsec)
> +{
> + struct sock_xprt *lower_transport =
> + container_of(lower_xprt, struct sock_xprt, xprt);
> + struct tls_handshake_args args = {
> + .ta_sock = lower_transport->sock,
> + .ta_done = xs_tls_handshake_done,
> + .ta_data = xprt_get(lower_xprt),
> + .ta_peername = lower_xprt->servername,
This part isn't compiling for me on v6.4-rc2:
net/sunrpc/xprtsock.c:2538:4: error: field designator 'ta_peername'
does not refer to any field in type 'struct tls_handshake_args'
.ta_peername = lower_xprt->servername,
^
1 error generated.
Am I missing a patch, or did this struct get changed somewhere along the line?
Anna
> + };
> + struct sock *sk = lower_transport->inet;
> + int rc;
>
> - rpc_shutdown_client(clnt);
> + init_completion(&lower_transport->handshake_done);
> + set_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
>
> -out_unlock:
> - return;
> + lower_transport->xprt_err = -ETIMEDOUT;
> + switch (xprtsec->policy) {
> + case RPC_XPRTSEC_TLS_ANON:
> + rc = tls_client_hello_anon(&args, GFP_KERNEL);
> + if (rc)
> + goto out_put_xprt;
> + break;
> + case RPC_XPRTSEC_TLS_X509:
> + args.ta_my_cert = xprtsec->cert_serial;
> + args.ta_my_privkey = xprtsec->privkey_serial;
> + rc = tls_client_hello_x509(&args, GFP_KERNEL);
> + if (rc)
> + goto out_put_xprt;
> + break;
> + default:
> + rc = -EACCES;
> + goto out_put_xprt;
> + }
> +
> + rc = wait_for_completion_interruptible_timeout(&lower_transport->handshake_done,
> + XS_TLS_HANDSHAKE_TO);
> + if (rc <= 0) {
> + if (!tls_handshake_cancel(sk)) {
> + if (rc == 0)
> + rc = -ETIMEDOUT;
> + goto out_put_xprt;
> + }
> + }
> +
> + rc = lower_transport->xprt_err;
> +
> +out:
> + xs_stream_reset_connect(lower_transport);
> + clear_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
> + return rc;
> +
> +out_put_xprt:
> + xprt_put(lower_xprt);
> + goto out;
> }
>
> -static void xs_set_transport_clnt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
> +/**
> + * xs_tls_connect - establish a TLS session on a socket
> + * @work: queued work item
> + *
> + * For RPC-with-TLS, there is a two-stage connection process.
> + *
> + * The "upper-layer xprt" is visible to the RPC consumer. Once it has
> + * been marked connected, the consumer knows that a TCP connection and
> + * a TLS session have been established.
> + *
> + * A "lower-layer xprt", created in this function, handles the mechanics
> + * of connecting the TCP socket, performing the RPC_AUTH_TLS probe, and
> + * then driving the TLS handshake. Once all that is complete, the upper
> + * layer xprt is marked connected.
> + */
> +static void xs_tls_connect(struct work_struct *work)
> {
> - struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
> + struct sock_xprt *upper_transport =
> + container_of(work, struct sock_xprt, connect_worker.work);
> + struct rpc_clnt *upper_clnt = upper_transport->clnt;
> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
> struct rpc_create_args args = {
> - .net = xprt->xprt_net,
> - .protocol = xprt->prot,
> - .address = (struct sockaddr *)&xprt->addr,
> - .addrsize = xprt->addrlen,
> - .timeout = clnt->cl_timeout,
> - .servername = xprt->servername,
> - .nodename = clnt->cl_nodename,
> - .program = clnt->cl_program,
> - .prognumber = clnt->cl_prog,
> - .version = clnt->cl_vers,
> + .net = upper_xprt->xprt_net,
> + .protocol = upper_xprt->prot,
> + .address = (struct sockaddr *)&upper_xprt->addr,
> + .addrsize = upper_xprt->addrlen,
> + .timeout = upper_clnt->cl_timeout,
> + .servername = upper_xprt->servername,
> + .nodename = upper_clnt->cl_nodename,
> + .program = upper_clnt->cl_program,
> + .prognumber = upper_clnt->cl_prog,
> + .version = upper_clnt->cl_vers,
> .authflavor = RPC_AUTH_TLS,
> - .cred = clnt->cl_cred,
> + .cred = upper_clnt->cl_cred,
> .xprtsec = {
> .policy = RPC_XPRTSEC_NONE,
> },
> - .flags = RPC_CLNT_CREATE_NOPING,
> };
> + unsigned int pflags = current->flags;
> + struct rpc_clnt *lower_clnt;
> + struct rpc_xprt *lower_xprt;
> + int status;
>
> - switch (xprt->xprtsec.policy) {
> - case RPC_XPRTSEC_TLS_ANON:
> - case RPC_XPRTSEC_TLS_X509:
> - transport->clnt = rpc_create(&args);
> - break;
> - default:
> - transport->clnt = ERR_PTR(-ENOTCONN);
> + if (atomic_read(&upper_xprt->swapper))
> + current->flags |= PF_MEMALLOC;
> +
> + xs_stream_start_connect(upper_transport);
> +
> + /* This implicitly sends an RPC_AUTH_TLS probe */
> + lower_clnt = rpc_create(&args);
> + if (IS_ERR(lower_clnt)) {
> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> + xprt_clear_connecting(upper_xprt);
> + xprt_wake_pending_tasks(upper_xprt, PTR_ERR(lower_clnt));
> + smp_mb__before_atomic();
> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> + goto out_unlock;
> }
> +
> + /* RPC_AUTH_TLS probe was successful. Try a TLS handshake on
> + * the lower xprt.
> + */
> + rcu_read_lock();
> + lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
> + rcu_read_unlock();
> + status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
> + if (status)
> + goto out_close;
> +
> + status = xs_tls_finish_connecting(lower_xprt, upper_transport);
> + if (status)
> + goto out_close;
> +
> + trace_rpc_socket_connect(upper_xprt, upper_transport->sock, 0);
> + if (!xprt_test_and_set_connected(upper_xprt)) {
> + upper_xprt->connect_cookie++;
> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> + xprt_clear_connecting(upper_xprt);
> +
> + upper_xprt->stat.connect_count++;
> + upper_xprt->stat.connect_time += (long)jiffies -
> + upper_xprt->stat.connect_start;
> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> + }
> + rpc_shutdown_client(lower_clnt);
> +
> +out_unlock:
> + current_restore_flags(pflags, PF_MEMALLOC);
> + upper_transport->clnt = NULL;
> + xprt_unlock_connect(upper_xprt, upper_transport);
> + return;
> +
> +out_close:
> + rpc_shutdown_client(lower_clnt);
> +
> + /* xprt_force_disconnect() wakes tasks with a fixed tk_status code.
> + * Wake them first here to ensure they get our tk_status code.
> + */
> + xprt_wake_pending_tasks(upper_xprt, status);
> + xs_tcp_force_close(upper_xprt);
> + xprt_clear_connecting(upper_xprt);
> + goto out_unlock;
> }
>
> /**
> @@ -2498,8 +2714,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
> } else
> dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
>
> - xs_set_transport_clnt(task->tk_client, xprt);
> -
> + transport->clnt = task->tk_client;
> queue_delayed_work(xprtiod_workqueue,
> &transport->connect_worker,
> delay);
>
>
> On May 19, 2023, at 2:19 PM, Anna Schumaker <[email protected]> wrote:
>
> Hi Chuck,
>
> On Tue, May 16, 2023 at 3:52 PM Chuck Lever <[email protected]> wrote:
>>
>> From: Chuck Lever <[email protected]>
>>
>> Use the new TLS handshake API to enable the SunRPC client code
>> to request a TLS handshake. This implements support for RFC 9289.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> include/linux/sunrpc/xprtsock.h | 1
>> net/sunrpc/xprtsock.c | 289 ++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 253 insertions(+), 37 deletions(-)
>>
>> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
>> index 574a6a5391ba..700a1e6c047c 100644
>> --- a/include/linux/sunrpc/xprtsock.h
>> +++ b/include/linux/sunrpc/xprtsock.h
>> @@ -57,6 +57,7 @@ struct sock_xprt {
>> struct work_struct error_worker;
>> struct work_struct recv_worker;
>> struct mutex recv_mutex;
>> + struct completion handshake_done;
>> struct sockaddr_storage srcaddr;
>> unsigned short srcport;
>> int xprt_err;
>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>> index 7ea5984a52a3..686dd313f89f 100644
>> --- a/net/sunrpc/xprtsock.c
>> +++ b/net/sunrpc/xprtsock.c
>> @@ -48,6 +48,7 @@
>> #include <net/udp.h>
>> #include <net/tcp.h>
>> #include <net/tls.h>
>> +#include <net/handshake.h>
>>
>> #include <linux/bvec.h>
>> #include <linux/highmem.h>
>> @@ -189,6 +190,11 @@ static struct ctl_table xs_tunables_table[] = {
>> */
>> #define XS_IDLE_DISC_TO (5U * 60 * HZ)
>>
>> +/*
>> + * TLS handshake timeout.
>> + */
>> +#define XS_TLS_HANDSHAKE_TO (10U * HZ)
>> +
>> #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>> # undef RPC_DEBUG_DATA
>> # define RPCDBG_FACILITY RPCDBG_TRANS
>> @@ -1238,6 +1244,10 @@ static void xs_reset_transport(struct sock_xprt *transport)
>> if (atomic_read(&transport->xprt.swapper))
>> sk_clear_memalloc(sk);
>>
>> + /* XXX: Maybe also send a TLS Closure alert? */
>> +
>> + tls_handshake_cancel(sk);
>> +
>> kernel_sock_shutdown(sock, SHUT_RDWR);
>>
>> mutex_lock(&transport->recv_mutex);
>> @@ -2411,60 +2421,266 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>> current_restore_flags(pflags, PF_MEMALLOC);
>> }
>>
>> +/*
>> + * Transfer the connected socket to @upper_transport, then mark that
>> + * xprt CONNECTED.
>> + */
>> +static int xs_tls_finish_connecting(struct rpc_xprt *lower_xprt,
>> + struct sock_xprt *upper_transport)
>> +{
>> + struct sock_xprt *lower_transport =
>> + container_of(lower_xprt, struct sock_xprt, xprt);
>> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
>> +
>> + if (!upper_transport->inet) {
>> + struct socket *sock = lower_transport->sock;
>> + struct sock *sk = sock->sk;
>> +
>> + /* Avoid temporary address, they are bad for long-lived
>> + * connections such as NFS mounts.
>> + * RFC4941, section 3.6 suggests that:
>> + * Individual applications, which have specific
>> + * knowledge about the normal duration of connections,
>> + * MAY override this as appropriate.
>> + */
>> + if (xs_addr(upper_xprt)->sa_family == PF_INET6) {
>> + ip6_sock_set_addr_preferences(sk,
>> + IPV6_PREFER_SRC_PUBLIC);
>> + }
>> +
>> + xs_tcp_set_socket_timeouts(upper_xprt, sock);
>> + tcp_sock_set_nodelay(sk);
>> +
>> + lock_sock(sk);
>> +
>> + /*
>> + * @sk is already connected, so it now has the RPC callbacks.
>> + * Reach into @lower_transport to save the original ones.
>> + */
>> + upper_transport->old_data_ready = lower_transport->old_data_ready;
>> + upper_transport->old_state_change = lower_transport->old_state_change;
>> + upper_transport->old_write_space = lower_transport->old_write_space;
>> + upper_transport->old_error_report = lower_transport->old_error_report;
>> + sk->sk_user_data = upper_xprt;
>> +
>> + /* socket options */
>> + sock_reset_flag(sk, SOCK_LINGER);
>> +
>> + xprt_clear_connected(upper_xprt);
>> +
>> + upper_transport->sock = sock;
>> + upper_transport->inet = sk;
>> + upper_transport->file = lower_transport->file;
>> +
>> + release_sock(sk);
>> +
>> + /* Reset lower_transport before shutting down its clnt */
>> + mutex_lock(&lower_transport->recv_mutex);
>> + lower_transport->inet = NULL;
>> + lower_transport->sock = NULL;
>> + lower_transport->file = NULL;
>> +
>> + xprt_clear_connected(lower_xprt);
>> + xs_sock_reset_connection_flags(lower_xprt);
>> + xs_stream_reset_connect(lower_transport);
>> + mutex_unlock(&lower_transport->recv_mutex);
>> + }
>> +
>> + if (!xprt_bound(upper_xprt))
>> + return -ENOTCONN;
>> +
>> + xs_set_memalloc(upper_xprt);
>> +
>> + if (!xprt_test_and_set_connected(upper_xprt)) {
>> + upper_xprt->connect_cookie++;
>> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
>> + xprt_clear_connecting(upper_xprt);
>> +
>> + upper_xprt->stat.connect_count++;
>> + upper_xprt->stat.connect_time += (long)jiffies -
>> + upper_xprt->stat.connect_start;
>> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
>> + }
>> + return 0;
>> +}
>> +
>> /**
>> - * xs_tls_connect - establish a TLS session on a socket
>> - * @work: queued work item
>> + * xs_tls_handshake_done - TLS handshake completion handler
>> + * @data: address of xprt to wake
>> + * @status: status of handshake
>> + * @peerid: serial number of key containing the remote's identity
>> *
>> */
>> -static void xs_tls_connect(struct work_struct *work)
>> +static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid)
>> {
>> - struct sock_xprt *transport =
>> - container_of(work, struct sock_xprt, connect_worker.work);
>> - struct rpc_clnt *clnt;
>> + struct rpc_xprt *lower_xprt = data;
>> + struct sock_xprt *lower_transport =
>> + container_of(lower_xprt, struct sock_xprt, xprt);
>>
>> - clnt = transport->clnt;
>> - transport->clnt = NULL;
>> - if (IS_ERR(clnt))
>> - goto out_unlock;
>> + lower_transport->xprt_err = status ? -EACCES : 0;
>> + complete(&lower_transport->handshake_done);
>> + xprt_put(lower_xprt);
>> +}
>>
>> - xs_tcp_setup_socket(work);
>> +static int xs_tls_handshake_sync(struct rpc_xprt *lower_xprt, struct xprtsec_parms *xprtsec)
>> +{
>> + struct sock_xprt *lower_transport =
>> + container_of(lower_xprt, struct sock_xprt, xprt);
>> + struct tls_handshake_args args = {
>> + .ta_sock = lower_transport->sock,
>> + .ta_done = xs_tls_handshake_done,
>> + .ta_data = xprt_get(lower_xprt),
>> + .ta_peername = lower_xprt->servername,
>
> This part isn't compiling for me on v6.4-rc2:
>
> net/sunrpc/xprtsock.c:2538:4: error: field designator 'ta_peername'
> does not refer to any field in type 'struct tls_handshake_args'
> .ta_peername = lower_xprt->servername,
> ^
> 1 error generated.
>
> Am I missing a patch, or did this struct get changed somewhere along the line?
The patch series is based on net-next, which includes a patch
that changes this code.
I had expected those patches to have been merged, but they are
still pending.
> Anna
>
>> + };
>> + struct sock *sk = lower_transport->inet;
>> + int rc;
>>
>> - rpc_shutdown_client(clnt);
>> + init_completion(&lower_transport->handshake_done);
>> + set_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
>>
>> -out_unlock:
>> - return;
>> + lower_transport->xprt_err = -ETIMEDOUT;
>> + switch (xprtsec->policy) {
>> + case RPC_XPRTSEC_TLS_ANON:
>> + rc = tls_client_hello_anon(&args, GFP_KERNEL);
>> + if (rc)
>> + goto out_put_xprt;
>> + break;
>> + case RPC_XPRTSEC_TLS_X509:
>> + args.ta_my_cert = xprtsec->cert_serial;
>> + args.ta_my_privkey = xprtsec->privkey_serial;
>> + rc = tls_client_hello_x509(&args, GFP_KERNEL);
>> + if (rc)
>> + goto out_put_xprt;
>> + break;
>> + default:
>> + rc = -EACCES;
>> + goto out_put_xprt;
>> + }
>> +
>> + rc = wait_for_completion_interruptible_timeout(&lower_transport->handshake_done,
>> + XS_TLS_HANDSHAKE_TO);
>> + if (rc <= 0) {
>> + if (!tls_handshake_cancel(sk)) {
>> + if (rc == 0)
>> + rc = -ETIMEDOUT;
>> + goto out_put_xprt;
>> + }
>> + }
>> +
>> + rc = lower_transport->xprt_err;
>> +
>> +out:
>> + xs_stream_reset_connect(lower_transport);
>> + clear_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
>> + return rc;
>> +
>> +out_put_xprt:
>> + xprt_put(lower_xprt);
>> + goto out;
>> }
>>
>> -static void xs_set_transport_clnt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
>> +/**
>> + * xs_tls_connect - establish a TLS session on a socket
>> + * @work: queued work item
>> + *
>> + * For RPC-with-TLS, there is a two-stage connection process.
>> + *
>> + * The "upper-layer xprt" is visible to the RPC consumer. Once it has
>> + * been marked connected, the consumer knows that a TCP connection and
>> + * a TLS session have been established.
>> + *
>> + * A "lower-layer xprt", created in this function, handles the mechanics
>> + * of connecting the TCP socket, performing the RPC_AUTH_TLS probe, and
>> + * then driving the TLS handshake. Once all that is complete, the upper
>> + * layer xprt is marked connected.
>> + */
>> +static void xs_tls_connect(struct work_struct *work)
>> {
>> - struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
>> + struct sock_xprt *upper_transport =
>> + container_of(work, struct sock_xprt, connect_worker.work);
>> + struct rpc_clnt *upper_clnt = upper_transport->clnt;
>> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
>> struct rpc_create_args args = {
>> - .net = xprt->xprt_net,
>> - .protocol = xprt->prot,
>> - .address = (struct sockaddr *)&xprt->addr,
>> - .addrsize = xprt->addrlen,
>> - .timeout = clnt->cl_timeout,
>> - .servername = xprt->servername,
>> - .nodename = clnt->cl_nodename,
>> - .program = clnt->cl_program,
>> - .prognumber = clnt->cl_prog,
>> - .version = clnt->cl_vers,
>> + .net = upper_xprt->xprt_net,
>> + .protocol = upper_xprt->prot,
>> + .address = (struct sockaddr *)&upper_xprt->addr,
>> + .addrsize = upper_xprt->addrlen,
>> + .timeout = upper_clnt->cl_timeout,
>> + .servername = upper_xprt->servername,
>> + .nodename = upper_clnt->cl_nodename,
>> + .program = upper_clnt->cl_program,
>> + .prognumber = upper_clnt->cl_prog,
>> + .version = upper_clnt->cl_vers,
>> .authflavor = RPC_AUTH_TLS,
>> - .cred = clnt->cl_cred,
>> + .cred = upper_clnt->cl_cred,
>> .xprtsec = {
>> .policy = RPC_XPRTSEC_NONE,
>> },
>> - .flags = RPC_CLNT_CREATE_NOPING,
>> };
>> + unsigned int pflags = current->flags;
>> + struct rpc_clnt *lower_clnt;
>> + struct rpc_xprt *lower_xprt;
>> + int status;
>>
>> - switch (xprt->xprtsec.policy) {
>> - case RPC_XPRTSEC_TLS_ANON:
>> - case RPC_XPRTSEC_TLS_X509:
>> - transport->clnt = rpc_create(&args);
>> - break;
>> - default:
>> - transport->clnt = ERR_PTR(-ENOTCONN);
>> + if (atomic_read(&upper_xprt->swapper))
>> + current->flags |= PF_MEMALLOC;
>> +
>> + xs_stream_start_connect(upper_transport);
>> +
>> + /* This implicitly sends an RPC_AUTH_TLS probe */
>> + lower_clnt = rpc_create(&args);
>> + if (IS_ERR(lower_clnt)) {
>> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
>> + xprt_clear_connecting(upper_xprt);
>> + xprt_wake_pending_tasks(upper_xprt, PTR_ERR(lower_clnt));
>> + smp_mb__before_atomic();
>> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
>> + goto out_unlock;
>> }
>> +
>> + /* RPC_AUTH_TLS probe was successful. Try a TLS handshake on
>> + * the lower xprt.
>> + */
>> + rcu_read_lock();
>> + lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
>> + rcu_read_unlock();
>> + status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
>> + if (status)
>> + goto out_close;
>> +
>> + status = xs_tls_finish_connecting(lower_xprt, upper_transport);
>> + if (status)
>> + goto out_close;
>> +
>> + trace_rpc_socket_connect(upper_xprt, upper_transport->sock, 0);
>> + if (!xprt_test_and_set_connected(upper_xprt)) {
>> + upper_xprt->connect_cookie++;
>> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
>> + xprt_clear_connecting(upper_xprt);
>> +
>> + upper_xprt->stat.connect_count++;
>> + upper_xprt->stat.connect_time += (long)jiffies -
>> + upper_xprt->stat.connect_start;
>> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
>> + }
>> + rpc_shutdown_client(lower_clnt);
>> +
>> +out_unlock:
>> + current_restore_flags(pflags, PF_MEMALLOC);
>> + upper_transport->clnt = NULL;
>> + xprt_unlock_connect(upper_xprt, upper_transport);
>> + return;
>> +
>> +out_close:
>> + rpc_shutdown_client(lower_clnt);
>> +
>> + /* xprt_force_disconnect() wakes tasks with a fixed tk_status code.
>> + * Wake them first here to ensure they get our tk_status code.
>> + */
>> + xprt_wake_pending_tasks(upper_xprt, status);
>> + xs_tcp_force_close(upper_xprt);
>> + xprt_clear_connecting(upper_xprt);
>> + goto out_unlock;
>> }
>>
>> /**
>> @@ -2498,8 +2714,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
>> } else
>> dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
>>
>> - xs_set_transport_clnt(task->tk_client, xprt);
>> -
>> + transport->clnt = task->tk_client;
>> queue_delayed_work(xprtiod_workqueue,
>> &transport->connect_worker,
>> delay);
--
Chuck Lever
On Fri, May 19, 2023 at 2:33 PM Chuck Lever III <[email protected]> wrote:
>
>
>
> > On May 19, 2023, at 2:19 PM, Anna Schumaker <[email protected]> wrote:
> >
> > Hi Chuck,
> >
> > On Tue, May 16, 2023 at 3:52 PM Chuck Lever <[email protected]> wrote:
> >>
> >> From: Chuck Lever <[email protected]>
> >>
> >> Use the new TLS handshake API to enable the SunRPC client code
> >> to request a TLS handshake. This implements support for RFC 9289.
> >>
> >> Signed-off-by: Chuck Lever <[email protected]>
> >> ---
> >> include/linux/sunrpc/xprtsock.h | 1
> >> net/sunrpc/xprtsock.c | 289 ++++++++++++++++++++++++++++++++++-----
> >> 2 files changed, 253 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
> >> index 574a6a5391ba..700a1e6c047c 100644
> >> --- a/include/linux/sunrpc/xprtsock.h
> >> +++ b/include/linux/sunrpc/xprtsock.h
> >> @@ -57,6 +57,7 @@ struct sock_xprt {
> >> struct work_struct error_worker;
> >> struct work_struct recv_worker;
> >> struct mutex recv_mutex;
> >> + struct completion handshake_done;
> >> struct sockaddr_storage srcaddr;
> >> unsigned short srcport;
> >> int xprt_err;
> >> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> >> index 7ea5984a52a3..686dd313f89f 100644
> >> --- a/net/sunrpc/xprtsock.c
> >> +++ b/net/sunrpc/xprtsock.c
> >> @@ -48,6 +48,7 @@
> >> #include <net/udp.h>
> >> #include <net/tcp.h>
> >> #include <net/tls.h>
> >> +#include <net/handshake.h>
> >>
> >> #include <linux/bvec.h>
> >> #include <linux/highmem.h>
> >> @@ -189,6 +190,11 @@ static struct ctl_table xs_tunables_table[] = {
> >> */
> >> #define XS_IDLE_DISC_TO (5U * 60 * HZ)
> >>
> >> +/*
> >> + * TLS handshake timeout.
> >> + */
> >> +#define XS_TLS_HANDSHAKE_TO (10U * HZ)
> >> +
> >> #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> >> # undef RPC_DEBUG_DATA
> >> # define RPCDBG_FACILITY RPCDBG_TRANS
> >> @@ -1238,6 +1244,10 @@ static void xs_reset_transport(struct sock_xprt *transport)
> >> if (atomic_read(&transport->xprt.swapper))
> >> sk_clear_memalloc(sk);
> >>
> >> + /* XXX: Maybe also send a TLS Closure alert? */
> >> +
> >> + tls_handshake_cancel(sk);
> >> +
> >> kernel_sock_shutdown(sock, SHUT_RDWR);
> >>
> >> mutex_lock(&transport->recv_mutex);
> >> @@ -2411,60 +2421,266 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> >> current_restore_flags(pflags, PF_MEMALLOC);
> >> }
> >>
> >> +/*
> >> + * Transfer the connected socket to @upper_transport, then mark that
> >> + * xprt CONNECTED.
> >> + */
> >> +static int xs_tls_finish_connecting(struct rpc_xprt *lower_xprt,
> >> + struct sock_xprt *upper_transport)
> >> +{
> >> + struct sock_xprt *lower_transport =
> >> + container_of(lower_xprt, struct sock_xprt, xprt);
> >> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
> >> +
> >> + if (!upper_transport->inet) {
> >> + struct socket *sock = lower_transport->sock;
> >> + struct sock *sk = sock->sk;
> >> +
> >> + /* Avoid temporary address, they are bad for long-lived
> >> + * connections such as NFS mounts.
> >> + * RFC4941, section 3.6 suggests that:
> >> + * Individual applications, which have specific
> >> + * knowledge about the normal duration of connections,
> >> + * MAY override this as appropriate.
> >> + */
> >> + if (xs_addr(upper_xprt)->sa_family == PF_INET6) {
> >> + ip6_sock_set_addr_preferences(sk,
> >> + IPV6_PREFER_SRC_PUBLIC);
> >> + }
> >> +
> >> + xs_tcp_set_socket_timeouts(upper_xprt, sock);
> >> + tcp_sock_set_nodelay(sk);
> >> +
> >> + lock_sock(sk);
> >> +
> >> + /*
> >> + * @sk is already connected, so it now has the RPC callbacks.
> >> + * Reach into @lower_transport to save the original ones.
> >> + */
> >> + upper_transport->old_data_ready = lower_transport->old_data_ready;
> >> + upper_transport->old_state_change = lower_transport->old_state_change;
> >> + upper_transport->old_write_space = lower_transport->old_write_space;
> >> + upper_transport->old_error_report = lower_transport->old_error_report;
> >> + sk->sk_user_data = upper_xprt;
> >> +
> >> + /* socket options */
> >> + sock_reset_flag(sk, SOCK_LINGER);
> >> +
> >> + xprt_clear_connected(upper_xprt);
> >> +
> >> + upper_transport->sock = sock;
> >> + upper_transport->inet = sk;
> >> + upper_transport->file = lower_transport->file;
> >> +
> >> + release_sock(sk);
> >> +
> >> + /* Reset lower_transport before shutting down its clnt */
> >> + mutex_lock(&lower_transport->recv_mutex);
> >> + lower_transport->inet = NULL;
> >> + lower_transport->sock = NULL;
> >> + lower_transport->file = NULL;
> >> +
> >> + xprt_clear_connected(lower_xprt);
> >> + xs_sock_reset_connection_flags(lower_xprt);
> >> + xs_stream_reset_connect(lower_transport);
> >> + mutex_unlock(&lower_transport->recv_mutex);
> >> + }
> >> +
> >> + if (!xprt_bound(upper_xprt))
> >> + return -ENOTCONN;
> >> +
> >> + xs_set_memalloc(upper_xprt);
> >> +
> >> + if (!xprt_test_and_set_connected(upper_xprt)) {
> >> + upper_xprt->connect_cookie++;
> >> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> >> + xprt_clear_connecting(upper_xprt);
> >> +
> >> + upper_xprt->stat.connect_count++;
> >> + upper_xprt->stat.connect_time += (long)jiffies -
> >> + upper_xprt->stat.connect_start;
> >> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> /**
> >> - * xs_tls_connect - establish a TLS session on a socket
> >> - * @work: queued work item
> >> + * xs_tls_handshake_done - TLS handshake completion handler
> >> + * @data: address of xprt to wake
> >> + * @status: status of handshake
> >> + * @peerid: serial number of key containing the remote's identity
> >> *
> >> */
> >> -static void xs_tls_connect(struct work_struct *work)
> >> +static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid)
> >> {
> >> - struct sock_xprt *transport =
> >> - container_of(work, struct sock_xprt, connect_worker.work);
> >> - struct rpc_clnt *clnt;
> >> + struct rpc_xprt *lower_xprt = data;
> >> + struct sock_xprt *lower_transport =
> >> + container_of(lower_xprt, struct sock_xprt, xprt);
> >>
> >> - clnt = transport->clnt;
> >> - transport->clnt = NULL;
> >> - if (IS_ERR(clnt))
> >> - goto out_unlock;
> >> + lower_transport->xprt_err = status ? -EACCES : 0;
> >> + complete(&lower_transport->handshake_done);
> >> + xprt_put(lower_xprt);
> >> +}
> >>
> >> - xs_tcp_setup_socket(work);
> >> +static int xs_tls_handshake_sync(struct rpc_xprt *lower_xprt, struct xprtsec_parms *xprtsec)
> >> +{
> >> + struct sock_xprt *lower_transport =
> >> + container_of(lower_xprt, struct sock_xprt, xprt);
> >> + struct tls_handshake_args args = {
> >> + .ta_sock = lower_transport->sock,
> >> + .ta_done = xs_tls_handshake_done,
> >> + .ta_data = xprt_get(lower_xprt),
> >> + .ta_peername = lower_xprt->servername,
> >
> > This part isn't compiling for me on v6.4-rc2:
> >
> > net/sunrpc/xprtsock.c:2538:4: error: field designator 'ta_peername'
> > does not refer to any field in type 'struct tls_handshake_args'
> > .ta_peername = lower_xprt->servername,
> > ^
> > 1 error generated.
> >
> > Am I missing a patch, or did this struct get changed somewhere along the line?
>
> The patch series is based on net-next, which includes a patch
> that changes this code.
>
> I had expected those patches to have been merged, but they are
> still pending.
Makes sense! I wonder what the hold up is. Oh well, I'll rebase on top
of that and try again :)
Anna
>
>
> > Anna
> >
> >> + };
> >> + struct sock *sk = lower_transport->inet;
> >> + int rc;
> >>
> >> - rpc_shutdown_client(clnt);
> >> + init_completion(&lower_transport->handshake_done);
> >> + set_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
> >>
> >> -out_unlock:
> >> - return;
> >> + lower_transport->xprt_err = -ETIMEDOUT;
> >> + switch (xprtsec->policy) {
> >> + case RPC_XPRTSEC_TLS_ANON:
> >> + rc = tls_client_hello_anon(&args, GFP_KERNEL);
> >> + if (rc)
> >> + goto out_put_xprt;
> >> + break;
> >> + case RPC_XPRTSEC_TLS_X509:
> >> + args.ta_my_cert = xprtsec->cert_serial;
> >> + args.ta_my_privkey = xprtsec->privkey_serial;
> >> + rc = tls_client_hello_x509(&args, GFP_KERNEL);
> >> + if (rc)
> >> + goto out_put_xprt;
> >> + break;
> >> + default:
> >> + rc = -EACCES;
> >> + goto out_put_xprt;
> >> + }
> >> +
> >> + rc = wait_for_completion_interruptible_timeout(&lower_transport->handshake_done,
> >> + XS_TLS_HANDSHAKE_TO);
> >> + if (rc <= 0) {
> >> + if (!tls_handshake_cancel(sk)) {
> >> + if (rc == 0)
> >> + rc = -ETIMEDOUT;
> >> + goto out_put_xprt;
> >> + }
> >> + }
> >> +
> >> + rc = lower_transport->xprt_err;
> >> +
> >> +out:
> >> + xs_stream_reset_connect(lower_transport);
> >> + clear_bit(XPRT_SOCK_IGNORE_RECV, &lower_transport->sock_state);
> >> + return rc;
> >> +
> >> +out_put_xprt:
> >> + xprt_put(lower_xprt);
> >> + goto out;
> >> }
> >>
> >> -static void xs_set_transport_clnt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
> >> +/**
> >> + * xs_tls_connect - establish a TLS session on a socket
> >> + * @work: queued work item
> >> + *
> >> + * For RPC-with-TLS, there is a two-stage connection process.
> >> + *
> >> + * The "upper-layer xprt" is visible to the RPC consumer. Once it has
> >> + * been marked connected, the consumer knows that a TCP connection and
> >> + * a TLS session have been established.
> >> + *
> >> + * A "lower-layer xprt", created in this function, handles the mechanics
> >> + * of connecting the TCP socket, performing the RPC_AUTH_TLS probe, and
> >> + * then driving the TLS handshake. Once all that is complete, the upper
> >> + * layer xprt is marked connected.
> >> + */
> >> +static void xs_tls_connect(struct work_struct *work)
> >> {
> >> - struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
> >> + struct sock_xprt *upper_transport =
> >> + container_of(work, struct sock_xprt, connect_worker.work);
> >> + struct rpc_clnt *upper_clnt = upper_transport->clnt;
> >> + struct rpc_xprt *upper_xprt = &upper_transport->xprt;
> >> struct rpc_create_args args = {
> >> - .net = xprt->xprt_net,
> >> - .protocol = xprt->prot,
> >> - .address = (struct sockaddr *)&xprt->addr,
> >> - .addrsize = xprt->addrlen,
> >> - .timeout = clnt->cl_timeout,
> >> - .servername = xprt->servername,
> >> - .nodename = clnt->cl_nodename,
> >> - .program = clnt->cl_program,
> >> - .prognumber = clnt->cl_prog,
> >> - .version = clnt->cl_vers,
> >> + .net = upper_xprt->xprt_net,
> >> + .protocol = upper_xprt->prot,
> >> + .address = (struct sockaddr *)&upper_xprt->addr,
> >> + .addrsize = upper_xprt->addrlen,
> >> + .timeout = upper_clnt->cl_timeout,
> >> + .servername = upper_xprt->servername,
> >> + .nodename = upper_clnt->cl_nodename,
> >> + .program = upper_clnt->cl_program,
> >> + .prognumber = upper_clnt->cl_prog,
> >> + .version = upper_clnt->cl_vers,
> >> .authflavor = RPC_AUTH_TLS,
> >> - .cred = clnt->cl_cred,
> >> + .cred = upper_clnt->cl_cred,
> >> .xprtsec = {
> >> .policy = RPC_XPRTSEC_NONE,
> >> },
> >> - .flags = RPC_CLNT_CREATE_NOPING,
> >> };
> >> + unsigned int pflags = current->flags;
> >> + struct rpc_clnt *lower_clnt;
> >> + struct rpc_xprt *lower_xprt;
> >> + int status;
> >>
> >> - switch (xprt->xprtsec.policy) {
> >> - case RPC_XPRTSEC_TLS_ANON:
> >> - case RPC_XPRTSEC_TLS_X509:
> >> - transport->clnt = rpc_create(&args);
> >> - break;
> >> - default:
> >> - transport->clnt = ERR_PTR(-ENOTCONN);
> >> + if (atomic_read(&upper_xprt->swapper))
> >> + current->flags |= PF_MEMALLOC;
> >> +
> >> + xs_stream_start_connect(upper_transport);
> >> +
> >> + /* This implicitly sends an RPC_AUTH_TLS probe */
> >> + lower_clnt = rpc_create(&args);
> >> + if (IS_ERR(lower_clnt)) {
> >> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> >> + xprt_clear_connecting(upper_xprt);
> >> + xprt_wake_pending_tasks(upper_xprt, PTR_ERR(lower_clnt));
> >> + smp_mb__before_atomic();
> >> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> >> + goto out_unlock;
> >> }
> >> +
> >> + /* RPC_AUTH_TLS probe was successful. Try a TLS handshake on
> >> + * the lower xprt.
> >> + */
> >> + rcu_read_lock();
> >> + lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
> >> + rcu_read_unlock();
> >> + status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
> >> + if (status)
> >> + goto out_close;
> >> +
> >> + status = xs_tls_finish_connecting(lower_xprt, upper_transport);
> >> + if (status)
> >> + goto out_close;
> >> +
> >> + trace_rpc_socket_connect(upper_xprt, upper_transport->sock, 0);
> >> + if (!xprt_test_and_set_connected(upper_xprt)) {
> >> + upper_xprt->connect_cookie++;
> >> + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state);
> >> + xprt_clear_connecting(upper_xprt);
> >> +
> >> + upper_xprt->stat.connect_count++;
> >> + upper_xprt->stat.connect_time += (long)jiffies -
> >> + upper_xprt->stat.connect_start;
> >> + xs_run_error_worker(upper_transport, XPRT_SOCK_WAKE_PENDING);
> >> + }
> >> + rpc_shutdown_client(lower_clnt);
> >> +
> >> +out_unlock:
> >> + current_restore_flags(pflags, PF_MEMALLOC);
> >> + upper_transport->clnt = NULL;
> >> + xprt_unlock_connect(upper_xprt, upper_transport);
> >> + return;
> >> +
> >> +out_close:
> >> + rpc_shutdown_client(lower_clnt);
> >> +
> >> + /* xprt_force_disconnect() wakes tasks with a fixed tk_status code.
> >> + * Wake them first here to ensure they get our tk_status code.
> >> + */
> >> + xprt_wake_pending_tasks(upper_xprt, status);
> >> + xs_tcp_force_close(upper_xprt);
> >> + xprt_clear_connecting(upper_xprt);
> >> + goto out_unlock;
> >> }
> >>
> >> /**
> >> @@ -2498,8 +2714,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
> >> } else
> >> dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
> >>
> >> - xs_set_transport_clnt(task->tk_client, xprt);
> >> -
> >> + transport->clnt = task->tk_client;
> >> queue_delayed_work(xprtiod_workqueue,
> >> &transport->connect_worker,
> >> delay);
>
>
> --
> Chuck Lever
>
>
Hi Chuck,
On Tue, May 16, 2023 at 3:42 PM Chuck Lever <[email protected]> wrote:
>
> From: Chuck Lever <[email protected]>
>
> I'm about to add a use case that does not want RPC_TASK_NULLCREDS.
> Refactor rpc_call_null_helper() so that callers provide NULLCREDS
> when they need it.
>
> Reviewed-by: Jeff Layton <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/clnt.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 4cdb539b5854..2dca0ae489ec 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2811,8 +2811,7 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
> .rpc_op_cred = cred,
> .callback_ops = ops ?: &rpc_null_ops,
> .callback_data = data,
> - .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
> - RPC_TASK_NULLCREDS,
> + .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
> };
>
> return rpc_run_task(&task_setup_data);
> @@ -2820,7 +2819,8 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
>
> struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int flags)
> {
> - return rpc_call_null_helper(clnt, NULL, cred, flags, NULL, NULL);
> + return rpc_call_null_helper(clnt, NULL, cred, flags | RPC_TASK_NULLCREDS,
> + NULL, NULL);
> }
> EXPORT_SYMBOL_GPL(rpc_call_null);
I think you missed updating rpc_ping() right below this function as
well, I'm unable to mount without the flag. Although I do wonder if it
would be easier to slightly rename rpc_call_null_helper(), and then
create a new rpc_call_null_helper() that appends the
RPC_TASK_NULLCREDS flag. Then we don't need to touch current callers,
and your new use case could call the renamed function.
What do you think?
Anna
>
> @@ -2920,12 +2920,13 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
> goto success;
> }
>
> - task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
> - &rpc_cb_add_xprt_call_ops, data);
> + task = rpc_call_null_helper(clnt, xprt, NULL,
> + RPC_TASK_ASYNC | RPC_TASK_NULLCREDS,
> + &rpc_cb_add_xprt_call_ops, data);
> if (IS_ERR(task))
> return PTR_ERR(task);
> -
> data->xps->xps_nunique_destaddr_xprts++;
> +
> rpc_put_task(task);
> success:
> return 1;
> @@ -2940,7 +2941,8 @@ static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
> int status = -EADDRINUSE;
>
> /* Test the connection */
> - task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
> + task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_NULLCREDS,
> + NULL, NULL);
> if (IS_ERR(task))
> return PTR_ERR(task);
>
>
>
> On May 22, 2023, at 4:35 PM, Anna Schumaker <[email protected]> wrote:
>
> Hi Chuck,
>
> On Tue, May 16, 2023 at 3:42 PM Chuck Lever <[email protected]> wrote:
>>
>> From: Chuck Lever <[email protected]>
>>
>> I'm about to add a use case that does not want RPC_TASK_NULLCREDS.
>> Refactor rpc_call_null_helper() so that callers provide NULLCREDS
>> when they need it.
>>
>> Reviewed-by: Jeff Layton <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> net/sunrpc/clnt.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index 4cdb539b5854..2dca0ae489ec 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -2811,8 +2811,7 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
>> .rpc_op_cred = cred,
>> .callback_ops = ops ?: &rpc_null_ops,
>> .callback_data = data,
>> - .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
>> - RPC_TASK_NULLCREDS,
>> + .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
>> };
>>
>> return rpc_run_task(&task_setup_data);
>> @@ -2820,7 +2819,8 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
>>
>> struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int flags)
>> {
>> - return rpc_call_null_helper(clnt, NULL, cred, flags, NULL, NULL);
>> + return rpc_call_null_helper(clnt, NULL, cred, flags | RPC_TASK_NULLCREDS,
>> + NULL, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpc_call_null);
>
> I think you missed updating rpc_ping() right below this function as
> well, I'm unable to mount without the flag.
Hrm, I haven't seen that problem. But I can take a look.
This patch has been around for more than a year. I'm sure forward
porting so many times has left some kruft.
> Although I do wonder if it
> would be easier to slightly rename rpc_call_null_helper(), and then
> create a new rpc_call_null_helper() that appends the
> RPC_TASK_NULLCREDS flag. Then we don't need to touch current callers,
> and your new use case could call the renamed function.
>
> What do you think?
> Anna
>
>>
>> @@ -2920,12 +2920,13 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>> goto success;
>> }
>>
>> - task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
>> - &rpc_cb_add_xprt_call_ops, data);
>> + task = rpc_call_null_helper(clnt, xprt, NULL,
>> + RPC_TASK_ASYNC | RPC_TASK_NULLCREDS,
>> + &rpc_cb_add_xprt_call_ops, data);
>> if (IS_ERR(task))
>> return PTR_ERR(task);
>> -
>> data->xps->xps_nunique_destaddr_xprts++;
>> +
>> rpc_put_task(task);
>> success:
>> return 1;
>> @@ -2940,7 +2941,8 @@ static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
>> int status = -EADDRINUSE;
>>
>> /* Test the connection */
>> - task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
>> + task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_NULLCREDS,
>> + NULL, NULL);
>> if (IS_ERR(task))
>> return PTR_ERR(task);
--
Chuck Lever
> On May 22, 2023, at 4:35 PM, Anna Schumaker <[email protected]> wrote:
>
> Hi Chuck,
>
> On Tue, May 16, 2023 at 3:42 PM Chuck Lever <[email protected]> wrote:
>>
>> From: Chuck Lever <[email protected]>
>>
>> I'm about to add a use case that does not want RPC_TASK_NULLCREDS.
>> Refactor rpc_call_null_helper() so that callers provide NULLCREDS
>> when they need it.
>>
>> Reviewed-by: Jeff Layton <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> net/sunrpc/clnt.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index 4cdb539b5854..2dca0ae489ec 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -2811,8 +2811,7 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
>> .rpc_op_cred = cred,
>> .callback_ops = ops ?: &rpc_null_ops,
>> .callback_data = data,
>> - .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
>> - RPC_TASK_NULLCREDS,
>> + .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
>> };
>>
>> return rpc_run_task(&task_setup_data);
>> @@ -2820,7 +2819,8 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
>>
>> struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int flags)
>> {
>> - return rpc_call_null_helper(clnt, NULL, cred, flags, NULL, NULL);
>> + return rpc_call_null_helper(clnt, NULL, cred, flags | RPC_TASK_NULLCREDS,
>> + NULL, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpc_call_null);
>
> I think you missed updating rpc_ping() right below this function
That was on purpose, IIRC. rpc_ping() is supposed to pick up the
authentication flavor from the rpc_clnt. In the case of a ping
with TLS, it will use RPC_AUTH_TLS. Everyone else should be using
NULL creds.
What credential are you seeing on the wire for the NULL request?
> as well, I'm unable to mount without the flag. Although I do wonder if it
> would be easier to slightly rename rpc_call_null_helper(), and then
> create a new rpc_call_null_helper() that appends the
> RPC_TASK_NULLCREDS flag. Then we don't need to touch current callers,
> and your new use case could call the renamed function.
>
> What do you think?
> Anna
>
>>
>> @@ -2920,12 +2920,13 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>> goto success;
>> }
>>
>> - task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
>> - &rpc_cb_add_xprt_call_ops, data);
>> + task = rpc_call_null_helper(clnt, xprt, NULL,
>> + RPC_TASK_ASYNC | RPC_TASK_NULLCREDS,
>> + &rpc_cb_add_xprt_call_ops, data);
>> if (IS_ERR(task))
>> return PTR_ERR(task);
>> -
>> data->xps->xps_nunique_destaddr_xprts++;
>> +
>> rpc_put_task(task);
>> success:
>> return 1;
>> @@ -2940,7 +2941,8 @@ static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
>> int status = -EADDRINUSE;
>>
>> /* Test the connection */
>> - task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
>> + task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_NULLCREDS,
>> + NULL, NULL);
>> if (IS_ERR(task))
>> return PTR_ERR(task);
>>
>>
>>
--
Chuck Lever
From: Chuck Lever <[email protected]>
When connecting, we don't want to send both a NULL ping and an
RPC_AUTH_TLS probe, because, except for the detection of RPC-with-
TLS support, both serve effectively the same purpose.
Modify rpc_ping() so it can send a TLS probe when @clnt's flavor
is RPC_AUTH_TLS. All other callers continue to use AUTH_NONE.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/clnt.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
Does it help to replace 4/12 with this? Compile-tested only.
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 4cdb539b5854..274ad74cb2bd 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2826,10 +2826,22 @@ EXPORT_SYMBOL_GPL(rpc_call_null);
static int rpc_ping(struct rpc_clnt *clnt)
{
+ struct rpc_message msg = {
+ .rpc_proc = &rpcproc_null,
+ };
+ struct rpc_task_setup task_setup_data = {
+ .rpc_client = clnt,
+ .rpc_message = &msg,
+ .callback_ops = &rpc_null_ops,
+ .flags = RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
+ };
struct rpc_task *task;
int status;
- task = rpc_call_null_helper(clnt, NULL, NULL, 0, NULL, NULL);
+ if (clnt->cl_auth->au_flavor != RPC_AUTH_TLS)
+ flags |= RPC_TASK_NULLCREDS;
+
+ task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return PTR_ERR(task);
status = task->tk_status;
On Mon, May 22, 2023 at 05:21:22PM -0400, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> When connecting, we don't want to send both a NULL ping and an
> RPC_AUTH_TLS probe, because, except for the detection of RPC-with-
> TLS support, both serve effectively the same purpose.
>
> Modify rpc_ping() so it can send a TLS probe when @clnt's flavor
> is RPC_AUTH_TLS. All other callers continue to use AUTH_NONE.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/clnt.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> Does it help to replace 4/12 with this? Compile-tested only.
>
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 4cdb539b5854..274ad74cb2bd 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2826,10 +2826,22 @@ EXPORT_SYMBOL_GPL(rpc_call_null);
>
> static int rpc_ping(struct rpc_clnt *clnt)
> {
> + struct rpc_message msg = {
> + .rpc_proc = &rpcproc_null,
> + };
> + struct rpc_task_setup task_setup_data = {
> + .rpc_client = clnt,
> + .rpc_message = &msg,
> + .callback_ops = &rpc_null_ops,
> + .flags = RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
> + };
> struct rpc_task *task;
> int status;
>
> - task = rpc_call_null_helper(clnt, NULL, NULL, 0, NULL, NULL);
> + if (clnt->cl_auth->au_flavor != RPC_AUTH_TLS)
> + flags |= RPC_TASK_NULLCREDS;
Obviously this needs to be:
task_setup_data.flags |= RPC_TASK_NULLCREDS;
This has been fixed in the topic-rpc-with-tls-upcall branch.
> +
> + task = rpc_run_task(&task_setup_data);
> if (IS_ERR(task))
> return PTR_ERR(task);
> status = task->tk_status;
>
>
On Mon, 2023-05-22 at 22:37 -0400, Chuck Lever wrote:
> On Mon, May 22, 2023 at 05:21:22PM -0400, Chuck Lever wrote:
> > From: Chuck Lever <[email protected]>
> >
> > When connecting, we don't want to send both a NULL ping and an
> > RPC_AUTH_TLS probe, because, except for the detection of RPC-with-
> > TLS support, both serve effectively the same purpose.
> >
> > Modify rpc_ping() so it can send a TLS probe when @clnt's flavor
> > is RPC_AUTH_TLS. All other callers continue to use AUTH_NONE.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > net/sunrpc/clnt.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > Does it help to replace 4/12 with this? Compile-tested only.
> >
> >
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index 4cdb539b5854..274ad74cb2bd 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -2826,10 +2826,22 @@ EXPORT_SYMBOL_GPL(rpc_call_null);
> >
> > static int rpc_ping(struct rpc_clnt *clnt)
> > {
> > + struct rpc_message msg = {
> > + .rpc_proc = &rpcproc_null,
> > + };
> > + struct rpc_task_setup task_setup_data = {
> > + .rpc_client = clnt,
> > + .rpc_message = &msg,
> > + .callback_ops = &rpc_null_ops,
> > + .flags = RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
> > + };
> > struct rpc_task *task;
> > int status;
> >
> > - task = rpc_call_null_helper(clnt, NULL, NULL, 0, NULL,
> > NULL);
> > + if (clnt->cl_auth->au_flavor != RPC_AUTH_TLS)
> > + flags |= RPC_TASK_NULLCREDS;
>
> Obviously this needs to be:
>
> task_setup_data.flags |= RPC_TASK_NULLCREDS;
>
> This has been fixed in the topic-rpc-with-tls-upcall branch.
rpc_ping() should not need to know about the existence of AUTH_TLS.
Let's rather add in a struct rpc_authops callback connect_ping() (?)
that knows how to call rpc_ping() with the appropriate creds. For the
AUTH types other than AUTH_TLS, that will be a generic call with the
AUTH_NULL cred.
Then let's get rid of the RPC_TASK_NULLCREDS wart altogether.
>
> > +
> > + task = rpc_run_task(&task_setup_data);
> > if (IS_ERR(task))
> > return PTR_ERR(task);
> > status = task->tk_status;
> >
> >
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]