From owner-dev-commits-src-all@freebsd.org Sat Jun 5 23:58:08 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C511764CBD2; Sat, 5 Jun 2021 23:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FyGmJ55FDz4n13; Sat, 5 Jun 2021 23:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 977A719C60; Sat, 5 Jun 2021 23:58:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 155Nw82W092672; Sat, 5 Jun 2021 23:58:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 155Nw8RW092670; Sat, 5 Jun 2021 23:58:08 GMT (envelope-from git) Date: Sat, 5 Jun 2021 23:58:08 GMT Message-Id: <202106052358.155Nw8RW092670@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: a5df139ec614 - main - nfsd: Fix when NFSERR_WRONGSEC may be replied to NFSv4 clients MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a5df139ec614c34f505bce9de3447fe7b49016e6 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Jun 2021 23:58:08 -0000 The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=a5df139ec614c34f505bce9de3447fe7b49016e6 commit a5df139ec614c34f505bce9de3447fe7b49016e6 Author: Rick Macklem AuthorDate: 2021-06-05 23:53:07 +0000 Commit: Rick Macklem 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) {