All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
@ 2022-04-18 16:51 Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready Chuck Lever
                   ` (15 more replies)
  0 siblings, 16 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

This series implements RPC-with-TLS in the Linux kernel:

https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpc-tls/

This prototype is based on the previously posted mechanism for
providing a TLS handshake facility to in-kernel TLS consumers.

For the purpose of demonstration, the Linux NFS client is modified
to add a new mount option: xprtsec = [ none|auto|tls ] . Updates
to the nfs(5) man page are being developed separately.

The new mount option enables client administrators to require in-
transit encryption for their NFS traffic, protecting the weak
security of AUTH_SYS. An x.509 certificate is not required on the
client for this protection.

This prototype has been tested against prototype TLS-capable NFS
servers. The Linux NFS server itself does not yet have support for
RPC-with-TLS, but it is planned.

At a later time, the Linux NFS client will also get support for
x.509 authentication (for which a certificate will be required on
the client) and PSK. For this demonstration, only authentication-
less TLS (encryption-only) is supported.

---

Chuck Lever (15):
      SUNRPC: Replace dprintk() call site in xs_data_ready
      SUNRPC: Ignore data_ready callbacks during TLS handshakes
      SUNRPC: Capture cmsg metadata on client-side receive
      SUNRPC: Fail faster on bad verifier
      SUNRPC: Widen rpc_task::tk_flags
      SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor
      SUNRPC: Refactor rpc_call_null_helper()
      SUNRPC: Add RPC_TASK_CORK flag
      SUNRPC: Add a cl_xprtsec_policy field
      SUNRPC: Expose TLS policy via the rpc_create() API
      SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe
      SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect
      NFS: Replace fs_context-related dprintk() call sites with tracepoints
      NFS: Have struct nfs_client carry a TLS policy field
      NFS: Add an "xprtsec=" NFS mount option


 fs/nfs/client.c                 |  22 ++++
 fs/nfs/fs_context.c             |  70 ++++++++--
 fs/nfs/internal.h               |   2 +
 fs/nfs/nfs3client.c             |   1 +
 fs/nfs/nfs4client.c             |  16 ++-
 fs/nfs/nfstrace.h               |  77 +++++++++++
 fs/nfs/super.c                  |  10 ++
 include/linux/nfs_fs_sb.h       |   7 +-
 include/linux/sunrpc/auth.h     |   1 +
 include/linux/sunrpc/clnt.h     |  14 +-
 include/linux/sunrpc/sched.h    |  36 +++---
 include/linux/sunrpc/xprt.h     |  14 ++
 include/linux/sunrpc/xprtsock.h |   2 +
 include/net/tls.h               |   2 +
 include/trace/events/sunrpc.h   | 157 ++++++++++++++++++++--
 net/sunrpc/Makefile             |   2 +-
 net/sunrpc/auth.c               |   2 +
 net/sunrpc/auth_tls.c           | 117 +++++++++++++++++
 net/sunrpc/clnt.c               | 222 +++++++++++++++++++++++++++++---
 net/sunrpc/debugfs.c            |   2 +-
 net/sunrpc/xprt.c               |   3 +
 net/sunrpc/xprtsock.c           | 211 +++++++++++++++++++++++++++++-
 22 files changed, 920 insertions(+), 70 deletions(-)
 create mode 100644 net/sunrpc/auth_tls.c

--
Chuck Lever


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes Chuck Lever
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/trace/events/sunrpc.h |   20 ++++++++++++++++++++
 net/sunrpc/xprtsock.c         |    6 ++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 3995c58a1c51..a66aa1f59ed8 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1266,6 +1266,26 @@ TRACE_EVENT(xprt_reserve,
 	)
 );
 
+TRACE_EVENT(xs_data_ready,
+	TP_PROTO(
+		const struct rpc_xprt *xprt
+	),
+
+	TP_ARGS(xprt),
+
+	TP_STRUCT__entry(
+		__string(addr, xprt->address_strings[RPC_DISPLAY_ADDR])
+		__string(port, xprt->address_strings[RPC_DISPLAY_PORT])
+	),
+
+	TP_fast_assign(
+		__assign_str(addr, xprt->address_strings[RPC_DISPLAY_ADDR]);
+		__assign_str(port, xprt->address_strings[RPC_DISPLAY_PORT]);
+	),
+
+	TP_printk("peer=[%s]:%s", __get_str(addr), __get_str(port))
+);
+
 TRACE_EVENT(xs_stream_read_data,
 	TP_PROTO(struct rpc_xprt *xprt, ssize_t err, size_t total),
 
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 8ab64ea46870..e62d339ba589 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1378,7 +1378,7 @@ static void xs_udp_data_receive_workfn(struct work_struct *work)
 }
 
 /**
- * xs_data_ready - "data ready" callback for UDP sockets
+ * xs_data_ready - "data ready" callback for sockets
  * @sk: socket with data to read
  *
  */
@@ -1386,11 +1386,13 @@ static void xs_data_ready(struct sock *sk)
 {
 	struct rpc_xprt *xprt;
 
-	dprintk("RPC:       xs_data_ready...\n");
 	xprt = xprt_from_sock(sk);
 	if (xprt != NULL) {
 		struct sock_xprt *transport = container_of(xprt,
 				struct sock_xprt, xprt);
+
+		trace_xs_data_ready(xprt);
+
 		transport->old_data_ready(sk);
 		/* Any data means we had a useful conversation, so
 		 * then we don't need to delay the next reconnect



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 03/15] SUNRPC: Capture cmsg metadata on client-side receive Chuck Lever
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

The RPC header parser doesn't recognize TLS handshake traffic, so it
will close the connection prematurely. To avoid that, shunt the
transport's data_ready callback when there is a TLS handshake in
progress.

The ignore_dr boolean will be toggled by code added in a subsequent
patch.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/xprtsock.h |    1 +
 net/sunrpc/xprtsock.c           |    7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 38284f25eddf..426c3bd516fe 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -40,6 +40,7 @@ struct sock_xprt {
 				len;
 
 		unsigned long	copied;
+		bool		ignore_dr;
 	} recv;
 
 	/*
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index e62d339ba589..b5bc03c52b9b 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -703,6 +703,8 @@ static void xs_poll_check_readable(struct sock_xprt *transport)
 {
 
 	clear_bit(XPRT_SOCK_DATA_READY, &transport->sock_state);
+	if (transport->recv.ignore_dr)
+		return;
 	if (!xs_poll_socket_readable(transport))
 		return;
 	if (!test_and_set_bit(XPRT_SOCK_DATA_READY, &transport->sock_state))
@@ -1394,6 +1396,10 @@ static void xs_data_ready(struct sock *sk)
 		trace_xs_data_ready(xprt);
 
 		transport->old_data_ready(sk);
+
+		if (transport->recv.ignore_dr)
+			return;
+
 		/* Any data means we had a useful conversation, so
 		 * then we don't need to delay the next reconnect
 		 */
@@ -2779,6 +2785,7 @@ static struct rpc_xprt *xs_setup_xprt(struct xprt_create *args,
 	}
 
 	new = container_of(xprt, struct sock_xprt, xprt);
+	new->recv.ignore_dr = false;
 	mutex_init(&new->recv_mutex);
 	memcpy(&xprt->addr, args->dstaddr, args->addrlen);
 	xprt->addrlen = args->addrlen;



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 03/15] SUNRPC: Capture cmsg metadata on client-side receive
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 04/15] SUNRPC: Fail faster on bad verifier Chuck Lever
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

kTLS sockets use cmsg to report decryption errors and the need
for session re-keying. 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 <chuck.lever@oracle.com>
---
 include/net/tls.h             |    2 ++
 include/trace/events/sunrpc.h |   40 +++++++++++++++++++++++++++++++++
 net/sunrpc/xprtsock.c         |   49 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 6b1bf46daa34..54bccb2e4014 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -71,6 +71,8 @@ static inline struct tlsh_sock *tlsh_sk(struct sock *sk)
 
 #define TLS_CRYPTO_INFO_READY(info)	((info)->cipher_type)
 
+#define TLS_RECORD_TYPE_ALERT		0x15
+#define TLS_RECORD_TYPE_HANDSHAKE	0x16
 #define TLS_RECORD_TYPE_DATA		0x17
 
 #define TLS_AAD_SPACE_SIZE		13
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index a66aa1f59ed8..49b748c03610 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1286,6 +1286,46 @@ TRACE_EVENT(xs_data_ready,
 	TP_printk("peer=[%s]:%s", __get_str(addr), __get_str(port))
 );
 
+/*
+ * From https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml
+ *
+ * Captured March 2022. Other values are unassigned or reserved.
+ */
+#define rpc_show_tls_content_type(type) \
+	__print_symbolic(type, \
+		{ 20,		"change cipher spec" }, \
+		{ 21,		"alert" }, \
+		{ 22,		"handshake" }, \
+		{ 23,		"application data" }, \
+		{ 24,		"heartbeat" }, \
+		{ 25,		"tls12_cid" }, \
+		{ 26,		"ACK" })
+
+TRACE_EVENT(xs_tls_contenttype,
+	TP_PROTO(
+		const struct rpc_xprt *xprt,
+		unsigned char ctype
+	),
+
+	TP_ARGS(xprt, ctype),
+
+	TP_STRUCT__entry(
+		__string(addr, xprt->address_strings[RPC_DISPLAY_ADDR])
+		__string(port, xprt->address_strings[RPC_DISPLAY_PORT])
+		__field(unsigned long, ctype)
+	),
+
+	TP_fast_assign(
+		__assign_str(addr, xprt->address_strings[RPC_DISPLAY_ADDR]);
+		__assign_str(port, xprt->address_strings[RPC_DISPLAY_PORT]);
+		__entry->ctype = ctype;
+	),
+
+	TP_printk("peer=[%s]:%s: %s", __get_str(addr), __get_str(port),
+		rpc_show_tls_content_type(__entry->ctype)
+	)
+);
+
 TRACE_EVENT(xs_stream_read_data,
 	TP_PROTO(struct rpc_xprt *xprt, ssize_t err, size_t total),
 
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index b5bc03c52b9b..e42ae84d7359 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>
@@ -350,13 +352,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));
+
+		trace_xs_tls_contenttype(xprt_from_sock(sock->sk), content_type);
+		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;
 }
 
@@ -382,7 +427,7 @@ xs_read_discard(struct socket *sock, struct msghdr *msg, int flags,
 		size_t count)
 {
 	iov_iter_discard(&msg->msg_iter, READ, count);
-	return sock_recvmsg(sock, msg, flags);
+	return xs_sock_recv_cmsg(sock, msg, flags);
 }
 
 #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 04/15] SUNRPC: Fail faster on bad verifier
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (2 preceding siblings ...)
  2022-04-18 16:51 ` [PATCH RFC 03/15] SUNRPC: Capture cmsg metadata on client-side receive Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 05/15] SUNRPC: Widen rpc_task::tk_flags Chuck Lever
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

A bad verifier is not a garbage argument, it's an authentication
failure. Retrying it doesn't make the problem go away, and delays
upper layer recovery steps.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/clnt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index af0174d7ce5a..ef3d7e4a26e7 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2639,7 +2639,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr)
 
 out_verifier:
 	trace_rpc_bad_verifier(task);
-	goto out_garbage;
+	goto out_err;
 
 out_msg_denied:
 	error = -EACCES;



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 05/15] SUNRPC: Widen rpc_task::tk_flags
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (3 preceding siblings ...)
  2022-04-18 16:51 ` [PATCH RFC 04/15] SUNRPC: Fail faster on bad verifier Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor Chuck Lever
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

There is just one unused bit left in rpc_task::tk_flags, and I will
need two in subsequent patches. Double the width of the field to
accommodate more flag bits.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/clnt.h  |    6 ++++--
 include/linux/sunrpc/sched.h |   32 ++++++++++++++++----------------
 net/sunrpc/clnt.c            |   11 ++++++-----
 net/sunrpc/debugfs.c         |    2 +-
 4 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 267b7aeaf1a6..ec0d241730cb 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -192,11 +192,13 @@ void rpc_prepare_reply_pages(struct rpc_rqst *req, struct page **pages,
 			     unsigned int hdrsize);
 void		rpc_call_start(struct rpc_task *);
 int		rpc_call_async(struct rpc_clnt *clnt,
-			       const struct rpc_message *msg, int flags,
+			       const struct rpc_message *msg,
+			       unsigned int flags,
 			       const struct rpc_call_ops *tk_ops,
 			       void *calldata);
 int		rpc_call_sync(struct rpc_clnt *clnt,
-			      const struct rpc_message *msg, int flags);
+			      const struct rpc_message *msg,
+			      unsigned int flags);
 struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred,
 			       int flags);
 int		rpc_restart_call_prepare(struct rpc_task *);
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 1d7a3e51b795..d4b7ebd0a99c 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -82,7 +82,7 @@ struct rpc_task {
 	ktime_t			tk_start;	/* RPC task init timestamp */
 
 	pid_t			tk_owner;	/* Process id for batching tasks */
-	unsigned short		tk_flags;	/* misc flags */
+	unsigned int		tk_flags;	/* misc flags */
 	unsigned short		tk_timeouts;	/* maj timeouts */
 
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) || IS_ENABLED(CONFIG_TRACEPOINTS)
@@ -112,27 +112,27 @@ struct rpc_task_setup {
 	const struct rpc_call_ops *callback_ops;
 	void *callback_data;
 	struct workqueue_struct *workqueue;
-	unsigned short flags;
+	unsigned int flags;
 	signed char priority;
 };
 
 /*
  * RPC task flags
  */
-#define RPC_TASK_ASYNC		0x0001		/* is an async task */
-#define RPC_TASK_SWAPPER	0x0002		/* is swapping in/out */
-#define RPC_TASK_MOVEABLE	0x0004		/* nfs4.1+ rpc tasks */
-#define RPC_TASK_NULLCREDS	0x0010		/* Use AUTH_NULL credential */
-#define RPC_CALL_MAJORSEEN	0x0020		/* major timeout seen */
-#define RPC_TASK_DYNAMIC	0x0080		/* task was kmalloc'ed */
-#define	RPC_TASK_NO_ROUND_ROBIN	0x0100		/* send requests on "main" xprt */
-#define RPC_TASK_SOFT		0x0200		/* Use soft timeouts */
-#define RPC_TASK_SOFTCONN	0x0400		/* Fail if can't connect */
-#define RPC_TASK_SENT		0x0800		/* message was sent */
-#define RPC_TASK_TIMEOUT	0x1000		/* fail with ETIMEDOUT on timeout */
-#define RPC_TASK_NOCONNECT	0x2000		/* return ENOTCONN if not connected */
-#define RPC_TASK_NO_RETRANS_TIMEOUT	0x4000		/* wait forever for a reply */
-#define RPC_TASK_CRED_NOREF	0x8000		/* No refcount on the credential */
+#define RPC_TASK_ASYNC			0x00000001	/* is an async task */
+#define RPC_TASK_SWAPPER		0x00000002	/* is swapping in/out */
+#define RPC_TASK_MOVEABLE		0x00000004	/* nfs4.1+ rpc tasks */
+#define RPC_TASK_NULLCREDS		0x00000010	/* Use AUTH_NULL credential */
+#define RPC_CALL_MAJORSEEN		0x00000020	/* major timeout seen */
+#define RPC_TASK_DYNAMIC		0x00000080	/* task was kmalloc'ed */
+#define	RPC_TASK_NO_ROUND_ROBIN		0x00000100	/* send requests on "main" xprt */
+#define RPC_TASK_SOFT			0x00000200	/* Use soft timeouts */
+#define RPC_TASK_SOFTCONN		0x00000400	/* Fail if can't connect */
+#define RPC_TASK_SENT			0x00000800	/* message was sent */
+#define RPC_TASK_TIMEOUT		0x00001000	/* fail with ETIMEDOUT on timeout */
+#define RPC_TASK_NOCONNECT		0x00002000	/* return ENOTCONN if not connected */
+#define RPC_TASK_NO_RETRANS_TIMEOUT	0x00004000	/* wait forever for a reply */
+#define RPC_TASK_CRED_NOREF		0x00008000	/* No refcount on the credential */
 
 #define RPC_IS_ASYNC(t)		((t)->tk_flags & RPC_TASK_ASYNC)
 #define RPC_IS_SWAPPER(t)	((t)->tk_flags & RPC_TASK_SWAPPER)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index ef3d7e4a26e7..3d3be55205bb 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1151,7 +1151,8 @@ EXPORT_SYMBOL_GPL(rpc_run_task);
  * @msg: RPC call parameters
  * @flags: RPC call flags
  */
-int rpc_call_sync(struct rpc_clnt *clnt, const struct rpc_message *msg, int flags)
+int rpc_call_sync(struct rpc_clnt *clnt, const struct rpc_message *msg,
+		  unsigned int flags)
 {
 	struct rpc_task	*task;
 	struct rpc_task_setup task_setup_data = {
@@ -1186,9 +1187,9 @@ EXPORT_SYMBOL_GPL(rpc_call_sync);
  * @tk_ops: RPC call ops
  * @data: user call data
  */
-int
-rpc_call_async(struct rpc_clnt *clnt, const struct rpc_message *msg, int flags,
-	       const struct rpc_call_ops *tk_ops, void *data)
+int rpc_call_async(struct rpc_clnt *clnt, const struct rpc_message *msg,
+		   unsigned int flags, const struct rpc_call_ops *tk_ops,
+		   void *data)
 {
 	struct rpc_task	*task;
 	struct rpc_task_setup task_setup_data = {
@@ -3043,7 +3044,7 @@ static void rpc_show_task(const struct rpc_clnt *clnt,
 	if (RPC_IS_QUEUED(task))
 		rpc_waitq = rpc_qname(task->tk_waitqueue);
 
-	printk(KERN_INFO "%5u %04x %6d %8p %8p %8ld %8p %sv%u %s a:%ps q:%s\n",
+	printk(KERN_INFO "%5u %08x %6d %8p %8p %8ld %8p %sv%u %s a:%ps q:%s\n",
 		task->tk_pid, task->tk_flags, task->tk_status,
 		clnt, task->tk_rqstp, rpc_task_timeout(task), task->tk_ops,
 		clnt->cl_program->name, clnt->cl_vers, rpc_proc_name(task),
diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
index 7dc9cc929bfd..8b66235a3a49 100644
--- a/net/sunrpc/debugfs.c
+++ b/net/sunrpc/debugfs.c
@@ -30,7 +30,7 @@ tasks_show(struct seq_file *f, void *v)
 	if (task->tk_rqstp)
 		xid = be32_to_cpu(task->tk_rqstp->rq_xid);
 
-	seq_printf(f, "%5u %04x %6d 0x%x 0x%x %8ld %ps %sv%u %s a:%ps q:%s\n",
+	seq_printf(f, "%5u %08x %6d 0x%x 0x%x %8ld %ps %sv%u %s a:%ps q:%s\n",
 		task->tk_pid, task->tk_flags, task->tk_status,
 		clnt->cl_clid, xid, rpc_task_timeout(task), task->tk_ops,
 		clnt->cl_program->name, clnt->cl_vers, rpc_proc_name(task),



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (4 preceding siblings ...)
  2022-04-18 16:51 ` [PATCH RFC 05/15] SUNRPC: Widen rpc_task::tk_flags Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:51 ` [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper() Chuck Lever
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

The new authentication flavor is used to discover peer support for
RPC-over-TLS.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/auth.h   |    1 
 include/linux/sunrpc/sched.h  |    1 
 include/trace/events/sunrpc.h |    1 
 net/sunrpc/Makefile           |    2 -
 net/sunrpc/auth.c             |    2 +
 net/sunrpc/auth_tls.c         |  117 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 123 insertions(+), 1 deletion(-)
 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/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index d4b7ebd0a99c..599133fb3c63 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -122,6 +122,7 @@ struct rpc_task_setup {
 #define RPC_TASK_ASYNC			0x00000001	/* is an async task */
 #define RPC_TASK_SWAPPER		0x00000002	/* is swapping in/out */
 #define RPC_TASK_MOVEABLE		0x00000004	/* nfs4.1+ rpc tasks */
+#define RPC_TASK_TLSCRED		0x00000008	/* Use AUTH_TLS credential */
 #define RPC_TASK_NULLCREDS		0x00000010	/* Use AUTH_NULL credential */
 #define RPC_CALL_MAJORSEEN		0x00000020	/* major timeout seen */
 #define RPC_TASK_DYNAMIC		0x00000080	/* task was kmalloc'ed */
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 49b748c03610..811187c47ebb 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -309,6 +309,7 @@ TRACE_EVENT(rpc_request,
 		{ RPC_TASK_ASYNC, "ASYNC" },				\
 		{ RPC_TASK_SWAPPER, "SWAPPER" },			\
 		{ RPC_TASK_MOVEABLE, "MOVEABLE" },			\
+		{ RPC_TASK_TLSCRED, "TLSCRED" },			\
 		{ RPC_TASK_NULLCREDS, "NULLCREDS" },			\
 		{ RPC_CALL_MAJORSEEN, "MAJORSEEN" },			\
 		{ RPC_TASK_DYNAMIC, "DYNAMIC" },			\
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 682fcd24bf43..8a9b358f7ed7 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -674,6 +674,8 @@ rpcauth_bindcred(struct rpc_task *task, const struct cred *cred, int flags)
 		new = rpcauth_bind_root_cred(task, lookupflags);
 	else if (flags & RPC_TASK_NULLCREDS)
 		new = authnull_ops.lookup_cred(NULL, NULL, 0);
+	else if (flags & RPC_TASK_TLSCRED)
+		new = authtls_ops.lookup_cred(NULL, NULL, 0);
 	else
 		new = rpcauth_bind_new_cred(task, lookupflags);
 	if (IS_ERR(new))
diff --git a/net/sunrpc/auth_tls.c b/net/sunrpc/auth_tls.c
new file mode 100644
index 000000000000..244f0890d471
--- /dev/null
+++ b/net/sunrpc/auth_tls.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 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 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, 8) != 8)
+		return -EIO;
+	if (memcmp(str, "STARTTLS", 8))
+		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,
+};



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper()
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (5 preceding siblings ...)
  2022-04-18 16:51 ` [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor Chuck Lever
@ 2022-04-18 16:51 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag Chuck Lever
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:51 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

We need to use RPC_TASK_TLSCRED instead of RPC_TASK_NULLCREDS in one
upcoming case.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/clnt.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 3d3be55205bb..3fb601d8a531 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2734,8 +2734,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);
@@ -2743,7 +2742,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);
 
@@ -2821,9 +2821,11 @@ 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);
 	data->xps->xps_nunique_destaddr_xprts++;
+
 	rpc_put_task(task);
 success:
 	return 1;
@@ -2864,7 +2866,8 @@ int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *clnt,
 		goto out_err;
 
 	/* 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)) {
 		status = PTR_ERR(task);
 		goto out_err;



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (6 preceding siblings ...)
  2022-04-18 16:51 ` [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper() Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-19  2:57   ` Trond Myklebust
  2022-04-18 16:52 ` [PATCH RFC 09/15] SUNRPC: Add a cl_xprtsec_policy field Chuck Lever
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Introduce a mechanism to cause xprt_transmit() to break out of its
sending loop at a specific rpc_rqst, rather than draining the whole
transmit queue.

This enables the client to send just an RPC TLS probe and then wait
for the response before proceeding with the rest of the queue.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/sched.h  |    2 ++
 include/trace/events/sunrpc.h |    1 +
 net/sunrpc/xprt.c             |    2 ++
 3 files changed, 5 insertions(+)

diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 599133fb3c63..f8c09638fa69 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -125,6 +125,7 @@ struct rpc_task_setup {
 #define RPC_TASK_TLSCRED		0x00000008	/* Use AUTH_TLS credential */
 #define RPC_TASK_NULLCREDS		0x00000010	/* Use AUTH_NULL credential */
 #define RPC_CALL_MAJORSEEN		0x00000020	/* major timeout seen */
+#define RPC_TASK_CORK			0x00000040	/* cork the xmit queue */
 #define RPC_TASK_DYNAMIC		0x00000080	/* task was kmalloc'ed */
 #define	RPC_TASK_NO_ROUND_ROBIN		0x00000100	/* send requests on "main" xprt */
 #define RPC_TASK_SOFT			0x00000200	/* Use soft timeouts */
@@ -137,6 +138,7 @@ struct rpc_task_setup {
 
 #define RPC_IS_ASYNC(t)		((t)->tk_flags & RPC_TASK_ASYNC)
 #define RPC_IS_SWAPPER(t)	((t)->tk_flags & RPC_TASK_SWAPPER)
+#define RPC_IS_CORK(t)		((t)->tk_flags & RPC_TASK_CORK)
 #define RPC_IS_SOFT(t)		((t)->tk_flags & (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
 #define RPC_IS_SOFTCONN(t)	((t)->tk_flags & RPC_TASK_SOFTCONN)
 #define RPC_WAS_SENT(t)		((t)->tk_flags & RPC_TASK_SENT)
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 811187c47ebb..e8d6adff1a50 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
 		{ RPC_TASK_TLSCRED, "TLSCRED" },			\
 		{ RPC_TASK_NULLCREDS, "NULLCREDS" },			\
 		{ RPC_CALL_MAJORSEEN, "MAJORSEEN" },			\
+		{ RPC_TASK_CORK, "CORK" },				\
 		{ RPC_TASK_DYNAMIC, "DYNAMIC" },			\
 		{ RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN" },		\
 		{ RPC_TASK_SOFT, "SOFT" },				\
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 86d62cffba0d..4b303b945b51 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
 		if (xprt_request_data_received(task) &&
 		    !test_bit(RPC_TASK_NEED_XMIT, &task->tk_runstate))
 			break;
+		if (RPC_IS_CORK(task))
+			break;
 		cond_resched_lock(&xprt->queue_lock);
 	}
 	spin_unlock(&xprt->queue_lock);



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 09/15] SUNRPC: Add a cl_xprtsec_policy field
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (7 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 10/15] SUNRPC: Expose TLS policy via the rpc_create() API Chuck Lever
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

The Linux RPC client will support one  transport security policy for
each struct rpc_clnt:

Three basic ones are added initially:

 = None: No transport security for this rpc_clnt.

 = TLS: The RPC client will establish a TLS session with encryption
   for this rpc_clnt.

 = mTLS: The RPC client will perform mutual peer authentication and
   establish a TLS session with encryption for this rpc_clnt.

Add tracepoints that are used to create an audit trail of TLS usage,
as REQUIRED by Section 7.1 of draft-ietf-nfsv4-rpc-tls-11 .

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/clnt.h   |    7 +++++
 include/trace/events/sunrpc.h |   54 +++++++++++++++++++++++++++++++++++++++++
 net/sunrpc/clnt.c             |    2 ++
 3 files changed, 63 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index ec0d241730cb..14f169aec5c8 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -32,6 +32,12 @@
 struct rpc_inode;
 struct rpc_sysfs_client;
 
+enum rpc_xprtsec {
+	RPC_XPRTSEC_NONE,	/* No transport security is used */
+	RPC_XPRTSEC_TLS,	/* RPC-with-TLS, encryption only */
+	RPC_XPRTSEC_MTLS,	/* RPC-with-TLS, peer auth plus encryption */
+};
+
 /*
  * The high-level client handle
  */
@@ -58,6 +64,7 @@ struct rpc_clnt {
 				cl_noretranstimeo: 1,/* No retransmit timeouts */
 				cl_autobind : 1,/* use getport() */
 				cl_chatty   : 1;/* be verbose */
+	enum rpc_xprtsec	cl_xprtsec_policy;	/* transport security policy */
 
 	struct rpc_rtt *	cl_rtt;		/* RTO estimator data */
 	const struct rpc_timeout *cl_timeout;	/* Timeout strategy */
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index e8d6adff1a50..8ffc9c07bc69 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -139,6 +139,16 @@ 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);
+TRACE_DEFINE_ENUM(RPC_XPRTSEC_MTLS);
+
+#define rpc_show_xprtsec_policy(policy)					\
+	__print_symbolic(policy,					\
+		{ RPC_XPRTSEC_NONE,		"none" },		\
+		{ RPC_XPRTSEC_TLS,		"tls" },		\
+		{ RPC_XPRTSEC_MTLS,		"mtls" })
+
 TRACE_EVENT(rpc_clnt_new,
 	TP_PROTO(
 		const struct rpc_clnt *clnt,
@@ -1535,6 +1545,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/clnt.c b/net/sunrpc/clnt.c
index 3fb601d8a531..856581018f10 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -477,6 +477,7 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
 	if (IS_ERR(clnt))
 		return clnt;
 
+	clnt->cl_xprtsec_policy = RPC_XPRTSEC_NONE;
 	if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
 		int err = rpc_ping(clnt);
 		if (err != 0) {
@@ -643,6 +644,7 @@ static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args,
 	new->cl_noretranstimeo = clnt->cl_noretranstimeo;
 	new->cl_discrtry = clnt->cl_discrtry;
 	new->cl_chatty = clnt->cl_chatty;
+	new->cl_xprtsec_policy = clnt->cl_xprtsec_policy;
 	new->cl_principal = clnt->cl_principal;
 	return new;
 



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 10/15] SUNRPC: Expose TLS policy via the rpc_create() API
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (8 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 09/15] SUNRPC: Add a cl_xprtsec_policy field Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe Chuck Lever
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Consumers use this API to specify RPC-over-TLS security policy for
each rpc_clnt. For the moment, this checks for TLS availability only
at the time the struct rpc_clnt is created.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/clnt.h     |    1 
 include/linux/sunrpc/xprt.h     |    1 
 include/linux/sunrpc/xprtsock.h |    1 
 include/trace/events/sunrpc.h   |   41 +++++++++++++----
 net/sunrpc/clnt.c               |   81 +++++++++++++++++++++++++++++++---
 net/sunrpc/xprtsock.c           |   93 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 201 insertions(+), 17 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 14f169aec5c8..e10a19d136ca 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -146,6 +146,7 @@ struct rpc_create_args {
 	struct svc_xprt		*bc_xprt;	/* NFSv4.1 backchannel */
 	const struct cred	*cred;
 	unsigned int		max_connect;
+	enum rpc_xprtsec	xprtsec_policy;
 };
 
 struct rpc_add_xprt_test {
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 522bbf937957..8d654bc35dce 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -158,6 +158,7 @@ struct rpc_xprt_ops {
 	int		(*enable_swap)(struct rpc_xprt *xprt);
 	void		(*disable_swap)(struct rpc_xprt *xprt);
 	void		(*inject_disconnect)(struct rpc_xprt *xprt);
+	int		(*tls_handshake_sync)(struct rpc_xprt *xprt);
 	int		(*bc_setup)(struct rpc_xprt *xprt,
 				    unsigned int min_reqs);
 	size_t		(*bc_maxpayload)(struct rpc_xprt *xprt);
diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 426c3bd516fe..d738a302b38b 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -58,6 +58,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/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 8ffc9c07bc69..a73b68e25a8c 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -149,36 +149,56 @@ TRACE_DEFINE_ENUM(RPC_XPRTSEC_MTLS);
 		{ RPC_XPRTSEC_TLS,		"tls" },		\
 		{ RPC_XPRTSEC_MTLS,		"mtls" })
 
+#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,
@@ -1585,6 +1605,7 @@ DECLARE_EVENT_CLASS(rpc_tls_class,
 			), \
 			TP_ARGS(clnt, xprt))
 
+DEFINE_RPC_TLS_EVENT(unsupported);
 DEFINE_RPC_TLS_EVENT(unavailable);
 DEFINE_RPC_TLS_EVENT(not_started);
 
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 856581018f10..1601a06deaf5 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -76,6 +76,7 @@ static int	rpc_encode_header(struct rpc_task *task,
 static int	rpc_decode_header(struct rpc_task *task,
 				  struct xdr_stream *xdr);
 static int	rpc_ping(struct rpc_clnt *clnt);
+static int	rpc_starttls_sync(struct rpc_clnt *clnt);
 static void	rpc_check_timeout(struct rpc_task *task);
 
 static void rpc_register_client(struct rpc_clnt *clnt)
@@ -433,7 +434,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:
@@ -457,6 +458,7 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
 {
 	struct rpc_clnt *clnt = NULL;
 	struct rpc_xprt_switch *xps;
+	int err;
 
 	if (args->bc_xprt && args->bc_xprt->xpt_bc_xps) {
 		WARN_ON_ONCE(!(args->protocol & XPRT_TRANSPORT_BC));
@@ -477,13 +479,23 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
 	if (IS_ERR(clnt))
 		return clnt;
 
-	clnt->cl_xprtsec_policy = RPC_XPRTSEC_NONE;
-	if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
-		int err = rpc_ping(clnt);
-		if (err != 0) {
-			rpc_shutdown_client(clnt);
-			return ERR_PTR(err);
+	clnt->cl_xprtsec_policy = args->xprtsec_policy;
+	switch (args->xprtsec_policy) {
+	case RPC_XPRTSEC_NONE:
+		if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
+			err = rpc_ping(clnt);
+			if (err != 0)
+				goto out_shutdown;
 		}
+		break;
+	case RPC_XPRTSEC_TLS:
+		err = rpc_starttls_sync(clnt);
+		if (err != 0)
+			goto out_shutdown;
+		break;
+	default:
+		err = -EINVAL;
+		goto out_shutdown;
 	}
 
 	clnt->cl_softrtry = 1;
@@ -503,6 +515,10 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
 		clnt->cl_chatty = 1;
 
 	return clnt;
+
+out_shutdown:
+	rpc_shutdown_client(clnt);
+	return ERR_PTR(err);
 }
 
 /**
@@ -690,6 +706,7 @@ rpc_clone_client_set_auth(struct rpc_clnt *clnt, rpc_authflavor_t flavor)
 		.version	= clnt->cl_vers,
 		.authflavor	= flavor,
 		.cred		= clnt->cl_cred,
+		.xprtsec_policy	= clnt->cl_xprtsec_policy,
 	};
 	return __rpc_clone_client(&args, clnt);
 }
@@ -2762,6 +2779,56 @@ static int rpc_ping(struct rpc_clnt *clnt)
 	return status;
 }
 
+/**
+ * rpc_starttls_sync - Synchronously establish a TLS session
+ * @clnt: A fresh RPC client
+ *
+ * Return values:
+ *   %0: TLS session established
+ *   %-ENOPROTOOPT: underlying xprt does not support TLS
+ *   %-EPERM: peer does not support TLS
+ *   %-EACCES: TLS session could not be established
+ */
+static int rpc_starttls_sync(struct rpc_clnt *clnt)
+{
+	struct rpc_xprt *xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
+	struct rpc_message msg = {
+		.rpc_proc = &rpcproc_null,
+	};
+	int err;
+
+	if (!xprt->ops->tls_handshake_sync) {
+		trace_rpc_tls_unsupported(clnt, xprt);
+		err = -ENOPROTOOPT;
+		goto out_put;
+	}
+
+	err = rpc_call_sync(clnt, &msg,
+			    RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
+			    RPC_TASK_TLSCRED);
+	switch (err) {
+	case 0:
+		break;
+	case -EACCES:
+	case -EIO:
+		trace_rpc_tls_unavailable(clnt, xprt);
+		err = -EPERM;
+		fallthrough;
+	default:
+		goto out_put;
+	}
+
+	if (xprt->ops->tls_handshake_sync(xprt)) {
+		trace_rpc_tls_not_started(clnt, xprt);
+		err = -EACCES;
+		goto out_put;
+	}
+
+out_put:
+	xprt_put(xprt);
+	return err;
+}
+
 struct rpc_cb_add_xprt_calldata {
 	struct rpc_xprt_switch *xps;
 	struct rpc_xprt *xprt;
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index e42ae84d7359..bbba9747f68d 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/tlsh.h>
 
 #include <linux/bvec.h>
 #include <linux/highmem.h>
@@ -197,6 +198,11 @@ static struct ctl_table sunrpc_table[] = {
  */
 #define XS_IDLE_DISC_TO		(5U * 60 * HZ)
 
+/*
+ * TLS handshake timeout.
+ */
+#define XS_TLS_HANDSHAKE_TO	(20U * HZ)
+
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 # undef  RPC_DEBUG_DATA
 # define RPCDBG_FACILITY	RPCDBG_TRANS
@@ -2304,6 +2310,92 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
 	return kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
 }
 
+#if IS_ENABLED(CONFIG_TLS)
+
+static void xs_tcp_clear_discard(struct sock_xprt *transport)
+{
+	transport->recv.ignore_dr = false;
+	transport->recv.copied = 0;
+	transport->recv.offset = 0;
+	transport->recv.len = 0;
+}
+
+/**
+ * xs_tcp_tls_handshake_done - TLS handshake completion handler
+ * @data: address of xprt to wake
+ * @status: status of handshake
+ *
+ */
+static void xs_tcp_tls_handshake_done(void *data, int status)
+{
+	struct rpc_xprt *xprt = data;
+	struct sock_xprt *transport =
+				container_of(xprt, struct sock_xprt, xprt);
+
+	transport->xprt_err = status ? -EACCES : 0;
+	complete(&transport->handshake_done);
+	xprt_put(xprt);
+}
+
+/**
+ * xs_tcp_tls_handshake_sync - Perform a full TLS client handshake
+ * @xprt: transport on which to perform handshake
+ *
+ * Caller ensures there will be no other traffic on this transport.
+ *
+ * Return values:
+ *   %0: Handshake completed successfully.
+ *   Negative errno: handshake not started, or failed.
+ */
+static int xs_tcp_tls_handshake_sync(struct rpc_xprt *xprt)
+{
+	struct sock_xprt *transport =
+				container_of(xprt, struct sock_xprt, xprt);
+	int rc;
+
+	/* XXX: make it an XPRT_ flag instead? */
+	WRITE_ONCE(transport->recv.ignore_dr, true);
+
+	init_completion(&transport->handshake_done);
+
+	transport->xprt_err = -ETIMEDOUT;
+	rc = tls_client_hello_x509(transport->sock,
+				   xs_tcp_tls_handshake_done, xprt_get(xprt),
+				   TLSH_DEFAULT_PRIORITIES, TLSH_NO_PEERID,
+				   TLSH_NO_CERT);
+	if (rc)
+		goto out;
+
+	rc = wait_for_completion_interruptible_timeout(&transport->handshake_done,
+						       XS_TLS_HANDSHAKE_TO);
+	if (rc < 0)
+		goto out;
+
+	rc = transport->xprt_err;
+
+out:
+	xs_tcp_clear_discard(transport);
+	return rc;
+}
+
+#else /* CONFIG_TLS */
+
+/**
+ * xs_tcp_tls_handshake_sync - Perform a full TLS client handshake
+ * @xprt: transport on which to perform handshake
+ *
+ * Caller ensures there will be no other traffic on this transport.
+ *
+ * Return values:
+ *   %-EACCES: handshake was not started.
+ */
+static int xs_tcp_tls_handshake_sync(struct rpc_xprt *xprt)
+{
+	return -EACCES;
+}
+
+#endif /*CONFIG_TLS */
+
 /**
  * xs_tcp_setup_socket - create a TCP socket and connect to a remote endpoint
  * @work: queued work item
@@ -2752,6 +2844,7 @@ static const struct rpc_xprt_ops xs_tcp_ops = {
 	.enable_swap		= xs_enable_swap,
 	.disable_swap		= xs_disable_swap,
 	.inject_disconnect	= xs_inject_disconnect,
+	.tls_handshake_sync	= xs_tcp_tls_handshake_sync,
 #ifdef CONFIG_SUNRPC_BACKCHANNEL
 	.bc_setup		= xprt_setup_bc,
 	.bc_maxpayload		= xs_tcp_bc_maxpayload,



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (9 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 10/15] SUNRPC: Expose TLS policy via the rpc_create() API Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect Chuck Lever
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Introduce helpers for sending an RPC_AUTH_TLS probe, waiting for
it, and then parsing the reply. These will be used to handle the
reconnect case.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/clnt.h |    1 +
 include/linux/sunrpc/xprt.h |   13 ++++++++++
 net/sunrpc/clnt.c           |   58 +++++++++++++++++++++++++++++++++++++++++++
 net/sunrpc/xprt.c           |    1 +
 net/sunrpc/xprtsock.c       |   56 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 129 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index e10a19d136ca..15fd84e4c321 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -209,6 +209,7 @@ int		rpc_call_sync(struct rpc_clnt *clnt,
 			      unsigned int flags);
 struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred,
 			       int flags);
+void		rpc_starttls_async(struct rpc_task *task);
 int		rpc_restart_call_prepare(struct rpc_task *);
 int		rpc_restart_call(struct rpc_task *);
 void		rpc_setbufsize(struct rpc_clnt *, unsigned int, unsigned int);
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 8d654bc35dce..cbb0fe738d97 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -159,6 +159,7 @@ struct rpc_xprt_ops {
 	void		(*disable_swap)(struct rpc_xprt *xprt);
 	void		(*inject_disconnect)(struct rpc_xprt *xprt);
 	int		(*tls_handshake_sync)(struct rpc_xprt *xprt);
+	int		(*tls_handshake_async)(struct rpc_xprt *xprt);
 	int		(*bc_setup)(struct rpc_xprt *xprt,
 				    unsigned int min_reqs);
 	size_t		(*bc_maxpayload)(struct rpc_xprt *xprt);
@@ -204,6 +205,7 @@ struct rpc_xprt {
 	size_t			max_payload;	/* largest RPC payload size,
 						   in bytes */
 
+	struct rpc_wait_queue	probing;	/* requests waiting on TLS probe */
 	struct rpc_wait_queue	binding;	/* requests waiting on rpcbind */
 	struct rpc_wait_queue	sending;	/* requests waiting to send */
 	struct rpc_wait_queue	pending;	/* requests in flight */
@@ -436,6 +438,7 @@ void			xprt_release_write(struct rpc_xprt *, struct rpc_task *);
 #define XPRT_CWND_WAIT		(10)
 #define XPRT_WRITE_SPACE	(11)
 #define XPRT_SND_IS_COOKIE	(12)
+#define XPRT_PROBING		(13)
 
 static inline void xprt_set_connected(struct rpc_xprt *xprt)
 {
@@ -506,4 +509,14 @@ static inline int xprt_test_and_set_binding(struct rpc_xprt *xprt)
 	return test_and_set_bit(XPRT_BINDING, &xprt->state);
 }
 
+static inline void xprt_clear_probing(struct rpc_xprt *xprt)
+{
+	clear_bit(XPRT_PROBING, &xprt->state);
+}
+
+static inline int xprt_test_and_set_probing(struct rpc_xprt *xprt)
+{
+	return test_and_set_bit(XPRT_PROBING, &xprt->state);
+}
+
 #endif /* _LINUX_SUNRPC_XPRT_H */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 1601a06deaf5..e9a6622dba68 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2829,6 +2829,64 @@ static int rpc_starttls_sync(struct rpc_clnt *clnt)
 	return err;
 }
 
+static void rpc_probe_tls_done(struct rpc_task *task, void *data)
+{
+	struct rpc_xprt *xprt = data;
+
+	if (task->tk_status < 0) {
+		trace_rpc_tls_unavailable(task->tk_client, xprt);
+		xprt_clear_probing(xprt);
+		rpc_wake_up_status(&xprt->probing, task->tk_status);
+		return;
+	}
+
+	/* Send ClientHello; a second callback will wake the waiting task */
+	if (xprt->ops->tls_handshake_async(xprt)) {
+		trace_rpc_tls_not_started(task->tk_client, xprt);
+		xprt_clear_probing(xprt);
+		rpc_wake_up_status(&xprt->probing, -EACCES);
+	}
+}
+
+static void rpc_probe_tls_release(void *data)
+{
+	struct rpc_xprt *xprt = data;
+
+	xprt_put(xprt);
+}
+
+static const struct rpc_call_ops rpc_ops_probe_tls = {
+	.rpc_call_done	= rpc_probe_tls_done,
+	.rpc_release	= rpc_probe_tls_release,
+};
+
+/**
+ * rpc_starttls_async - Asynchronously establish a TLS session
+ * @task: an RPC task waiting for a TLS session
+ *
+ */
+void rpc_starttls_async(struct rpc_task *task)
+{
+	struct rpc_xprt *xprt = xprt_get(task->tk_xprt);
+
+	rpc_sleep_on_timeout(&xprt->probing, task, NULL,
+			     jiffies + xprt->connect_timeout);
+	if (xprt_test_and_set_probing(xprt)) {
+		xprt_put(xprt);
+		return;
+	}
+
+	/*
+	 * RPC_TASK_TLSCRED: use an RPC_AUTH_TLS cred instead of AUTH_NULL
+	 * RPC_TASK_SWAPPER: insert the task at the head of the transmit queue
+	 * RPC_TASK_CORK: stop sending after this Call is transmitted
+	 */
+	rpc_put_task(rpc_call_null_helper(task->tk_client, xprt, NULL,
+		     RPC_TASK_TLSCRED | RPC_TASK_SWAPPER | RPC_TASK_CORK,
+		     &rpc_ops_probe_tls, xprt));
+}
+EXPORT_SYMBOL_GPL(rpc_starttls_async);
+
 struct rpc_cb_add_xprt_calldata {
 	struct rpc_xprt_switch *xps;
 	struct rpc_xprt *xprt;
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 4b303b945b51..762281dba077 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -2016,6 +2016,7 @@ static void xprt_init(struct rpc_xprt *xprt, struct net *net)
 	xprt->cwnd = RPC_INITCWND;
 	xprt->bind_index = 0;
 
+	rpc_init_wait_queue(&xprt->probing, "xprt_probing");
 	rpc_init_wait_queue(&xprt->binding, "xprt_binding");
 	rpc_init_wait_queue(&xprt->pending, "xprt_pending");
 	rpc_init_wait_queue(&xprt->sending, "xprt_sending");
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index bbba9747f68d..bb8c32002ce7 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2378,6 +2378,46 @@ static int xs_tcp_tls_handshake_sync(struct rpc_xprt *xprt)
 	return rc;
 }
 
+/**
+ * xs_tcp_tls_handshake_wake - TLS handshake completion handler
+ * @data: address of xprt to wake
+ * @status: status of handshake
+ *
+ */
+static void xs_tcp_tls_handshake_wake(void *data, int status)
+{
+	struct rpc_xprt *xprt = data;
+	struct sock_xprt *transport =
+				container_of(xprt, struct sock_xprt, xprt);
+
+	xs_tcp_clear_discard(transport);
+	xprt_clear_probing(xprt);
+	rpc_wake_up_status(&xprt->probing, status < 0 ? -EACCES : 0);
+	xprt_put(xprt);
+}
+
+/**
+ * xs_tcp_tls_handshake_async - Start a TLS handshake
+ * @xprt: transport on which to perform handshake
+ *
+ * Caller ensures there will be no other traffic on this transport.
+ *
+ * Return values:
+ *   %0: Handshake initiated; @xprt will be awoken when it's done.
+ *   Negative errno: handshake was not started.
+ */
+static int xs_tcp_tls_handshake_async(struct rpc_xprt *xprt)
+{
+	struct sock_xprt *transport =
+				container_of(xprt, struct sock_xprt, xprt);
+
+	WRITE_ONCE(transport->recv.ignore_dr, true);
+	return tls_client_hello_x509(transport->sock,
+				     xs_tcp_tls_handshake_wake, xprt_get(xprt),
+				     TLSH_DEFAULT_PRIORITIES, TLSH_NO_PEERID,
+				     TLSH_NO_CERT);
+}
+
 #else /* CONFIG_TLS */
 
 /**
@@ -2394,6 +2434,21 @@ static int xs_tcp_tls_handshake_sync(struct rpc_xprt *xprt)
 	return -EACCES;
 }
 
+/**
+ * xs_tcp_tls_handshake_async - Start a TLS handshake
+ * @xprt: transport on which to perform handshake
+ *
+ * Caller ensures there will be no other traffic on this transport.
+ *
+ * Return values:
+ *   %0: Handshake initiated; @xprt will be awoken when it's done.
+ *   Negative errno: handshake was not started.
+ */
+static int xs_tcp_tls_handshake_async(struct rpc_xprt *xprt)
+{
+	return -EACCES;
+}
+
 #endif /*CONFIG_TLS */
 
 /**
@@ -2845,6 +2900,7 @@ static const struct rpc_xprt_ops xs_tcp_ops = {
 	.disable_swap		= xs_disable_swap,
 	.inject_disconnect	= xs_inject_disconnect,
 	.tls_handshake_sync	= xs_tcp_tls_handshake_sync,
+	.tls_handshake_async	= xs_tcp_tls_handshake_async,
 #ifdef CONFIG_SUNRPC_BACKCHANNEL
 	.bc_setup		= xprt_setup_bc,
 	.bc_maxpayload		= xs_tcp_bc_maxpayload,



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (10 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 13/15] NFS: Replace fs_context-related dprintk() call sites with tracepoints Chuck Lever
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Try STARTTLS with the RPC server peer as soon as a transport
connection is established.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/clnt.h  |    1 -
 include/linux/sunrpc/sched.h |    1 +
 net/sunrpc/clnt.c            |   59 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 15fd84e4c321..e10a19d136ca 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -209,7 +209,6 @@ int		rpc_call_sync(struct rpc_clnt *clnt,
 			      unsigned int flags);
 struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred,
 			       int flags);
-void		rpc_starttls_async(struct rpc_task *task);
 int		rpc_restart_call_prepare(struct rpc_task *);
 int		rpc_restart_call(struct rpc_task *);
 void		rpc_setbufsize(struct rpc_clnt *, unsigned int, unsigned int);
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index f8c09638fa69..0d1ae89a2339 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -139,6 +139,7 @@ struct rpc_task_setup {
 #define RPC_IS_ASYNC(t)		((t)->tk_flags & RPC_TASK_ASYNC)
 #define RPC_IS_SWAPPER(t)	((t)->tk_flags & RPC_TASK_SWAPPER)
 #define RPC_IS_CORK(t)		((t)->tk_flags & RPC_TASK_CORK)
+#define RPC_IS_TLSPROBE(t)	((t)->tk_flags & RPC_TASK_TLSCRED)
 #define RPC_IS_SOFT(t)		((t)->tk_flags & (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
 #define RPC_IS_SOFTCONN(t)	((t)->tk_flags & RPC_TASK_SOFTCONN)
 #define RPC_WAS_SENT(t)		((t)->tk_flags & RPC_TASK_SENT)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index e9a6622dba68..0506971410f7 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -70,6 +70,8 @@ static void	call_refresh(struct rpc_task *task);
 static void	call_refreshresult(struct rpc_task *task);
 static void	call_connect(struct rpc_task *task);
 static void	call_connect_status(struct rpc_task *task);
+static void	call_start_tls(struct rpc_task *task);
+static void	call_tls_status(struct rpc_task *task);
 
 static int	rpc_encode_header(struct rpc_task *task,
 				  struct xdr_stream *xdr);
@@ -77,6 +79,7 @@ static int	rpc_decode_header(struct rpc_task *task,
 				  struct xdr_stream *xdr);
 static int	rpc_ping(struct rpc_clnt *clnt);
 static int	rpc_starttls_sync(struct rpc_clnt *clnt);
+static void	rpc_starttls_async(struct rpc_task *task);
 static void	rpc_check_timeout(struct rpc_task *task);
 
 static void rpc_register_client(struct rpc_clnt *clnt)
@@ -2163,7 +2166,7 @@ call_connect_status(struct rpc_task *task)
 	rpc_call_rpcerror(task, status);
 	return;
 out_next:
-	task->tk_action = call_transmit;
+	task->tk_action = call_start_tls;
 	return;
 out_retry:
 	/* Check for timeouts before looping back to call_bind */
@@ -2171,6 +2174,53 @@ call_connect_status(struct rpc_task *task)
 	rpc_check_timeout(task);
 }
 
+static void
+call_start_tls(struct rpc_task *task)
+{
+	struct rpc_xprt *xprt = task->tk_rqstp->rq_xprt;
+	struct rpc_clnt *clnt = task->tk_client;
+
+	task->tk_action = call_transmit;
+	if (RPC_IS_TLSPROBE(task))
+		return;
+
+	switch (clnt->cl_xprtsec_policy) {
+	case RPC_XPRTSEC_TLS:
+	case RPC_XPRTSEC_MTLS:
+		if (xprt->ops->tls_handshake_async) {
+			task->tk_action = call_tls_status;
+			rpc_starttls_async(task);
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static void
+call_tls_status(struct rpc_task *task)
+{
+	struct rpc_xprt *xprt = task->tk_rqstp->rq_xprt;
+	struct rpc_clnt *clnt = task->tk_client;
+
+	task->tk_action = call_transmit;
+	if (!task->tk_status)
+		return;
+
+	xprt_force_disconnect(xprt);
+
+	switch (clnt->cl_xprtsec_policy) {
+	case RPC_XPRTSEC_TLS:
+	case RPC_XPRTSEC_MTLS:
+		rpc_delay(task, 5*HZ /* arbitrary */);
+		break;
+	default:
+		task->tk_action = call_bind;
+	}
+
+	rpc_check_timeout(task);
+}
+
 /*
  * 5.	Transmit the RPC request, and wait for reply
  */
@@ -2355,7 +2405,7 @@ call_status(struct rpc_task *task)
 	struct rpc_clnt	*clnt = task->tk_client;
 	int		status;
 
-	if (!task->tk_msg.rpc_proc->p_proc)
+	if (!task->tk_msg.rpc_proc->p_proc && !RPC_IS_TLSPROBE(task))
 		trace_xprt_ping(task->tk_xprt, task->tk_status);
 
 	status = task->tk_status;
@@ -2663,6 +2713,8 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr)
 
 out_msg_denied:
 	error = -EACCES;
+	if (RPC_IS_TLSPROBE(task))
+		goto out_err;
 	p = xdr_inline_decode(xdr, sizeof(*p));
 	if (!p)
 		goto out_unparsable;
@@ -2865,7 +2917,7 @@ static const struct rpc_call_ops rpc_ops_probe_tls = {
  * @task: an RPC task waiting for a TLS session
  *
  */
-void rpc_starttls_async(struct rpc_task *task)
+static void rpc_starttls_async(struct rpc_task *task)
 {
 	struct rpc_xprt *xprt = xprt_get(task->tk_xprt);
 
@@ -2885,7 +2937,6 @@ void rpc_starttls_async(struct rpc_task *task)
 		     RPC_TASK_TLSCRED | RPC_TASK_SWAPPER | RPC_TASK_CORK,
 		     &rpc_ops_probe_tls, xprt));
 }
-EXPORT_SYMBOL_GPL(rpc_starttls_async);
 
 struct rpc_cb_add_xprt_calldata {
 	struct rpc_xprt_switch *xps;



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 13/15] NFS: Replace fs_context-related dprintk() call sites with tracepoints
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (11 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field Chuck Lever
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

Contributed as part of the long patch series that converts NFS from
using dprintk to tracepoints for observability.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/fs_context.c |   25 ++++++++++-------
 fs/nfs/nfstrace.h   |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index e2d59bb5e6bb..b52362735d12 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -21,6 +21,8 @@
 #include "nfs.h"
 #include "internal.h"
 
+#include "nfstrace.h"
+
 #define NFSDBG_FACILITY		NFSDBG_MOUNT
 
 #if IS_ENABLED(CONFIG_NFS_V3)
@@ -284,7 +286,7 @@ static int nfs_verify_server_address(struct sockaddr *addr)
 	}
 	}
 
-	dfprintk(MOUNT, "NFS: Invalid IP address specified\n");
+	trace_nfs_mount_addr_err(addr);
 	return 0;
 }
 
@@ -378,7 +380,7 @@ static int nfs_parse_security_flavors(struct fs_context *fc,
 	char *string = param->string, *p;
 	int ret;
 
-	dfprintk(MOUNT, "NFS: parsing %s=%s option\n", param->key, param->string);
+	trace_nfs_mount_assign(param->key, string);
 
 	while ((p = strsep(&string, ":")) != NULL) {
 		if (!*p)
@@ -480,7 +482,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 	unsigned int len;
 	int ret, opt;
 
-	dfprintk(MOUNT, "NFS:   parsing nfs mount option '%s'\n", param->key);
+	trace_nfs_mount_option(param);
 
 	opt = fs_parse(fc, nfs_fs_parameters, param, &result);
 	if (opt < 0)
@@ -683,6 +685,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 			return ret;
 		break;
 	case Opt_vers:
+		trace_nfs_mount_assign(param->key, param->string);
 		ret = nfs_parse_version_string(fc, param->string);
 		if (ret < 0)
 			return ret;
@@ -694,6 +697,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		break;
 
 	case Opt_proto:
+		trace_nfs_mount_assign(param->key, param->string);
 		protofamily = AF_INET;
 		switch (lookup_constant(nfs_xprt_protocol_tokens, param->string, -1)) {
 		case Opt_xprt_udp6:
@@ -729,6 +733,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		break;
 
 	case Opt_mountproto:
+		trace_nfs_mount_assign(param->key, param->string);
 		mountfamily = AF_INET;
 		switch (lookup_constant(nfs_xprt_protocol_tokens, param->string, -1)) {
 		case Opt_xprt_udp6:
@@ -751,6 +756,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		break;
 
 	case Opt_addr:
+		trace_nfs_mount_assign(param->key, param->string);
 		len = rpc_pton(fc->net_ns, param->string, param->size,
 			       &ctx->nfs_server.address,
 			       sizeof(ctx->nfs_server._address));
@@ -759,16 +765,19 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		ctx->nfs_server.addrlen = len;
 		break;
 	case Opt_clientaddr:
+		trace_nfs_mount_assign(param->key, param->string);
 		kfree(ctx->client_address);
 		ctx->client_address = param->string;
 		param->string = NULL;
 		break;
 	case Opt_mounthost:
+		trace_nfs_mount_assign(param->key, param->string);
 		kfree(ctx->mount_server.hostname);
 		ctx->mount_server.hostname = param->string;
 		param->string = NULL;
 		break;
 	case Opt_mountaddr:
+		trace_nfs_mount_assign(param->key, param->string);
 		len = rpc_pton(fc->net_ns, param->string, param->size,
 			       &ctx->mount_server.address,
 			       sizeof(ctx->mount_server._address));
@@ -846,7 +855,6 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		 */
 	case Opt_sloppy:
 		ctx->sloppy = true;
-		dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
 		break;
 	}
 
@@ -879,10 +887,8 @@ static int nfs_parse_source(struct fs_context *fc,
 	size_t len;
 	const char *end;
 
-	if (unlikely(!dev_name || !*dev_name)) {
-		dfprintk(MOUNT, "NFS: device name not specified\n");
+	if (unlikely(!dev_name || !*dev_name))
 		return -EINVAL;
-	}
 
 	/* Is the host name protected with square brakcets? */
 	if (*dev_name == '[') {
@@ -922,7 +928,7 @@ static int nfs_parse_source(struct fs_context *fc,
 	if (!ctx->nfs_server.export_path)
 		goto out_nomem;
 
-	dfprintk(MOUNT, "NFS: MNTPATH: '%s'\n", ctx->nfs_server.export_path);
+	trace_nfs_mount_path(ctx->nfs_server.export_path);
 	return 0;
 
 out_bad_devname:
@@ -1116,7 +1122,6 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
 	return nfs_invalf(fc, "NFS: nfs_mount_data version supports only AUTH_SYS");
 
 out_nomem:
-	dfprintk(MOUNT, "NFS: not enough memory to handle mount options");
 	return -ENOMEM;
 
 out_no_address:
@@ -1248,7 +1253,7 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
 	if (IS_ERR(c))
 		return PTR_ERR(c);
 	ctx->nfs_server.export_path = c;
-	dfprintk(MOUNT, "NFS: MNTPATH: '%s'\n", c);
+	trace_nfs_mount_path(c);
 
 	c = strndup_user(data->client_addr.data, 16);
 	if (IS_ERR(c))
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 012bd7339862..ccaeae42ee77 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -1609,6 +1609,83 @@ TRACE_EVENT(nfs_fh_to_dentry,
 		)
 );
 
+TRACE_EVENT(nfs_mount_addr_err,
+	TP_PROTO(
+		const struct sockaddr *sap
+	),
+
+	TP_ARGS(sap),
+
+	TP_STRUCT__entry(
+		__array(unsigned char, addr, sizeof(struct sockaddr_in6))
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->addr, sap, sizeof(__entry->addr));
+	),
+
+	TP_printk("addr=%pISpc", __entry->addr)
+);
+
+TRACE_EVENT(nfs_mount_assign,
+	TP_PROTO(
+		const char *option,
+		const char *value
+	),
+
+	TP_ARGS(option, value),
+
+	TP_STRUCT__entry(
+		__string(option, option)
+		__string(value, value)
+	),
+
+	TP_fast_assign(
+		__assign_str(option, option);
+		__assign_str(value, value);
+	),
+
+	TP_printk("option %s=%s",
+		__get_str(option), __get_str(value)
+	)
+);
+
+TRACE_EVENT(nfs_mount_option,
+	TP_PROTO(
+		const struct fs_parameter *param
+	),
+
+	TP_ARGS(param),
+
+	TP_STRUCT__entry(
+		__string(option, param->key)
+	),
+
+	TP_fast_assign(
+		__assign_str(option, param->key);
+	),
+
+	TP_printk("option %s", __get_str(option))
+);
+
+TRACE_EVENT(nfs_mount_path,
+	TP_PROTO(
+		const char *path
+	),
+
+	TP_ARGS(path),
+
+	TP_STRUCT__entry(
+		__string(path, path)
+	),
+
+	TP_fast_assign(
+		__assign_str(path, path);
+	),
+
+	TP_printk("path='%s'", __get_str(path))
+);
+
 DECLARE_EVENT_CLASS(nfs_xdr_event,
 		TP_PROTO(
 			const struct xdr_stream *xdr,



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (12 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 13/15] NFS: Replace fs_context-related dprintk() call sites with tracepoints Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-18 16:52 ` [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option Chuck Lever
  2022-04-19  3:31 ` [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Trond Myklebust
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

The new field is used to match struct nfs_clients that have the same
TLS policy setting.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/client.c           |    6 ++++++
 fs/nfs/internal.h         |    1 +
 fs/nfs/nfs3client.c       |    1 +
 fs/nfs/nfs4client.c       |   16 +++++++++++-----
 include/linux/nfs_fs_sb.h |    7 ++++++-
 5 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index e828504cc396..0896e4f047d1 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_policy;
 	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 != data->xprtsec_policy)
+			continue;
+
 		refcount_inc(&clp->cl_count);
 		return clp;
 	}
@@ -675,6 +680,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 = NFS_CS_XPRTSEC_NONE,
 	};
 	struct nfs_client *clp;
 	int error;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 7eefa16ed381..0a3512c39376 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;
+	unsigned int xprtsec_policy;
 };
 
 /*
diff --git a/fs/nfs/nfs3client.c b/fs/nfs/nfs3client.c
index 5601e47360c2..d7c5db1f5825 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_policy = 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 47a6cf892c95..682d47e5977b 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -895,7 +895,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,
+		unsigned int xprtsec_policy)
 {
 	struct nfs_client_initdata cl_init = {
 		.hostname = hostname,
@@ -908,6 +909,7 @@ static int nfs4_set_client(struct nfs_server *server,
 		.net = net,
 		.timeparms = timeparms,
 		.cred = server->cred,
+		.xprtsec_policy = xprtsec_policy,
 	};
 	struct nfs_client *clp;
 
@@ -1156,7 +1158,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,
+				NFS_CS_XPRTSEC_NONE);
 	if (error < 0)
 		return error;
 
@@ -1246,7 +1249,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) */
@@ -1262,7 +1266,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;
 
@@ -1335,7 +1340,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 157d2bd6b241..7ec7d3b37d19 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -63,7 +63,12 @@ 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 */
+	unsigned int		cl_xprtsec;	/* xprt security policy */
+#define NFS_CS_XPRTSEC_NONE	(0)
+#define NFS_CS_XPRTSEC_AUTO	(1)
+#define NFS_CS_XPRTSEC_TLS	(2)
+#define NFS_CS_XPRTSEC_MTLS	(3)
 
 #if IS_ENABLED(CONFIG_NFS_V4)
 	struct list_head	cl_ds_clients; /* auth flavor data servers */



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (13 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field Chuck Lever
@ 2022-04-18 16:52 ` Chuck Lever
  2022-04-19  3:31 ` [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Trond Myklebust
  15 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2022-04-18 16:52 UTC (permalink / raw)
  To: netdev, linux-nfs, linux-nvme, linux-cifs, linux-fsdevel; +Cc: ak, borisp, simo

After some discussion, we decided that controlling the user authen-
tication flavor should be separate from the setting for transport
layer security policy. 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 disabled.

  xprtsec=auto     - Try to establish a TLS session, but proceed
                     with no transport layer security if that fails.

  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.

The default is xprtsec=auto.

An update to nfs(5) will be sent under separate cover.

To support client peer authentication, the plan is to add another
xprtsec= choice called "mtls" which will require a second mount
option that specifies the pathname of a directory containing the
private key and an x.509 certificate.

Similarly, pre-shared key authentication can be supported by adding
support for "xprtsec=psk" along with a second mount option that
specifies the name of a file containing the key.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/client.c     |   18 +++++++++++++++++-
 fs/nfs/fs_context.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/internal.h   |    1 +
 fs/nfs/nfs4client.c |    2 +-
 fs/nfs/super.c      |   10 ++++++++++
 5 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 0896e4f047d1..edb2cfd7262e 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -530,11 +530,27 @@ int nfs_create_rpc_client(struct nfs_client *clp,
 	if (test_bit(NFS_CS_REUSEPORT, &clp->cl_flags))
 		args.flags |= RPC_CLNT_CREATE_REUSEPORT;
 
+	switch (clp->cl_xprtsec) {
+	case NFS_CS_XPRTSEC_AUTO:
+	case NFS_CS_XPRTSEC_TLS:
+		args.xprtsec_policy = RPC_XPRTSEC_TLS;
+		break;
+	default:
+		args.xprtsec_policy = RPC_XPRTSEC_NONE;
+	}
+
 	if (!IS_ERR(clp->cl_rpcclient))
 		return 0;
 
+retry:
 	clnt = rpc_create(&args);
 	if (IS_ERR(clnt)) {
+		if (clp->cl_xprtsec == NFS_CS_XPRTSEC_AUTO &&
+		    args.xprtsec_policy == RPC_XPRTSEC_TLS) {
+			args.xprtsec_policy = RPC_XPRTSEC_NONE;
+			goto retry;
+		}
+
 		dprintk("%s: cannot create RPC client. Error = %ld\n",
 				__func__, PTR_ERR(clnt));
 		return PTR_ERR(clnt);
@@ -680,7 +696,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 = NFS_CS_XPRTSEC_NONE,
+		.xprtsec_policy = ctx->xprtsec_policy,
 	};
 	struct nfs_client *clp;
 	int error;
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index b52362735d12..bfc31ac9af65 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -88,6 +88,7 @@ enum nfs_param {
 	Opt_vers,
 	Opt_wsize,
 	Opt_write,
+	Opt_xprtsec,
 };
 
 enum {
@@ -194,6 +195,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 +269,20 @@ static const struct constant_table nfs_secflavor_tokens[] = {
 	{}
 };
 
+enum {
+	Opt_xprtsec_none,
+	Opt_xprtsec_auto,
+	Opt_xprtsec_tls,
+	nr__Opt_xprtsec
+};
+
+static const struct constant_table nfs_xprtsec_policies[] = {
+	{ "none",	Opt_xprtsec_none },
+	{ "auto",	Opt_xprtsec_auto },
+	{ "tls",	Opt_xprtsec_tls },
+	{}
+};
+
 /*
  * Sanity-check a server address provided by the mount command.
  *
@@ -431,6 +447,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 = NFS_CS_XPRTSEC_NONE;
+		break;
+	case Opt_xprtsec_auto:
+		ctx->xprtsec_policy = NFS_CS_XPRTSEC_AUTO;
+		break;
+	case Opt_xprtsec_tls:
+		ctx->xprtsec_policy = NFS_CS_XPRTSEC_TLS;
+		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)
 {
@@ -695,6 +734,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:
 		trace_nfs_mount_assign(param->key, param->string);
@@ -1564,6 +1608,7 @@ 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	= NFS_CS_XPRTSEC_AUTO;
 
 		fc->s_iflags		|= SB_I_STABLE_WRITES;
 	}
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 0a3512c39376..bc60a556ad92 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;
+	unsigned int		xprtsec_policy;
 	char			*client_address;
 	unsigned int		version;
 	unsigned int		minorversion;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 682d47e5977b..8dbdb00859fe 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -1159,7 +1159,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,
-				NFS_CS_XPRTSEC_NONE);
+				ctx->xprtsec_policy);
 	if (error < 0)
 		return error;
 
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 6ab5eeb000dc..0c2371bbf21d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -491,6 +491,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) {
+	case NFS_CS_XPRTSEC_AUTO:
+		seq_printf(m, ",xprtsec=auto");
+		break;
+	case NFS_CS_XPRTSEC_TLS:
+		seq_printf(m, ",xprtsec=tls");
+		break;
+	default:
+		break;
+	}
 
 	if (version != 4)
 		nfs_show_mountd_options(m, nfss, showdefaults);



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-18 16:52 ` [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag Chuck Lever
@ 2022-04-19  2:57   ` Trond Myklebust
  2022-04-19 18:16     ` Chuck Lever III
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2022-04-19  2:57 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel, netdev, linux-nvme, chuck.lever, linux-cifs
  Cc: borisp, simo, ak

On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
> Introduce a mechanism to cause xprt_transmit() to break out of its
> sending loop at a specific rpc_rqst, rather than draining the whole
> transmit queue.
> 
> This enables the client to send just an RPC TLS probe and then wait
> for the response before proceeding with the rest of the queue.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  include/linux/sunrpc/sched.h  |    2 ++
>  include/trace/events/sunrpc.h |    1 +
>  net/sunrpc/xprt.c             |    2 ++
>  3 files changed, 5 insertions(+)
> 
> diff --git a/include/linux/sunrpc/sched.h
> b/include/linux/sunrpc/sched.h
> index 599133fb3c63..f8c09638fa69 100644
> --- a/include/linux/sunrpc/sched.h
> +++ b/include/linux/sunrpc/sched.h
> @@ -125,6 +125,7 @@ struct rpc_task_setup {
>  #define RPC_TASK_TLSCRED               0x00000008      /* Use
> AUTH_TLS credential */
>  #define RPC_TASK_NULLCREDS             0x00000010      /* Use
> AUTH_NULL credential */
>  #define RPC_CALL_MAJORSEEN             0x00000020      /* major
> timeout seen */
> +#define RPC_TASK_CORK                  0x00000040      /* cork the
> xmit queue */
>  #define RPC_TASK_DYNAMIC               0x00000080      /* task was
> kmalloc'ed */
>  #define        RPC_TASK_NO_ROUND_ROBIN         0x00000100      /*
> send requests on "main" xprt */
>  #define RPC_TASK_SOFT                  0x00000200      /* Use soft
> timeouts */
> @@ -137,6 +138,7 @@ struct rpc_task_setup {
>  
>  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
> RPC_TASK_ASYNC)
>  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags & RPC_TASK_SWAPPER)
> +#define RPC_IS_CORK(t)         ((t)->tk_flags & RPC_TASK_CORK)
>  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
> (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
>  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags & RPC_TASK_SOFTCONN)
>  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
> RPC_TASK_SENT)
> diff --git a/include/trace/events/sunrpc.h
> b/include/trace/events/sunrpc.h
> index 811187c47ebb..e8d6adff1a50 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
>                 { RPC_TASK_TLSCRED, "TLSCRED"
> },                        \
>                 { RPC_TASK_NULLCREDS, "NULLCREDS"
> },                    \
>                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
> },                    \
> +               { RPC_TASK_CORK, "CORK"
> },                              \
>                 { RPC_TASK_DYNAMIC, "DYNAMIC"
> },                        \
>                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
> },          \
>                 { RPC_TASK_SOFT, "SOFT"
> },                              \
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 86d62cffba0d..4b303b945b51 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
>                 if (xprt_request_data_received(task) &&
>                     !test_bit(RPC_TASK_NEED_XMIT, &task-
> >tk_runstate))
>                         break;
> +               if (RPC_IS_CORK(task))
> +                       break;
>                 cond_resched_lock(&xprt->queue_lock);
>         }
>         spin_unlock(&xprt->queue_lock);
> 
> 

This is entirely the wrong place for this kind of control mechanism.

TLS vs not-TLS needs to be decided up front when we initialise the
transport (i.e. at mount time or whenever the pNFS channels are set
up). Otherwise, we're vulnerable to downgrade attacks.

Once we've decided that TLS is the right thing to do, then we shouldn't
declare to the RPC layer that the TLS-enabled transport is connected
until the underlying transport connection is established, and the TLS
handshake is done.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
  2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
                   ` (14 preceding siblings ...)
  2022-04-18 16:52 ` [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option Chuck Lever
@ 2022-04-19  3:31 ` Trond Myklebust
  2022-04-19 16:00   ` Chuck Lever III
  15 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2022-04-19  3:31 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel, netdev, linux-nvme, chuck.lever, linux-cifs
  Cc: borisp, simo, ak

On Mon, 2022-04-18 at 12:51 -0400, Chuck Lever wrote:
> This series implements RPC-with-TLS in the Linux kernel:
> 
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpc-tls/
> 
> This prototype is based on the previously posted mechanism for
> providing a TLS handshake facility to in-kernel TLS consumers.
> 
> For the purpose of demonstration, the Linux NFS client is modified
> to add a new mount option: xprtsec = [ none|auto|tls ] . Updates
> to the nfs(5) man page are being developed separately.
> 

I'm fine with having a userspace level 'auto' option if that's a
requirement for someone, however I see no reason why we would need to
implement that in the kernel.

Let's just have a robust mechanism for immediately returning an error
if the user supplies a 'tls' option on the client that the server
doesn't support, and let the negotiation policy be worked out in
userspace by the 'mount.nfs' utility. Otherwise we'll rathole into
another twisty maze of policy decisions that generate kernel level CVEs
instead of a set of more gentle fixes.

> The new mount option enables client administrators to require in-
> transit encryption for their NFS traffic, protecting the weak
> security of AUTH_SYS. An x.509 certificate is not required on the
> client for this protection.

That doesn't really do much to 'protect the weak security of AUTH_SYS'.
It just means that nobody can tamper with our AUTH_SYS credential while
in flight. It is still quite possible for the client to spoof both its
IP address and user/group credentials.

A better recommendation would be to have users select sys=krb5 when
they have the ability to do so. Doing so ensures that both the client
and server are authenticating to one another, while also guaranteeing
RPC message integrity and privacy.

> This prototype has been tested against prototype TLS-capable NFS
> servers. The Linux NFS server itself does not yet have support for
> RPC-with-TLS, but it is planned.
> 
> At a later time, the Linux NFS client will also get support for
> x.509 authentication (for which a certificate will be required on
> the client) and PSK. For this demonstration, only authentication-
> less TLS (encryption-only) is supported.
> 
> ---
> 
> Chuck Lever (15):
>       SUNRPC: Replace dprintk() call site in xs_data_ready
>       SUNRPC: Ignore data_ready callbacks during TLS handshakes
>       SUNRPC: Capture cmsg metadata on client-side receive
>       SUNRPC: Fail faster on bad verifier
>       SUNRPC: Widen rpc_task::tk_flags
>       SUNRPC: Add RPC client support for the RPC_AUTH_TLS
> authentication flavor
>       SUNRPC: Refactor rpc_call_null_helper()
>       SUNRPC: Add RPC_TASK_CORK flag
>       SUNRPC: Add a cl_xprtsec_policy field
>       SUNRPC: Expose TLS policy via the rpc_create() API
>       SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe
>       SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect
>       NFS: Replace fs_context-related dprintk() call sites with
> tracepoints
>       NFS: Have struct nfs_client carry a TLS policy field
>       NFS: Add an "xprtsec=" NFS mount option
> 
> 
>  fs/nfs/client.c                 |  22 ++++
>  fs/nfs/fs_context.c             |  70 ++++++++--
>  fs/nfs/internal.h               |   2 +
>  fs/nfs/nfs3client.c             |   1 +
>  fs/nfs/nfs4client.c             |  16 ++-
>  fs/nfs/nfstrace.h               |  77 +++++++++++
>  fs/nfs/super.c                  |  10 ++
>  include/linux/nfs_fs_sb.h       |   7 +-
>  include/linux/sunrpc/auth.h     |   1 +
>  include/linux/sunrpc/clnt.h     |  14 +-
>  include/linux/sunrpc/sched.h    |  36 +++---
>  include/linux/sunrpc/xprt.h     |  14 ++
>  include/linux/sunrpc/xprtsock.h |   2 +
>  include/net/tls.h               |   2 +
>  include/trace/events/sunrpc.h   | 157 ++++++++++++++++++++--
>  net/sunrpc/Makefile             |   2 +-
>  net/sunrpc/auth.c               |   2 +
>  net/sunrpc/auth_tls.c           | 117 +++++++++++++++++
>  net/sunrpc/clnt.c               | 222 +++++++++++++++++++++++++++++-
> --
>  net/sunrpc/debugfs.c            |   2 +-
>  net/sunrpc/xprt.c               |   3 +
>  net/sunrpc/xprtsock.c           | 211 +++++++++++++++++++++++++++++-
>  22 files changed, 920 insertions(+), 70 deletions(-)
>  create mode 100644 net/sunrpc/auth_tls.c
> 
> --
> Chuck Lever
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
  2022-04-19  3:31 ` [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Trond Myklebust
@ 2022-04-19 16:00   ` Chuck Lever III
  2022-04-19 18:48     ` Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Chuck Lever III @ 2022-04-19 16:00 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: Linux NFS Mailing List, linux-fsdevel, netdev, linux-nvme,
	linux-cifs, borisp, simo, ak

Hi Trond-

Thanks for the early review!


> On Apr 18, 2022, at 11:31 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Mon, 2022-04-18 at 12:51 -0400, Chuck Lever wrote:
>> This series implements RPC-with-TLS in the Linux kernel:
>> 
>> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpc-tls/
>> 
>> This prototype is based on the previously posted mechanism for
>> providing a TLS handshake facility to in-kernel TLS consumers.
>> 
>> For the purpose of demonstration, the Linux NFS client is modified
>> to add a new mount option: xprtsec = [ none|auto|tls ] . Updates
>> to the nfs(5) man page are being developed separately.
>> 
> 
> I'm fine with having a userspace level 'auto' option if that's a
> requirement for someone, however I see no reason why we would need to
> implement that in the kernel.
> 
> Let's just have a robust mechanism for immediately returning an error
> if the user supplies a 'tls' option on the client that the server
> doesn't support, and let the negotiation policy be worked out in
> userspace by the 'mount.nfs' utility. Otherwise we'll rathole into
> another twisty maze of policy decisions that generate kernel level CVEs
> instead of a set of more gentle fixes.

Noted.

However, one of Rick's preferences is that "auto" not use
transport-layer security unless the server requires it via
a SECINFO/MNT pseudoflavor, which only the kernel would be
privy to. I'll have to think about whether we want to make
that happen.


>> The new mount option enables client administrators to require in-
>> transit encryption for their NFS traffic, protecting the weak
>> security of AUTH_SYS. An x.509 certificate is not required on the
>> client for this protection.
> 
> That doesn't really do much to 'protect the weak security of AUTH_SYS'.

My description doesn't really explain the whole plan, it
introduces only what's in the current prototype. Eventually
I'd like to do this:

  xprtsec= none | auto | tls | mtls | psk | ...

where
  none: transport-layer security is explicitly disabled
  auto: pick one based on what authentication material is available
  tls: encryption-only TLSv1.3 (no client cert needed)
  mtls: encryption and mutual authentication (client cert required)
  psk: pre-shared key
  ...: we could require wiregard, EAP, or IPSEC if someone cares
       to implement one or more of them


> It just means that nobody can tamper with our AUTH_SYS credential while
> in flight. It is still quite possible for the client to spoof both its
> IP address and user/group credentials.

True enough. But some folks are interested only in encryption.
They trust their clients, but not the network.


> A better recommendation would be to have users select sys=krb5 when
> they have the ability to do so. Doing so ensures that both the client
> and server are authenticating to one another, while also guaranteeing
> RPC message integrity and privacy.

With xprtsec=mtls (see above), the server and client mutually
authenticate, which provides a higher degree of security as you
describe here.

I agree that xprtsec=tls + sec=krb5 is probably the ultimate
combination of security with the least performance compromise.
The prototype posted here should support this combination right
now.


>> This prototype has been tested against prototype TLS-capable NFS
>> servers. The Linux NFS server itself does not yet have support for
>> RPC-with-TLS, but it is planned.
>> 
>> At a later time, the Linux NFS client will also get support for
>> x.509 authentication (for which a certificate will be required on
>> the client) and PSK. For this demonstration, only authentication-
>> less TLS (encryption-only) is supported.
>> 
>> ---
>> 
>> Chuck Lever (15):
>>       SUNRPC: Replace dprintk() call site in xs_data_ready
>>       SUNRPC: Ignore data_ready callbacks during TLS handshakes
>>       SUNRPC: Capture cmsg metadata on client-side receive
>>       SUNRPC: Fail faster on bad verifier
>>       SUNRPC: Widen rpc_task::tk_flags
>>       SUNRPC: Add RPC client support for the RPC_AUTH_TLS
>> authentication flavor
>>       SUNRPC: Refactor rpc_call_null_helper()
>>       SUNRPC: Add RPC_TASK_CORK flag
>>       SUNRPC: Add a cl_xprtsec_policy field
>>       SUNRPC: Expose TLS policy via the rpc_create() API
>>       SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe
>>       SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect
>>       NFS: Replace fs_context-related dprintk() call sites with
>> tracepoints
>>       NFS: Have struct nfs_client carry a TLS policy field
>>       NFS: Add an "xprtsec=" NFS mount option
>> 
>> 
>>  fs/nfs/client.c                 |  22 ++++
>>  fs/nfs/fs_context.c             |  70 ++++++++--
>>  fs/nfs/internal.h               |   2 +
>>  fs/nfs/nfs3client.c             |   1 +
>>  fs/nfs/nfs4client.c             |  16 ++-
>>  fs/nfs/nfstrace.h               |  77 +++++++++++
>>  fs/nfs/super.c                  |  10 ++
>>  include/linux/nfs_fs_sb.h       |   7 +-
>>  include/linux/sunrpc/auth.h     |   1 +
>>  include/linux/sunrpc/clnt.h     |  14 +-
>>  include/linux/sunrpc/sched.h    |  36 +++---
>>  include/linux/sunrpc/xprt.h     |  14 ++
>>  include/linux/sunrpc/xprtsock.h |   2 +
>>  include/net/tls.h               |   2 +
>>  include/trace/events/sunrpc.h   | 157 ++++++++++++++++++++--
>>  net/sunrpc/Makefile             |   2 +-
>>  net/sunrpc/auth.c               |   2 +
>>  net/sunrpc/auth_tls.c           | 117 +++++++++++++++++
>>  net/sunrpc/clnt.c               | 222 +++++++++++++++++++++++++++++-
>> --
>>  net/sunrpc/debugfs.c            |   2 +-
>>  net/sunrpc/xprt.c               |   3 +
>>  net/sunrpc/xprtsock.c           | 211 +++++++++++++++++++++++++++++-
>>  22 files changed, 920 insertions(+), 70 deletions(-)
>>  create mode 100644 net/sunrpc/auth_tls.c
>> 
>> --
>> Chuck Lever
>> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

--
Chuck Lever




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-19  2:57   ` Trond Myklebust
@ 2022-04-19 18:16     ` Chuck Lever III
  2022-04-19 19:04       ` Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Chuck Lever III @ 2022-04-19 18:16 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: Linux NFS Mailing List, linux-fsdevel, netdev, linux-nvme,
	linux-cifs, borisp, simo, ak



> On Apr 18, 2022, at 10:57 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
>> Introduce a mechanism to cause xprt_transmit() to break out of its
>> sending loop at a specific rpc_rqst, rather than draining the whole
>> transmit queue.
>> 
>> This enables the client to send just an RPC TLS probe and then wait
>> for the response before proceeding with the rest of the queue.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  include/linux/sunrpc/sched.h  |    2 ++
>>  include/trace/events/sunrpc.h |    1 +
>>  net/sunrpc/xprt.c             |    2 ++
>>  3 files changed, 5 insertions(+)
>> 
>> diff --git a/include/linux/sunrpc/sched.h
>> b/include/linux/sunrpc/sched.h
>> index 599133fb3c63..f8c09638fa69 100644
>> --- a/include/linux/sunrpc/sched.h
>> +++ b/include/linux/sunrpc/sched.h
>> @@ -125,6 +125,7 @@ struct rpc_task_setup {
>>  #define RPC_TASK_TLSCRED               0x00000008      /* Use
>> AUTH_TLS credential */
>>  #define RPC_TASK_NULLCREDS             0x00000010      /* Use
>> AUTH_NULL credential */
>>  #define RPC_CALL_MAJORSEEN             0x00000020      /* major
>> timeout seen */
>> +#define RPC_TASK_CORK                  0x00000040      /* cork the
>> xmit queue */
>>  #define RPC_TASK_DYNAMIC               0x00000080      /* task was
>> kmalloc'ed */
>>  #define        RPC_TASK_NO_ROUND_ROBIN         0x00000100      /*
>> send requests on "main" xprt */
>>  #define RPC_TASK_SOFT                  0x00000200      /* Use soft
>> timeouts */
>> @@ -137,6 +138,7 @@ struct rpc_task_setup {
>>  
>>  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
>> RPC_TASK_ASYNC)
>>  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags & RPC_TASK_SWAPPER)
>> +#define RPC_IS_CORK(t)         ((t)->tk_flags & RPC_TASK_CORK)
>>  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
>> (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
>>  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags & RPC_TASK_SOFTCONN)
>>  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
>> RPC_TASK_SENT)
>> diff --git a/include/trace/events/sunrpc.h
>> b/include/trace/events/sunrpc.h
>> index 811187c47ebb..e8d6adff1a50 100644
>> --- a/include/trace/events/sunrpc.h
>> +++ b/include/trace/events/sunrpc.h
>> @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
>>                 { RPC_TASK_TLSCRED, "TLSCRED"
>> },                        \
>>                 { RPC_TASK_NULLCREDS, "NULLCREDS"
>> },                    \
>>                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
>> },                    \
>> +               { RPC_TASK_CORK, "CORK"
>> },                              \
>>                 { RPC_TASK_DYNAMIC, "DYNAMIC"
>> },                        \
>>                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
>> },          \
>>                 { RPC_TASK_SOFT, "SOFT"
>> },                              \
>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
>> index 86d62cffba0d..4b303b945b51 100644
>> --- a/net/sunrpc/xprt.c
>> +++ b/net/sunrpc/xprt.c
>> @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
>>                 if (xprt_request_data_received(task) &&
>>                     !test_bit(RPC_TASK_NEED_XMIT, &task-
>>> tk_runstate))
>>                         break;
>> +               if (RPC_IS_CORK(task))
>> +                       break;
>>                 cond_resched_lock(&xprt->queue_lock);
>>         }
>>         spin_unlock(&xprt->queue_lock);
>> 
>> 
> 
> This is entirely the wrong place for this kind of control mechanism.

I'm not sure I entirely understand your concern, so bear with
me while I try to clarify.


> TLS vs not-TLS needs to be decided up front when we initialise the
> transport (i.e. at mount time or whenever the pNFS channels are set
> up). Otherwise, we're vulnerable to downgrade attacks.

Downgrade attacks are prevented by using "xprtsec=tls" because
in that case, transport creation fails if either the AUTH_TLS
fails or the handshake fails.

The TCP connection has to be established first, though. Then the
client can send the RPC_AUTH_TLS probe, which is the same as the
NULL ping that it already sends. That mechanism is independent
of the lower layer transport (TCP in this case).

Therefore, RPC traffic must be stoppered while the client:

1. waits for the AUTH_TLS probe's reply, and

2. waits for the handshake to complete

Because an RPC message is involved in this interaction, I didn't
see a way to implement it completely within xprtsock's TCP
connection logic. IMO, driving the handshake has to be done by
the generic RPC client.

So, do you mean that I need to replace RPC_TASK_CORK with a
special return code from xs_tcp_send_request() ?


> Once we've decided that TLS is the right thing to do, then we shouldn't
> declare to the RPC layer that the TLS-enabled transport is connected
> until the underlying transport connection is established, and the TLS
> handshake is done.

That logic is handled in patch 10/15.

Reconnecting and re-establishing a TLS session is handled in
patches 11/15 and 12/15. Again, if the transport's policy setting
is "must use TLS" then the client ensures that a TLS session is in
use before allowing more RPC traffic on the new connection.


--
Chuck Lever




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
  2022-04-19 16:00   ` Chuck Lever III
@ 2022-04-19 18:48     ` Trond Myklebust
  2022-04-19 18:53       ` Chuck Lever III
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2022-04-19 18:48 UTC (permalink / raw)
  To: chuck.lever
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak,
	linux-nfs, borisp

On Tue, 2022-04-19 at 16:00 +0000, Chuck Lever III wrote:
> Hi Trond-
> 
> Thanks for the early review!
> 
> 
> > On Apr 18, 2022, at 11:31 PM, Trond Myklebust
> > <trondmy@hammerspace.com> wrote:
> > 
> > On Mon, 2022-04-18 at 12:51 -0400, Chuck Lever wrote:
> > > This series implements RPC-with-TLS in the Linux kernel:
> > > 
> > > https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpc-tls/
> > > 
> > > This prototype is based on the previously posted mechanism for
> > > providing a TLS handshake facility to in-kernel TLS consumers.
> > > 
> > > For the purpose of demonstration, the Linux NFS client is
> > > modified
> > > to add a new mount option: xprtsec = [ none|auto|tls ] . Updates
> > > to the nfs(5) man page are being developed separately.
> > > 
> > 
> > I'm fine with having a userspace level 'auto' option if that's a
> > requirement for someone, however I see no reason why we would need
> > to
> > implement that in the kernel.
> > 
> > Let's just have a robust mechanism for immediately returning an
> > error
> > if the user supplies a 'tls' option on the client that the server
> > doesn't support, and let the negotiation policy be worked out in
> > userspace by the 'mount.nfs' utility. Otherwise we'll rathole into
> > another twisty maze of policy decisions that generate kernel level
> > CVEs
> > instead of a set of more gentle fixes.
> 
> Noted.
> 
> However, one of Rick's preferences is that "auto" not use
> transport-layer security unless the server requires it via
> a SECINFO/MNT pseudoflavor, which only the kernel would be
> privy to. I'll have to think about whether we want to make
> that happen.

That sounds like a terrible protocol hack. TLS is not an authentication
flavour but a transport level protection.

That said, I don't see how this invalidates my argument. When told to
use TLS, the kernel client can still return a mount time error if the
server fails to advertise support through this pseudoflavour and leave
it up to userspace to decide how to deal with that.

> > 
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
  2022-04-19 18:48     ` Trond Myklebust
@ 2022-04-19 18:53       ` Chuck Lever III
  2022-04-19 20:49         ` Rick Macklem
  0 siblings, 1 reply; 27+ messages in thread
From: Chuck Lever III @ 2022-04-19 18:53 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak,
	Linux NFS Mailing List, borisp



> On Apr 19, 2022, at 2:48 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2022-04-19 at 16:00 +0000, Chuck Lever III wrote:
>> Hi Trond-
>> 
>> Thanks for the early review!
>> 
>> 
>>> On Apr 18, 2022, at 11:31 PM, Trond Myklebust
>>> <trondmy@hammerspace.com> wrote:
>>> 
>>> On Mon, 2022-04-18 at 12:51 -0400, Chuck Lever wrote:
>>>> This series implements RPC-with-TLS in the Linux kernel:
>>>> 
>>>> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpc-tls/
>>>> 
>>>> This prototype is based on the previously posted mechanism for
>>>> providing a TLS handshake facility to in-kernel TLS consumers.
>>>> 
>>>> For the purpose of demonstration, the Linux NFS client is
>>>> modified
>>>> to add a new mount option: xprtsec = [ none|auto|tls ] . Updates
>>>> to the nfs(5) man page are being developed separately.
>>>> 
>>> 
>>> I'm fine with having a userspace level 'auto' option if that's a
>>> requirement for someone, however I see no reason why we would need
>>> to
>>> implement that in the kernel.
>>> 
>>> Let's just have a robust mechanism for immediately returning an
>>> error
>>> if the user supplies a 'tls' option on the client that the server
>>> doesn't support, and let the negotiation policy be worked out in
>>> userspace by the 'mount.nfs' utility. Otherwise we'll rathole into
>>> another twisty maze of policy decisions that generate kernel level
>>> CVEs
>>> instead of a set of more gentle fixes.
>> 
>> Noted.
>> 
>> However, one of Rick's preferences is that "auto" not use
>> transport-layer security unless the server requires it via
>> a SECINFO/MNT pseudoflavor, which only the kernel would be
>> privy to. I'll have to think about whether we want to make
>> that happen.
> 
> That sounds like a terrible protocol hack. TLS is not an authentication
> flavour but a transport level protection.

Fair enough. We've been discussing this on nfsv4@ietf.org, and
it's certainly not written in stone yet.

I invite you to join the conversation and share your concerns
(and possibly any alternative solutions you might have).


> That said, I don't see how this invalidates my argument. When told to
> use TLS, the kernel client can still return a mount time error if the
> server fails to advertise support through this pseudoflavour and leave
> it up to userspace to decide how to deal with that.

Sure. I'm just saying I haven't thought it through yet. I don't
think it will be a problem to move more (or all) of the transport
security policy to mount.nfs.


--
Chuck Lever




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-19 18:16     ` Chuck Lever III
@ 2022-04-19 19:04       ` Trond Myklebust
  2022-04-19 19:40         ` Chuck Lever III
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2022-04-19 19:04 UTC (permalink / raw)
  To: chuck.lever
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak,
	linux-nfs, borisp

On Tue, 2022-04-19 at 18:16 +0000, Chuck Lever III wrote:
> 
> 
> > On Apr 18, 2022, at 10:57 PM, Trond Myklebust
> > <trondmy@hammerspace.com> wrote:
> > 
> > On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
> > > Introduce a mechanism to cause xprt_transmit() to break out of
> > > its
> > > sending loop at a specific rpc_rqst, rather than draining the
> > > whole
> > > transmit queue.
> > > 
> > > This enables the client to send just an RPC TLS probe and then
> > > wait
> > > for the response before proceeding with the rest of the queue.
> > > 
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > >  include/linux/sunrpc/sched.h  |    2 ++
> > >  include/trace/events/sunrpc.h |    1 +
> > >  net/sunrpc/xprt.c             |    2 ++
> > >  3 files changed, 5 insertions(+)
> > > 
> > > diff --git a/include/linux/sunrpc/sched.h
> > > b/include/linux/sunrpc/sched.h
> > > index 599133fb3c63..f8c09638fa69 100644
> > > --- a/include/linux/sunrpc/sched.h
> > > +++ b/include/linux/sunrpc/sched.h
> > > @@ -125,6 +125,7 @@ struct rpc_task_setup {
> > >  #define RPC_TASK_TLSCRED               0x00000008      /* Use
> > > AUTH_TLS credential */
> > >  #define RPC_TASK_NULLCREDS             0x00000010      /* Use
> > > AUTH_NULL credential */
> > >  #define RPC_CALL_MAJORSEEN             0x00000020      /* major
> > > timeout seen */
> > > +#define RPC_TASK_CORK                  0x00000040      /* cork
> > > the
> > > xmit queue */
> > >  #define RPC_TASK_DYNAMIC               0x00000080      /* task
> > > was
> > > kmalloc'ed */
> > >  #define        RPC_TASK_NO_ROUND_ROBIN         0x00000100     
> > > /*
> > > send requests on "main" xprt */
> > >  #define RPC_TASK_SOFT                  0x00000200      /* Use
> > > soft
> > > timeouts */
> > > @@ -137,6 +138,7 @@ struct rpc_task_setup {
> > >  
> > >  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
> > > RPC_TASK_ASYNC)
> > >  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags &
> > > RPC_TASK_SWAPPER)
> > > +#define RPC_IS_CORK(t)         ((t)->tk_flags & RPC_TASK_CORK)
> > >  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
> > > (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
> > >  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags &
> > > RPC_TASK_SOFTCONN)
> > >  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
> > > RPC_TASK_SENT)
> > > diff --git a/include/trace/events/sunrpc.h
> > > b/include/trace/events/sunrpc.h
> > > index 811187c47ebb..e8d6adff1a50 100644
> > > --- a/include/trace/events/sunrpc.h
> > > +++ b/include/trace/events/sunrpc.h
> > > @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
> > >                 { RPC_TASK_TLSCRED, "TLSCRED"
> > > },                        \
> > >                 { RPC_TASK_NULLCREDS, "NULLCREDS"
> > > },                    \
> > >                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
> > > },                    \
> > > +               { RPC_TASK_CORK, "CORK"
> > > },                              \
> > >                 { RPC_TASK_DYNAMIC, "DYNAMIC"
> > > },                        \
> > >                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
> > > },          \
> > >                 { RPC_TASK_SOFT, "SOFT"
> > > },                              \
> > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > > index 86d62cffba0d..4b303b945b51 100644
> > > --- a/net/sunrpc/xprt.c
> > > +++ b/net/sunrpc/xprt.c
> > > @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
> > >                 if (xprt_request_data_received(task) &&
> > >                     !test_bit(RPC_TASK_NEED_XMIT, &task-
> > > > tk_runstate))
> > >                         break;
> > > +               if (RPC_IS_CORK(task))
> > > +                       break;
> > >                 cond_resched_lock(&xprt->queue_lock);
> > >         }
> > >         spin_unlock(&xprt->queue_lock);
> > > 
> > > 
> > 
> > This is entirely the wrong place for this kind of control
> > mechanism.
> 
> I'm not sure I entirely understand your concern, so bear with
> me while I try to clarify.
> 
> 
> > TLS vs not-TLS needs to be decided up front when we initialise the
> > transport (i.e. at mount time or whenever the pNFS channels are set
> > up). Otherwise, we're vulnerable to downgrade attacks.
> 
> Downgrade attacks are prevented by using "xprtsec=tls" because
> in that case, transport creation fails if either the AUTH_TLS
> fails or the handshake fails.
> 
> The TCP connection has to be established first, though. Then the
> client can send the RPC_AUTH_TLS probe, which is the same as the
> NULL ping that it already sends. That mechanism is independent
> of the lower layer transport (TCP in this case).
> 
> Therefore, RPC traffic must be stoppered while the client:
> 
> 1. waits for the AUTH_TLS probe's reply, and
> 
> 2. waits for the handshake to complete
> 
> Because an RPC message is involved in this interaction, I didn't
> see a way to implement it completely within xprtsock's TCP
> connection logic. IMO, driving the handshake has to be done by
> the generic RPC client.
> 
> So, do you mean that I need to replace RPC_TASK_CORK with a
> special return code from xs_tcp_send_request() ?


I mean the right mechanism for controlling whether or not the transport
is ready to serve RPC requests is through the XPRT_CONNECTED flag. All
the existing generic RPC error handling, congestion handling, etc
depends on that flag being set correctly.

Until the TLS socket has completed its handshake protocol and is ready
to transmit data, it should not be declared connected. The distinction
between the two states 'TCP is unconnected' and 'TLS handshake is
incomplete' is a socket/transport setup detail as far as the RPC xprt
layer is concerned: just another set of intermediate states between
SYN_SENT and ESTABLISHED.

> > Once we've decided that TLS is the right thing to do, then we
> > shouldn't
> > declare to the RPC layer that the TLS-enabled transport is
> > connected
> > until the underlying transport connection is established, and the
> > TLS
> > handshake is done.
> 
> That logic is handled in patch 10/15.
> 
> Reconnecting and re-establishing a TLS session is handled in
> patches 11/15 and 12/15. Again, if the transport's policy setting
> is "must use TLS" then the client ensures that a TLS session is in
> use before allowing more RPC traffic on the new connection.
> 
> 
> --
> Chuck Lever
> 
> 
> 

-- 
Trond Myklebust
CTO, Hammerspace Inc
4984 El Camino Real, Suite 208
Los Altos, CA 94022
​
www.hammer.space


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-19 19:04       ` Trond Myklebust
@ 2022-04-19 19:40         ` Chuck Lever III
  2022-04-19 22:08           ` Trond Myklebust
  0 siblings, 1 reply; 27+ messages in thread
From: Chuck Lever III @ 2022-04-19 19:40 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak,
	Linux NFS Mailing List, borisp



> On Apr 19, 2022, at 3:04 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2022-04-19 at 18:16 +0000, Chuck Lever III wrote:
>> 
>> 
>>> On Apr 18, 2022, at 10:57 PM, Trond Myklebust
>>> <trondmy@hammerspace.com> wrote:
>>> 
>>> On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
>>>> Introduce a mechanism to cause xprt_transmit() to break out of
>>>> its
>>>> sending loop at a specific rpc_rqst, rather than draining the
>>>> whole
>>>> transmit queue.
>>>> 
>>>> This enables the client to send just an RPC TLS probe and then
>>>> wait
>>>> for the response before proceeding with the rest of the queue.
>>>> 
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>>  include/linux/sunrpc/sched.h  |    2 ++
>>>>  include/trace/events/sunrpc.h |    1 +
>>>>  net/sunrpc/xprt.c             |    2 ++
>>>>  3 files changed, 5 insertions(+)
>>>> 
>>>> diff --git a/include/linux/sunrpc/sched.h
>>>> b/include/linux/sunrpc/sched.h
>>>> index 599133fb3c63..f8c09638fa69 100644
>>>> --- a/include/linux/sunrpc/sched.h
>>>> +++ b/include/linux/sunrpc/sched.h
>>>> @@ -125,6 +125,7 @@ struct rpc_task_setup {
>>>>  #define RPC_TASK_TLSCRED               0x00000008      /* Use
>>>> AUTH_TLS credential */
>>>>  #define RPC_TASK_NULLCREDS             0x00000010      /* Use
>>>> AUTH_NULL credential */
>>>>  #define RPC_CALL_MAJORSEEN             0x00000020      /* major
>>>> timeout seen */
>>>> +#define RPC_TASK_CORK                  0x00000040      /* cork
>>>> the
>>>> xmit queue */
>>>>  #define RPC_TASK_DYNAMIC               0x00000080      /* task
>>>> was
>>>> kmalloc'ed */
>>>>  #define        RPC_TASK_NO_ROUND_ROBIN         0x00000100     
>>>> /*
>>>> send requests on "main" xprt */
>>>>  #define RPC_TASK_SOFT                  0x00000200      /* Use
>>>> soft
>>>> timeouts */
>>>> @@ -137,6 +138,7 @@ struct rpc_task_setup {
>>>>  
>>>>  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
>>>> RPC_TASK_ASYNC)
>>>>  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags &
>>>> RPC_TASK_SWAPPER)
>>>> +#define RPC_IS_CORK(t)         ((t)->tk_flags & RPC_TASK_CORK)
>>>>  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
>>>> (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
>>>>  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags &
>>>> RPC_TASK_SOFTCONN)
>>>>  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
>>>> RPC_TASK_SENT)
>>>> diff --git a/include/trace/events/sunrpc.h
>>>> b/include/trace/events/sunrpc.h
>>>> index 811187c47ebb..e8d6adff1a50 100644
>>>> --- a/include/trace/events/sunrpc.h
>>>> +++ b/include/trace/events/sunrpc.h
>>>> @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
>>>>                 { RPC_TASK_TLSCRED, "TLSCRED"
>>>> },                        \
>>>>                 { RPC_TASK_NULLCREDS, "NULLCREDS"
>>>> },                    \
>>>>                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
>>>> },                    \
>>>> +               { RPC_TASK_CORK, "CORK"
>>>> },                              \
>>>>                 { RPC_TASK_DYNAMIC, "DYNAMIC"
>>>> },                        \
>>>>                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
>>>> },          \
>>>>                 { RPC_TASK_SOFT, "SOFT"
>>>> },                              \
>>>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
>>>> index 86d62cffba0d..4b303b945b51 100644
>>>> --- a/net/sunrpc/xprt.c
>>>> +++ b/net/sunrpc/xprt.c
>>>> @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
>>>>                 if (xprt_request_data_received(task) &&
>>>>                     !test_bit(RPC_TASK_NEED_XMIT, &task-
>>>>> tk_runstate))
>>>>                         break;
>>>> +               if (RPC_IS_CORK(task))
>>>> +                       break;
>>>>                 cond_resched_lock(&xprt->queue_lock);
>>>>         }
>>>>         spin_unlock(&xprt->queue_lock);
>>>> 
>>>> 
>>> 
>>> This is entirely the wrong place for this kind of control
>>> mechanism.
>> 
>> I'm not sure I entirely understand your concern, so bear with
>> me while I try to clarify.
>> 
>> 
>>> TLS vs not-TLS needs to be decided up front when we initialise the
>>> transport (i.e. at mount time or whenever the pNFS channels are set
>>> up). Otherwise, we're vulnerable to downgrade attacks.
>> 
>> Downgrade attacks are prevented by using "xprtsec=tls" because
>> in that case, transport creation fails if either the AUTH_TLS
>> fails or the handshake fails.
>> 
>> The TCP connection has to be established first, though. Then the
>> client can send the RPC_AUTH_TLS probe, which is the same as the
>> NULL ping that it already sends. That mechanism is independent
>> of the lower layer transport (TCP in this case).
>> 
>> Therefore, RPC traffic must be stoppered while the client:
>> 
>> 1. waits for the AUTH_TLS probe's reply, and
>> 
>> 2. waits for the handshake to complete
>> 
>> Because an RPC message is involved in this interaction, I didn't
>> see a way to implement it completely within xprtsock's TCP
>> connection logic. IMO, driving the handshake has to be done by
>> the generic RPC client.
>> 
>> So, do you mean that I need to replace RPC_TASK_CORK with a
>> special return code from xs_tcp_send_request() ?
> 
> 
> I mean the right mechanism for controlling whether or not the transport
> is ready to serve RPC requests is through the XPRT_CONNECTED flag. All
> the existing generic RPC error handling, congestion handling, etc
> depends on that flag being set correctly.
> 
> Until the TLS socket has completed its handshake protocol and is ready
> to transmit data, it should not be declared connected. The distinction
> between the two states 'TCP is unconnected' and 'TLS handshake is
> incomplete' is a socket/transport setup detail as far as the RPC xprt
> layer is concerned: just another set of intermediate states between
> SYN_SENT and ESTABLISHED.

First, TLS is technically an upper layer protocol. It's not
part of the transport protocol. This is exactly how it's
implemented in the Linux kernel. And, TLS works on transports
other than TCP, so that makes it a reasonable candidate for
treatment in the generic client rather than in a particular
transport mechanism.

Second, the "intermediate states" would be /outside/ of SYN_SENT
and ESTABLISHED. A TCP transport has to be in the ESTABLISHED
state (ie, the transport's connection handshake has to be
complete) before any TLS traffic can go over it.

Most importantly, the client has to send an RPC message first
before it can start a TLS handshake. The RPC-with-TLS protocol
specification requires that the handshake be preceded with the
NULL AUTH_TLS request, which is an RPC. Otherwise, there's no
way for the server end to know when to expect a handshake.

In today's RPC client, the underlying connection has to be in
the XPRT_CONNECTED state before the RPC client can exchange any
RPC transaction, including AUTH_TLS NULL.

To make it work the way you've suggested, we would have to build
a mechanism that could send the AUTH_TLS NULL and receive and
parse its reply /before/ the client has put the transport into
the XPRT_CONNECTED state, and that NULL request would have to
be driven from inside the transport instance (not via the FSM
where all other RPC traffic originates).

Do you have any suggestions about how to make this last point
less painful?


--
Chuck Lever




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS
  2022-04-19 18:53       ` Chuck Lever III
@ 2022-04-19 20:49         ` Rick Macklem
  0 siblings, 0 replies; 27+ messages in thread
From: Rick Macklem @ 2022-04-19 20:49 UTC (permalink / raw)
  To: Chuck Lever III, Trond Myklebust
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak,
	Linux NFS Mailing List, borisp

> Chuck Lever III <chuck.lever@oracle.com> wrote:
> > On Apr 19, 2022, at 2:48 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> >
> > On Tue, 2022-04-19 at 16:00 +0000, Chuck Lever III wrote:
> >> Hi Trond-
> >>
> >> Thanks for the early review!
>>
[good stuff snipped]
> >>
> >> However, one of Rick's preferences is that "auto" not use
> >> transport-layer security unless the server requires it via
> >> a SECINFO/MNT pseudoflavor, which only the kernel would be
> >> privy to. I'll have to think about whether we want to make
> >> that happen.
Just fyi, the above was not exactly what I thought.

My concern with "xprtsec=auto" was that the client (user/admin doing the
mount) would not know if on the wire encryption was happening or not.
As such, this case in not implemented in the FreeBSD client at this time.
(I may do so in order to ne Linux compatible, but I doubt it will be the
 default. Of course, it is really up to the "FreeBSD collective" and not
 just me.)

For the "xprtsec=auto" case, I am fine with the client attempting the
NULL AUTH_TLS RPC probe as soon as a connection is established,
followed by a TLS handshake, if the NULL AUTH_TLS RPC probe succeeds.

At this time, the FreeBSD client does not use indications from the server,
such as SECINFO to decide to do the NULL AUTH_TLS RPC. The current
implementation does it optionally (just called "tls", which is the
equivalent of "xprtsec=tls"), as soon as a connection is established.

> >
> > That sounds like a terrible protocol hack. TLS is not an authentication
> > flavour but a transport level protection.
Not sure if I lost the "context" w.r.t. this comment, but I argued that this
should not be more "sec=XXX" options, since it was related to the transport
and not user authentication.

> Fair enough. We've been discussing this on nfsv4@ietf.org, and
> it's certainly not written in stone yet.
Yes. I cannot guarantee FreeBSD will become Linux compatible, but what
Linux chooses is certainly up to the Linux community. Since Linux is the
"big player", I do attempt to keep FreeBSD's mount options compatible,
whenever practical.

> I invite you to join the conversation and share your concerns
> (and possibly any alternative solutions you might have).
>
>
> > That said, I don't see how this invalidates my argument. When told to
> > use TLS, the kernel client can still return a mount time error if the
> > server fails to advertise support through this pseudoflavour and leave
> > it up to userspace to decide how to deal with that.
>
> Sure. I'm just saying I haven't thought it through yet. I don't
> think it will be a problem to move more (or all) of the transport
> security policy to mount.nfs.
It happens to be implemented in the kernel for FreeBSD, but that
was just what was convenient for FreeBSD. (New TCP connections
for RPCs, including reconnects, are done in the krpc for FreeBSD,
so that is where it needed to know whether or not to do the
NULL AUTH_TLS RPC probe.)

rick

--
Chuck Lever





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-19 19:40         ` Chuck Lever III
@ 2022-04-19 22:08           ` Trond Myklebust
  2022-04-20  0:34             ` Chuck Lever III
  0 siblings, 1 reply; 27+ messages in thread
From: Trond Myklebust @ 2022-04-19 22:08 UTC (permalink / raw)
  To: chuck.lever
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak, borisp,
	linux-nfs

On Tue, 2022-04-19 at 19:40 +0000, Chuck Lever III wrote:
> 
> 
> > On Apr 19, 2022, at 3:04 PM, Trond Myklebust
> > <trondmy@hammerspace.com> wrote:
> > 
> > On Tue, 2022-04-19 at 18:16 +0000, Chuck Lever III wrote:
> > > 
> > > 
> > > > On Apr 18, 2022, at 10:57 PM, Trond Myklebust
> > > > <trondmy@hammerspace.com> wrote:
> > > > 
> > > > On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
> > > > > Introduce a mechanism to cause xprt_transmit() to break out
> > > > > of
> > > > > its
> > > > > sending loop at a specific rpc_rqst, rather than draining the
> > > > > whole
> > > > > transmit queue.
> > > > > 
> > > > > This enables the client to send just an RPC TLS probe and
> > > > > then
> > > > > wait
> > > > > for the response before proceeding with the rest of the
> > > > > queue.
> > > > > 
> > > > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > > > ---
> > > > >  include/linux/sunrpc/sched.h  |    2 ++
> > > > >  include/trace/events/sunrpc.h |    1 +
> > > > >  net/sunrpc/xprt.c             |    2 ++
> > > > >  3 files changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/include/linux/sunrpc/sched.h
> > > > > b/include/linux/sunrpc/sched.h
> > > > > index 599133fb3c63..f8c09638fa69 100644
> > > > > --- a/include/linux/sunrpc/sched.h
> > > > > +++ b/include/linux/sunrpc/sched.h
> > > > > @@ -125,6 +125,7 @@ struct rpc_task_setup {
> > > > >  #define RPC_TASK_TLSCRED               0x00000008      /*
> > > > > Use
> > > > > AUTH_TLS credential */
> > > > >  #define RPC_TASK_NULLCREDS             0x00000010      /*
> > > > > Use
> > > > > AUTH_NULL credential */
> > > > >  #define RPC_CALL_MAJORSEEN             0x00000020      /*
> > > > > major
> > > > > timeout seen */
> > > > > +#define RPC_TASK_CORK                  0x00000040      /*
> > > > > cork
> > > > > the
> > > > > xmit queue */
> > > > >  #define RPC_TASK_DYNAMIC               0x00000080      /*
> > > > > task
> > > > > was
> > > > > kmalloc'ed */
> > > > >  #define        RPC_TASK_NO_ROUND_ROBIN        
> > > > > 0x00000100     
> > > > > /*
> > > > > send requests on "main" xprt */
> > > > >  #define RPC_TASK_SOFT                  0x00000200      /*
> > > > > Use
> > > > > soft
> > > > > timeouts */
> > > > > @@ -137,6 +138,7 @@ struct rpc_task_setup {
> > > > >  
> > > > >  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
> > > > > RPC_TASK_ASYNC)
> > > > >  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags &
> > > > > RPC_TASK_SWAPPER)
> > > > > +#define RPC_IS_CORK(t)         ((t)->tk_flags &
> > > > > RPC_TASK_CORK)
> > > > >  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
> > > > > (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
> > > > >  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags &
> > > > > RPC_TASK_SOFTCONN)
> > > > >  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
> > > > > RPC_TASK_SENT)
> > > > > diff --git a/include/trace/events/sunrpc.h
> > > > > b/include/trace/events/sunrpc.h
> > > > > index 811187c47ebb..e8d6adff1a50 100644
> > > > > --- a/include/trace/events/sunrpc.h
> > > > > +++ b/include/trace/events/sunrpc.h
> > > > > @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
> > > > >                 { RPC_TASK_TLSCRED, "TLSCRED"
> > > > > },                        \
> > > > >                 { RPC_TASK_NULLCREDS, "NULLCREDS"
> > > > > },                    \
> > > > >                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
> > > > > },                    \
> > > > > +               { RPC_TASK_CORK, "CORK"
> > > > > },                              \
> > > > >                 { RPC_TASK_DYNAMIC, "DYNAMIC"
> > > > > },                        \
> > > > >                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
> > > > > },          \
> > > > >                 { RPC_TASK_SOFT, "SOFT"
> > > > > },                              \
> > > > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > > > > index 86d62cffba0d..4b303b945b51 100644
> > > > > --- a/net/sunrpc/xprt.c
> > > > > +++ b/net/sunrpc/xprt.c
> > > > > @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
> > > > >                 if (xprt_request_data_received(task) &&
> > > > >                     !test_bit(RPC_TASK_NEED_XMIT, &task-
> > > > > > tk_runstate))
> > > > >                         break;
> > > > > +               if (RPC_IS_CORK(task))
> > > > > +                       break;
> > > > >                 cond_resched_lock(&xprt->queue_lock);
> > > > >         }
> > > > >         spin_unlock(&xprt->queue_lock);
> > > > > 
> > > > > 
> > > > 
> > > > This is entirely the wrong place for this kind of control
> > > > mechanism.
> > > 
> > > I'm not sure I entirely understand your concern, so bear with
> > > me while I try to clarify.
> > > 
> > > 
> > > > TLS vs not-TLS needs to be decided up front when we initialise
> > > > the
> > > > transport (i.e. at mount time or whenever the pNFS channels are
> > > > set
> > > > up). Otherwise, we're vulnerable to downgrade attacks.
> > > 
> > > Downgrade attacks are prevented by using "xprtsec=tls" because
> > > in that case, transport creation fails if either the AUTH_TLS
> > > fails or the handshake fails.
> > > 
> > > The TCP connection has to be established first, though. Then the
> > > client can send the RPC_AUTH_TLS probe, which is the same as the
> > > NULL ping that it already sends. That mechanism is independent
> > > of the lower layer transport (TCP in this case).
> > > 
> > > Therefore, RPC traffic must be stoppered while the client:
> > > 
> > > 1. waits for the AUTH_TLS probe's reply, and
> > > 
> > > 2. waits for the handshake to complete
> > > 
> > > Because an RPC message is involved in this interaction, I didn't
> > > see a way to implement it completely within xprtsock's TCP
> > > connection logic. IMO, driving the handshake has to be done by
> > > the generic RPC client.
> > > 
> > > So, do you mean that I need to replace RPC_TASK_CORK with a
> > > special return code from xs_tcp_send_request() ?
> > 
> > 
> > I mean the right mechanism for controlling whether or not the
> > transport
> > is ready to serve RPC requests is through the XPRT_CONNECTED flag.
> > All
> > the existing generic RPC error handling, congestion handling, etc
> > depends on that flag being set correctly.
> > 
> > Until the TLS socket has completed its handshake protocol and is
> > ready
> > to transmit data, it should not be declared connected. The
> > distinction
> > between the two states 'TCP is unconnected' and 'TLS handshake is
> > incomplete' is a socket/transport setup detail as far as the RPC
> > xprt
> > layer is concerned: just another set of intermediate states between
> > SYN_SENT and ESTABLISHED.
> 
> First, TLS is technically an upper layer protocol. It's not
> part of the transport protocol. This is exactly how it's
> implemented in the Linux kernel. And, TLS works on transports
> other than TCP, so that makes it a reasonable candidate for
> treatment in the generic client rather than in a particular
> transport mechanism.

Sorry, but no! As far as the RPC layer is concerned, there is no
difference between a TLS socket and a TCP socket. The xprt layer should
not have to know or care about the existence of TLS other that as a
transport option to be configured at connection time.

> 
> Second, the "intermediate states" would be /outside/ of SYN_SENT
> and ESTABLISHED. A TCP transport has to be in the ESTABLISHED
> state (ie, the transport's connection handshake has to be
> complete) before any TLS traffic can go over it.
> 

My point is we don't give a damn about the intermediate states in the
RPC layer.

> Most importantly, the client has to send an RPC message first
> before it can start a TLS handshake. The RPC-with-TLS protocol
> specification requires that the handshake be preceded with the
> NULL AUTH_TLS request, which is an RPC. Otherwise, there's no
> way for the server end to know when to expect a handshake.
> 

Sure, but those are 2 non-overlapping states. The socket is first in a
state where it needs to do a NULL ping using regular RPC/TCP. Then it
needs to do the TLS handshake. Then it transitions into the state where
it can act like any other transport.

> In today's RPC client, the underlying connection has to be in
> the XPRT_CONNECTED state before the RPC client can exchange any
> RPC transaction, including AUTH_TLS NULL.
> 
> To make it work the way you've suggested, we would have to build
> a mechanism that could send the AUTH_TLS NULL and receive and
> parse its reply /before/ the client has put the transport into
> the XPRT_CONNECTED state, and that NULL request would have to
> be driven from inside the transport instance (not via the FSM
> where all other RPC traffic originates).
> 
> Do you have any suggestions about how to make this last point
> less painful?

This isn't too different from what we already do with the rpcbind call
for performing port discovery. The only difference is that the NULL
ping needs to happen on the same transport as the one being constructed
and that it needs to happen after the TCP connection is complete.

So I'd suggest that TLS/TCP needs to be a different xprt_class than the
base TCP, then doing the whole "do-NULL-ping-and-TLS-handshake" in the
connect() callback for that new class.

The connect() callback can set up a private rpc client and do the NULL
call asynchronously just like we do in rpcb_getport_async(). When the
RPC call completes, we steal the resulting socket from that private rpc
client and kick off the TLS handshake on it. All that can be done in
the rpc_call_done callback (i.e. the equivalent of rpcb_getport_done).

Once the TLS handshake is done, you can set the XPRT_CONNECTED state
and call xprt_wake_pending_tasks().

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
  2022-04-19 22:08           ` Trond Myklebust
@ 2022-04-20  0:34             ` Chuck Lever III
  0 siblings, 0 replies; 27+ messages in thread
From: Chuck Lever III @ 2022-04-20  0:34 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-cifs, linux-fsdevel, linux-nvme, netdev, simo, ak, borisp,
	linux-nfs


> On Apr 19, 2022, at 6:09 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2022-04-19 at 19:40 +0000, Chuck Lever III wrote:
>> 
>> 
>>> On Apr 19, 2022, at 3:04 PM, Trond Myklebust
>>> <trondmy@hammerspace.com> wrote:
>>> 
>>> On Tue, 2022-04-19 at 18:16 +0000, Chuck Lever III wrote:
>>>> 
>>>> 
>>>>> On Apr 18, 2022, at 10:57 PM, Trond Myklebust
>>>>> <trondmy@hammerspace.com> wrote:
>>>>> 
>>>>>> On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
>>>>>>> Introduce a mechanism to cause xprt_transmit() to break out
>>>>>>> of
>>>>>>> its
>>>>>>> sending loop at a specific rpc_rqst, rather than draining the
>>>>>>> whole
>>>>>>> transmit queue.
>>>>>>> 
>>>>>>> This enables the client to send just an RPC TLS probe and
>>>>>>> then
>>>>>>> wait
>>>>>>> for the response before proceeding with the rest of the
>>>>>>> queue.
>>>>>>> 
>>>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>>>>> ---
>>>>>>>  include/linux/sunrpc/sched.h  |    2 ++
>>>>>>>  include/trace/events/sunrpc.h |    1 +
>>>>>>>  net/sunrpc/xprt.c             |    2 ++
>>>>>>>  3 files changed, 5 insertions(+)
>>>>>>> 
>>>>>>> diff --git a/include/linux/sunrpc/sched.h
>>>>>>> b/include/linux/sunrpc/sched.h
>>>>>>> index 599133fb3c63..f8c09638fa69 100644
>>>>>>> --- a/include/linux/sunrpc/sched.h
>>>>>>> +++ b/include/linux/sunrpc/sched.h
>>>>>>> @@ -125,6 +125,7 @@ struct rpc_task_setup {
>>>>>>>  #define RPC_TASK_TLSCRED               0x00000008      /*
>>>>>>> Use
>>>>>>> AUTH_TLS credential */
>>>>>>>  #define RPC_TASK_NULLCREDS             0x00000010      /*
>>>>>>> Use
>>>>>>> AUTH_NULL credential */
>>>>>>>  #define RPC_CALL_MAJORSEEN             0x00000020      /*
>>>>>>> major
>>>>>>> timeout seen */
>>>>>>> +#define RPC_TASK_CORK                  0x00000040      /*
>>>>>>> cork
>>>>>>> the
>>>>>>> xmit queue */
>>>>>>>  #define RPC_TASK_DYNAMIC               0x00000080      /*
>>>>>>> task
>>>>>>> was
>>>>>>> kmalloc'ed */
>>>>>>>  #define        RPC_TASK_NO_ROUND_ROBIN        
>>>>>>> 0x00000100     
>>>>>>> /*
>>>>>>> send requests on "main" xprt */
>>>>>>>  #define RPC_TASK_SOFT                  0x00000200      /*
>>>>>>> Use
>>>>>>> soft
>>>>>>> timeouts */
>>>>>>> @@ -137,6 +138,7 @@ struct rpc_task_setup {
>>>>>>>  
>>>>>>>  #define RPC_IS_ASYNC(t)                ((t)->tk_flags &
>>>>>>> RPC_TASK_ASYNC)
>>>>>>>  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags &
>>>>>>> RPC_TASK_SWAPPER)
>>>>>>> +#define RPC_IS_CORK(t)         ((t)->tk_flags &
>>>>>>> RPC_TASK_CORK)
>>>>>>>  #define RPC_IS_SOFT(t)         ((t)->tk_flags &
>>>>>>> (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
>>>>>>>  #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags &
>>>>>>> RPC_TASK_SOFTCONN)
>>>>>>>  #define RPC_WAS_SENT(t)                ((t)->tk_flags &
>>>>>>> RPC_TASK_SENT)
>>>>>>> diff --git a/include/trace/events/sunrpc.h
>>>>>>> b/include/trace/events/sunrpc.h
>>>>>>> index 811187c47ebb..e8d6adff1a50 100644
>>>>>>> --- a/include/trace/events/sunrpc.h
>>>>>>> +++ b/include/trace/events/sunrpc.h
>>>>>>> @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
>>>>>>>                 { RPC_TASK_TLSCRED, "TLSCRED"
>>>>>>> },                        \
>>>>>>>                 { RPC_TASK_NULLCREDS, "NULLCREDS"
>>>>>>> },                    \
>>>>>>>                 { RPC_CALL_MAJORSEEN, "MAJORSEEN"
>>>>>>> },                    \
>>>>>>> +               { RPC_TASK_CORK, "CORK"
>>>>>>> },                              \
>>>>>>>                 { RPC_TASK_DYNAMIC, "DYNAMIC"
>>>>>>> },                        \
>>>>>>>                 { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
>>>>>>> },          \
>>>>>>>                 { RPC_TASK_SOFT, "SOFT"
>>>>>>> },                              \
>>>>>>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
>>>>>>> index 86d62cffba0d..4b303b945b51 100644
>>>>>>> --- a/net/sunrpc/xprt.c
>>>>>>> +++ b/net/sunrpc/xprt.c
>>>>>>> @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
>>>>>>>                 if (xprt_request_data_received(task) &&
>>>>>>>                     !test_bit(RPC_TASK_NEED_XMIT, &task-
>>>>>>>> tk_runstate))
>>>>>>>                         break;
>>>>>>> +               if (RPC_IS_CORK(task))
>>>>>>> +                       break;
>>>>>>>                 cond_resched_lock(&xprt->queue_lock);
>>>>>>>         }
>>>>>>>         spin_unlock(&xprt->queue_lock);
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> This is entirely the wrong place for this kind of control
>>>>>> mechanism.
>>>>> 
>>>>> I'm not sure I entirely understand your concern, so bear with
>>>>> me while I try to clarify.
>>>>> 
>>>>> 
>>>>>> TLS vs not-TLS needs to be decided up front when we initialise
>>>>>> the
>>>>>> transport (i.e. at mount time or whenever the pNFS channels are
>>>>>> set
>>>>>> up). Otherwise, we're vulnerable to downgrade attacks.
>>>>> 
>>>>> Downgrade attacks are prevented by using "xprtsec=tls" because
>>>>> in that case, transport creation fails if either the AUTH_TLS
>>>>> fails or the handshake fails.
>>>>> 
>>>>> The TCP connection has to be established first, though. Then the
>>>>> client can send the RPC_AUTH_TLS probe, which is the same as the
>>>>> NULL ping that it already sends. That mechanism is independent
>>>>> of the lower layer transport (TCP in this case).
>>>>> 
>>>>> Therefore, RPC traffic must be stoppered while the client:
>>>>> 
>>>>> 1. waits for the AUTH_TLS probe's reply, and
>>>>> 
>>>>> 2. waits for the handshake to complete
>>>>> 
>>>>> Because an RPC message is involved in this interaction, I didn't
>>>>> see a way to implement it completely within xprtsock's TCP
>>>>> connection logic. IMO, driving the handshake has to be done by
>>>>> the generic RPC client.
>>>>> 
>>>>> So, do you mean that I need to replace RPC_TASK_CORK with a
>>>>> special return code from xs_tcp_send_request() ?
>>> 
>>> 
>>> I mean the right mechanism for controlling whether or not the
>>> transport
>>> is ready to serve RPC requests is through the XPRT_CONNECTED flag.
>>> All
>>> the existing generic RPC error handling, congestion handling, etc
>>> depends on that flag being set correctly.
>>> 
>>> Until the TLS socket has completed its handshake protocol and is
>>> ready
>>> to transmit data, it should not be declared connected. The
>>> distinction
>>> between the two states 'TCP is unconnected' and 'TLS handshake is
>>> incomplete' is a socket/transport setup detail as far as the RPC
>>> xprt
>>> layer is concerned: just another set of intermediate states between
>>> SYN_SENT and ESTABLISHED.
>> 
>> First, TLS is technically an upper layer protocol. It's not
>> part of the transport protocol. This is exactly how it's
>> implemented in the Linux kernel. And, TLS works on transports
>> other than TCP, so that makes it a reasonable candidate for
>> treatment in the generic client rather than in a particular
>> transport mechanism.
> 
> Sorry, but no! As far as the RPC layer is concerned, there is no
> difference between a TLS socket and a TCP socket. The xprt layer should
> not have to know or care about the existence of TLS other that as a
> transport option to be configured at connection time.
> 
>> 
>> Second, the "intermediate states" would be /outside/ of SYN_SENT
>> and ESTABLISHED. A TCP transport has to be in the ESTABLISHED
>> state (ie, the transport's connection handshake has to be
>> complete) before any TLS traffic can go over it.
>> 
> 
> My point is we don't give a damn about the intermediate states in the
> RPC layer.
> 
>> Most importantly, the client has to send an RPC message first
>> before it can start a TLS handshake. The RPC-with-TLS protocol
>> specification requires that the handshake be preceded with the
>> NULL AUTH_TLS request, which is an RPC. Otherwise, there's no
>> way for the server end to know when to expect a handshake.
>> 
> 
> Sure, but those are 2 non-overlapping states. The socket is first in a
> state where it needs to do a NULL ping using regular RPC/TCP. Then it
> needs to do the TLS handshake. Then it transitions into the state where
> it can act like any other transport.

Understood: this architecture more-or-less mimics what the RPC client would see for a transport like QUIC where connection establishment and the security handshake are integrated and handled concurrently.

The reality is that the RPC client’s transport layer will have to deal with these steps separately for TLS-on-UDP and TLS-on-TCP, but hiding the details under the transport switch is fine with me as long as there is a way to send the AUTH_TLS probe before the transport is marked “connected” (see below).


>> In today's RPC client, the underlying connection has to be in
>> the XPRT_CONNECTED state before the RPC client can exchange any
>> RPC transaction, including AUTH_TLS NULL.
>> 
>> To make it work the way you've suggested, we would have to build
>> a mechanism that could send the AUTH_TLS NULL and receive and
>> parse its reply /before/ the client has put the transport into
>> the XPRT_CONNECTED state, and that NULL request would have to
>> be driven from inside the transport instance (not via the FSM
>> where all other RPC traffic originates).
>> 
>> Do you have any suggestions about how to make this last point
>> less painful?
> 
> This isn't too different from what we already do with the rpcbind call
> for performing port discovery. The only difference is that the NULL
> ping needs to happen on the same transport as the one being constructed
> and that it needs to happen after the TCP connection is complete.
> 
> So I'd suggest that TLS/TCP needs to be a different xprt_class than the
> base TCP, then doing the whole "do-NULL-ping-and-TLS-handshake" in the
> connect() callback for that new class.
> 
> The connect() callback can set up a private rpc client and do the NULL
> call asynchronously just like we do in rpcb_getport_async().

“Set up a separate rpc_client to handle the AUTH_TLS probe” was the piece I was missing. The rest of this is pretty much what the RPC changes in this patch series already do, but organized a little differently (for example, they use a “done” callback as you describe below, so we’re already about halfway there).

The idea is to stopper the stream of RPC messages by leaving the xprt marked “not connected” instead of by adding and setting an RPC_TASK_CORK flag. The FSM will no longer be involved at all in dealing with TLS.


> When the
> RPC call completes, we steal the resulting socket from that private rpc
> client and kick off the TLS handshake on it. All that can be done in
> the rpc_call_done callback (i.e. the equivalent of rpcb_getport_done).
> 
> Once the TLS handshake is done, you can set the XPRT_CONNECTED state
> and call xprt_wake_pending_tasks().

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-04-20  0:35 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 03/15] SUNRPC: Capture cmsg metadata on client-side receive Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 04/15] SUNRPC: Fail faster on bad verifier Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 05/15] SUNRPC: Widen rpc_task::tk_flags Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper() Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag Chuck Lever
2022-04-19  2:57   ` Trond Myklebust
2022-04-19 18:16     ` Chuck Lever III
2022-04-19 19:04       ` Trond Myklebust
2022-04-19 19:40         ` Chuck Lever III
2022-04-19 22:08           ` Trond Myklebust
2022-04-20  0:34             ` Chuck Lever III
2022-04-18 16:52 ` [PATCH RFC 09/15] SUNRPC: Add a cl_xprtsec_policy field Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 10/15] SUNRPC: Expose TLS policy via the rpc_create() API Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 13/15] NFS: Replace fs_context-related dprintk() call sites with tracepoints Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option Chuck Lever
2022-04-19  3:31 ` [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Trond Myklebust
2022-04-19 16:00   ` Chuck Lever III
2022-04-19 18:48     ` Trond Myklebust
2022-04-19 18:53       ` Chuck Lever III
2022-04-19 20:49         ` Rick Macklem

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.