From owner-dev-commits-src-all@freebsd.org Sat Jun 26 22:56:44 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 B7D3B65B39E; Sat, 26 Jun 2021 22:56:44 +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 4GC8Pm4mytz3tlY; Sat, 26 Jun 2021 22:56:44 +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 8BDE02721D; Sat, 26 Jun 2021 22:56:44 +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 15QMuiRD055319; Sat, 26 Jun 2021 22:56:44 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 15QMui3G055318; Sat, 26 Jun 2021 22:56:44 GMT (envelope-from git) Date: Sat, 26 Jun 2021 22:56:44 GMT Message-Id: <202106262256.15QMui3G055318@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Rick Macklem Subject: git: 9f581cbd5aac - stable/13 - 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/stable/13 X-Git-Reftype: branch X-Git-Commit: 9f581cbd5aac096b3b08f343b9019c0f2432083a 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, 26 Jun 2021 22:56:44 -0000 The branch stable/13 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=9f581cbd5aac096b3b08f343b9019c0f2432083a commit 9f581cbd5aac096b3b08f343b9019c0f2432083a Author: Rick Macklem AuthorDate: 2021-06-05 23:53:07 +0000 Commit: Rick Macklem CommitDate: 2021-06-26 22:52:30 +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. (cherry picked from commit a5df139ec614c34f505bce9de3447fe7b49016e6) --- 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 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) {