linux-kernel - Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZwcSC4ZWihv/PyV2@tissot.1015granger.net>
Date: Wed, 9 Oct 2024 19:30:19 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: NeilBrown <neilb@...e.de>
Cc: Pali Rohár <pali@...nel.org>,
        Jeff Layton <jlayton@...nel.org>,
        Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <dai.ngo@...cle.com>,
        Tom Talpey <tom@...pey.com>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and
 NFSD_MAY_BYPASS_GSS_ON_ROOT

On Thu, Oct 10, 2024 at 07:14:07AM +1100, NeilBrown wrote:
> On Thu, 10 Oct 2024, Chuck Lever wrote:
> > On Tue, Oct 08, 2024 at 05:47:55PM -0400, NeilBrown wrote:
> > > And NFSD_MAY_LOCK should be discarded, and nlm_fopen() should set
> > > NFSD_MAY_BYPASS_SEC.
> > 
> > 366         /*                                                                      
> > 367          * pseudoflavor restrictions are not enforced on NLM,                   
> > 
> > Wrt the mention of "NLM", nfsd4_lock() also sets NFSD_MAY_LOCK.
> 
> True, but it shouldn't.  NFSD_MAY_LOCK is only used to bypass the GSS
> requirement.  It must have been copied into nfsd4_lock() without a full
> understanding of its purpose.

nfsd4_lock()'s use of MAY_LOCK goes back before the git era, so it's
difficult to say with certainty.

I would like to keep such subtle changes bisectable. To me, it seems
like it would be a basic first step to change the fh_verify() call
in nfsd4_lock() to use (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE)
instead of NFSD_MAY_LOCK, as a separate patch.


> > 368          * which clients virtually always use auth_sys for,                     
> > 369          * even while using RPCSEC_GSS for NFS.                                 
> > 370          */                                                                     
> > 371         if (access & NFSD_MAY_LOCK)                                             
> > 372                 goto skip_pseudoflavor_check;                                   
> > 373         if (access & NFSD_MAY_BYPASS_GSS)                                       
> > 374                 may_bypass_gss = true;
> > 375         /*                                                                      
> > 376          * Clients may expect to be able to use auth_sys during mount,          
> > 377          * even if they use gss for everything else; see section 2.3.2          
> > 378          * of rfc 2623.                                                         
> > 379          */                                                                     
> > 380         if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT                                
> > 381                         && exp->ex_path.dentry == dentry)                       
> > 382                 may_bypass_gss = true;                                          
> > 383                                                                                 
> > 384         error = check_nfsd_access(exp, rqstp, may_bypass_gss);                  
> > 385         if (error)                                                              
> > 386                 goto out;                                                       
> > 387                                                                                 
> > 388 skip_pseudoflavor_check:                                                        
> > 389         /* Finally, check access permissions. */                                
> > 390         error = nfsd_permission(cred, exp, dentry, access);     
> > 
> > MAY_LOCK is checked in nfsd_permission() and __fh_verify().
> > 
> > But MAY_BYPASS_GSS is set in loads of places that use those two
> > functions. How can we be certain that the two flags are equivalent? 
> 
> We can be certain by looking at the effect.  Before a recent patch they
> both did "goto skip_pseudoflavor_check" and nothing else.

I'm still not convinced MAY_LOCK and MAY_BYPASS_GSS are 100%
equivalent.  nfsd_permission() checks for MAY_LOCK, but does not
check for MAY_BYPASS_GSS:

        if (acc & NFSD_MAY_LOCK) {
                /* If we cannot rely on authentication in NLM requests,
                 * just allow locks, otherwise require read permission, or
                 * ownership
                 */
                if (exp->ex_flags & NFSEXP_NOAUTHNLM)
                        return 0;
                else 
                        acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
        } 

The only consumer of MAY_BYPASS_GSS seems to be OP_PUTFH, now that
I'm looking closely for it. But I don't think we want the
no_auth_nlm export option to modify the way PUTFH behaves.


-- 
Chuck Lever