On Mon, 2022-06-06 at 10:52 -0400, Chuck Lever wrote:
> After some discussion, we decided that controlling transport layer
> security policy should be separate from the setting for the user
> authentication flavor. To accomplish this, add a new NFS mount
> option to select a transport layer security policy for RPC
> operations associated with the mount point.
>
> xprtsec=none - Transport layer security is disabled.
>
> 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 mount.nfs command will provide an addition setting:
>
> xprtsec=auto - Try to establish a TLS session, but proceed
> with no transport layer security if that fails.
>
> Updates to mount.nfs and nfs(5) will be sent under separate cover.
>
Seems like a reasonable interface.
> Future work:
>
> 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.
>
^^^
We might want something more flexible than this over the long haul.
Container orchestration like kubernetes would probably prefer to manage
this without sprinkling files all over.
Maybe we can allow this info to be set in the kernel keyring of the
mounting process instead of requiring files? In any case, we can cross
that bridge when we come to it.
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfs/client.c | 10 +++++++++-
> fs/nfs/fs_context.c | 40 ++++++++++++++++++++++++++++++++++++++++
> fs/nfs/internal.h | 1 +
> fs/nfs/nfs4client.c | 2 +-
> fs/nfs/super.c | 7 +++++++
> 5 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 0896e4f047d1..1f26c1ad18b3 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -530,6 +530,14 @@ 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_TLS:
> + args.xprtsec = RPC_XPRTSEC_TLS_X509;
> + break;
> + default:
> + args.xprtsec = RPC_XPRTSEC_NONE;
> + }
> +
> if (!IS_ERR(clp->cl_rpcclient))
> return 0;
>
> @@ -680,7 +688,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 35e400a557b9..3e29dd88b59b 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,18 @@ static const struct constant_table nfs_secflavor_tokens[] = {
> {}
> };
>
> +enum {
> + Opt_xprtsec_none,
> + Opt_xprtsec_tls,
> + nr__Opt_xprtsec
> +};
> +
> +static const struct constant_table nfs_xprtsec_policies[] = {
> + { "none", Opt_xprtsec_none },
> + { "tls", Opt_xprtsec_tls },
> + {}
> +};
> +
> /*
> * Sanity-check a server address provided by the mount command.
> *
> @@ -431,6 +445,26 @@ 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_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 +729,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 +1603,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_NONE;
>
> 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..66da994500f9 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -491,6 +491,13 @@ 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_TLS:
> + seq_printf(m, ",xprtsec=tls");
> + break;
> + default:
> + break;
> + }
>
> if (version != 4)
> nfs_show_mountd_options(m, nfss, showdefaults);
>
>
--
Jeff Layton <[email protected]>
> On Jul 18, 2022, at 4:24 PM, Jeff Layton <[email protected]> wrote:
>
> On Mon, 2022-06-06 at 10:52 -0400, Chuck Lever wrote:
>> After some discussion, we decided that controlling transport layer
>> security policy should be separate from the setting for the user
>> authentication flavor. To accomplish this, add a new NFS mount
>> option to select a transport layer security policy for RPC
>> operations associated with the mount point.
>>
>> xprtsec=none - Transport layer security is disabled.
>>
>> 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 mount.nfs command will provide an addition setting:
>>
>> xprtsec=auto - Try to establish a TLS session, but proceed
>> with no transport layer security if that fails.
>>
>> Updates to mount.nfs and nfs(5) will be sent under separate cover.
>>
>
> Seems like a reasonable interface.
Note we are trying to keep the administrative interfaces on all
implementations as similar as is possible. Some of this may feel
a little non-Linuxy, and that might be why.
>> Future work:
>>
>> 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.
>>
>
>
> ^^^
> We might want something more flexible than this over the long haul.
> Container orchestration like kubernetes would probably prefer to manage
> this without sprinkling files all over.
>
> Maybe we can allow this info to be set in the kernel keyring of the
> mounting process instead of requiring files? In any case, we can cross
> that bridge when we come to it.
In the prototype I'm working on now, mount.nfs will read the client's
certificate from a file and use add_key(2) to inject it into the
kernel. It passes the serial number of that key to the kernel via
mount(2).
During the TLS handshake, the kernel gives a connected socket and
the key's serial number to tlshd to do the handshake with client
TLS authentication.
When we get to the server-side, it's likely that rpc.nfsd or
some other utility will do the same for the server's certificate.
So we do indeed plan to use keyrings for this.
Meanwhile, if you have some specific recommendations for handling
the orchestration requirements, or know someone who has informed
opinions about it that I should talk to, please hook me up!
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> fs/nfs/client.c | 10 +++++++++-
>> fs/nfs/fs_context.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> fs/nfs/internal.h | 1 +
>> fs/nfs/nfs4client.c | 2 +-
>> fs/nfs/super.c | 7 +++++++
>> 5 files changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index 0896e4f047d1..1f26c1ad18b3 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -530,6 +530,14 @@ 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_TLS:
>> + args.xprtsec = RPC_XPRTSEC_TLS_X509;
>> + break;
>> + default:
>> + args.xprtsec = RPC_XPRTSEC_NONE;
>> + }
>> +
>> if (!IS_ERR(clp->cl_rpcclient))
>> return 0;
>>
>> @@ -680,7 +688,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 35e400a557b9..3e29dd88b59b 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,18 @@ static const struct constant_table nfs_secflavor_tokens[] = {
>> {}
>> };
>>
>> +enum {
>> + Opt_xprtsec_none,
>> + Opt_xprtsec_tls,
>> + nr__Opt_xprtsec
>> +};
>> +
>> +static const struct constant_table nfs_xprtsec_policies[] = {
>> + { "none", Opt_xprtsec_none },
>> + { "tls", Opt_xprtsec_tls },
>> + {}
>> +};
>> +
>> /*
>> * Sanity-check a server address provided by the mount command.
>> *
>> @@ -431,6 +445,26 @@ 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_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 +729,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 +1603,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_NONE;
>>
>> 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..66da994500f9 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -491,6 +491,13 @@ 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_TLS:
>> + seq_printf(m, ",xprtsec=tls");
>> + break;
>> + default:
>> + break;
>> + }
>>
>> if (version != 4)
>> nfs_show_mountd_options(m, nfss, showdefaults);
>>
>>
>
> --
> Jeff Layton <[email protected]>
--
Chuck Lever