Hi Trond- Thanks for the early review! > On Apr 18, 2022, at 11:31 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx -- Chuck Lever
