git: a5df139ec614 - main - nfsd: Fix when NFSERR_WRONGSEC may be replied to NFSv4 clients
Rick Macklem
rmacklem at FreeBSD.org
Sat Jun 5 23:58:08 UTC 2021
The branch main has been updated by rmacklem:
URL: https://cgit.FreeBSD.org/src/commit/?id=a5df139ec614c34f505bce9de3447fe7b49016e6
commit a5df139ec614c34f505bce9de3447fe7b49016e6
Author: Rick Macklem <rmacklem at FreeBSD.org>
AuthorDate: 2021-06-05 23:53:07 +0000
Commit: Rick Macklem <rmacklem at FreeBSD.org>
CommitDate: 2021-06-05 23:53:07 +0000
nfsd: Fix when NFSERR_WRONGSEC may be replied to NFSv4 clients
Commit d224f05fcfc1 pre-parsed the next operation number for
the put file handle operations. This patch uses this next
operation number, plus the type of the file handle being set by
the put file handle operation, to implement the rules in RFC5661
Sec. 2.6 with respect to replying NFSERR_WRONGSEC.
This patch also adds a check to see if NFSERR_WRONGSEC should be
replied when about to perform Lookup, Lookupp or Open with a file
name component, so that the NFSERR_WRONGSEC reply is done for
these operations, as required by RFC5661 Sec. 2.6.
This patch does not have any practical effect for the FreeBSD NFSv4
client and I believe that the same is true for the Linux client,
since NFSERR_WRONGSEC is considered a fatal error at this time.
MFC after: 2 weeks
---
sys/fs/nfs/nfs_var.h | 6 ++-
sys/fs/nfsserver/nfs_nfsdport.c | 91 ++++++++++++++++++++++++++-------------
sys/fs/nfsserver/nfs_nfsdserv.c | 19 ++++++--
sys/fs/nfsserver/nfs_nfsdsocket.c | 53 ++++++++++++++---------
4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h
index c1ca7c03af39..9db8b92f44e7 100644
--- a/sys/fs/nfs/nfs_var.h
+++ b/sys/fs/nfs/nfs_var.h
@@ -381,8 +381,9 @@ int nfscl_request(struct nfsrv_descript *, vnode_t,
/* nfs_nfsdsubs.c */
void nfsd_fhtovp(struct nfsrv_descript *, struct nfsrvfh *, int,
- vnode_t *, struct nfsexstuff *, mount_t *, int);
-int nfsd_excred(struct nfsrv_descript *, struct nfsexstuff *, struct ucred *);
+ vnode_t *, struct nfsexstuff *, mount_t *, int, int);
+int nfsd_excred(struct nfsrv_descript *, struct nfsexstuff *, struct ucred *,
+ bool);
int nfsrv_mtofh(struct nfsrv_descript *, struct nfsrvfh *);
int nfsrv_putattrbit(struct nfsrv_descript *, nfsattrbit_t *);
void nfsrv_wcc(struct nfsrv_descript *, int, struct nfsvattr *, int,
@@ -759,6 +760,7 @@ int nfsvno_listxattr(struct vnode *, uint64_t, struct ucred *, struct thread *,
u_char **, uint32_t *, bool *);
void nfsm_trimtrailing(struct nfsrv_descript *, struct mbuf *, char *, int,
int);
+bool nfsrv_checkwrongsec(struct nfsrv_descript *, int, enum vtype);
/* nfs_commonkrpc.c */
int newnfs_nmcancelreqs(struct nfsmount *);
diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c
index 4d19c73dfa06..7bcbc738d61b 100644
--- a/sys/fs/nfsserver/nfs_nfsdport.c
+++ b/sys/fs/nfsserver/nfs_nfsdport.c
@@ -3103,9 +3103,9 @@ nfsmout:
*/
int
nfsd_excred(struct nfsrv_descript *nd, struct nfsexstuff *exp,
- struct ucred *credanon)
+ struct ucred *credanon, bool testsec)
{
- int error = 0;
+ int error;
/*
* Check/setup credentials.
@@ -3115,18 +3115,12 @@ nfsd_excred(struct nfsrv_descript *nd, struct nfsexstuff *exp,
/*
* Check to see if the operation is allowed for this security flavor.
- * RFC2623 suggests that the NFSv3 Fsinfo RPC be allowed to
- * AUTH_NONE or AUTH_SYS for file systems requiring RPCSEC_GSS.
- * Also, allow Secinfo, so that it can acquire the correct flavor(s).
*/
- if (nfsvno_testexp(nd, exp) &&
- nd->nd_procnum != NFSV4OP_SECINFO &&
- nd->nd_procnum != NFSPROC_FSINFO) {
- if (nd->nd_flag & ND_NFSV4)
- error = NFSERR_WRONGSEC;
- else
- error = (NFSERR_AUTHERR | AUTH_TOOWEAK);
- goto out;
+ error = 0;
+ if (testsec) {
+ error = nfsvno_testexp(nd, exp);
+ if (error != 0)
+ goto out;
}
/*
@@ -3246,7 +3240,7 @@ nfsvno_fhtovp(struct mount *mp, fhandle_t *fhp, struct sockaddr *nam,
void
nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
struct vnode **vpp, struct nfsexstuff *exp,
- struct mount **mpp, int startwrite)
+ struct mount **mpp, int startwrite, int nextop)
{
struct mount *mp, *mpw;
struct ucred *credanon;
@@ -3292,19 +3286,6 @@ nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
nd->nd_repstat = EACCES;
}
- /*
- * If TLS is required by the export, check the flags in nd_flag.
- */
- if (nd->nd_repstat == 0 && ((NFSVNO_EXTLS(exp) &&
- (nd->nd_flag & ND_TLS) == 0) ||
- (NFSVNO_EXTLSCERT(exp) &&
- (nd->nd_flag & ND_TLSCERT) == 0) ||
- (NFSVNO_EXTLSCERTUSER(exp) &&
- (nd->nd_flag & ND_TLSCERTUSER) == 0))) {
- vput(*vpp);
- nd->nd_repstat = NFSERR_ACCES;
- }
-
/*
* Personally, I've never seen any point in requiring a
* reserved port#, since only in the rare case where the
@@ -3342,7 +3323,8 @@ nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
*/
if (!nd->nd_repstat) {
nd->nd_saveduid = nd->nd_cred->cr_uid;
- nd->nd_repstat = nfsd_excred(nd, exp, credanon);
+ nd->nd_repstat = nfsd_excred(nd, exp, credanon,
+ nfsrv_checkwrongsec(nd, nextop, (*vpp)->v_type));
if (nd->nd_repstat)
vput(*vpp);
}
@@ -3981,6 +3963,24 @@ nfsvno_testexp(struct nfsrv_descript *nd, struct nfsexstuff *exp)
{
int i;
+ /*
+ * Allow NFSv3 Fsinfo per RFC2623.
+ */
+ if (((nd->nd_flag & ND_NFSV4) != 0 ||
+ nd->nd_procnum != NFSPROC_FSINFO) &&
+ ((NFSVNO_EXTLS(exp) && (nd->nd_flag & ND_TLS) == 0) ||
+ (NFSVNO_EXTLSCERT(exp) &&
+ (nd->nd_flag & ND_TLSCERT) == 0) ||
+ (NFSVNO_EXTLSCERTUSER(exp) &&
+ (nd->nd_flag & ND_TLSCERTUSER) == 0))) {
+ if ((nd->nd_flag & ND_NFSV4) != 0)
+ return (NFSERR_WRONGSEC);
+ else if ((nd->nd_flag & ND_TLS) == 0)
+ return (NFSERR_AUTHERR | AUTH_NEEDS_TLS);
+ else
+ return (NFSERR_AUTHERR | AUTH_NEEDS_TLS_MUTUAL_HOST);
+ }
+
/*
* This seems odd, but allow the case where the security flavor
* list is empty. This happens when NFSv4 is traversing non-exported
@@ -4008,7 +4008,9 @@ nfsvno_testexp(struct nfsrv_descript *nd, struct nfsexstuff *exp)
(nd->nd_flag & ND_GSS) == 0)
return (0);
}
- return (1);
+ if ((nd->nd_flag & ND_NFSV4) != 0)
+ return (NFSERR_WRONGSEC);
+ return (NFSERR_AUTHERR | AUTH_TOOWEAK);
}
/*
@@ -6616,6 +6618,37 @@ nfsm_trimtrailing(struct nfsrv_descript *nd, struct mbuf *mb, char *bpos,
nd->nd_bpos = bpos;
}
+
+/*
+ * Check to see if a put file handle operation should test for
+ * NFSERR_WRONGSEC, although NFSv3 actually returns NFSERR_AUTHERR.
+ * When Open is the next operation, NFSERR_WRONGSEC cannot be
+ * replied for the Open cases that use a component. Thia can
+ * be identified by the fact that the file handle's type is VDIR.
+ */
+bool
+nfsrv_checkwrongsec(struct nfsrv_descript *nd, int nextop, enum vtype vtyp)
+{
+
+ if ((nd->nd_flag & ND_NFSV4) == 0) {
+ if (nd->nd_procnum == NFSPROC_FSINFO)
+ return (false);
+ return (true);
+ }
+
+ if ((nd->nd_flag & ND_LASTOP) != 0)
+ return (false);
+
+ if (nextop == NFSV4OP_PUTROOTFH || nextop == NFSV4OP_PUTFH ||
+ nextop == NFSV4OP_PUTPUBFH || nextop == NFSV4OP_RESTOREFH ||
+ nextop == NFSV4OP_LOOKUP || nextop == NFSV4OP_LOOKUPP ||
+ nextop == NFSV4OP_SECINFO || nextop == NFSV4OP_SECINFONONAME)
+ return (false);
+ if (nextop == NFSV4OP_OPEN && vtyp == VDIR)
+ return (false);
+ return (true);
+}
+
extern int (*nfsd_call_nfsd)(struct thread *, struct nfssvc_args *);
/*
diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c
index d7049ba1fed8..e564a6a48b79 100644
--- a/sys/fs/nfsserver/nfs_nfsdserv.c
+++ b/sys/fs/nfsserver/nfs_nfsdserv.c
@@ -1669,7 +1669,7 @@ nfsrvd_rename(struct nfsrv_descript *nd, int isdgram,
NFSVOPUNLOCK(dp);
nd->nd_cred->cr_uid = nd->nd_saveduid;
nfsd_fhtovp(nd, &tfh, LK_EXCLUSIVE, &tdp, &tnes, NULL,
- 0); /* Locks tdp. */
+ 0, -1); /* Locks tdp. */
if (tdp) {
tdirfor_ret = nfsvno_getattr(tdp, &tdirfor, nd,
p, 1, NULL);
@@ -1800,7 +1800,8 @@ nfsrvd_link(struct nfsrv_descript *nd, int isdgram,
/* tovp is always NULL unless NFSv4 */
goto out;
}
- nfsd_fhtovp(nd, &dfh, LK_EXCLUSIVE, &dp, &tnes, NULL, 0);
+ nfsd_fhtovp(nd, &dfh, LK_EXCLUSIVE, &dp, &tnes, NULL,
+ 0, -1);
if (dp)
NFSVOPUNLOCK(dp);
}
@@ -3695,7 +3696,12 @@ nfsrvd_secinfo(struct nfsrv_descript *nd, int isdgram,
vput(vp);
savflag = nd->nd_flag;
if (!nd->nd_repstat) {
- nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0);
+ /*
+ * Pretend the next op is Secinfo, so that no wrongsec
+ * test will be done.
+ */
+ nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0,
+ NFSV4OP_SECINFO);
if (vp)
vput(vp);
}
@@ -3830,7 +3836,12 @@ nfsrvd_secinfononame(struct nfsrv_descript *nd, int isdgram,
vput(vp);
savflag = nd->nd_flag;
if (nd->nd_repstat == 0) {
- nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0);
+ /*
+ * Pretend the next op is Secinfo, so that no wrongsec
+ * test will be done.
+ */
+ nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0,
+ NFSV4OP_SECINFO);
if (vp != NULL)
vput(vp);
}
diff --git a/sys/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsdsocket.c
index f40569da0097..85771974be2f 100644
--- a/sys/fs/nfsserver/nfs_nfsdsocket.c
+++ b/sys/fs/nfsserver/nfs_nfsdsocket.c
@@ -584,10 +584,10 @@ tryagain:
lktype = LK_EXCLUSIVE;
if (nd->nd_flag & ND_PUBLOOKUP)
nfsd_fhtovp(nd, &nfs_pubfh, lktype, &vp, &nes,
- &mp, nfsrv_writerpc[nd->nd_procnum]);
+ &mp, nfsrv_writerpc[nd->nd_procnum], -1);
else
nfsd_fhtovp(nd, &fh, lktype, &vp, &nes,
- &mp, nfsrv_writerpc[nd->nd_procnum]);
+ &mp, nfsrv_writerpc[nd->nd_procnum], -1);
if (nd->nd_repstat == NFSERR_PROGNOTV4)
goto out;
}
@@ -702,7 +702,7 @@ static void
nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
int taglen, u_int32_t minorvers)
{
- int i, lktype, op, op0 = 0, statsinprog = 0;
+ int i, lktype, op, op0 = 0, rstat, statsinprog = 0;
u_int32_t *tl;
struct nfsclient *clp, *nclp;
int error = 0, igotlock, nextop, numops, savefhcnt;
@@ -983,7 +983,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
}
if (!nd->nd_repstat)
nfsd_fhtovp(nd, &fh, LK_SHARED, &nvp, &nes,
- NULL, 0);
+ NULL, 0, nextop);
/* For now, allow this for non-export FHs */
if (!nd->nd_repstat) {
if (vp)
@@ -1017,7 +1017,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
i < numops - 1);
}
nfsd_fhtovp(nd, &nfs_pubfh, LK_SHARED, &nvp,
- &nes, NULL, 0);
+ &nes, NULL, 0, nextop);
} else
nd->nd_repstat = NFSERR_NOFILEHANDLE;
if (!nd->nd_repstat) {
@@ -1052,7 +1052,7 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
i < numops - 1);
}
nfsd_fhtovp(nd, &nfs_rootfh, LK_SHARED, &nvp,
- &nes, NULL, 0);
+ &nes, NULL, 0, nextop);
if (!nd->nd_repstat) {
if (vp)
vrele(vp);
@@ -1110,13 +1110,21 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
nd->nd_repstat = 0;
/* If vp == savevp, a no-op */
if (vp != savevp) {
- VREF(savevp);
- vrele(vp);
- vp = savevp;
- vpnes = savevpnes;
- cur_fsid = save_fsid;
+ if (nfsrv_checkwrongsec(nd, nextop,
+ savevp->v_type))
+ nd->nd_repstat =
+ nfsvno_testexp(nd,
+ &savevpnes);
+ if (nd->nd_repstat == 0) {
+ VREF(savevp);
+ vrele(vp);
+ vp = savevp;
+ vpnes = savevpnes;
+ cur_fsid = save_fsid;
+ }
}
- if ((nd->nd_flag & ND_SAVEDCURSTATEID) != 0) {
+ if (nd->nd_repstat == 0 &&
+ (nd->nd_flag & ND_SAVEDCURSTATEID) != 0) {
nd->nd_curstateid =
nd->nd_savedcurstateid;
nd->nd_flag |= ND_CURSTATEID;
@@ -1143,14 +1151,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
op != NFSV4OP_GETFH &&
op != NFSV4OP_ACCESS &&
op != NFSV4OP_READLINK &&
- op != NFSV4OP_SECINFO)
+ op != NFSV4OP_SECINFO &&
+ op != NFSV4OP_SECINFONONAME)
nd->nd_repstat = NFSERR_NOFILEHANDLE;
- else if (nfsvno_testexp(nd, &vpnes) &&
- op != NFSV4OP_LOOKUP &&
- op != NFSV4OP_GETFH &&
- op != NFSV4OP_GETATTR &&
- op != NFSV4OP_SECINFO)
- nd->nd_repstat = NFSERR_WRONGSEC;
if (nd->nd_repstat) {
if (op == NFSV4OP_SETATTR) {
/*
@@ -1183,6 +1186,16 @@ tryagain:
nd->nd_repstat = NFSERR_NOFILEHANDLE;
break;
}
+ if (NFSVNO_EXPORTED(&vpnes) && (op == NFSV4OP_LOOKUP ||
+ op == NFSV4OP_LOOKUPP || (op == NFSV4OP_OPEN &&
+ vp->v_type == VDIR))) {
+ /* Check for wrong security. */
+ rstat = nfsvno_testexp(nd, &vpnes);
+ if (rstat != 0) {
+ nd->nd_repstat = rstat;
+ break;
+ }
+ }
VREF(vp);
if (nfsv4_opflag[op].modifyfs)
vn_start_write(vp, &temp_mp, V_WAIT);
@@ -1197,7 +1210,7 @@ tryagain:
nd->nd_nam, &nes, &credanon);
if (!nd->nd_repstat)
nd->nd_repstat = nfsd_excred(nd,
- &nes, credanon);
+ &nes, credanon, true);
if (credanon != NULL)
crfree(credanon);
if (!nd->nd_repstat) {
More information about the dev-commits-src-all
mailing list