Date: Sat, 02 Nov 2024 23:41:38 -0700 From: Cy Schubert <Cy.Schubert@cschubert.com> To: Ravi Pokala <rpokala@freebsd.org> Cc: Olivier Certner <olce@FreeBSD.org>, 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 Message-ID: <20241103064138.C6B06286@slippy.cwsent.com> In-Reply-To: <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com> References: <202411022039.4A2KdbAE046580@gitrepo.freebsd.org> <9307D0CC-6D10-4F86-AE3B-43E7D6DA19A9@panasas.com>
next in thread | previous in thread | raw e-mail | index | archive | help
It also causes a panic when mountd is started. I'll post details later. -- Cheers, Cy Schubert <Cy.Schubert@cschubert.com> FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org NTP: <cy@nwtime.org> 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: <owner-src-committers@freebsd.org <mailto:owner-src-committers@freebs= > d.org>> on behalf of Olivier Certner <olce@FreeBSD.org <mailto:olce@FreeBSD.= > org>> > Date: Saturday, November 2, 2024 at 13:39 > To: <src-committers@FreeBSD.org <mailto:src-committers@FreeBSD.org>>, <dev-= > commits-src-all@FreeBSD.org <mailto:dev-commits-src-all@FreeBSD.org>>, <dev-= > commits-src-main@FreeBSD.org <mailto:dev-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 <https://cgit.FreeBSD.org/src/commit/?id=3Dcfbe7a62dc62e8a5d7520cb > 5= > eb8ad7c4a9418e26> > > > commit cfbe7a62dc62e8a5d7520cb5eb8ad7c4a9418e26 > Author: Olivier Certner <olce@FreeBSD.org <mailto:olce@FreeBSD.org>> > AuthorDate: 2024-10-02 14:28:59 +0000 > Commit: Olivier Certner <olce@FreeBSD.org <mailto:olce@FreeBSD.org>> > 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 <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 <rpc/types.h> > #include <rpc/auth.h> > > > +#include <fs/nfs/nfsport.h> > + > +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; > > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20241103064138.C6B06286>