git: 7b20967a1a17 - stable/14 - nfs, rpc: Ensure kernel credentials have at least one group
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 15 Nov 2024 10:49:03 UTC
The branch stable/14 has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=7b20967a1a172c6398b7f1a56ec96440d78469d2
commit 7b20967a1a172c6398b7f1a56ec96440d78469d2
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-10-02 14:28:59 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-11-15 10:47:43 +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/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 >= 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 9f0e98bc93ce..29f5a9569b12 100644
--- a/sys/fs/nfs/nfs_commonsubs.c
+++ b/sys/fs/nfs/nfs_commonsubs.c
@@ -4037,8 +4037,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 767bdcd80709..84b579def954 100644
--- a/sys/fs/nfsserver/nfs_nfsdport.c
+++ b/sys/fs/nfsserver/nfs_nfsdport.c
@@ -3293,7 +3293,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/fs/nfsserver/nfs_nfsdsocket.c b/sys/fs/nfsserver/nfs_nfsdsocket.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 = crget();
cr->cr_uid = cr->cr_ruid = cr->cr_svuid = UID_ROOT;
- grp = GID_WHEEL;
- crsetgroups(cr, 1, &grp);
- cr->cr_rgid = cr->cr_svgid = cr->cr_groups[0];
+ crsetgroups_fallback(cr, 0, NULL, GID_WHEEL);
+ 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/kern/vfs_export.c b/sys/kern/vfs_export.c
index 3ff9608b0614..4649a05665e5 100644
--- a/sys/kern/vfs_export.c
+++ b/sys/kern/vfs_export.c
@@ -63,6 +63,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 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 89526544639a..64038240ab37 100644
--- a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
+++ b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
@@ -538,7 +538,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;