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.

(cherry picked from commit a5df139ec614c34f505bce9de3447fe7b49016e6)

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;
@@ -3293,19 +3287,6 @@ nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
 	}
 
 	/*
-	 * 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
 	 * clients are all boxes with secure system privileges,
@@ -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);
 	}
@@ -3982,6 +3964,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
 	 * file systems. Exported file systems should always have a non-empty
@@ -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 f4d6dbe42a21..adb66d3d794d 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);
 	}
@@ -3805,7 +3811,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) {