git: fb880a5a119d - stable/13 - 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 13:00:54 UTC
The branch stable/13 has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=fb880a5a119d37d5ca35780235646d56fc21d4aa
commit fb880a5a119d37d5ca35780235646d56fc21d4aa
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-10-02 14:28:59 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
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 <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 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;