2022-04-19 02:47:24

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS

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


2022-04-19 03:57:45

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes

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 <[email protected]>
---
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;


2022-04-19 06:05:48

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe

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 <[email protected]>
---
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,


2022-04-19 11:36:32

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper()

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

Signed-off-by: Chuck Lever <[email protected]>
---
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;


2022-04-19 13:43:12

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field

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

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfs/client.c | 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 */


2022-04-19 14:31:06

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor

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

Signed-off-by: Chuck Lever <[email protected]>
---
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,
+};


2022-04-19 16:11:23

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option

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 <[email protected]>
---
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);


2022-04-19 16:47:41

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS

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
[email protected]


2022-04-19 16:52:57

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS

Hi Trond-

Thanks for the early review!


> On Apr 18, 2022, at 11:31 PM, Trond Myklebust <[email protected]> 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
> [email protected]

--
Chuck Lever



2022-04-20 02:46:25

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS

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
> > <[email protected]> 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
[email protected]


2022-04-20 09:22:14

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS



> On Apr 19, 2022, at 2:48 PM, Trond Myklebust <[email protected]> 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
>>> <[email protected]> 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 [email protected], 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



2022-04-20 14:17:56

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect

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

Signed-off-by: Chuck Lever <[email protected]>
---
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;


2022-04-20 15:24:33

by Rick Macklem

[permalink] [raw]
Subject: Re: [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS

> Chuck Lever III <[email protected]> wrote:
> > On Apr 19, 2022, at 2:48 PM, Trond Myklebust <[email protected]> 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 [email protected], 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