From nobody Sun Nov 03 03:58:12 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 4Xh16K6x5Gz5bgXM; Sun, 03 Nov 2024 03:58:17 +0000 (UTC) (envelope-from rpokala@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Xh16K4xr0z4tZ7; Sun, 3 Nov 2024 03:58:17 +0000 (UTC) (envelope-from rpokala@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730606297; 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: in-reply-to:in-reply-to:references:references; bh=oOUwQ0ndfENZKJGvhFht+h1lGikK9FaSwCLFNUmMv5Y=; b=N/P8095bK5jryvjmdBA7azRuEyN/l7HFfu6DgsPv73JLgq1AI1O0fMY8sBZw6yi+N6hwdy nhu33RRlE8beqGAICnJFdt4nB+ucP3DGKAomtrhtwE/4Q7uq4Nwp0IxyCGpNRsQUd1Kq80 N5v5+OI37FLKrVTxDO4hq6CZJuDeekFaZ2w+SfnphloIR3D3/ciRCRbb13ItfrDUyDZe8m hAViQbr0IFZAEsCz398ymb/7/NCk8EaGQZtr10svVAFki3AhjzPFB1w1kY2KCOjb9CaZ+c CKRL6ii9FYulrmrSQ54l7u7v53WhX+nPK9JkpDA+7it/hFU33Byvwj3tUaF81Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730606297; 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: in-reply-to:in-reply-to:references:references; bh=oOUwQ0ndfENZKJGvhFht+h1lGikK9FaSwCLFNUmMv5Y=; b=V/g37OSZSf12SpiFTkPZnQ2EToN+C922UjuPFk0WZKMUnHezSSzZc2gVOwP+mefu9wXdpx Z7Y74+FO/IuW1lIRPS6kun5YmsSfYJS8zpMvJtWzmU9Q6RkL/E3B8WH0rnCWGs3v3/oqtx TnQ/WtszS0N8Thctd52hgUNViULB2TUalBt3oDHa15wXrxPdk6ty5kKVlcVH/mYWxrBrn0 tPPPHLe6IJ3S9rUUL1Kk9N6lWmG1fFawvjcOSW/TUV3nvzg9ikRlc6x8mGp4+aQwNju/e6 mz1IX06rVcc3C0Tnp9ndLh+TDm+7xxBrBu3i8Sv1qUVehojJEL+nNwfaWzPRNQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1730606297; a=rsa-sha256; cv=none; b=Bv858a9JoDO6EICGfXrpoJspskSGZBI5pFiuMyQwcR3p4GMafATDQFxbZ0o8tg0kJd/846 hQZB8clhuyWzBLW3BY9sflRLeNyQdK9i5N6YKwHf9UyP2EvzWuJ7NxYJCY0KE4oGt1PR6o VC39+l1/9o/NBUDYgw3m5Xf2xM583lzkPxdYtLm1ju0cSyTM2CGlzVb2a/AhfQMVhGGrmo HdnwJe3mHFaCqfVcDtvJqvt0PFioJmqdoXoLbnjGjmDrns9gAI+FeQRPNF3WUSGB91jj04 U1GGxYBCBw6KLuKoZZqMBE8HvaLeCAXz+DQTZO7BK7X0yxXzMaoIFSXcOLxfeA== Received: from [192.168.1.10] (c-73-231-46-254.hsd1.ca.comcast.net [73.231.46.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: rpokala) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Xh16J6dVWz17mC; Sun, 3 Nov 2024 03:58:16 +0000 (UTC) (envelope-from rpokala@freebsd.org) User-Agent: Microsoft-MacOutlook/16.90.24110120 Date: Sat, 02 Nov 2024 20:58:12 -0700 Subject: Re: cfbe7a62dc62 - main - nfs, rpc: Ensure kernel credentials have at least one group From: Ravi Pokala To: Olivier Certner , , , Message-ID: <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com> Thread-Topic: cfbe7a62dc62 - main - nfs, rpc: Ensure kernel credentials have at least one group References: <202411022039.4A2KdbAE046580@gitrepo.freebsd.org> In-Reply-To: <202411022039.4A2KdbAE046580@gitrepo.freebsd.org> 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: quoted-printable Hi Olivier, This appears to break amd64.MINIMAL and amd64.MINIMALUP: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D % less _.amd64.MINIMAL linking kernel.full ld: error: undefined symbol: vnet_entry_nfsrv_defaultgid >>> referenced by vfs_export.c:141 (sys/kern/vfs_export.c:141) >>> vfs_export.o:(vfs_export) >>> referenced by vfs_export.c:220 (sys/kern/vfs_export.c:220) >>> vfs_export.o:(vfs_export) --- kernel.full --- *** [kernel.full] Error code 1 make[5]: stopped making "all" in amd64.amd64/sys/MINIMAL =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Thanks, Ravi (rpokala@) =EF=BB=BF-----Original Message----- From: > on behalf of Olivier Certner > Date: Saturday, November 2, 2024 at 13:39 To: >, >, > Subject: git: cfbe7a62dc62 - main - nfs, rpc: Ensure kernel credentials hav= e at least one group The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=3Dcfbe7a62dc62e8a5d7520cb5eb8ad7= c4a9418e26 commit cfbe7a62dc62e8a5d7520cb5eb8ad7c4a9418e26 Author: Olivier Certner > AuthorDate: 2024-10-02 14:28:59 +0000 Commit: Olivier Certner > CommitDate: 2024-11-02 20:37:42 +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 --- sys/fs/nfs/nfs_commonport.c | 4 +++- sys/fs/nfs/nfs_commonsubs.c | 5 +++-- sys/fs/nfsserver/nfs_nfsdport.c | 6 +++++- sys/fs/nfsserver/nfs_nfsdsocket.c | 6 ++---- sys/kern/vfs_export.c | 12 ++++++++---- sys/rpc/rpcsec_gss/svc_rpcsec_gss.c | 2 +- sys/rpc/svc_auth.c | 8 ++++++-- 7 files changed, 28 insertions(+), 15 deletions(-) diff --git a/sys/fs/nfs/nfs_commonport.c b/sys/fs/nfs/nfs_commonport.c index 2db9af5b9ea9..11f31d1a0e9f 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 *); @@ -258,7 +259,8 @@ newnfs_copycred(struct nfscred *nfscr, struct ucred *cr= ) KASSERT(nfscr->nfsc_ngroups >=3D 0, ("newnfs_copycred: negative nfsc_ngroups")); cr->cr_uid =3D 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 90b30f462106..ce4b0052714e 100644 --- a/sys/fs/nfs/nfs_commonsubs.c +++ b/sys/fs/nfs/nfs_commonsubs.c @@ -4051,8 +4051,9 @@ nfssvc_idname(struct nfsd_idargs *nidp) */ cr =3D crget(); cr->cr_uid =3D cr->cr_ruid =3D cr->cr_svuid =3D nidp->nid_uid; - crsetgroups(cr, nidp->nid_ngroup, grps); - cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_groups[0]; + crsetgroups_fallback(cr, nidp->nid_ngroup, grps, + NFSD_VNET(nfsrv_defaultgid)); + cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_gid; cr->cr_prison =3D 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_nfsdpor= t.c index 8a2a39052506..5160645ad73c 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -3311,7 +3311,11 @@ nfsd_excred(struct nfsrv_descript *nd, struct nfsexs= tuff *exp, NFSVNO_EXPORTANON(exp) || (nd->nd_flag & ND_AUTHNONE) !=3D 0) { nd->nd_cred->cr_uid =3D credanon->cr_uid; - nd->nd_cred->cr_gid =3D 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) =3D=3D 0) { diff --git a/sys/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsds= ocket.c index df0c0edd1b59..d1b6198ba0e1 100644 --- a/sys/fs/nfsserver/nfs_nfsdsocket.c +++ b/sys/fs/nfsserver/nfs_nfsdsocket.c @@ -1422,13 +1422,11 @@ static struct ucred * nfsrv_createrootcred(void) { struct ucred *cr; - gid_t grp; cr =3D crget(); cr->cr_uid =3D cr->cr_ruid =3D cr->cr_svuid =3D UID_ROOT; - grp =3D GID_WHEEL; - crsetgroups(cr, 1, &grp); - cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_groups[0]; + crsetgroups_fallback(cr, 0, NULL, GID_WHEEL); + cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_gid; cr->cr_prison =3D curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); #ifdef MAC diff --git a/sys/kern/vfs_export.c b/sys/kern/vfs_export.c index 996f3f74193f..c0337b1fe858 100644 --- a/sys/kern/vfs_export.c +++ b/sys/kern/vfs_export.c @@ -61,6 +61,10 @@ #include #include +#include + +NFSD_VNET_DECLARE(gid_t, nfsrv_defaultgid); + static MALLOC_DEFINE(M_NETADDR, "export_host", "Export host address structu= re"); #if defined(INET) || defined(INET6) @@ -133,8 +137,8 @@ vfs_hang_addrlist(struct mount *mp, struct netexport *n= ep, np->netc_exflags =3D argp->ex_flags; np->netc_anon =3D crget(); np->netc_anon->cr_uid =3D 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 =3D &prison0; prison_hold(np->netc_anon->cr_prison); np->netc_numsecflavors =3D argp->ex_numsecflavors; @@ -212,8 +216,8 @@ vfs_hang_addrlist(struct mount *mp, struct netexport *n= ep, np->netc_exflags =3D argp->ex_flags; np->netc_anon =3D crget(); np->netc_anon->cr_uid =3D 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 =3D &prison0; prison_hold(np->netc_anon->cr_prison); np->netc_numsecflavors =3D argp->ex_numsecflavors; diff --git a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c b/sys/rpc/rpcsec_gss/svc_r= pcsec_gss.c index 1e6e71fa10ac..b1790dd167d5 100644 --- a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c +++ b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c @@ -537,7 +537,7 @@ rpc_gss_svc_getcred(struct svc_req *req, struct ucred *= *crp, int *flavorp) cr =3D client->cl_cred =3D crget(); cr->cr_uid =3D cr->cr_ruid =3D cr->cr_svuid =3D uc->uid; cr->cr_rgid =3D cr->cr_svgid =3D uc->gid; - crsetgroups(cr, uc->gidlen, uc->gidlist); + crsetgroups_fallback(cr, uc->gidlen, uc->gidlist, uc->gid); cr->cr_prison =3D curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp =3D crhold(cr); diff --git a/sys/rpc/svc_auth.c b/sys/rpc/svc_auth.c index 6acb1fb0d4b9..92f1ee0f2844 100644 --- a/sys/rpc/svc_auth.c +++ b/sys/rpc/svc_auth.c @@ -187,10 +187,12 @@ svc_getcred(struct svc_req *rqst, struct ucred **crp,= int *flavorp) if ((xprt->xp_tls & (RPCTLS_FLAGS_CERTUSER | RPCTLS_FLAGS_DISABLED)) =3D=3D RPCTLS_FLAGS_CERTUSER && flavor =3D=3D AUTH_UNIX) { + if (xprt->xp_ngrps <=3D 0) + return (FALSE); cr =3D crget(); cr->cr_uid =3D cr->cr_ruid =3D cr->cr_svuid =3D xprt->xp_uid; crsetgroups(cr, xprt->xp_ngrps, xprt->xp_gidp); - cr->cr_rgid =3D cr->cr_svgid =3D xprt->xp_gidp[0]; + cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_gid; cr->cr_prison =3D curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp =3D cr; @@ -200,10 +202,12 @@ svc_getcred(struct svc_req *rqst, struct ucred **crp,= int *flavorp) switch (flavor) { case AUTH_UNIX: xcr =3D (struct xucred *) rqst->rq_clntcred; + if (xcr->cr_ngroups <=3D 0) + return (FALSE); cr =3D crget(); cr->cr_uid =3D cr->cr_ruid =3D cr->cr_svuid =3D xcr->cr_uid; crsetgroups(cr, xcr->cr_ngroups, xcr->cr_groups); - cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_groups[0]; + cr->cr_rgid =3D cr->cr_svgid =3D cr->cr_gid; cr->cr_prison =3D curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); *crp =3D cr;