From: Andrew Deason Date: Fri, 10 Jan 2020 18:01:50 +0000 (-0600) Subject: OPENAFS-SA-2024-001: afs: Introduce afs_genpag() X-Git-Tag: openafs-stable-1_8_13~22 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=20c22347b41eea2ebbdc0ab15f16c822af44df51 OPENAFS-SA-2024-001: afs: Introduce afs_genpag() CVE-2024-10394 Currently, several areas in the code call genpag() to generate a new PAG id, but the signature of genpag() is very limited. To allow for the code in genpag() to return errors and to examine the calling user's credentials, introduce a new function, afs_genpag(), that does the same thing as genpag(), but accepts creds and allows errors to be returned. Convert all existing callers to use afs_genpag() and to handle any errors, though no errors are ever returned in this commit on its own. To ensure there are no old callers of genpag() left around, change the existing genpag() to be called genpagval(), and declare it static. FIXES 135062 Reviewed-on: https://gerrit.openafs.org/14090 Reviewed-by: Benjamin Kaduk Tested-by: Benjamin Kaduk (cherry picked from commit f701f704c7bc93cf5fd7cffaaa043cef6a99e77f) Change-Id: I675d6cb111ca74638a3b856a3c989dcb2fe6d534 Reviewed-on: https://gerrit.openafs.org/15927 Reviewed-by: Benjamin Kaduk Tested-by: Benjamin Kaduk --- --- a/src/afs/AIX/osi_groups.c +++ b/src/afs/AIX/osi_groups.c @@ -86,6 +86,14 @@ setpag(cred, pagvalue, newpag, change_pa int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return (setuerror(code), code); + } + } + #ifndef AFS_AIX51_ENV ngroups = afs_getgroups(*cred, NGROUPS, gidset); if (afs_get_pag_from_groups(gidset[0], gidset[1]) == NOPAG) { @@ -99,7 +107,7 @@ setpag(cred, pagvalue, newpag, change_pa ngroups += 2; } #endif - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; #ifdef AFS_AIX51_ENV if (change_parent) { code = kcred_setpag(*cred, PAG_AFS, *newpag); --- a/src/afs/DARWIN/osi_groups.c +++ b/src/afs/DARWIN/osi_groups.c @@ -97,6 +97,14 @@ setpag(proc, cred, pagvalue, newpag, cha int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } + ngroups = afs_getgroups(*cred, NGROUPS, gidset); if (afs_get_pag_from_groups(gidset[1], gidset[2]) == NOPAG) { /* We will have to shift grouplist to make room for pag */ @@ -108,7 +116,7 @@ setpag(proc, cred, pagvalue, newpag, cha } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[1], &gidset[2]); code = afs_setgroups(proc, cred, ngroups, gidset, change_parent); return code; --- a/src/afs/FBSD/osi_groups.c +++ b/src/afs/FBSD/osi_groups.c @@ -95,6 +95,14 @@ setpag(struct thread *td, struct ucred * int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } + #ifdef AFS_FBSD80_ENV gidset = osi_Alloc(gidset_len * sizeof(gid_t)); #endif @@ -109,7 +117,7 @@ setpag(struct thread *td, struct ucred * } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[1], &gidset[2]); code = afs_setgroups(td, cred, ngroups, gidset, change_parent); #ifdef AFS_FBSD80_ENV --- a/src/afs/HPUX/osi_groups.c +++ b/src/afs/HPUX/osi_groups.c @@ -70,6 +70,14 @@ setpag(cred, pagvalue, newpag, change_pa int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return (setuerror(code), code); + } + } + ngroups = afs_getgroups(*cred, NGROUPS, gidset); if (afs_get_pag_from_groups(gidset[0], gidset[1]) == NOPAG) { /* We will have to shift grouplist to make room for pag */ @@ -81,7 +89,7 @@ setpag(cred, pagvalue, newpag, change_pa } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[0], &gidset[1]); if (code = afs_setgroups(cred, ngroups, gidset, change_parent)) { --- a/src/afs/IRIX/osi_groups.c +++ b/src/afs/IRIX/osi_groups.c @@ -276,6 +276,17 @@ setpag(cred, pagvalue, newpag, change_pa AFS_STATCNT(setpag); + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { +#if defined(KERNEL_HAVE_UERROR) + return (setuerror(code), code); +#else + return code; +#endif + } + } + ngroups = afs_getgroups(*cred, NGROUPS, gidset); if (afs_get_pag_from_groups(gidset[0], gidset[1]) == NOPAG) { /* We will have to shift grouplist to make room for pag */ @@ -291,7 +302,7 @@ setpag(cred, pagvalue, newpag, change_pa } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[0], &gidset[1]); if (code = afs_setgroups(cred, ngroups, gidset, change_parent)) { #if defined(KERNEL_HAVE_UERROR) --- a/src/afs/LINUX/osi_groups.c +++ b/src/afs/LINUX/osi_groups.c @@ -144,11 +144,19 @@ __setpag(cred_t **cr, afs_uint32 pagvalu { struct group_info *group_info; struct group_info *tmp; + int code; + + if (pagvalue == -1) { + code = afs_genpag(*cr, &pagvalue); + if (code != 0) { + return code; + } + } get_group_info(afs_cr_group_info(*cr)); group_info = afs_cr_group_info(*cr); - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_linux_pag_to_groups(*newpag, group_info, &tmp); if (old_groups) { --- a/src/afs/NBSD/osi_groups.c +++ b/src/afs/NBSD/osi_groups.c @@ -83,6 +83,13 @@ setpag(afs_proc_t *proc, afs_ucred_t **c int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } ngroups = osi_getgroups(*cred, NGROUPS, gidset); if (afs_get_pag_from_groups(gidset[1], gidset[2]) == NOPAG) { /* We will have to shift grouplist to make room for pag */ @@ -94,7 +101,7 @@ setpag(afs_proc_t *proc, afs_ucred_t **c } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[1], &gidset[2]); code = osi_setgroups(proc, cred, ngroups, gidset, change_parent); return code; --- a/src/afs/OBSD/osi_groups.c +++ b/src/afs/OBSD/osi_groups.c @@ -80,6 +80,14 @@ setpag(struct proc *proc, struct ucred * int j; AFS_STATCNT(setpag); + + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } + ngroups = afs_getgroups(*cred, NGROUPS, gidset); /* * If the group list is empty, use the task's primary group as the group @@ -101,7 +109,7 @@ setpag(struct proc *proc, struct ucred * } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[1], &gidset[2]); code = afs_setgroups(proc, cred, ngroups, gidset, change_parent); return code; --- a/src/afs/SOLARIS/osi_groups.c +++ b/src/afs/SOLARIS/osi_groups.c @@ -165,6 +165,13 @@ setpag(cred, pagvalue, newpag, change_pa AFS_STATCNT(setpag); + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } + /* Derive gidset size from running kernel's ngroups_max; * default 16, but configurable up to 32 (Sol10) or * 1024 (Sol11). @@ -174,8 +181,6 @@ setpag(cred, pagvalue, newpag, change_pa /* must use osi_Alloc, osi_AllocSmallSpace may not be enough. */ gidset = osi_Alloc(gidset_sz); - pagvalue = (pagvalue == -1 ? genpag() : pagvalue); - mutex_enter(&curproc->p_crlock); ngroups = afs_getgroups(*cred, gidset); --- a/src/afs/UKERNEL/osi_groups.c +++ b/src/afs/UKERNEL/osi_groups.c @@ -74,6 +74,13 @@ usr_setpag(struct usr_ucred **cred, afs_ AFS_STATCNT(setpag); + if (pagvalue == -1) { + code = afs_genpag(*cred, &pagvalue); + if (code != 0) { + return code; + } + } + gidset = osi_AllocSmallSpace(AFS_SMALLOCSIZ); ngroups = afs_getgroups(*cred, gidset); @@ -88,7 +95,7 @@ usr_setpag(struct usr_ucred **cred, afs_ } ngroups += 2; } - *newpag = (pagvalue == -1 ? genpag() : pagvalue); + *newpag = pagvalue; afs_get_groups_from_pag(*newpag, &gidset[0], &gidset[1]); if ((code = afs_setgroups(cred, ngroups, gidset, change_parent))) { osi_FreeSmallSpace((char *)gidset); --- a/src/afs/afs_osi_pag.c +++ b/src/afs/afs_osi_pag.c @@ -9,7 +9,7 @@ /* * Implements: - * genpag + * genpagval * getpag * afs_setpag * AddPag @@ -67,8 +67,8 @@ afs_uint32 pagCounter = 0; * secure (although of course not absolutely secure). */ #if !defined(UKERNEL) -afs_uint32 -genpag(void) +static afs_uint32 +genpagval(void) { AFS_STATCNT(genpag); #ifdef AFS_LINUX20_ENV @@ -96,8 +96,8 @@ getpag(void) /* Web enhancement: we don't need to restrict pags to 41XXXXXX since * we are not sharing the space with anyone. So we use the full 32 bits. */ -afs_uint32 -genpag(void) +static afs_uint32 +genpagval(void) { AFS_STATCNT(genpag); #ifdef AFS_LINUX20_ENV @@ -182,6 +182,13 @@ afs_pag_wait(afs_ucred_t **acred) return code; } +afs_int32 +afs_genpag(afs_ucred_t *acred, afs_uint32 *apag) +{ + *apag = genpagval(); + return 0; +} + int #if defined(AFS_SUN5_ENV) afs_setpag(afs_ucred_t **credpp) @@ -205,6 +212,7 @@ afs_setpag(void) #endif int code = 0; + afs_uint32 pag; #if defined(AFS_SGI53_ENV) && defined(MP) /* This is our first chance to get the global lock. */ @@ -218,15 +226,19 @@ afs_setpag(void) goto done; } + code = afs_genpag(acred, &pag); + if (code) { + goto done; + } #if defined(AFS_SUN5_ENV) - code = AddPag(genpag(), credpp); + code = AddPag(pag, credpp); #elif defined(AFS_FBSD_ENV) - code = AddPag(td, genpag(), &td->td_ucred); + code = AddPag(td, pag, &td->td_ucred); #elif defined(AFS_NBSD40_ENV) - code = AddPag(p, genpag(), &p->l_proc->p_cred); + code = AddPag(p, pag, &p->l_proc->p_cred); #elif defined(AFS_XBSD_ENV) - code = AddPag(p, genpag(), &p->p_rcred); + code = AddPag(p, pag, &p->p_rcred); #elif defined(AFS_AIX41_ENV) { struct ucred *credp; @@ -234,7 +246,7 @@ afs_setpag(void) credp = crref(); credp0 = credp; - code = AddPag(genpag(), &credp); + code = AddPag(pag, &credp); /* If AddPag() didn't make a new cred, then free our cred ref */ if (credp == credp0) { crfree(credp); @@ -243,36 +255,36 @@ afs_setpag(void) #elif defined(AFS_HPUX110_ENV) { struct ucred *credp = p_cred(u.u_procp); - code = AddPag(genpag(), &credp); + code = AddPag(pag, &credp); } #elif defined(AFS_SGI_ENV) { cred_t *credp; credp = OSI_GET_CURRENT_CRED(); - code = AddPag(genpag(), &credp); + code = AddPag(pag, &credp); } #elif defined(AFS_LINUX20_ENV) { afs_ucred_t *credp = crref(); - code = AddPag(genpag(), &credp); + code = AddPag(pag, &credp); crfree(credp); } #elif defined(AFS_DARWIN80_ENV) { afs_ucred_t *credp = kauth_cred_proc_ref(p); - code = AddPag(p, genpag(), &credp); + code = AddPag(p, pag, &credp); crfree(credp); } #elif defined(AFS_DARWIN_ENV) { afs_ucred_t *credp = crdup(p->p_cred->pc_ucred); - code = AddPag(p, genpag(), &credp); + code = AddPag(p, pag, &credp); crfree(credp); } #elif defined(UKERNEL) - code = AddPag(genpag(), &(get_user_struct()->u_cred)); + code = AddPag(pag, &(get_user_struct()->u_cred)); #else - code = AddPag(genpag(), &u.u_cred); + code = AddPag(pag, &u.u_cred); #endif done: @@ -294,7 +306,7 @@ afs_setpag(void) /* * afs_setpag_val * This function is like setpag but sets the current thread's pag id to a - * caller-provided value instead of calling genpag(). This implements a + * caller-provided value instead of calling afs_genpag(). This implements a * form of token caching since the caller can recall a particular pag value * for the thread to restore tokens, rather than reauthenticating. */ --- a/src/afs/afs_pioctl.c +++ b/src/afs/afs_pioctl.c @@ -4646,7 +4646,13 @@ HandleClientContext(struct afs_ioctl *ab *acred = newcred; if (!code && *com == PSETPAG) { /* Special case for 'setpag' */ - afs_uint32 pagvalue = genpag(); + afs_uint32 pagvalue; + + code = afs_genpag(*acred, &pagvalue); + if (code != 0) { + EXP_RELE(outexporter); + return code; + } au = afs_GetUser(pagvalue, -1, WRITE_LOCK); /* a new unixuser struct */ /* --- a/src/afs/afs_prototypes.h +++ b/src/afs/afs_prototypes.h @@ -588,7 +588,7 @@ extern int afs_setpag(afs_proc_t *p, voi extern int afs_setpag(void); #endif -extern afs_uint32 genpag(void); +extern afs_int32 afs_genpag(afs_ucred_t *acred, afs_uint32 *apag); extern afs_uint32 getpag(void); #if defined(AFS_FBSD_ENV) extern int AddPag(struct thread *td, afs_int32 aval, afs_ucred_t **credpp); --- a/src/afs/afs_stats.h +++ b/src/afs/afs_stats.h @@ -545,7 +545,7 @@ struct afs_MeanStats { AFS_CS(afs_swapvp) /* afs_vfsops.c */ \ AFS_CS(afs_AddMarinerName) /* afs_vnodeops.c */ \ AFS_CS(afs_setpag) /* afs_vnodeops.c */ \ - AFS_CS(genpag) /* afs_vnodeops.c */ \ + AFS_CS(genpag) /* afs_vnodeops.c, renamed to genpagval() */ \ AFS_CS(getpag) /* afs_vnodeops.c */ \ AFS_CS(afs_GetMariner) /* afs_vnodeops.c */ \ AFS_CS(afs_badop) /* afs_vnodeops.c */ \