| Message ID | 20240912221917.23802-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT | expand |
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 > >
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 > > > > >
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 > > > > >
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 >
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
> 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
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 > > >
> 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
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 > >
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.
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 > > >
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 >
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.
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
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.
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 >
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; /*
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(-)