From nobody Sun Nov 03 06:41:38 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 4Xh4kv60qSz5bxh3; Sun, 03 Nov 2024 06:41:43 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from omta001.cacentral1.a.cloudfilter.net (omta001.cacentral1.a.cloudfilter.net [3.97.99.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Xh4kv1hhZz49vC; Sun, 3 Nov 2024 06:41:43 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Authentication-Results: mx1.freebsd.org; none Received: from shw-obgw-4001a.ext.cloudfilter.net ([10.228.9.142]) by cmsmtp with ESMTPS id 7UFEtapQkAHbZ7UIut3GIG; Sun, 03 Nov 2024 06:41:40 +0000 Received: from spqr.komquats.com ([70.66.152.170]) by cmsmtp with ESMTPSA id 7UIttM5U4GvSV7UIuty6GK; Sun, 03 Nov 2024 06:41:40 +0000 X-Auth-User: cschuber X-Authority-Analysis: v=2.4 cv=FpSm/Hrq c=1 sm=1 tr=0 ts=67271b24 a=y8EK/9tc/U6QY+pUhnbtgQ==:117 a=y8EK/9tc/U6QY+pUhnbtgQ==:17 a=kj9zAlcOel0A:10 a=VlfZXiiP6vEA:10 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=EkcXrb_YAAAA:8 a=cOLrLDMxAAAA:8 a=vUPWEWiMAAAA:8 a=fY1FzLYsdD8ry1HIQnYA:9 a=CjuIK1q_8ugA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=LK5xJRSDVpKd5WXXoEvA:22 a=P0s3qUPvOpV5zndjNR8V:22 a=s3Yi14Of9AgBIP63TAoC:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTP id EAE673D3; Sat, 02 Nov 2024 23:41:38 -0700 (PDT) Received: by slippy.cwsent.com (Postfix, from userid 1000) id C6B06286; Sat, 02 Nov 2024 23:41:38 -0700 (PDT) X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.8+dev Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Ravi Pokala cc: Olivier Certner , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: cfbe7a62dc62 - main - nfs, rpc: Ensure kernel credentials have at least one group In-reply-to: <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com> References: <202411022039.4A2KdbAE046580@gitrepo.freebsd.org> <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com> Comments: In-reply-to Ravi Pokala message dated "Sat, 02 Nov 2024 20:58:12 -0700." 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=us-ascii Date: Sat, 02 Nov 2024 23:41:38 -0700 Message-Id: <20241103064138.C6B06286@slippy.cwsent.com> X-CMAE-Envelope: MS4xfPDdSmstVY4HyyP2bSnOfdR7H03OhDK2mGRlW04zXLpDmB1bqCJYsXTJNoUzsNt8pKH0Kfy/iRNCR3AkSS701ekSFhXRDmKBG2v8p5Gt9UkYhWQC8foc KgtZuDTBL/vjzBvAL1D+O5p69u3Gyu97+yrHYsmVbc/suFEfEOiGL5Y0LweJ8Ip63ioSouoczgHYxxm2Xy8AOaJV/gCRbdeEOOpNggbHKe41W5E1ykBCIikW ydYbkur4drVg1F1jXhcs8ThyySwJToBARBHgzOyI2hI0Fr4mXJeuAnsN/4kgFiDZSo3BTf3Dd/sKKRJTuF5fTmxMG/GLy5vJLlDOhHk8aFoSan13iqNtIDCH 3EDQ4Fsv X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:16509, ipnet:3.96.0.0/15, country:US] X-Rspamd-Queue-Id: 4Xh4kv1hhZz49vC X-Spamd-Bar: ---- It also causes a panic when mountd is started. I'll post details later. -- Cheers, Cy Schubert FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org e^(i*pi)+1=0 In message <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com>, Ravi Pokala writ es: > 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=3 > D=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=3 > D=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: d.org>> on behalf of Olivier Certner org>> > Date: Saturday, November 2, 2024 at 13:39 > To: >, commits-src-all@FreeBSD.org >, commits-src-main@FreeBSD.org > > 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 5= > eb8ad7c4a9418e26> > > > 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 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; > > > >