Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT

[Chuck Lever III <chuck.lever@oracle.com>]

> On Oct 6, 2024, at 6:29 PM, Pali Rohár <pali@kernel.org> wrote:
> 
> On Monday 07 October 2024 09:13:17 NeilBrown wrote:
>> On Mon, 07 Oct 2024, Chuck Lever wrote:
>>> On Fri, Sep 13, 2024 at 08:52:20AM +1000, NeilBrown wrote:
>>>> On Fri, 13 Sep 2024, Pali Rohár wrote:
>>>>> Currently NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT do not bypass
>>>>> only GSS, but bypass any authentication method. This is problem specially
>>>>> for NFS3 AUTH_NULL-only exports.
>>>>> 
>>>>> The purpose of NFSD_MAY_BYPASS_GSS_ON_ROOT is described in RFC 2623,
>>>>> section 2.3.2, to allow mounting NFS2/3 GSS-only export without
>>>>> authentication. So few procedures which do not expose security risk used
>>>>> during mount time can be called also with AUTH_NONE or AUTH_SYS, to allow
>>>>> client mount operation to finish successfully.
>>>>> 
>>>>> The problem with current implementation is that for AUTH_NULL-only exports,
>>>>> the NFSD_MAY_BYPASS_GSS_ON_ROOT is active also for NFS3 AUTH_UNIX mount
>>>>> attempts which confuse NFS3 clients, and make them think that AUTH_UNIX is
>>>>> enabled and is working. Linux NFS3 client never switches from AUTH_UNIX to
>>>>> AUTH_NONE on active mount, which makes the mount inaccessible.
>>>>> 
>>>>> Fix the NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT implementation
>>>>> and really allow to bypass only exports which have some GSS auth flavor
>>>>> enabled.
>>>>> 
>>>>> The result would be: For AUTH_NULL-only export if client attempts to do
>>>>> mount with AUTH_UNIX flavor then it will receive access errors, which
>>>>> instruct client that AUTH_UNIX flavor is not usable and will either try
>>>>> other auth flavor (AUTH_NULL if enabled) or fails mount procedure.
>>>>> 
>>>>> This should fix problems with AUTH_NULL-only or AUTH_UNIX-only exports if
>>>>> client attempts to mount it with other auth flavor (e.g. with AUTH_NULL for
>>>>> AUTH_UNIX-only export, or with AUTH_UNIX for AUTH_NULL-only export).
>>>> 
>>>> The MAY_BYPASS_GSS flag currently also bypasses TLS restrictions.  With
>>>> your change it doesn't.  I don't think we want to make that change.
>>> 
>>> Neil, I'm not seeing this, I must be missing something.
>>> 
>>> RPC_AUTH_TLS is used only on NULL procedures.
>>> 
>>> The export's xprtsec= setting determines whether a TLS session must
>>> be present to access the files on the export. If the TLS session
>>> meets the xprtsec= policy, then the normal user authentication
>>> settings apply. In other words, I don't think execution gets close
>>> to check_nfsd_access() unless the xprtsec policy setting is met.
>> 
>> check_nfsd_access() is literally the ONLY place that ->ex_xprtsec_modes
>> is tested and that seems to be where xprtsec= export settings are stored.
>> 
>>> 
>>> I'm not convinced check_nfsd_access() needs to care about
>>> RPC_AUTH_TLS. Can you expand a little on your concern?
>> 
>> Probably it doesn't care about RPC_AUTH_TLS which as you say is only
>> used on NULL procedures when setting up the TLS connection.
>> 
>> But it *does* care about NFS_XPRTSEC_MTLS etc.
>> 
>> But I now see that RPC_AUTH_TLS is never reported by OP_SECINFO as an
>> acceptable flavour, so the client cannot dynamically determine that TLS
>> is required.
> 
> Why is not RPC_AUTH_TLS announced in NFS4 OP_SECINFO? Should not NFS4
> OP_SECINFO report all possible auth methods for particular filehandle?

SECINFO reports user authentication flavors and pseudoflavors.

RPC_AUTH_TLS is not a user authentication flavor, it is merely
a query to see if the server peer supports RPC-with-TLS.

So far the nfsv4 WG has not been able to come to consensus
about how a server's transport layer security policies should
be reported to clients. There does not seem to be a clean way
to do that with existing NFSv4 protocol elements, so a
protocol extension might be needed.


>> So there is no value in giving non-tls clients access to
>> xprtsec=mtls exports so they can discover that for themselves.  The
>> client needs to explicitly mount with tls, or possibly the client can
>> opportunistically try TLS in every case, and call back.
>> 
>> So the original patch is OK.
>> 
>> NeilBrown


--
Chuck Lever