what should happen for proto=rdma,xprtsec=tls

what should happen for proto=rdma,xprtsec=tls

[Olga Kornievskaia]

Hi Chuck,

I did
[aglo@unknown000c29dba616 ~]$ sudo mount -o
vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
192.168.1.106:/nfsshare /mnt
which succeeded and then I did "mount" which displays:
192.168.1.106:/nfsshare on /mnt type nfs4
(rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
Naturally no TLS happens for rdma but I'm not sure if this mount
should fail? or If the "mount" afterwards shouldn't display/claim that
it's doing xprtsec=tls?

Thoughts?

Re: what should happen for proto=rdma,xprtsec=tls

[Chuck Lever III]

> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> 
> Hi Chuck,
> 
> I did
> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> 192.168.1.106:/nfsshare /mnt
> which succeeded and then I did "mount" which displays:
> 192.168.1.106:/nfsshare on /mnt type nfs4
> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
> Naturally no TLS happens for rdma but I'm not sure if this mount
> should fail? or If the "mount" afterwards shouldn't display/claim that
> it's doing xprtsec=tls?

That mount request should have failed.


--
Chuck Lever

Re: what should happen for proto=rdma,xprtsec=tls

[Olga Kornievskaia]

On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > Hi Chuck,
> >
> > I did
> > [aglo@unknown000c29dba616 ~]$ sudo mount -o
> > vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> > 192.168.1.106:/nfsshare /mnt
> > which succeeded and then I did "mount" which displays:
> > 192.168.1.106:/nfsshare on /mnt type nfs4
> > (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
> > Naturally no TLS happens for rdma but I'm not sure if this mount
> > should fail? or If the "mount" afterwards shouldn't display/claim that
> > it's doing xprtsec=tls?
>
> That mount request should have failed.

So perhaps something like this:

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index e49e3d18ef88..5fc229a7690b 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
fs_context *fc,
                        goto out_invalid_transport_udp;
                break;
        case XPRT_TRANSPORT_TCP:
+               break;
        case XPRT_TRANSPORT_RDMA:
+               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
+                       goto out_invalid_transport_rdma;
                break;
        default:
                ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
@@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
fs_context *fc,
        return 0;
 out_invalid_transport_udp:
        return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
+out_invalid_transport_rdma:
+       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
 }

[aglo@unknown000c29dba616 ~]$sudo mount -o
vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
192.168.1.106:/nfsshare /mnt
mount.nfs: an incorrect mount option was specified

I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
the "an incorrect mount option was specified". But if I try to do
"proto=udp" I don't see the "unsupported transport protocol udp" so
I'm not sure what this nfs_invalf() is supposed to do.
I
>
>
> --
> Chuck Lever
>
>

Re: what should happen for proto=rdma,xprtsec=tls

[Chuck Lever III]

> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> 
> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>> 
>>> Hi Chuck,
>>> 
>>> I did
>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>> 192.168.1.106:/nfsshare /mnt
>>> which succeeded and then I did "mount" which displays:
>>> 192.168.1.106:/nfsshare on /mnt type nfs4
>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
>>> Naturally no TLS happens for rdma but I'm not sure if this mount
>>> should fail? or If the "mount" afterwards shouldn't display/claim that
>>> it's doing xprtsec=tls?
>> 
>> That mount request should have failed.
> 
> So perhaps something like this:
> 
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index e49e3d18ef88..5fc229a7690b 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
> fs_context *fc,
>                        goto out_invalid_transport_udp;
>                break;
>        case XPRT_TRANSPORT_TCP:
> +               break;
>        case XPRT_TRANSPORT_RDMA:
> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
> +                       goto out_invalid_transport_rdma;
>                break;
>        default:
>                ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
> fs_context *fc,
>        return 0;
> out_invalid_transport_udp:
>        return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> +out_invalid_transport_rdma:
> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
> }
> 
> [aglo@unknown000c29dba616 ~]$sudo mount -o
> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> 192.168.1.106:/nfsshare /mnt
> mount.nfs: an incorrect mount option was specified
> 
> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
> the "an incorrect mount option was specified". But if I try to do
> "proto=udp" I don't see the "unsupported transport protocol udp" so
> I'm not sure what this nfs_invalf() is supposed to do.

Architecturally, the NFS client should pass these option
settings down to RPC and let the transport tell it that
TLS isn't available.

I thought RDMA was missing the functions that marked it
as supporting TLS, and so it should have failed the mount.
I must have missed something.


--
Chuck Lever

Re: what should happen for proto=rdma,xprtsec=tls

[Olga Kornievskaia]

On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
> >>
> >>
> >>
> >>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >>>
> >>> Hi Chuck,
> >>>
> >>> I did
> >>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> >>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> >>> 192.168.1.106:/nfsshare /mnt
> >>> which succeeded and then I did "mount" which displays:
> >>> 192.168.1.106:/nfsshare on /mnt type nfs4
> >>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
> >>> Naturally no TLS happens for rdma but I'm not sure if this mount
> >>> should fail? or If the "mount" afterwards shouldn't display/claim that
> >>> it's doing xprtsec=tls?
> >>
> >> That mount request should have failed.
> >
> > So perhaps something like this:
> >
> > diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> > index e49e3d18ef88..5fc229a7690b 100644
> > --- a/fs/nfs/fs_context.c
> > +++ b/fs/nfs/fs_context.c
> > @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
> > fs_context *fc,
> >                        goto out_invalid_transport_udp;
> >                break;
> >        case XPRT_TRANSPORT_TCP:
> > +               break;
> >        case XPRT_TRANSPORT_RDMA:
> > +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
> > +                       goto out_invalid_transport_rdma;
> >                break;
> >        default:
> >                ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> > @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
> > fs_context *fc,
> >        return 0;
> > out_invalid_transport_udp:
> >        return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> > +out_invalid_transport_rdma:
> > +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
> > }
> >
> > [aglo@unknown000c29dba616 ~]$sudo mount -o
> > vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> > 192.168.1.106:/nfsshare /mnt
> > mount.nfs: an incorrect mount option was specified
> >
> > I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
> > the "an incorrect mount option was specified". But if I try to do
> > "proto=udp" I don't see the "unsupported transport protocol udp" so
> > I'm not sure what this nfs_invalf() is supposed to do.
>
> Architecturally, the NFS client should pass these option
> settings down to RPC and let the transport tell it that
> TLS isn't available.
>
> I thought RDMA was missing the functions that marked it
> as supporting TLS, and so it should have failed the mount.
> I must have missed something.

Ah, ok.

So in that case here's another (failure) one for you --- mount over
UDP with xprtsec=tls.

[aglo@unknown000c29dba616 ~]$ sudo mount -o
vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt

192.168.1.125:/vol1 on /mnt type nfs
(rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)

>
> --
> Chuck Lever
>
>

Re: what should happen for proto=rdma,xprtsec=tls

[Chuck Lever III]

> On May 31, 2023, at 10:38 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
> 
> On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>> 
>>> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>> 
>>>>> Hi Chuck,
>>>>> 
>>>>> I did
>>>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>>>> 192.168.1.106:/nfsshare /mnt
>>>>> which succeeded and then I did "mount" which displays:
>>>>> 192.168.1.106:/nfsshare on /mnt type nfs4
>>>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
>>>>> Naturally no TLS happens for rdma but I'm not sure if this mount
>>>>> should fail? or If the "mount" afterwards shouldn't display/claim that
>>>>> it's doing xprtsec=tls?
>>>> 
>>>> That mount request should have failed.
>>> 
>>> So perhaps something like this:
>>> 
>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>>> index e49e3d18ef88..5fc229a7690b 100644
>>> --- a/fs/nfs/fs_context.c
>>> +++ b/fs/nfs/fs_context.c
>>> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
>>> fs_context *fc,
>>>                       goto out_invalid_transport_udp;
>>>               break;
>>>       case XPRT_TRANSPORT_TCP:
>>> +               break;
>>>       case XPRT_TRANSPORT_RDMA:
>>> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
>>> +                       goto out_invalid_transport_rdma;
>>>               break;
>>>       default:
>>>               ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
>>> fs_context *fc,
>>>       return 0;
>>> out_invalid_transport_udp:
>>>       return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
>>> +out_invalid_transport_rdma:
>>> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
>>> }
>>> 
>>> [aglo@unknown000c29dba616 ~]$sudo mount -o
>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>> 192.168.1.106:/nfsshare /mnt
>>> mount.nfs: an incorrect mount option was specified
>>> 
>>> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
>>> the "an incorrect mount option was specified". But if I try to do
>>> "proto=udp" I don't see the "unsupported transport protocol udp" so
>>> I'm not sure what this nfs_invalf() is supposed to do.
>> 
>> Architecturally, the NFS client should pass these option
>> settings down to RPC and let the transport tell it that
>> TLS isn't available.
>> 
>> I thought RDMA was missing the functions that marked it
>> as supporting TLS, and so it should have failed the mount.
>> I must have missed something.
> 
> Ah, ok.
> 
> So in that case here's another (failure) one for you --- mount over
> UDP with xprtsec=tls.
> 
> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt
> 
> 192.168.1.125:/vol1 on /mnt type nfs
> (rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)

If we ever add DTLS support, that should work. It won't today,
though, and the mount should fail. If you ask for xprtsec=tls
and the transport cannot provide it, the mount needs to fail.

I'm going to have to rewrite all of this due to Trond's most
recent review comment, so I'll be sure to try the other
transports before shipping the next version.


--
Chuck Lever

Re: what should happen for proto=rdma,xprtsec=tls

[Chuck Lever III]

> On May 31, 2023, at 2:55 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On May 31, 2023, at 10:38 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
>> 
>> On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>> 
>>> 
>>> 
>>>> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>>> 
>>>> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>>> 
>>>>>> Hi Chuck,
>>>>>> 
>>>>>> I did
>>>>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>>>>> 192.168.1.106:/nfsshare /mnt
>>>>>> which succeeded and then I did "mount" which displays:
>>>>>> 192.168.1.106:/nfsshare on /mnt type nfs4
>>>>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
>>>>>> Naturally no TLS happens for rdma but I'm not sure if this mount
>>>>>> should fail? or If the "mount" afterwards shouldn't display/claim that
>>>>>> it's doing xprtsec=tls?
>>>>> 
>>>>> That mount request should have failed.
>>>> 
>>>> So perhaps something like this:
>>>> 
>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>>>> index e49e3d18ef88..5fc229a7690b 100644
>>>> --- a/fs/nfs/fs_context.c
>>>> +++ b/fs/nfs/fs_context.c
>>>> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
>>>> fs_context *fc,
>>>>                      goto out_invalid_transport_udp;
>>>>              break;
>>>>      case XPRT_TRANSPORT_TCP:
>>>> +               break;
>>>>      case XPRT_TRANSPORT_RDMA:
>>>> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
>>>> +                       goto out_invalid_transport_rdma;
>>>>              break;
>>>>      default:
>>>>              ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
>>>> fs_context *fc,
>>>>      return 0;
>>>> out_invalid_transport_udp:
>>>>      return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
>>>> +out_invalid_transport_rdma:
>>>> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
>>>> }
>>>> 
>>>> [aglo@unknown000c29dba616 ~]$sudo mount -o
>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>>> 192.168.1.106:/nfsshare /mnt
>>>> mount.nfs: an incorrect mount option was specified
>>>> 
>>>> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
>>>> the "an incorrect mount option was specified". But if I try to do
>>>> "proto=udp" I don't see the "unsupported transport protocol udp" so
>>>> I'm not sure what this nfs_invalf() is supposed to do.
>>> 
>>> Architecturally, the NFS client should pass these option
>>> settings down to RPC and let the transport tell it that
>>> TLS isn't available.
>>> 
>>> I thought RDMA was missing the functions that marked it
>>> as supporting TLS, and so it should have failed the mount.
>>> I must have missed something.
>> 
>> Ah, ok.
>> 
>> So in that case here's another (failure) one for you --- mount over
>> UDP with xprtsec=tls.
>> 
>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>> vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt
>> 
>> 192.168.1.125:/vol1 on /mnt type nfs
>> (rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)
> 
> If we ever add DTLS support, that should work. It won't today,
> though, and the mount should fail. If you ask for xprtsec=tls
> and the transport cannot provide it, the mount needs to fail.
> 
> I'm going to have to rewrite all of this due to Trond's most
> recent review comment, so I'll be sure to try the other
> transports before shipping the next version.

To support using an xprt_class as Trond suggested, the NFS
client will have to check the specified combination of netid
and xprtsec policy, just as you suggested. Testing now.


--
Chuck Lever

Re: what should happen for proto=rdma,xprtsec=tls

[Chuck Lever III]

> On Jun 1, 2023, at 3:28 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
>> On May 31, 2023, at 2:55 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
>> 
>>> On May 31, 2023, at 10:38 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>> 
>>> On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>>> 
>>>>> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>> 
>>>>> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>>>>> 
>>>>>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>>>> 
>>>>>>> Hi Chuck,
>>>>>>> 
>>>>>>> I did
>>>>>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>>>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>>>>>> 192.168.1.106:/nfsshare /mnt
>>>>>>> which succeeded and then I did "mount" which displays:
>>>>>>> 192.168.1.106:/nfsshare on /mnt type nfs4
>>>>>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
>>>>>>> Naturally no TLS happens for rdma but I'm not sure if this mount
>>>>>>> should fail? or If the "mount" afterwards shouldn't display/claim that
>>>>>>> it's doing xprtsec=tls?
>>>>>> 
>>>>>> That mount request should have failed.
>>>>> 
>>>>> So perhaps something like this:
>>>>> 
>>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>>>>> index e49e3d18ef88..5fc229a7690b 100644
>>>>> --- a/fs/nfs/fs_context.c
>>>>> +++ b/fs/nfs/fs_context.c
>>>>> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
>>>>> fs_context *fc,
>>>>>                     goto out_invalid_transport_udp;
>>>>>             break;
>>>>>     case XPRT_TRANSPORT_TCP:
>>>>> +               break;
>>>>>     case XPRT_TRANSPORT_RDMA:
>>>>> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
>>>>> +                       goto out_invalid_transport_rdma;
>>>>>             break;
>>>>>     default:
>>>>>             ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>>> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
>>>>> fs_context *fc,
>>>>>     return 0;
>>>>> out_invalid_transport_udp:
>>>>>     return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
>>>>> +out_invalid_transport_rdma:
>>>>> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
>>>>> }
>>>>> 
>>>>> [aglo@unknown000c29dba616 ~]$sudo mount -o
>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
>>>>> 192.168.1.106:/nfsshare /mnt
>>>>> mount.nfs: an incorrect mount option was specified
>>>>> 
>>>>> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
>>>>> the "an incorrect mount option was specified". But if I try to do
>>>>> "proto=udp" I don't see the "unsupported transport protocol udp" so
>>>>> I'm not sure what this nfs_invalf() is supposed to do.
>>>> 
>>>> Architecturally, the NFS client should pass these option
>>>> settings down to RPC and let the transport tell it that
>>>> TLS isn't available.
>>>> 
>>>> I thought RDMA was missing the functions that marked it
>>>> as supporting TLS, and so it should have failed the mount.
>>>> I must have missed something.
>>> 
>>> Ah, ok.
>>> 
>>> So in that case here's another (failure) one for you --- mount over
>>> UDP with xprtsec=tls.
>>> 
>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
>>> vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt
>>> 
>>> 192.168.1.125:/vol1 on /mnt type nfs
>>> (rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)
>> 
>> If we ever add DTLS support, that should work. It won't today,
>> though, and the mount should fail. If you ask for xprtsec=tls
>> and the transport cannot provide it, the mount needs to fail.
>> 
>> I'm going to have to rewrite all of this due to Trond's most
>> recent review comment, so I'll be sure to try the other
>> transports before shipping the next version.
> 
> To support using an xprt_class as Trond suggested, the NFS
> client will have to check the specified combination of netid
> and xprtsec policy, just as you suggested. Testing now.

I've pushed an update to the topic-rpc-with-tls-upcall branch in:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git

The update attempts to address Trond's request for a new
xprt_class for RPC on TCP with TLS.

Please try it out. I'd like to post v4 of this series next week.


--
Chuck Lever

Re: what should happen for proto=rdma,xprtsec=tls

[Olga Kornievskaia]

On Fri, Jun 2, 2023 at 10:11 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
> > On Jun 1, 2023, at 3:28 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> >
> >> On May 31, 2023, at 2:55 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> >>
> >>> On May 31, 2023, at 10:38 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >>>
> >>> On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
> >>>>
> >>>>> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >>>>>
> >>>>> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
> >>>>>>
> >>>>>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> >>>>>>>
> >>>>>>> Hi Chuck,
> >>>>>>>
> >>>>>>> I did
> >>>>>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> >>>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> >>>>>>> 192.168.1.106:/nfsshare /mnt
> >>>>>>> which succeeded and then I did "mount" which displays:
> >>>>>>> 192.168.1.106:/nfsshare on /mnt type nfs4
> >>>>>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
> >>>>>>> Naturally no TLS happens for rdma but I'm not sure if this mount
> >>>>>>> should fail? or If the "mount" afterwards shouldn't display/claim that
> >>>>>>> it's doing xprtsec=tls?
> >>>>>>
> >>>>>> That mount request should have failed.
> >>>>>
> >>>>> So perhaps something like this:
> >>>>>
> >>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> >>>>> index e49e3d18ef88..5fc229a7690b 100644
> >>>>> --- a/fs/nfs/fs_context.c
> >>>>> +++ b/fs/nfs/fs_context.c
> >>>>> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
> >>>>> fs_context *fc,
> >>>>>                     goto out_invalid_transport_udp;
> >>>>>             break;
> >>>>>     case XPRT_TRANSPORT_TCP:
> >>>>> +               break;
> >>>>>     case XPRT_TRANSPORT_RDMA:
> >>>>> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
> >>>>> +                       goto out_invalid_transport_rdma;
> >>>>>             break;
> >>>>>     default:
> >>>>>             ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> >>>>> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
> >>>>> fs_context *fc,
> >>>>>     return 0;
> >>>>> out_invalid_transport_udp:
> >>>>>     return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> >>>>> +out_invalid_transport_rdma:
> >>>>> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
> >>>>> }
> >>>>>
> >>>>> [aglo@unknown000c29dba616 ~]$sudo mount -o
> >>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> >>>>> 192.168.1.106:/nfsshare /mnt
> >>>>> mount.nfs: an incorrect mount option was specified
> >>>>>
> >>>>> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
> >>>>> the "an incorrect mount option was specified". But if I try to do
> >>>>> "proto=udp" I don't see the "unsupported transport protocol udp" so
> >>>>> I'm not sure what this nfs_invalf() is supposed to do.
> >>>>
> >>>> Architecturally, the NFS client should pass these option
> >>>> settings down to RPC and let the transport tell it that
> >>>> TLS isn't available.
> >>>>
> >>>> I thought RDMA was missing the functions that marked it
> >>>> as supporting TLS, and so it should have failed the mount.
> >>>> I must have missed something.
> >>>
> >>> Ah, ok.
> >>>
> >>> So in that case here's another (failure) one for you --- mount over
> >>> UDP with xprtsec=tls.
> >>>
> >>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> >>> vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt
> >>>
> >>> 192.168.1.125:/vol1 on /mnt type nfs
> >>> (rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)
> >>
> >> If we ever add DTLS support, that should work. It won't today,
> >> though, and the mount should fail. If you ask for xprtsec=tls
> >> and the transport cannot provide it, the mount needs to fail.
> >>
> >> I'm going to have to rewrite all of this due to Trond's most
> >> recent review comment, so I'll be sure to try the other
> >> transports before shipping the next version.
> >
> > To support using an xprt_class as Trond suggested, the NFS
> > client will have to check the specified combination of netid
> > and xprtsec policy, just as you suggested. Testing now.
>
> I've pushed an update to the topic-rpc-with-tls-upcall branch in:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> The update attempts to address Trond's request for a new
> xprt_class for RPC on TCP with TLS.
>
> Please try it out. I'd like to post v4 of this series next week.

Will do.

>
>
> --
> Chuck Lever
>
>

Re: what should happen for proto=rdma,xprtsec=tls

[Olga Kornievskaia]

On Fri, Jun 2, 2023 at 12:13 PM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Fri, Jun 2, 2023 at 10:11 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
> >
> >
> > > On Jun 1, 2023, at 3:28 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> > >
> > >> On May 31, 2023, at 2:55 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> > >>
> > >>> On May 31, 2023, at 10:38 AM, Olga Kornievskaia <aglo@umich.edu> wrote:
> > >>>
> > >>> On Wed, May 31, 2023 at 10:03 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
> > >>>>
> > >>>>> On May 30, 2023, at 5:29 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> > >>>>>
> > >>>>> On Tue, May 30, 2023 at 5:10 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
> > >>>>>>
> > >>>>>>> On May 30, 2023, at 5:08 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> > >>>>>>>
> > >>>>>>> Hi Chuck,
> > >>>>>>>
> > >>>>>>> I did
> > >>>>>>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> > >>>>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> > >>>>>>> 192.168.1.106:/nfsshare /mnt
> > >>>>>>> which succeeded and then I did "mount" which displays:
> > >>>>>>> 192.168.1.106:/nfsshare on /mnt type nfs4
> > >>>>>>> (rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=rdma,port=20049,timeo=600,retrans=2,sec=sys,xprtsec=tls,clientaddr=192.168.1.119,local_lock=none,addr=192.168.1.106)
> > >>>>>>> Naturally no TLS happens for rdma but I'm not sure if this mount
> > >>>>>>> should fail? or If the "mount" afterwards shouldn't display/claim that
> > >>>>>>> it's doing xprtsec=tls?
> > >>>>>>
> > >>>>>> That mount request should have failed.
> > >>>>>
> > >>>>> So perhaps something like this:
> > >>>>>
> > >>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> > >>>>> index e49e3d18ef88..5fc229a7690b 100644
> > >>>>> --- a/fs/nfs/fs_context.c
> > >>>>> +++ b/fs/nfs/fs_context.c
> > >>>>> @@ -334,7 +334,10 @@ static int nfs_validate_transport_protocol(struct
> > >>>>> fs_context *fc,
> > >>>>>                     goto out_invalid_transport_udp;
> > >>>>>             break;
> > >>>>>     case XPRT_TRANSPORT_TCP:
> > >>>>> +               break;
> > >>>>>     case XPRT_TRANSPORT_RDMA:
> > >>>>> +               if (ctx->xprtsec.policy != RPC_XPRTSEC_NONE)
> > >>>>> +                       goto out_invalid_transport_rdma;
> > >>>>>             break;
> > >>>>>     default:
> > >>>>>             ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> > >>>>> @@ -342,6 +345,8 @@ static int nfs_validate_transport_protocol(struct
> > >>>>> fs_context *fc,
> > >>>>>     return 0;
> > >>>>> out_invalid_transport_udp:
> > >>>>>     return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> > >>>>> +out_invalid_transport_rdma:
> > >>>>> +       return nfs_invalf(fc, "NFS: Unsupported xprtsec option for rdma");
> > >>>>> }
> > >>>>>
> > >>>>> [aglo@unknown000c29dba616 ~]$sudo mount -o
> > >>>>> vers=4.2,sec=sys,xprtsec=tls,proto=rdma,port=20049
> > >>>>> 192.168.1.106:/nfsshare /mnt
> > >>>>> mount.nfs: an incorrect mount option was specified
> > >>>>>
> > >>>>> I was expecting to see "NFS: Unsupported xprtsec option for rdma" not
> > >>>>> the "an incorrect mount option was specified". But if I try to do
> > >>>>> "proto=udp" I don't see the "unsupported transport protocol udp" so
> > >>>>> I'm not sure what this nfs_invalf() is supposed to do.
> > >>>>
> > >>>> Architecturally, the NFS client should pass these option
> > >>>> settings down to RPC and let the transport tell it that
> > >>>> TLS isn't available.
> > >>>>
> > >>>> I thought RDMA was missing the functions that marked it
> > >>>> as supporting TLS, and so it should have failed the mount.
> > >>>> I must have missed something.
> > >>>
> > >>> Ah, ok.
> > >>>
> > >>> So in that case here's another (failure) one for you --- mount over
> > >>> UDP with xprtsec=tls.
> > >>>
> > >>> [aglo@unknown000c29dba616 ~]$ sudo mount -o
> > >>> vers=3,sec=sys,proto=udp,xprtsec=tls 192.168.1.125:/vol1 /mnt
> > >>>
> > >>> 192.168.1.125:/vol1 on /mnt type nfs
> > >>> (rw,relatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=udp,timeo=11,retrans=3,sec=sys,xprtsec=tls,mountaddr=192.168.1.125,mountvers=3,mountport=635,mountproto=udp,local_lock=none,addr=192.168.1.125)
> > >>
> > >> If we ever add DTLS support, that should work. It won't today,
> > >> though, and the mount should fail. If you ask for xprtsec=tls
> > >> and the transport cannot provide it, the mount needs to fail.
> > >>
> > >> I'm going to have to rewrite all of this due to Trond's most
> > >> recent review comment, so I'll be sure to try the other
> > >> transports before shipping the next version.
> > >
> > > To support using an xprt_class as Trond suggested, the NFS
> > > client will have to check the specified combination of netid
> > > and xprtsec policy, just as you suggested. Testing now.
> >
> > I've pushed an update to the topic-rpc-with-tls-upcall branch in:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
> >
> > The update attempts to address Trond's request for a new
> > xprt_class for RPC on TCP with TLS.
> >
> > Please try it out. I'd like to post v4 of this series next week.
>
> Will do.

I can confirm that mount with udp+tls and rdma+tls fail with
"mount.nfs: an incorrect mount option was specified".

> >
> >
> > --
> > Chuck Lever
> >
> >