From nobody Fri Nov 15 13:00:54 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XqcZv1GkBz5d6rB; Fri, 15 Nov 2024 13:00:55 +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 "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XqcZt5kzhz4pgy; Fri, 15 Nov 2024 13:00:54 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1731675654; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=1IX7EJ9KA2MJToFTnbQX0wVsiJuozVwuXnFlNA0xT1Y=; b=HofZLo5SolQZoPzwi88gkQ2COAO8086WIXhwduxC0Oo8TQK/sT46lhVrVddLzd25nKyAEc 5t4rlkBcsA+G4if7j9luZx/Dq17kNprTqKpg9VSW5C/VZlsjQa3wXnQbzX5ZzZO4v2YVAr xLtRmhBRWd/I5T6gUuSXsphKhHR/i6oLRzjTXPYvgYDHXUuAOcE5xWB7jqQwlRdRbE9KjP hpTa6wUK5Wc+d5c0YgJzrL8bQGK0jp4xEXuJa6jxAIHK1aKlurrZE4V0Fz9Qvzxw+lSn0p iyh/5izp6eNFuDAVMg9i87lcnuvUzbN890gKbBAaE6u741fsQT3VMXze9L2rWA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1731675654; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=1IX7EJ9KA2MJToFTnbQX0wVsiJuozVwuXnFlNA0xT1Y=; b=c/yglwP6VHp1PAecwq3UNLul7hTlo1KpjaOKaojiF0JX21e5McIehoTmeOPHVJ/uOGEYht kstjyFnHRQDaW2LPdKXa030+I6CgV6kye8c2M4XLLkT3JyoBy3Qp/9pO3O2tGzipSYaxNY nFEyCrz/Hyxnx00s+WR9EtS24TVYP0ZgNe+ptn5nhiw6oTbRoLa+jDF+cU2FrB9g8+Ro1n 2bWQhYd0gFQBWiqRgkirV8QG6JgbHx2LAKg1qBhq0GWxLsTomUk7YVrqs4qxWSuVLC/rKF j+8XEjflff+vRNflCx7SPjPp3ZadMg59ojDQdxCgqzaTfEPJ6ukdgv00NgS9kg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1731675654; a=rsa-sha256; cv=none; b=Lf5DLMKEQY5ABlLROKDvobp/PIXCyP3MaNxVi6HOOhaVLpxHXql3HO7BMhPTOzQGe6EX6y F3xtbB22CNs8V+7IoQr2tLFc/eqV2mFzqZYqmgxpY08bmOr52U3DO9Xmn8NMEUT5d478Q7 U8XGkPgI1Ge47fVICfSuuOptRWKzrni57ZUgEoD0sOn2pxf5YiS8GNJQCNv0RSdjLP2iXG rcXVV15u9C+xfWgOAzHhvIv/ka7KbHcxh/DHzlpN7qtUlkFA8OnowB4AojqQW9bUdS02yT LK6z26Ep5m7PW2q+zQTKfhEjEsZcSiCDJpW+td2CvhBNrs64WjiFPZLafa4QUw== 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 4XqcZt5LgrzPLH; Fri, 15 Nov 2024 13:00:54 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4AFD0sYt071380; Fri, 15 Nov 2024 13:00:54 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4AFD0sjX071377; Fri, 15 Nov 2024 13:00:54 GMT (envelope-from git) Date: Fri, 15 Nov 2024 13:00:54 GMT Message-Id: <202411151300.4AFD0sjX071377@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Olivier Certner Subject: git: fb880a5a119d - stable/13 - nfs, rpc: Ensure kernel credentials have at least one group List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: olce X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: fb880a5a119d37d5ca35780235646d56fc21d4aa Auto-Submitted: auto-generated The branch stable/13 has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=fb880a5a119d37d5ca35780235646d56fc21d4aa commit fb880a5a119d37d5ca35780235646d56fc21d4aa Author: Olivier Certner AuthorDate: 2024-10-02 14:28:59 +0000 Commit: Olivier Certner CommitDate: 2024-11-15 12:59:09 +0000 nfs, rpc: Ensure kernel credentials have at least one group This fixes several bugs where some 'struct ucred' in the kernel, constructed from user input (via nmount(2)) or obtained from other servers (e.g., gssd(8)), could have an unfilled 'cr_groups' field and whose 'cr_groups[0]' (or 'cr_gid', which is an alias) was later accessed, causing an uninitialized access giving random access rights. Use crsetgroups_fallback() to enforce a fallback group when possible. For NFS, the chosen fallback group is that of the NFS server in the current VNET (NFSD_VNET(nfsrv_defaultgid)). There does not seem to be any sensible fallback available in rpc code (sys/rpc/svc_auth.c, svc_getcred()) on AUTH_UNIX (TLS or not), so just fail credential retrieval there. Stock NSS sources, rpc.tlsservd(8) or rpc.tlsclntd(8) provide non-empty group lists, so will not be impacted. Discussed with: rmacklem (by mail) Approved by: markj (mentor) MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D46918 (cherry picked from commit cfbe7a62dc62e8a5d7520cb5eb8ad7c4a9418e26) Approved by: markj (mentor) --- sys/fs/nfs/nfs_commonport.c | 4 +++- sys/fs/nfs/nfs_commonsubs.c | 5 +++-- sys/fs/nfsserver/nfs_nfsdport.c | 6 +++++- sys/kern/vfs_export.c | 12 ++++++++---- sys/rpc/rpcsec_gss/svc_rpcsec_gss.c | 2 +- sys/rpc/svc_auth.c | 8 ++++++-- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/sys/fs/nfs/nfs_commonport.c b/sys/fs/nfs/nfs_commonport.c index b8e6bfc170a2..67dd8e14a22c 100644 --- a/sys/fs/nfs/nfs_commonport.c +++ b/sys/fs/nfs/nfs_commonport.c @@ -75,6 +75,7 @@ NFSD_VNET_DEFINE(struct nfsstatsv1 *, nfsstatsv1_p); NFSD_VNET_DECLARE(struct nfssockreq, nfsrv_nfsuserdsock); NFSD_VNET_DECLARE(nfsuserd_state, nfsrv_nfsuserd); +NFSD_VNET_DECLARE(gid_t, nfsrv_defaultgid); int nfs_pnfsio(task_fn_t *, void *); @@ -259,7 +260,8 @@ newnfs_copycred(struct nfscred *nfscr, struct ucred *cr) KASSERT(nfscr->nfsc_ngroups >= 0, ("newnfs_copycred: negative nfsc_ngroups")); cr->cr_uid = nfscr->nfsc_uid; - crsetgroups(cr, nfscr->nfsc_ngroups, nfscr->nfsc_groups); + crsetgroups_fallback(cr, nfscr->nfsc_ngroups, nfscr->nfsc_groups, + NFSD_VNET(nfsrv_defaultgid)); } /* diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c index a9659079ed5f..cbac4ee85167 100644 --- a/sys/fs/nfs/nfs_commonsubs.c +++ b/sys/fs/nfs/nfs_commonsubs.c @@ -3978,8 +3978,9 @@ nfssvc_idname(struct nfsd_idargs *nidp) */ cr = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = nidp->nid_uid; - crsetgroups(cr, nidp->nid_ngroup, grps); - cr->cr_rgid = cr->cr_svgid = cr->cr_groups[0]; + crsetgroups_fallback(cr, nidp->nid_ngroup, grps, + NFSD_VNET(nfsrv_defaultgid)); + cr->cr_rgid = cr->cr_svgid = cr->cr_gid; cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); #ifdef MAC diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c index 41443a5a79f4..caa63dc20a58 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -3304,7 +3304,11 @@ nfsd_excred(struct nfsrv_descript *nd, struct nfsexstuff *exp, NFSVNO_EXPORTANON(exp) || (nd->nd_flag & ND_AUTHNONE) != 0) { nd->nd_cred->cr_uid = credanon->cr_uid; - nd->nd_cred->cr_gid = credanon->cr_gid; + /* + * 'credanon' is already a 'struct ucred' that was built + * internally with calls to crsetgroups_fallback(), so + * we don't need a fallback here. + */ crsetgroups(nd->nd_cred, credanon->cr_ngroups, credanon->cr_groups); } else if ((nd->nd_flag & ND_GSS) == 0) { diff --git a/sys/kern/vfs_export.c b/sys/kern/vfs_export.c index d9686edb3769..6eb20c8ed3a9 100644 --- a/sys/kern/vfs_export.c +++ b/sys/kern/vfs_export.c @@ -63,6 +63,10 @@ #include #include +#include + +NFSD_VNET_DECLARE(gid_t, nfsrv_defaultgid); + static MALLOC_DEFINE(M_NETADDR, "export_host", "Export host address structure"); #if defined(INET) || defined(INET6) @@ -135,8 +139,8 @@ vfs_hang_addrlist(struct mount *mp, struct netexport *nep, np->netc_exflags = argp->ex_flags; np->netc_anon = crget(); np->netc_anon->cr_uid = argp->ex_uid; - crsetgroups(np->netc_anon, argp->ex_ngroups, - argp->ex_groups); + crsetgroups_fallback(np->netc_anon, argp->ex_ngroups, + argp->ex_groups, NFSD_VNET(nfsrv_defaultgid)); np->netc_anon->cr_prison = &prison0; prison_hold(np->netc_anon->cr_prison); np->netc_numsecflavors = argp->ex_numsecflavors; @@ -214,8 +218,8 @@ vfs_hang_addrlist(struct mount *mp, struct netexport *nep, np->netc_exflags = argp->ex_flags; np->netc_anon = crget(); np->netc_anon->cr_uid = argp->ex_uid; - crsetgroups(np->netc_anon, argp->ex_ngroups, - argp->ex_groups); + crsetgroups_fallback(np->netc_anon, argp->ex_ngroups, argp->ex_groups, + NFSD_VNET(nfsrv_defaultgid)); np->netc_anon->cr_prison = &prison0; prison_hold(np->netc_anon->cr_prison); np->netc_numsecflavors = argp->ex_numsecflavors; diff --git a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c index f5917a65a7b2..93a41dc045cc 100644 --- a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c +++ b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c @@ -510,7 +510,7 @@ rpc_gss_svc_getcred(struct svc_req *req, struct ucred **crp, int *flavorp) cr = client->cl_cred = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = uc->uid; cr->cr_rgid = cr->cr_svgid = uc->gid; - crsetgroups(cr, uc->gidlen, uc->gidlist); + crsetgroups_fallback(cr, uc->gidlen, uc->gidlist, uc->gid); cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp = crhold(cr); diff --git a/sys/rpc/svc_auth.c b/sys/rpc/svc_auth.c index 86ce7d9aefd2..edb11426bd72 100644 --- a/sys/rpc/svc_auth.c +++ b/sys/rpc/svc_auth.c @@ -192,10 +192,12 @@ svc_getcred(struct svc_req *rqst, struct ucred **crp, int *flavorp) if ((xprt->xp_tls & (RPCTLS_FLAGS_CERTUSER | RPCTLS_FLAGS_DISABLED)) == RPCTLS_FLAGS_CERTUSER && flavor == AUTH_UNIX) { + if (xprt->xp_ngrps <= 0) + return (FALSE); cr = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = xprt->xp_uid; crsetgroups(cr, xprt->xp_ngrps, xprt->xp_gidp); - cr->cr_rgid = cr->cr_svgid = xprt->xp_gidp[0]; + cr->cr_rgid = cr->cr_svgid = cr->cr_gid; cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp = cr; @@ -205,10 +207,12 @@ svc_getcred(struct svc_req *rqst, struct ucred **crp, int *flavorp) switch (flavor) { case AUTH_UNIX: xcr = (struct xucred *) rqst->rq_clntcred; + if (xcr->cr_ngroups <= 0) + return (FALSE); cr = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = xcr->cr_uid; crsetgroups(cr, xcr->cr_ngroups, xcr->cr_groups); - cr->cr_rgid = cr->cr_svgid = cr->cr_groups[0]; + cr->cr_rgid = cr->cr_svgid = cr->cr_gid; cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp = cr;