[PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT

Pali Rohár posted 1 patch 2 months, 2 weeks ago
There is a newer version of this series
fs/nfsd/export.c   | 19 ++++++++++++++++++-
fs/nfsd/export.h   |  2 +-
fs/nfsd/nfs4proc.c |  2 +-
fs/nfsd/nfs4xdr.c  |  2 +-
fs/nfsd/nfsfh.c    | 12 +++++++++---
fs/nfsd/vfs.c      |  2 +-
6 files changed, 31 insertions(+), 8 deletions(-)
[PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Pali Rohár 2 months, 2 weeks ago
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).

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/nfsd/export.c   | 19 ++++++++++++++++++-
 fs/nfsd/export.h   |  2 +-
 fs/nfsd/nfs4proc.c |  2 +-
 fs/nfsd/nfs4xdr.c  |  2 +-
 fs/nfsd/nfsfh.c    | 12 +++++++++---
 fs/nfsd/vfs.c      |  2 +-
 6 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 50b3135d07ac..eb11d3fdffe1 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
 	return exp;
 }
 
-__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
+__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
 {
 	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
 	struct svc_xprt *xprt = rqstp->rq_xprt;
@@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 	if (nfsd4_spo_must_allow(rqstp))
 		return 0;
 
+	/* Some calls may be processed without authentication
+	 * on GSS exports. For example NFS2/3 calls on root
+	 * directory, see section 2.3.2 of rfc 2623.
+	 * For "may_bypass_gss" check that export has really
+	 * enabled some GSS flavor and also check that the
+	 * used auth flavor is without auth (none or sys).
+	 */
+	if (may_bypass_gss && (
+	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
+	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
+		for (f = exp->ex_flavors; f < end; f++) {
+			if (f->pseudoflavor == RPC_AUTH_GSS ||
+			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
+				return 0;
+		}
+	}
+
 denied:
 	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
 }
diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
index ca9dc230ae3d..dc7cf4f6ac53 100644
--- a/fs/nfsd/export.h
+++ b/fs/nfsd/export.h
@@ -100,7 +100,7 @@ struct svc_expkey {
 #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
 
 int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
-__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
+__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
 
 /*
  * Function declarations
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2e39cf2e502a..0f67f4a7b8b2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 
 			if (current_fh->fh_export &&
 					need_wrongsec_check(rqstp))
-				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
+				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
 		}
 encode_op:
 		if (op->status == nfserr_replay_me) {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 97f583777972..b45ea5757652 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
 			nfserr = nfserrno(err);
 			goto out_put;
 		}
-		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
+		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
 		if (nfserr)
 			goto out_put;
 
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index dd4e11a703aa..ed0eabfa3cb0 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 {
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 	struct svc_export *exp = NULL;
+	bool may_bypass_gss = false;
 	struct dentry	*dentry;
 	__be32		error;
 
@@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 	 * which clients virtually always use auth_sys for,
 	 * even while using RPCSEC_GSS for NFS.
 	 */
-	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
+	if (access & NFSD_MAY_LOCK)
 		goto skip_pseudoflavor_check;
+	/*
+	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
+	 */
+	if (access & NFSD_MAY_BYPASS_GSS)
+		may_bypass_gss = true;
 	/*
 	 * Clients may expect to be able to use auth_sys during mount,
 	 * even if they use gss for everything else; see section 2.3.2
@@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 	 */
 	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
 			&& exp->ex_path.dentry == dentry)
-		goto skip_pseudoflavor_check;
+		may_bypass_gss = true;
 
-	error = check_nfsd_access(exp, rqstp);
+	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
 	if (error)
 		goto out;
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 29b1f3613800..b2f5ea7c2187 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
 	if (err)
 		return err;
-	err = check_nfsd_access(exp, rqstp);
+	err = check_nfsd_access(exp, rqstp, false);
 	if (err)
 		goto out;
 	/*
-- 
2.20.1

Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 2 months, 2 weeks ago
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.

I think that what you want to do makes sense.  Higher security can be
downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
AUTH_UNIX.

Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
for AUTH_UNIX and only if something other than AUTH_NULL is allowed.

Thanks,
NeilBrown



> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/nfsd/export.c   | 19 ++++++++++++++++++-
>  fs/nfsd/export.h   |  2 +-
>  fs/nfsd/nfs4proc.c |  2 +-
>  fs/nfsd/nfs4xdr.c  |  2 +-
>  fs/nfsd/nfsfh.c    | 12 +++++++++---
>  fs/nfsd/vfs.c      |  2 +-
>  6 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 50b3135d07ac..eb11d3fdffe1 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
>  	return exp;
>  }
>  
> -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
>  {
>  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
>  	struct svc_xprt *xprt = rqstp->rq_xprt;
> @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
>  	if (nfsd4_spo_must_allow(rqstp))
>  		return 0;
>  
> +	/* Some calls may be processed without authentication
> +	 * on GSS exports. For example NFS2/3 calls on root
> +	 * directory, see section 2.3.2 of rfc 2623.
> +	 * For "may_bypass_gss" check that export has really
> +	 * enabled some GSS flavor and also check that the
> +	 * used auth flavor is without auth (none or sys).
> +	 */
> +	if (may_bypass_gss && (
> +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> +		for (f = exp->ex_flavors; f < end; f++) {
> +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> +				return 0;
> +		}
> +	}
> +
>  denied:
>  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
>  }
> diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> index ca9dc230ae3d..dc7cf4f6ac53 100644
> --- a/fs/nfsd/export.h
> +++ b/fs/nfsd/export.h
> @@ -100,7 +100,7 @@ struct svc_expkey {
>  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
>  
>  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
>  
>  /*
>   * Function declarations
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2e39cf2e502a..0f67f4a7b8b2 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
>  
>  			if (current_fh->fh_export &&
>  					need_wrongsec_check(rqstp))
> -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
>  		}
>  encode_op:
>  		if (op->status == nfserr_replay_me) {
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 97f583777972..b45ea5757652 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
>  			nfserr = nfserrno(err);
>  			goto out_put;
>  		}
> -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
>  		if (nfserr)
>  			goto out_put;
>  
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index dd4e11a703aa..ed0eabfa3cb0 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
>  {
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>  	struct svc_export *exp = NULL;
> +	bool may_bypass_gss = false;
>  	struct dentry	*dentry;
>  	__be32		error;
>  
> @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
>  	 * which clients virtually always use auth_sys for,
>  	 * even while using RPCSEC_GSS for NFS.
>  	 */
> -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> +	if (access & NFSD_MAY_LOCK)
>  		goto skip_pseudoflavor_check;
> +	/*
> +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> +	 */
> +	if (access & NFSD_MAY_BYPASS_GSS)
> +		may_bypass_gss = true;
>  	/*
>  	 * Clients may expect to be able to use auth_sys during mount,
>  	 * even if they use gss for everything else; see section 2.3.2
> @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
>  	 */
>  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
>  			&& exp->ex_path.dentry == dentry)
> -		goto skip_pseudoflavor_check;
> +		may_bypass_gss = true;
>  
> -	error = check_nfsd_access(exp, rqstp);
> +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
>  	if (error)
>  		goto out;
>  
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 29b1f3613800..b2f5ea7c2187 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
>  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
>  	if (err)
>  		return err;
> -	err = check_nfsd_access(exp, rqstp);
> +	err = check_nfsd_access(exp, rqstp, false);
>  	if (err)
>  		goto out;
>  	/*
> -- 
> 2.20.1
> 
> 
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever 1 month, 3 weeks ago
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.

I'm not convinced check_nfsd_access() needs to care about
RPC_AUTH_TLS. Can you expand a little on your concern?


> I think that what you want to do makes sense.  Higher security can be
> downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
> AUTH_UNIX.
> 
> Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
> for AUTH_UNIX and only if something other than AUTH_NULL is allowed.
> 
> Thanks,
> NeilBrown
> 
> 
> 
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  fs/nfsd/export.c   | 19 ++++++++++++++++++-
> >  fs/nfsd/export.h   |  2 +-
> >  fs/nfsd/nfs4proc.c |  2 +-
> >  fs/nfsd/nfs4xdr.c  |  2 +-
> >  fs/nfsd/nfsfh.c    | 12 +++++++++---
> >  fs/nfsd/vfs.c      |  2 +-
> >  6 files changed, 31 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > index 50b3135d07ac..eb11d3fdffe1 100644
> > --- a/fs/nfsd/export.c
> > +++ b/fs/nfsd/export.c
> > @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> >  	return exp;
> >  }
> >  
> > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
> >  {
> >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> >  	struct svc_xprt *xprt = rqstp->rq_xprt;
> > @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> >  	if (nfsd4_spo_must_allow(rqstp))
> >  		return 0;
> >  
> > +	/* Some calls may be processed without authentication
> > +	 * on GSS exports. For example NFS2/3 calls on root
> > +	 * directory, see section 2.3.2 of rfc 2623.
> > +	 * For "may_bypass_gss" check that export has really
> > +	 * enabled some GSS flavor and also check that the
> > +	 * used auth flavor is without auth (none or sys).
> > +	 */
> > +	if (may_bypass_gss && (
> > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > +		for (f = exp->ex_flavors; f < end; f++) {
> > +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> > +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> > +				return 0;
> > +		}
> > +	}
> > +
> >  denied:
> >  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
> >  }
> > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> > index ca9dc230ae3d..dc7cf4f6ac53 100644
> > --- a/fs/nfsd/export.h
> > +++ b/fs/nfsd/export.h
> > @@ -100,7 +100,7 @@ struct svc_expkey {
> >  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
> >  
> >  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
> >  
> >  /*
> >   * Function declarations
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 2e39cf2e502a..0f67f4a7b8b2 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
> >  
> >  			if (current_fh->fh_export &&
> >  					need_wrongsec_check(rqstp))
> > -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> > +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
> >  		}
> >  encode_op:
> >  		if (op->status == nfserr_replay_me) {
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 97f583777972..b45ea5757652 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
> >  			nfserr = nfserrno(err);
> >  			goto out_put;
> >  		}
> > -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> > +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
> >  		if (nfserr)
> >  			goto out_put;
> >  
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index dd4e11a703aa..ed0eabfa3cb0 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  {
> >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> >  	struct svc_export *exp = NULL;
> > +	bool may_bypass_gss = false;
> >  	struct dentry	*dentry;
> >  	__be32		error;
> >  
> > @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  	 * which clients virtually always use auth_sys for,
> >  	 * even while using RPCSEC_GSS for NFS.
> >  	 */
> > -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> > +	if (access & NFSD_MAY_LOCK)
> >  		goto skip_pseudoflavor_check;
> > +	/*
> > +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> > +	 */
> > +	if (access & NFSD_MAY_BYPASS_GSS)
> > +		may_bypass_gss = true;
> >  	/*
> >  	 * Clients may expect to be able to use auth_sys during mount,
> >  	 * even if they use gss for everything else; see section 2.3.2
> > @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  	 */
> >  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
> >  			&& exp->ex_path.dentry == dentry)
> > -		goto skip_pseudoflavor_check;
> > +		may_bypass_gss = true;
> >  
> > -	error = check_nfsd_access(exp, rqstp);
> > +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
> >  	if (error)
> >  		goto out;
> >  
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 29b1f3613800..b2f5ea7c2187 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
> >  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
> >  	if (err)
> >  		return err;
> > -	err = check_nfsd_access(exp, rqstp);
> > +	err = check_nfsd_access(exp, rqstp, false);
> >  	if (err)
> >  		goto out;
> >  	/*
> > -- 
> > 2.20.1
> > 
> > 
> 

-- 
Chuck Lever
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 1 month, 3 weeks ago
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.  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


> 
> 
> > I think that what you want to do makes sense.  Higher security can be
> > downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
> > AUTH_UNIX.
> > 
> > Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
> > for AUTH_UNIX and only if something other than AUTH_NULL is allowed.
> > 
> > Thanks,
> > NeilBrown
> > 
> > 
> > 
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  fs/nfsd/export.c   | 19 ++++++++++++++++++-
> > >  fs/nfsd/export.h   |  2 +-
> > >  fs/nfsd/nfs4proc.c |  2 +-
> > >  fs/nfsd/nfs4xdr.c  |  2 +-
> > >  fs/nfsd/nfsfh.c    | 12 +++++++++---
> > >  fs/nfsd/vfs.c      |  2 +-
> > >  6 files changed, 31 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > > index 50b3135d07ac..eb11d3fdffe1 100644
> > > --- a/fs/nfsd/export.c
> > > +++ b/fs/nfsd/export.c
> > > @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> > >  	return exp;
> > >  }
> > >  
> > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
> > >  {
> > >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> > >  	struct svc_xprt *xprt = rqstp->rq_xprt;
> > > @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > >  	if (nfsd4_spo_must_allow(rqstp))
> > >  		return 0;
> > >  
> > > +	/* Some calls may be processed without authentication
> > > +	 * on GSS exports. For example NFS2/3 calls on root
> > > +	 * directory, see section 2.3.2 of rfc 2623.
> > > +	 * For "may_bypass_gss" check that export has really
> > > +	 * enabled some GSS flavor and also check that the
> > > +	 * used auth flavor is without auth (none or sys).
> > > +	 */
> > > +	if (may_bypass_gss && (
> > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > +		for (f = exp->ex_flavors; f < end; f++) {
> > > +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> > > +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> > > +				return 0;
> > > +		}
> > > +	}
> > > +
> > >  denied:
> > >  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
> > >  }
> > > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> > > index ca9dc230ae3d..dc7cf4f6ac53 100644
> > > --- a/fs/nfsd/export.h
> > > +++ b/fs/nfsd/export.h
> > > @@ -100,7 +100,7 @@ struct svc_expkey {
> > >  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
> > >  
> > >  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
> > >  
> > >  /*
> > >   * Function declarations
> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > index 2e39cf2e502a..0f67f4a7b8b2 100644
> > > --- a/fs/nfsd/nfs4proc.c
> > > +++ b/fs/nfsd/nfs4proc.c
> > > @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
> > >  
> > >  			if (current_fh->fh_export &&
> > >  					need_wrongsec_check(rqstp))
> > > -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> > > +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
> > >  		}
> > >  encode_op:
> > >  		if (op->status == nfserr_replay_me) {
> > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > index 97f583777972..b45ea5757652 100644
> > > --- a/fs/nfsd/nfs4xdr.c
> > > +++ b/fs/nfsd/nfs4xdr.c
> > > @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
> > >  			nfserr = nfserrno(err);
> > >  			goto out_put;
> > >  		}
> > > -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> > > +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
> > >  		if (nfserr)
> > >  			goto out_put;
> > >  
> > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > index dd4e11a703aa..ed0eabfa3cb0 100644
> > > --- a/fs/nfsd/nfsfh.c
> > > +++ b/fs/nfsd/nfsfh.c
> > > @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > >  {
> > >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > >  	struct svc_export *exp = NULL;
> > > +	bool may_bypass_gss = false;
> > >  	struct dentry	*dentry;
> > >  	__be32		error;
> > >  
> > > @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > >  	 * which clients virtually always use auth_sys for,
> > >  	 * even while using RPCSEC_GSS for NFS.
> > >  	 */
> > > -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> > > +	if (access & NFSD_MAY_LOCK)
> > >  		goto skip_pseudoflavor_check;
> > > +	/*
> > > +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> > > +	 */
> > > +	if (access & NFSD_MAY_BYPASS_GSS)
> > > +		may_bypass_gss = true;
> > >  	/*
> > >  	 * Clients may expect to be able to use auth_sys during mount,
> > >  	 * even if they use gss for everything else; see section 2.3.2
> > > @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > >  	 */
> > >  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
> > >  			&& exp->ex_path.dentry == dentry)
> > > -		goto skip_pseudoflavor_check;
> > > +		may_bypass_gss = true;
> > >  
> > > -	error = check_nfsd_access(exp, rqstp);
> > > +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
> > >  	if (error)
> > >  		goto out;
> > >  
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 29b1f3613800..b2f5ea7c2187 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
> > >  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
> > >  	if (err)
> > >  		return err;
> > > -	err = check_nfsd_access(exp, rqstp);
> > > +	err = check_nfsd_access(exp, rqstp, false);
> > >  	if (err)
> > >  		goto out;
> > >  	/*
> > > -- 
> > > 2.20.1
> > > 
> > > 
> > 
> 
> -- 
> Chuck Lever
> 
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever 1 month, 3 weeks ago
On Mon, Oct 07, 2024 at 09:13:17AM +1100, 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.  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.

May I add "Reviewed-by: NeilBrown <neilb@suse.de>" ?


> NeilBrown
> 
> 
> > 
> > 
> > > I think that what you want to do makes sense.  Higher security can be
> > > downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
> > > AUTH_UNIX.
> > > 
> > > Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
> > > for AUTH_UNIX and only if something other than AUTH_NULL is allowed.
> > > 
> > > Thanks,
> > > NeilBrown
> > > 
> > > 
> > > 
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > ---
> > > >  fs/nfsd/export.c   | 19 ++++++++++++++++++-
> > > >  fs/nfsd/export.h   |  2 +-
> > > >  fs/nfsd/nfs4proc.c |  2 +-
> > > >  fs/nfsd/nfs4xdr.c  |  2 +-
> > > >  fs/nfsd/nfsfh.c    | 12 +++++++++---
> > > >  fs/nfsd/vfs.c      |  2 +-
> > > >  6 files changed, 31 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > > > index 50b3135d07ac..eb11d3fdffe1 100644
> > > > --- a/fs/nfsd/export.c
> > > > +++ b/fs/nfsd/export.c
> > > > @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> > > >  	return exp;
> > > >  }
> > > >  
> > > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
> > > >  {
> > > >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> > > >  	struct svc_xprt *xprt = rqstp->rq_xprt;
> > > > @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > > >  	if (nfsd4_spo_must_allow(rqstp))
> > > >  		return 0;
> > > >  
> > > > +	/* Some calls may be processed without authentication
> > > > +	 * on GSS exports. For example NFS2/3 calls on root
> > > > +	 * directory, see section 2.3.2 of rfc 2623.
> > > > +	 * For "may_bypass_gss" check that export has really
> > > > +	 * enabled some GSS flavor and also check that the
> > > > +	 * used auth flavor is without auth (none or sys).
> > > > +	 */
> > > > +	if (may_bypass_gss && (
> > > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > > +		for (f = exp->ex_flavors; f < end; f++) {
> > > > +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> > > > +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> > > > +				return 0;
> > > > +		}
> > > > +	}
> > > > +
> > > >  denied:
> > > >  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
> > > >  }
> > > > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> > > > index ca9dc230ae3d..dc7cf4f6ac53 100644
> > > > --- a/fs/nfsd/export.h
> > > > +++ b/fs/nfsd/export.h
> > > > @@ -100,7 +100,7 @@ struct svc_expkey {
> > > >  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
> > > >  
> > > >  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> > > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> > > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
> > > >  
> > > >  /*
> > > >   * Function declarations
> > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > > index 2e39cf2e502a..0f67f4a7b8b2 100644
> > > > --- a/fs/nfsd/nfs4proc.c
> > > > +++ b/fs/nfsd/nfs4proc.c
> > > > @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
> > > >  
> > > >  			if (current_fh->fh_export &&
> > > >  					need_wrongsec_check(rqstp))
> > > > -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> > > > +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
> > > >  		}
> > > >  encode_op:
> > > >  		if (op->status == nfserr_replay_me) {
> > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > > index 97f583777972..b45ea5757652 100644
> > > > --- a/fs/nfsd/nfs4xdr.c
> > > > +++ b/fs/nfsd/nfs4xdr.c
> > > > @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
> > > >  			nfserr = nfserrno(err);
> > > >  			goto out_put;
> > > >  		}
> > > > -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> > > > +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
> > > >  		if (nfserr)
> > > >  			goto out_put;
> > > >  
> > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > index dd4e11a703aa..ed0eabfa3cb0 100644
> > > > --- a/fs/nfsd/nfsfh.c
> > > > +++ b/fs/nfsd/nfsfh.c
> > > > @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > >  {
> > > >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > > >  	struct svc_export *exp = NULL;
> > > > +	bool may_bypass_gss = false;
> > > >  	struct dentry	*dentry;
> > > >  	__be32		error;
> > > >  
> > > > @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > >  	 * which clients virtually always use auth_sys for,
> > > >  	 * even while using RPCSEC_GSS for NFS.
> > > >  	 */
> > > > -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> > > > +	if (access & NFSD_MAY_LOCK)
> > > >  		goto skip_pseudoflavor_check;
> > > > +	/*
> > > > +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> > > > +	 */
> > > > +	if (access & NFSD_MAY_BYPASS_GSS)
> > > > +		may_bypass_gss = true;
> > > >  	/*
> > > >  	 * Clients may expect to be able to use auth_sys during mount,
> > > >  	 * even if they use gss for everything else; see section 2.3.2
> > > > @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > >  	 */
> > > >  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
> > > >  			&& exp->ex_path.dentry == dentry)
> > > > -		goto skip_pseudoflavor_check;
> > > > +		may_bypass_gss = true;
> > > >  
> > > > -	error = check_nfsd_access(exp, rqstp);
> > > > +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
> > > >  	if (error)
> > > >  		goto out;
> > > >  
> > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > index 29b1f3613800..b2f5ea7c2187 100644
> > > > --- a/fs/nfsd/vfs.c
> > > > +++ b/fs/nfsd/vfs.c
> > > > @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
> > > >  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
> > > >  	if (err)
> > > >  		return err;
> > > > -	err = check_nfsd_access(exp, rqstp);
> > > > +	err = check_nfsd_access(exp, rqstp, false);
> > > >  	if (err)
> > > >  		goto out;
> > > >  	/*
> > > > -- 
> > > > 2.20.1
> > > > 
> > > > 
> > > 
> > 
> > -- 
> > Chuck Lever
> > 
> 

-- 
Chuck Lever
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 1 month, 3 weeks ago
On Wed, 09 Oct 2024, Chuck Lever wrote:
> On Mon, Oct 07, 2024 at 09:13:17AM +1100, 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.  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.
> 
> May I add "Reviewed-by: NeilBrown <neilb@suse.de>" ?

Yes, though I would prefer v2.

I also would love it if NFSD_MAY_BYPASS_GSS were renamed to
NFSD_MAY_BYPASS_SEC.
And NFSD_MAY_LOCK should be discarded, and nlm_fopen() should set
NFSD_MAY_BYPASS_SEC.
And I wonder if there is really any value in having
NFSD_MAY_BYPASS_GSS_ON_ROOT being separate from NFSD_MAY_BYPASS_GSS.

But I'm not offering patches - at least not today - and those concerns
don't need to block this patch.

Reviewed-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown

> 
> 
> > NeilBrown
> > 
> > 
> > > 
> > > 
> > > > I think that what you want to do makes sense.  Higher security can be
> > > > downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
> > > > AUTH_UNIX.
> > > > 
> > > > Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
> > > > for AUTH_UNIX and only if something other than AUTH_NULL is allowed.
> > > > 
> > > > Thanks,
> > > > NeilBrown
> > > > 
> > > > 
> > > > 
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > ---
> > > > >  fs/nfsd/export.c   | 19 ++++++++++++++++++-
> > > > >  fs/nfsd/export.h   |  2 +-
> > > > >  fs/nfsd/nfs4proc.c |  2 +-
> > > > >  fs/nfsd/nfs4xdr.c  |  2 +-
> > > > >  fs/nfsd/nfsfh.c    | 12 +++++++++---
> > > > >  fs/nfsd/vfs.c      |  2 +-
> > > > >  6 files changed, 31 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > > > > index 50b3135d07ac..eb11d3fdffe1 100644
> > > > > --- a/fs/nfsd/export.c
> > > > > +++ b/fs/nfsd/export.c
> > > > > @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> > > > >  	return exp;
> > > > >  }
> > > > >  
> > > > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > > > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
> > > > >  {
> > > > >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> > > > >  	struct svc_xprt *xprt = rqstp->rq_xprt;
> > > > > @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > > > >  	if (nfsd4_spo_must_allow(rqstp))
> > > > >  		return 0;
> > > > >  
> > > > > +	/* Some calls may be processed without authentication
> > > > > +	 * on GSS exports. For example NFS2/3 calls on root
> > > > > +	 * directory, see section 2.3.2 of rfc 2623.
> > > > > +	 * For "may_bypass_gss" check that export has really
> > > > > +	 * enabled some GSS flavor and also check that the
> > > > > +	 * used auth flavor is without auth (none or sys).
> > > > > +	 */
> > > > > +	if (may_bypass_gss && (
> > > > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > > > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > > > +		for (f = exp->ex_flavors; f < end; f++) {
> > > > > +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> > > > > +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> > > > > +				return 0;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > >  denied:
> > > > >  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
> > > > >  }
> > > > > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> > > > > index ca9dc230ae3d..dc7cf4f6ac53 100644
> > > > > --- a/fs/nfsd/export.h
> > > > > +++ b/fs/nfsd/export.h
> > > > > @@ -100,7 +100,7 @@ struct svc_expkey {
> > > > >  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
> > > > >  
> > > > >  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> > > > > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> > > > > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
> > > > >  
> > > > >  /*
> > > > >   * Function declarations
> > > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > > > index 2e39cf2e502a..0f67f4a7b8b2 100644
> > > > > --- a/fs/nfsd/nfs4proc.c
> > > > > +++ b/fs/nfsd/nfs4proc.c
> > > > > @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
> > > > >  
> > > > >  			if (current_fh->fh_export &&
> > > > >  					need_wrongsec_check(rqstp))
> > > > > -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> > > > > +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
> > > > >  		}
> > > > >  encode_op:
> > > > >  		if (op->status == nfserr_replay_me) {
> > > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > > > index 97f583777972..b45ea5757652 100644
> > > > > --- a/fs/nfsd/nfs4xdr.c
> > > > > +++ b/fs/nfsd/nfs4xdr.c
> > > > > @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
> > > > >  			nfserr = nfserrno(err);
> > > > >  			goto out_put;
> > > > >  		}
> > > > > -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> > > > > +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
> > > > >  		if (nfserr)
> > > > >  			goto out_put;
> > > > >  
> > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > index dd4e11a703aa..ed0eabfa3cb0 100644
> > > > > --- a/fs/nfsd/nfsfh.c
> > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > > >  {
> > > > >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > > > >  	struct svc_export *exp = NULL;
> > > > > +	bool may_bypass_gss = false;
> > > > >  	struct dentry	*dentry;
> > > > >  	__be32		error;
> > > > >  
> > > > > @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > > >  	 * which clients virtually always use auth_sys for,
> > > > >  	 * even while using RPCSEC_GSS for NFS.
> > > > >  	 */
> > > > > -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> > > > > +	if (access & NFSD_MAY_LOCK)
> > > > >  		goto skip_pseudoflavor_check;
> > > > > +	/*
> > > > > +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> > > > > +	 */
> > > > > +	if (access & NFSD_MAY_BYPASS_GSS)
> > > > > +		may_bypass_gss = true;
> > > > >  	/*
> > > > >  	 * Clients may expect to be able to use auth_sys during mount,
> > > > >  	 * even if they use gss for everything else; see section 2.3.2
> > > > > @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> > > > >  	 */
> > > > >  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
> > > > >  			&& exp->ex_path.dentry == dentry)
> > > > > -		goto skip_pseudoflavor_check;
> > > > > +		may_bypass_gss = true;
> > > > >  
> > > > > -	error = check_nfsd_access(exp, rqstp);
> > > > > +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
> > > > >  	if (error)
> > > > >  		goto out;
> > > > >  
> > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > index 29b1f3613800..b2f5ea7c2187 100644
> > > > > --- a/fs/nfsd/vfs.c
> > > > > +++ b/fs/nfsd/vfs.c
> > > > > @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
> > > > >  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
> > > > >  	if (err)
> > > > >  		return err;
> > > > > -	err = check_nfsd_access(exp, rqstp);
> > > > > +	err = check_nfsd_access(exp, rqstp, false);
> > > > >  	if (err)
> > > > >  		goto out;
> > > > >  	/*
> > > > > -- 
> > > > > 2.20.1
> > > > > 
> > > > > 
> > > > 
> > > 
> > > -- 
> > > Chuck Lever
> > > 
> > 
> 
> -- 
> Chuck Lever
> 
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever 1 month, 3 weeks ago
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.

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? 

Though I agree, simplifying this hot path would both help
performance scalability and reduce reader headaches. It might be a
little nicer to pass the NFSD_MAY flags directly to
check_nfsd_access(), for example.


-- 
Chuck Lever
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 1 month, 3 weeks ago
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.

> 
> 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.

> 
> Though I agree, simplifying this hot path would both help
> performance scalability and reduce reader headaches. It might be a
> little nicer to pass the NFSD_MAY flags directly to
> check_nfsd_access(), for example.

Yes, that might be cleaner.

Thanks,
NeilBrown
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever 1 month, 3 weeks ago
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
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 1 month, 2 weeks ago
On Thu, 10 Oct 2024, Chuck Lever wrote:
> 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.

Yes, that is sensible ...  though lockd used NFSD_MAY_WRITE for write
locks.
So if a process doesn't have read access to a file but does have write
access, and isn't the owner, then NLM would grant a write lock, but
NFSv4 would not.  check_fmode_for_setlk() makes the same choice, so a
local user could also get the lock.  Only NFSv4 would reject it.

> 
> 
> > > 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.

Thanks for fact-checking my claim!  I had forgotten about noauthnlm.

I'll suggest a patch which might make it all a bit clearer.

Thanks,
NeilBrown


> 
> 
> -- 
> Chuck Lever
> 
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Pali Rohár 1 month, 3 weeks ago
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.  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.

So if the original patch is also OK, you can choose which one you like
more. Original V1 restrict bypass to only GSS. V2 allows bypass for any
non-null/unix methods.

Let me know if something more is needed for this change.
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Pali Rohár 1 month, 3 weeks ago
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?

> 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
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever III 1 month, 3 weeks ago

> 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


Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by NeilBrown 1 month, 3 weeks ago
On Mon, 07 Oct 2024, Chuck Lever III wrote:
> 
> 
> > 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.

Interesting...

The distinction between RPC_AUTH_GSS_KRB5I and RPC_AUTH_GSS_KRB5P is not
about user authentication, it is about transport privacy.

And the distinction between xprtsec=tls and xprtsec=mtls seems to be
precisely about user authentication.

I would describe the current pseudo flavours as not "a clean way" to
advise the client of security requirements, but they are at least
established practice.

RPC_AUTH_SYS_TLS  seems to me to be an obvious sort of pseudo flavour.

But I suspect all these arguments and more have already been discussed
within the working group and people can sensibly have different
opinions.

Thanks for helping me understand NFS/TLS a bit better.

NeilBrown



> 
> 
> >> 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
> 
> 
> 
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Chuck Lever III 1 month, 3 weeks ago

> On Oct 6, 2024, at 7:36 PM, NeilBrown <neilb@suse.de> wrote:
> 
> On Mon, 07 Oct 2024, Chuck Lever III wrote:
>> 
>> 
>>> 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.
> 
> Interesting...
> 
> The distinction between RPC_AUTH_GSS_KRB5I and RPC_AUTH_GSS_KRB5P is not
> about user authentication, it is about transport privacy.

RPC_AUTH_GSS_KRB5I is Kerberos user authentication plus
Kerberos integrity protection.

RPC_AUTH_GSS_KRB5P is Kerberos user authentication plus
Kerberos confidentiality.

So, both of these pseudoflavors select Kerberos user
authentication (versus, say, RPC_AUTH_UNIX, which does
not).


> And the distinction between xprtsec=tls and xprtsec=mtls seems to be
> precisely about user authentication.

No: xprtsec authentication is /peer/ authentication. User
authentication is still set via sec= . See the final
paragraph in Section 4.2 of RFC 9289.


> I would describe the current pseudo flavours as not "a clean way" to
> advise the client of security requirements, but they are at least
> established practice.
> 
> RPC_AUTH_SYS_TLS  seems to me to be an obvious sort of pseudo flavour.
> 
> But I suspect all these arguments and more have already been discussed
> within the working group and people can sensibly have different
> opinions.

Yes, these arguments were discussed within the WG, and
I even wrote a draft (now expired) that treated the
various combinations of TLS and user authentication
flavors as unique pseudoflavors. The idea was rejected.


> Thanks for helping me understand NFS/TLS a bit better.
> 
> NeilBrown
> 
> 
> 
>> 
>> 
>>>> 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


--
Chuck Lever


Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Rick Macklem 1 month, 3 weeks ago
On Mon, Oct 7, 2024 at 8:51 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If in doubt, forward suspicious emails to IThelp@uoguelph.ca.
>
>
>
>
> > On Oct 6, 2024, at 7:36 PM, NeilBrown <neilb@suse.de> wrote:
> >
> > On Mon, 07 Oct 2024, Chuck Lever III wrote:
> >>
> >>
> >>> 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.
> >
> > Interesting...
> >
> > The distinction between RPC_AUTH_GSS_KRB5I and RPC_AUTH_GSS_KRB5P is not
> > about user authentication, it is about transport privacy.
>
> RPC_AUTH_GSS_KRB5I is Kerberos user authentication plus
> Kerberos integrity protection.
>
> RPC_AUTH_GSS_KRB5P is Kerberos user authentication plus
> Kerberos confidentiality.
>
> So, both of these pseudoflavors select Kerberos user
> authentication (versus, say, RPC_AUTH_UNIX, which does
> not).
I'd argue they also select on-the-wire protection. It just happens
that they use the session key for a user credential.
I'd agree with Neil, in that the 'p' refers to on-the-wire privacy.
>
>
> > And the distinction between xprtsec=tls and xprtsec=mtls seems to be
> > precisely about user authentication.
>
> No: xprtsec authentication is /peer/ authentication. User
> authentication is still set via sec= . See the final
> paragraph in Section 4.2 of RFC 9289.
True, but for krb5[ip] there is a (mis)use of a user principal for the
client's machine credential. (The user principal that does SetClientID
or ExchangeID.)
--> I'd argue that this user principal is really a client machine (or peer,
if you prefer) credential.
--> I think that the host based service principal in the client's keytab
      is a pita and maybe one of the reasons that krb5[ip] doesn't get
      used that much.

>
>
> > I would describe the current pseudo flavours as not "a clean way" to
> > advise the client of security requirements, but they are at least
> > established practice.
> >
> > RPC_AUTH_SYS_TLS  seems to me to be an obvious sort of pseudo flavour.
> >
> > But I suspect all these arguments and more have already been discussed
> > within the working group and people can sensibly have different
> > opinions.
>
> Yes, these arguments were discussed within the WG, and
> I even wrote a draft (now expired) that treated the
> various combinations of TLS and user authentication
> flavors as unique pseudoflavors. The idea was rejected.
I'll encourage NeilBrown to make comments related to the D. Noveck
security draft over on nfsv4@ieft.org. (I'll admit I have great difficulty
getting around to reading/commenting on these drafts, but I will try to
get around to the security one one of these days.)

The piece I'd like to see is mtls being accepted as a reasonable
alternative to krb5i/krb5p for SP4_MACH_CRED.

Personally, I think the pseudo-flavors make sense.
Maybe I/Neil can illicit further discussion w.r.t. them, rick

>
>
> > Thanks for helping me understand NFS/TLS a bit better.
> >
> > NeilBrown
> >
> >
> >
> >>
> >>
> >>>> 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
>
>
> --
> Chuck Lever
>
>
Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Pali Rohár 2 months, 2 weeks ago
On Friday 13 September 2024 08:52:20 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.

Ok. But this is not obvious really obvious ass the option has GSS in its name.

> I think that what you want to do makes sense.  Higher security can be
> downgraded to AUTH_UNIX, but AUTH_NULL mustn't be upgraded to to
> AUTH_UNIX.
> 
> Maybe that needs to be explicit in the code.  The bypass is ONLY allowed
> for AUTH_UNIX and only if something other than AUTH_NULL is allowed.

Ok, this sound good.

> Thanks,
> NeilBrown
> 
> 
> 
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  fs/nfsd/export.c   | 19 ++++++++++++++++++-
> >  fs/nfsd/export.h   |  2 +-
> >  fs/nfsd/nfs4proc.c |  2 +-
> >  fs/nfsd/nfs4xdr.c  |  2 +-
> >  fs/nfsd/nfsfh.c    | 12 +++++++++---
> >  fs/nfsd/vfs.c      |  2 +-
> >  6 files changed, 31 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > index 50b3135d07ac..eb11d3fdffe1 100644
> > --- a/fs/nfsd/export.c
> > +++ b/fs/nfsd/export.c
> > @@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> >  	return exp;
> >  }
> >  
> > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
> >  {
> >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> >  	struct svc_xprt *xprt = rqstp->rq_xprt;
> > @@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> >  	if (nfsd4_spo_must_allow(rqstp))
> >  		return 0;
> >  
> > +	/* Some calls may be processed without authentication
> > +	 * on GSS exports. For example NFS2/3 calls on root
> > +	 * directory, see section 2.3.2 of rfc 2623.
> > +	 * For "may_bypass_gss" check that export has really
> > +	 * enabled some GSS flavor and also check that the
> > +	 * used auth flavor is without auth (none or sys).
> > +	 */
> > +	if (may_bypass_gss && (
> > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > +	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > +		for (f = exp->ex_flavors; f < end; f++) {
> > +			if (f->pseudoflavor == RPC_AUTH_GSS ||
> > +			    f->pseudoflavor >= RPC_AUTH_GSS_KRB5)
> > +				return 0;
> > +		}
> > +	}
> > +
> >  denied:
> >  	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
> >  }
> > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
> > index ca9dc230ae3d..dc7cf4f6ac53 100644
> > --- a/fs/nfsd/export.h
> > +++ b/fs/nfsd/export.h
> > @@ -100,7 +100,7 @@ struct svc_expkey {
> >  #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
> >  
> >  int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
> > -__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
> > +__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
> >  
> >  /*
> >   * Function declarations
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 2e39cf2e502a..0f67f4a7b8b2 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
> >  
> >  			if (current_fh->fh_export &&
> >  					need_wrongsec_check(rqstp))
> > -				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
> > +				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
> >  		}
> >  encode_op:
> >  		if (op->status == nfserr_replay_me) {
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 97f583777972..b45ea5757652 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
> >  			nfserr = nfserrno(err);
> >  			goto out_put;
> >  		}
> > -		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
> > +		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
> >  		if (nfserr)
> >  			goto out_put;
> >  
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index dd4e11a703aa..ed0eabfa3cb0 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  {
> >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> >  	struct svc_export *exp = NULL;
> > +	bool may_bypass_gss = false;
> >  	struct dentry	*dentry;
> >  	__be32		error;
> >  
> > @@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  	 * which clients virtually always use auth_sys for,
> >  	 * even while using RPCSEC_GSS for NFS.
> >  	 */
> > -	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
> > +	if (access & NFSD_MAY_LOCK)
> >  		goto skip_pseudoflavor_check;
> > +	/*
> > +	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
> > +	 */
> > +	if (access & NFSD_MAY_BYPASS_GSS)
> > +		may_bypass_gss = true;
> >  	/*
> >  	 * Clients may expect to be able to use auth_sys during mount,
> >  	 * even if they use gss for everything else; see section 2.3.2
> > @@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
> >  	 */
> >  	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
> >  			&& exp->ex_path.dentry == dentry)
> > -		goto skip_pseudoflavor_check;
> > +		may_bypass_gss = true;
> >  
> > -	error = check_nfsd_access(exp, rqstp);
> > +	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
> >  	if (error)
> >  		goto out;
> >  
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 29b1f3613800..b2f5ea7c2187 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
> >  	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
> >  	if (err)
> >  		return err;
> > -	err = check_nfsd_access(exp, rqstp);
> > +	err = check_nfsd_access(exp, rqstp, false);
> >  	if (err)
> >  		goto out;
> >  	/*
> > -- 
> > 2.20.1
> > 
> > 
> 
[PATCH v2] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by Pali Rohár 1 month, 3 weeks ago
Currently NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT do not bypass
only GSS, but bypass any method. This is a 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 enabled some real
authentication (GSS, TLS, or any other).

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.
Similarly if client attempt to do mount with AUTH_NULL flavor and only
AUTH_UNIX flavor is enabled then the client will receive access error.

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).

Signed-off-by: Pali Rohár <pali@kernel.org>

---
Changes in v2:
* Apply whole NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT logic not
  only for GSS, but also for any method with the real authentication
  (anything except RPC_AUTH_NULL, RPC_AUTH_UNIX and RPC_AUTH_SHORT).
---
 fs/nfsd/export.c   | 19 ++++++++++++++++++-
 fs/nfsd/export.h   |  2 +-
 fs/nfsd/nfs4proc.c |  2 +-
 fs/nfsd/nfs4xdr.c  |  2 +-
 fs/nfsd/nfsfh.c    | 12 +++++++++---
 fs/nfsd/vfs.c      |  2 +-
 6 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 50b3135d07ac..8a12876b9335 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1074,7 +1074,7 @@ static struct svc_export *exp_find(struct cache_detail *cd,
 	return exp;
 }
 
-__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
+__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss)
 {
 	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
 	struct svc_xprt *xprt = rqstp->rq_xprt;
@@ -1120,6 +1120,23 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 	if (nfsd4_spo_must_allow(rqstp))
 		return 0;
 
+	/* Some calls may be processed without authentication
+	 * on GSS exports. For example NFS2/3 calls on root
+	 * directory, see section 2.3.2 of rfc 2623.
+	 * For "may_bypass_gss" check that export has really
+	 * enabled some flavor with authentication (GSS or any
+	 * other) and also check that the used auth flavor is
+	 * without authentication (none or sys).
+	 */
+	if (may_bypass_gss && (
+	     rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
+	     rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
+		for (f = exp->ex_flavors; f < end; f++) {
+			if (f->pseudoflavor >= RPC_AUTH_DES)
+				return 0;
+		}
+	}
+
 denied:
 	return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec;
 }
diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
index ca9dc230ae3d..dc7cf4f6ac53 100644
--- a/fs/nfsd/export.h
+++ b/fs/nfsd/export.h
@@ -100,7 +100,7 @@ struct svc_expkey {
 #define EX_WGATHER(exp)		((exp)->ex_flags & NFSEXP_GATHERED_WRITES)
 
 int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp);
-__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp);
+__be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp, bool may_bypass_gss);
 
 /*
  * Function declarations
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2e39cf2e502a..0f67f4a7b8b2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2791,7 +2791,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 
 			if (current_fh->fh_export &&
 					need_wrongsec_check(rqstp))
-				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
+				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
 		}
 encode_op:
 		if (op->status == nfserr_replay_me) {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 97f583777972..b45ea5757652 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3775,7 +3775,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
 			nfserr = nfserrno(err);
 			goto out_put;
 		}
-		nfserr = check_nfsd_access(exp, cd->rd_rqstp);
+		nfserr = check_nfsd_access(exp, cd->rd_rqstp, false);
 		if (nfserr)
 			goto out_put;
 
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index dd4e11a703aa..ed0eabfa3cb0 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -329,6 +329,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 {
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 	struct svc_export *exp = NULL;
+	bool may_bypass_gss = false;
 	struct dentry	*dentry;
 	__be32		error;
 
@@ -375,8 +376,13 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 	 * which clients virtually always use auth_sys for,
 	 * even while using RPCSEC_GSS for NFS.
 	 */
-	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
+	if (access & NFSD_MAY_LOCK)
 		goto skip_pseudoflavor_check;
+	/*
+	 * NFS4 PUTFH may bypass GSS (see nfsd4_putfh() in nfs4proc.c).
+	 */
+	if (access & NFSD_MAY_BYPASS_GSS)
+		may_bypass_gss = true;
 	/*
 	 * Clients may expect to be able to use auth_sys during mount,
 	 * even if they use gss for everything else; see section 2.3.2
@@ -384,9 +390,9 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
 	 */
 	if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
 			&& exp->ex_path.dentry == dentry)
-		goto skip_pseudoflavor_check;
+		may_bypass_gss = true;
 
-	error = check_nfsd_access(exp, rqstp);
+	error = check_nfsd_access(exp, rqstp, may_bypass_gss);
 	if (error)
 		goto out;
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 29b1f3613800..b2f5ea7c2187 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -320,7 +320,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
 	if (err)
 		return err;
-	err = check_nfsd_access(exp, rqstp);
+	err = check_nfsd_access(exp, rqstp, false);
 	if (err)
 		goto out;
 	/*
-- 
2.20.1

Re: [PATCH v2] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
Posted by cel@kernel.org 1 month, 3 weeks ago
From: Chuck Lever <chuck.lever@oracle.com>

On Sat, 05 Oct 2024 18:40:39 +0200, Pali Rohár wrote:                                              
> Currently NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT do not bypass
> only GSS, but bypass any method. This is a 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.
> 
> [...]                                                                        

Applied to nfsd-next for v6.13, thanks!                                                                

[1/1] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT
      commit: fa3d3ae84c5a6e9bd406c9ef75d3128a46cf1109                                                                      

--                                                                              
Chuck Lever