From fb28338b5310246e597404f7d2e85d449eca70ba Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Mon, 15 Jun 2026 13:14:34 +0900 Subject: [PATCH] Treat combined Match User/Group blocks as a conjunction in wolfsshd --- apps/wolfsshd/auth.c | 172 +++++++++++- apps/wolfsshd/auth.h | 3 + apps/wolfsshd/configuration.c | 50 +++- apps/wolfsshd/configuration.h | 2 +- apps/wolfsshd/include.am | 4 +- apps/wolfsshd/test/test_configuration.c | 331 ++++++++++++++++++------ 6 files changed, 460 insertions(+), 102 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 35c440e3c..50225637a 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1084,10 +1084,10 @@ static int DoCheckUser(const char* usr, WOLFSSHD_AUTH* auth) * scoped to this branch. * * A NULL return means the root user's configuration could not be - * resolved (e.g. getgrgid() failure inside wolfSSHD_AuthGetUserConf). - * Fail closed and reject the login in that case rather than falling - * back to the global node, since denying root on an unresolvable - * configuration is the safe choice. */ + * resolved (e.g. group set enumeration failed inside + * wolfSSHD_AuthGetUserConf). Fail closed and reject the login in that + * case rather than falling back to the global node, since denying root + * on an unresolvable configuration is the safe choice. */ usrConf = wolfSSHD_AuthGetUserConf(auth, usr, NULL, NULL, NULL, NULL, NULL); if (usrConf == NULL || wolfSSHD_ConfigGetPermitRoot(usrConf) == 0) { @@ -1194,7 +1194,7 @@ static int RequestAuthentication(WS_UserAuthData* authData, * non-Match users while enforcing Match restrictions. * * A NULL return here means the user's configuration could not be resolved - * (e.g. the primary group is unresolvable and getgrgid() failed inside + * (e.g. the user's group set could not be enumerated inside * wolfSSHD_AuthGetUserConf). DoCheckUser has already confirmed the user * exists, so this is a rare edge. Fail closed rather than fall back to the * permissive global node: such a user cannot complete a session anyway @@ -1752,6 +1752,144 @@ int wolfSSHD_AuthReducePermissions(WOLFSSHD_AUTH* auth) #else #define WGETGROUPLIST(x,y,z,w) getgrouplist((x),(y),(z),(w)) #endif + +/* Initial guess and upper bound for the number of groups a user can be in. + * getgrouplist cannot be reliably sized with a NULL probe (macOS returns + * success with size 0 and, when the buffer is too small, echoes the input size + * rather than the needed count), so the buffer grows from the guess up to the + * bound. */ +#ifndef WOLFSSHD_GROUP_LIST_INIT +#define WOLFSSHD_GROUP_LIST_INIT 32 +#endif +#ifndef WOLFSSHD_GROUP_LIST_MAX +#define WOLFSSHD_GROUP_LIST_MAX 65536 +#endif + +/* frees a group name array previously built by wolfSSHD_GetUserGroupNames */ +WOLFSSHD_STATIC void wolfSSHD_FreeUserGroupNames(void* heap, char** names, + word32 count) +{ + word32 i; + + if (names != NULL) { + for (i = 0; i < count; i++) { + WFREE(names[i], heap, DYNTYPE_SSHD); + } + WFREE(names, heap, DYNTYPE_SSHD); + } +} + +/* Builds the list of group names the user belongs to, primary plus + * supplementary, so Match Group can be evaluated against the full set. On + * success sets *outNames to an owned array of owned names and *outCount to the + * entry count; the caller frees them with wolfSSHD_FreeUserGroupNames. Returns + * WS_SUCCESS on success, leaving *outNames NULL on failure. */ +WOLFSSHD_STATIC int wolfSSHD_GetUserGroupNames(void* heap, const char* usr, + WGID_T primaryGid, char*** outNames, word32* outCount) +{ + int ret = WS_SUCCESS; + int grpListSz = 0; + int allocSz; + int res; + int i; + gid_t* grpList = NULL; + char** names = NULL; + struct group* g; + word32 count = 0; + + *outNames = NULL; + *outCount = 0; + +#if defined(__QNX__) || defined(__QNXNTO__) + /* QNX cannot report the size ahead of time, so allocate the max and fill + * once; getgrouplist returns 0 on success there. */ + allocSz = (int)sysconf(_SC_NGROUPS_MAX); + if (allocSz <= 0) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS) { + grpList = (gid_t*)WMALLOC(sizeof(gid_t) * allocSz, heap, DYNTYPE_SSHD); + if (grpList == NULL) { + ret = WS_MEMORY_E; + } + } + if (ret == WS_SUCCESS) { + grpListSz = allocSz; + res = WGETGROUPLIST(usr, primaryGid, grpList, &grpListSz); + if (res != 0) { + ret = WS_FATAL_ERROR; + } + } +#else + /* Grow the buffer until the lookup fits: a NULL probe is unreliable and a + * too-small buffer does not report the needed count on all platforms. On + * success grpListSz holds the actual number of groups. */ + allocSz = WOLFSSHD_GROUP_LIST_INIT; + res = -1; + while (ret == WS_SUCCESS && res < 0) { + grpList = (gid_t*)WMALLOC(sizeof(gid_t) * allocSz, heap, DYNTYPE_SSHD); + if (grpList == NULL) { + ret = WS_MEMORY_E; + break; + } + + grpListSz = allocSz; + res = WGETGROUPLIST(usr, primaryGid, grpList, &grpListSz); + if (res < 0) { + /* buffer too small: discard, grow, and retry up to the cap */ + WFREE(grpList, heap, DYNTYPE_SSHD); + grpList = NULL; + if (allocSz >= WOLFSSHD_GROUP_LIST_MAX) { + ret = WS_FATAL_ERROR; + break; + } + allocSz *= 2; + if (allocSz > WOLFSSHD_GROUP_LIST_MAX) { + allocSz = WOLFSSHD_GROUP_LIST_MAX; + } + } + } +#endif + + if (ret == WS_SUCCESS) { + names = (char**)WMALLOC(sizeof(char*) * grpListSz, heap, DYNTYPE_SSHD); + if (names == NULL) { + ret = WS_MEMORY_E; + } + } + + if (ret == WS_SUCCESS) { + for (i = 0; i < grpListSz; i++) { + /* Skip gids that do not resolve to a name rather than failing the + * login, matching OpenSSH. Copy immediately, since getgrgid reuses + * a static buffer the next call overwrites. */ + g = getgrgid(grpList[i]); + if (g == NULL || g->gr_name == NULL) { + continue; + } + names[count] = WSTRDUP(g->gr_name, heap, DYNTYPE_SSHD); + if (names[count] == NULL) { + ret = WS_MEMORY_E; + break; + } + count++; + } + } + + if (ret == WS_SUCCESS) { + *outNames = names; + *outCount = count; + } + else { + wolfSSHD_FreeUserGroupNames(heap, names, count); + } + + if (grpList != NULL) { + WFREE(grpList, heap, DYNTYPE_SSHD); + } + + return ret; +} #endif /* WIN32 */ /* sets the extended groups the user is in, returns WS_SUCCESS on success */ @@ -1827,32 +1965,38 @@ WOLFSSHD_CONFIG* wolfSSHD_AuthGetUserConf(const WOLFSSHD_AUTH* auth, const char* adr) { WOLFSSHD_CONFIG* ret = NULL; + char** grpNames = NULL; + word32 grpCount = 0; if (auth != NULL) { - char* gName = NULL; - if (usr != NULL) { #ifdef WIN32 - //LogonUserEx() + /* LogonUserEx(): group lookup is not implemented on Windows, so + * Match Group directives do not apply here */ #else struct passwd* p_passwd; - struct group* g = NULL; p_passwd = getpwnam((const char *)usr); if (p_passwd == NULL) { return NULL; } - g = getgrgid(p_passwd->pw_gid); - if (g == NULL) { + /* Resolve the full group set (primary and supplementary) so a + * Match Group directive matches on any of the user's groups. + * Fail closed if the groups cannot be enumerated. */ + if (wolfSSHD_GetUserGroupNames(auth->heap, usr, p_passwd->pw_gid, + &grpNames, &grpCount) != WS_SUCCESS) { return NULL; } - gName = g->gr_name; #endif } - ret = wolfSSHD_GetUserConf(auth->conf, usr, gName, host, localAdr, - localPort, RDomain, adr); + ret = wolfSSHD_GetUserConf(auth->conf, usr, (const char**)grpNames, + grpCount, host, localAdr, localPort, RDomain, adr); + +#ifndef WIN32 + wolfSSHD_FreeUserGroupNames(auth->heap, grpNames, grpCount); +#endif } return ret; } diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index e4b6178d0..298ea4690 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -84,6 +84,9 @@ int wolfSSHD_GetHomeDirectory(WOLFSSHD_AUTH* auth, WOLFSSH* ssh, WCHAR* out, int #ifndef _WIN32 extern int (*wsshd_setregid_cb)(WGID_T, WGID_T); extern int (*wsshd_setreuid_cb)(WUID_T, WUID_T); +int wolfSSHD_GetUserGroupNames(void* heap, const char* usr, WGID_T primaryGid, + char*** outNames, word32* outCount); +void wolfSSHD_FreeUserGroupNames(void* heap, char** names, word32 count); #endif #if defined(WOLFSSH_HAVE_LIBCRYPT) || defined(WOLFSSH_HAVE_LIBLOGIN) int CheckPasswordHashUnix(const char* input, char* stored); diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 3bb89b4ea..1a1c85dd4 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -1397,32 +1397,58 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth) /* returns the config associated with the user */ WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, - const char* usr, const char* grp, const char* host, + const char* usr, const char** grps, word32 grpCount, const char* host, const char* localAdr, word16* localPort, const char* RDomain, const char* adr) { WOLFSSHD_CONFIG* ret; WOLFSSHD_CONFIG* current; + int matches; + word32 i; /* default to return head of list */ ret = current = (WOLFSSHD_CONFIG*)conf; while (current != NULL) { - /* compare current configs user */ - if (usr != NULL && current->usrAppliesTo != NULL) { - if (XSTRCMP(current->usrAppliesTo, usr) == 0) { - ret = current; - break; + /* A node is a Match candidate only if it carries at least one + * selector. Every non-NULL selector on the node must match for the + * node to apply, so a combined 'Match User X Group Y' is treated as a + * conjunction the same way OpenSSH treats a Match line. A NULL + * selector acts as a wildcard. */ + matches = 0; + if (current->usrAppliesTo != NULL || current->groupAppliesTo != NULL) { + matches = 1; + + if (current->usrAppliesTo != NULL) { + if (usr == NULL || + XSTRCMP(current->usrAppliesTo, usr) != 0) { + matches = 0; + } } - } - /* compare current configs group */ - if (grp != NULL && current->groupAppliesTo != NULL) { - if (XSTRCMP(current->groupAppliesTo, grp) == 0) { - ret = current; - break; + /* The group selector matches when it equals any group the user + * belongs to, primary or supplementary, mirroring how OpenSSH + * evaluates 'Match Group'. An empty group list matches no group + * selector, so combined blocks fail closed. */ + if (matches && current->groupAppliesTo != NULL) { + matches = 0; + if (grps != NULL) { + for (i = 0; i < grpCount; i++) { + if (grps[i] == NULL) + continue; + if (XSTRCMP(current->groupAppliesTo, grps[i]) == 0) { + matches = 1; + break; + } + } + } } } + if (matches) { + ret = current; + break; + } + current = current->next; } diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index 64aa82053..8c8a542d8 100644 --- a/apps/wolfsshd/configuration.h +++ b/apps/wolfsshd/configuration.h @@ -63,7 +63,7 @@ long wolfSSHD_ConfigGetGraceTime(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPwAuth(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPubKeyAuth(const WOLFSSHD_CONFIG* conf); WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, - const char* usr, const char* grp, const char* host, + const char* usr, const char** grps, word32 grpCount, const char* host, const char* localAdr, word16* localPort, const char* RDomain, const char* adr); void wolfSSHD_ConfigSavePID(const WOLFSSHD_CONFIG* conf); diff --git a/apps/wolfsshd/include.am b/apps/wolfsshd/include.am index cea1afe16..60ddf8d0f 100644 --- a/apps/wolfsshd/include.am +++ b/apps/wolfsshd/include.am @@ -17,6 +17,8 @@ apps_wolfsshd_test_test_configuration_SOURCES = apps/wolfsshd/test/test_configur apps/wolfsshd/auth.h apps_wolfsshd_test_test_configuration_LDADD = src/libwolfssh.la apps_wolfsshd_test_test_configuration_DEPENDENCIES = src/libwolfssh.la -apps_wolfsshd_test_test_configuration_CPPFLAGS = $(AM_CPPFLAGS) -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -I$(srcdir)/apps/wolfsshd/ +# Force the smallest initial group buffer so test_GetUserGroupNames exercises +# the getgrouplist buffer-growth/realloc loop under the sanitizers. +apps_wolfsshd_test_test_configuration_CPPFLAGS = $(AM_CPPFLAGS) -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -DWOLFSSHD_GROUP_LIST_INIT=1 -I$(srcdir)/apps/wolfsshd/ endif BUILD_SSHD diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 86131bc0f..ab51cc1e4 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -17,6 +17,10 @@ #ifdef HAVE_CRYPT_H #include #endif +#ifndef _WIN32 + #include + #include +#endif #include #include @@ -334,7 +338,7 @@ static int test_ConfigCopy(void) /* retrieve match node from the list head */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "testuser", NULL, NULL, NULL, + match = wolfSSHD_GetUserConf(head, "testuser", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head) ret = WS_FATAL_ERROR; @@ -484,7 +488,7 @@ static int test_GetUserConfMatchOverride(void) /* resolving testuser must return the per-user node, not the global head */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "testuser", NULL, NULL, NULL, + match = wolfSSHD_GetUserConf(head, "testuser", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head) ret = WS_FATAL_ERROR; @@ -513,7 +517,7 @@ static int test_GetUserConfMatchOverride(void) /* a user with no Match block must fall back to the permissive global head, * confirming the default behavior is unchanged for non-Match users */ if (ret == WS_SUCCESS) { - other = wolfSSHD_GetUserConf(head, "otheruser", NULL, NULL, NULL, + other = wolfSSHD_GetUserConf(head, "otheruser", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (other != head) ret = WS_FATAL_ERROR; @@ -593,7 +597,199 @@ static int test_MatchUnsupportedSelector(void) } wolfSSHD_ConfigFree(head); } + return ret; +} + +/* A combined 'Match User X Group Y' directive is a conjunction: it applies only + * to a user who satisfies BOTH selectors, matching OpenSSH semantics. This + * locks in that wolfSSHD_GetUserConf does not return such a block for a user + * who satisfies only one selector (the policy-bypass case), while single + * selector 'Match User' and 'Match Group' blocks keep applying on their one + * selector alone. */ +static int test_GetUserConfMatchGroupAnd(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* head; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_CONFIG* combined; + WOLFSSHD_CONFIG* userOnly; + WOLFSSHD_CONFIG* groupOnly; + WOLFSSHD_CONFIG* match; + const char* grps[1]; + + head = wolfSSHD_ConfigNew(NULL); + if (head == NULL) + ret = WS_MEMORY_E; + conf = head; + +#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0) + /* restrictive global default: SFTP only for everyone */ + if (ret == WS_SUCCESS) ret = PCL("ForceCommand internal-sftp"); + + /* combined block relaxes the restriction, but only for a user who is both + * 'alice' AND in group 'admins' */ + if (ret == WS_SUCCESS) ret = PCL("Match User alice Group admins"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/sh"); + if (ret == WS_SUCCESS) combined = conf; + + /* single selector blocks must keep matching on their one selector */ + if (ret == WS_SUCCESS) ret = PCL("Match User bob"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/bob"); + if (ret == WS_SUCCESS) userOnly = conf; + + if (ret == WS_SUCCESS) ret = PCL("Match Group staff"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/staff"); + if (ret == WS_SUCCESS) groupOnly = conf; +#undef PCL + + /* alice in admins satisfies both selectors -> gets the combined block */ + if (ret == WS_SUCCESS) { + grps[0] = "admins"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != combined) + ret = WS_FATAL_ERROR; + } + + /* alice in a different group satisfies only the user selector -> must NOT + * get the combined block; falls back to the restrictive global head */ + if (ret == WS_SUCCESS) { + grps[0] = "users"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + + /* carol in admins satisfies only the group selector -> must NOT get the + * combined block; falls back to the restrictive global head */ + if (ret == WS_SUCCESS) { + grps[0] = "admins"; + match = wolfSSHD_GetUserConf(head, "carol", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + + /* alice with no resolved group must NOT get the combined block: an empty + * group set cannot satisfy the group selector, so it fails closed to the + * global head. This is the WIN32 / failed group-lookup path where + * wolfSSHD_AuthGetUserConf passes an empty group list. */ + if (ret == WS_SUCCESS) { + match = wolfSSHD_GetUserConf(head, "alice", NULL, 0, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + + /* single selector 'Match User bob' still applies on the user alone */ + if (ret == WS_SUCCESS) { + grps[0] = "anygroup"; + match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != userOnly) + ret = WS_FATAL_ERROR; + } + + /* single selector 'Match Group staff' still applies on the group alone */ + if (ret == WS_SUCCESS) { + grps[0] = "staff"; + match = wolfSSHD_GetUserConf(head, "anyuser", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != groupOnly) + ret = WS_FATAL_ERROR; + } + + wolfSSHD_ConfigFree(head); + return ret; +} + +/* A 'Match Group' directive must match on any of the user's groups, primary or + * supplementary, the way OpenSSH evaluates it. The matcher receives the full + * group set as a list, so a group named only in a secondary slot still selects + * the block. The combined 'Match User X Group Y' conjunction must likewise be + * satisfiable when the group is a secondary one. */ +static int test_GetUserConfMatchSecondaryGroup(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* head; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_CONFIG* blockWS; + WOLFSSHD_CONFIG* blockComb; + WOLFSSHD_CONFIG* match; + const char* grps[3]; + + head = wolfSSHD_ConfigNew(NULL); + if (head == NULL) + ret = WS_MEMORY_E; + conf = head; + +#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0) + /* restrictive global default */ + if (ret == WS_SUCCESS) ret = PCL("ForceCommand internal-sftp"); + + /* single group selector */ + if (ret == WS_SUCCESS) ret = PCL("Match Group wireshark"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/ws"); + if (ret == WS_SUCCESS) blockWS = conf; + + /* combined user AND group selector */ + if (ret == WS_SUCCESS) ret = PCL("Match User john Group admins"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/adm"); + if (ret == WS_SUCCESS) blockComb = conf; +#undef PCL + + /* wireshark present only in a secondary slot must still select the block */ + if (ret == WS_SUCCESS) { + grps[0] = "alice"; + grps[1] = "staff"; + grps[2] = "wireshark"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 3, NULL, NULL, + NULL, NULL, NULL); + if (match != blockWS) + ret = WS_FATAL_ERROR; + } + + /* combined block satisfied with the group in a secondary slot */ + if (ret == WS_SUCCESS) { + grps[0] = "john"; + grps[1] = "admins"; + match = wolfSSHD_GetUserConf(head, "john", grps, 2, NULL, NULL, + NULL, NULL, NULL); + if (match != blockComb) + ret = WS_FATAL_ERROR; + } + + /* combined block: user matches but group absent -> falls back to head */ + if (ret == WS_SUCCESS) { + grps[0] = "john"; + grps[1] = "users"; + match = wolfSSHD_GetUserConf(head, "john", grps, 2, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + + /* combined block: group matches but user differs -> falls back to head */ + if (ret == WS_SUCCESS) { + grps[0] = "bob"; + grps[1] = "admins"; + match = wolfSSHD_GetUserConf(head, "bob", grps, 2, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + /* user in none of the selector groups -> falls back to head */ + if (ret == WS_SUCCESS) { + grps[0] = "carol"; + match = wolfSSHD_GetUserConf(head, "carol", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != head) + ret = WS_FATAL_ERROR; + } + + wolfSSHD_ConfigFree(head); return ret; } @@ -611,6 +807,7 @@ static int test_GetUserConfMatchSubstring(void) WOLFSSHD_CONFIG* conf; WOLFSSHD_CONFIG* match; WOLFSSHD_CONFIG* ghost; + const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); if (head == NULL) @@ -627,7 +824,7 @@ static int test_GetUserConfMatchSubstring(void) /* lookup by the real user name must resolve to the Match node */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "GroupAdmin", NULL, NULL, NULL, + match = wolfSSHD_GetUserConf(head, "GroupAdmin", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head) ret = WS_FATAL_ERROR; @@ -640,7 +837,8 @@ static int test_GetUserConfMatchSubstring(void) /* lookup by the ghost group token ("Admin") must NOT match; it falls back * to the permissive-denied global head */ if (ret == WS_SUCCESS) { - ghost = wolfSSHD_GetUserConf(head, NULL, "Admin", NULL, NULL, + grps[0] = "Admin"; + ghost = wolfSSHD_GetUserConf(head, NULL, grps, 1, NULL, NULL, NULL, NULL, NULL); if (ghost != head) ret = WS_FATAL_ERROR; @@ -667,6 +865,7 @@ static int test_GetUserConfMatchSubstringGroup(void) WOLFSSHD_CONFIG* conf; WOLFSSHD_CONFIG* match; WOLFSSHD_CONFIG* ghost; + const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); if (head == NULL) @@ -683,7 +882,8 @@ static int test_GetUserConfMatchSubstringGroup(void) /* lookup by the real group name must resolve to the Match node */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, NULL, "UserStaff", NULL, NULL, + grps[0] = "UserStaff"; + match = wolfSSHD_GetUserConf(head, NULL, grps, 1, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head) ret = WS_FATAL_ERROR; @@ -696,7 +896,7 @@ static int test_GetUserConfMatchSubstringGroup(void) /* lookup by the ghost user token ("Staff") must NOT match; it falls back * to the permissive-denied global head */ if (ret == WS_SUCCESS) { - ghost = wolfSSHD_GetUserConf(head, "Staff", NULL, NULL, NULL, + ghost = wolfSSHD_GetUserConf(head, "Staff", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (ghost != head) ret = WS_FATAL_ERROR; @@ -726,6 +926,7 @@ static int test_GetUserConfMatchLiteralKeywordName(void) WOLFSSHD_CONFIG* conf; WOLFSSHD_CONFIG* match; WOLFSSHD_CONFIG* ghost; + const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); if (head == NULL) @@ -742,7 +943,7 @@ static int test_GetUserConfMatchLiteralKeywordName(void) /* lookup by the user name "Group" must resolve to the Match node */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "Group", NULL, NULL, NULL, + match = wolfSSHD_GetUserConf(head, "Group", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head) ret = WS_FATAL_ERROR; @@ -755,7 +956,8 @@ static int test_GetUserConfMatchLiteralKeywordName(void) /* the user name must NOT have leaked into groupAppliesTo: a lookup by group * "Group" must fall back to the permissive-denied global head */ if (ret == WS_SUCCESS) { - ghost = wolfSSHD_GetUserConf(head, NULL, "Group", NULL, NULL, + grps[0] = "Group"; + ghost = wolfSSHD_GetUserConf(head, NULL, grps, 1, NULL, NULL, NULL, NULL, NULL); if (ghost != head) ret = WS_FATAL_ERROR; @@ -835,7 +1037,7 @@ static int test_GetUserConfMatchRepeatedKeyword(void) /* lookup by the replacement name "b" must resolve to the Match node */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "b", NULL, NULL, NULL, + match = wolfSSHD_GetUserConf(head, "b", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (match == NULL || match == head || wolfSSHD_ConfigGetPwAuth(match) != 1) @@ -844,7 +1046,7 @@ static int test_GetUserConfMatchRepeatedKeyword(void) /* lookup by the replaced name "a" must NOT match; it falls back to head */ if (ret == WS_SUCCESS) { - old = wolfSSHD_GetUserConf(head, "a", NULL, NULL, NULL, + old = wolfSSHD_GetUserConf(head, "a", NULL, 0, NULL, NULL, NULL, NULL, NULL); if (old != head) ret = WS_FATAL_ERROR; @@ -858,68 +1060,6 @@ static int test_GetUserConfMatchRepeatedKeyword(void) return ret; } -/* A single Match line carrying both criteria ("Match User alice Group admins") - * is parsed as two keyword/name pairs, setting both usrAppliesTo="alice" and - * groupAppliesTo="admins" on one node. Both the user and the group lookup must - * resolve to that node, while unrelated names fall back to the global head. - * This locks in that the parser still extracts both names when both keywords - * share a line. */ -static int test_GetUserConfMatchUserAndGroup(void) -{ - int ret = WS_SUCCESS; - WOLFSSHD_CONFIG* head; - WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* byUsr; - WOLFSSHD_CONFIG* byGrp; - WOLFSSHD_CONFIG* other; - - head = wolfSSHD_ConfigNew(NULL); - if (head == NULL) - ret = WS_MEMORY_E; - conf = head; - -#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0) - if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no"); - - /* both criteria on one line */ - if (ret == WS_SUCCESS) ret = PCL("Match User alice Group admins"); - if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); -#undef PCL - - /* lookup by the user criterion must resolve to the Match node */ - if (ret == WS_SUCCESS) { - byUsr = wolfSSHD_GetUserConf(head, "alice", NULL, NULL, NULL, - NULL, NULL, NULL); - if (byUsr == NULL || byUsr == head || - wolfSSHD_ConfigGetPwAuth(byUsr) != 1) - ret = WS_FATAL_ERROR; - } - - /* lookup by the group criterion must resolve to the same Match node */ - if (ret == WS_SUCCESS) { - byGrp = wolfSSHD_GetUserConf(head, NULL, "admins", NULL, NULL, - NULL, NULL, NULL); - if (byGrp == NULL || byGrp == head || - wolfSSHD_ConfigGetPwAuth(byGrp) != 1) - ret = WS_FATAL_ERROR; - } - - /* unrelated user and group must fall back to the permissive-denied head */ - if (ret == WS_SUCCESS) { - other = wolfSSHD_GetUserConf(head, "bob", "staff", NULL, NULL, - NULL, NULL, NULL); - if (other != head) - ret = WS_FATAL_ERROR; - } - if (ret == WS_SUCCESS) { - if (wolfSSHD_ConfigGetPwAuth(head) != 0) - ret = WS_FATAL_ERROR; - } - - wolfSSHD_ConfigFree(head); - return ret; -} - /* Bounded recursion through Include directives: a self-including config * must fail with WS_BAD_ARGUMENT once the depth limit is hit, and the * config object must remain usable so a subsequent load of a normal @@ -1229,6 +1369,47 @@ static void InstallPrivDropStubs(int regidRet, int reuidRet, s_setreuid_arg0 = s_setreuid_arg1 = 0; } +/* Exercises the group-name enumeration helper against the account running the + * test. Covers the allocation and ownership contract end to end (the names + * array, each duplicated name, and the gid scratch buffer) so a sanitizer run + * guards the new auth.c cleanup paths. */ +static int test_GetUserGroupNames(void) +{ + int ret = WS_SUCCESS; + struct passwd* pw; + char** names = NULL; + word32 count = 0; + word32 i; + + pw = getpwuid(getuid()); + if (pw == NULL) { + /* no account for the running uid, nothing to exercise */ + return WS_SUCCESS; + } + + ret = wolfSSHD_GetUserGroupNames(NULL, pw->pw_name, pw->pw_gid, &names, + &count); + + /* every user belongs to at least their primary group */ + if (ret == WS_SUCCESS) { + if (names == NULL || count == 0) + ret = WS_FATAL_ERROR; + } + + /* unresolvable gids are skipped, so every populated entry is non-NULL */ + if (ret == WS_SUCCESS) { + for (i = 0; i < count; i++) { + if (names[i] == NULL) { + ret = WS_FATAL_ERROR; + break; + } + } + } + + wolfSSHD_FreeUserGroupNames(NULL, names, count); + return ret; +} + static int test_AuthReducePermissionsUser_ok(void) { int ret = WS_SUCCESS; @@ -1434,7 +1615,8 @@ const TEST_CASE testCases[] = { TEST_DECL(test_GetUserConfMatchLiteralKeywordName), TEST_DECL(test_GetUserConfMatchBareKeyword), TEST_DECL(test_GetUserConfMatchRepeatedKeyword), - TEST_DECL(test_GetUserConfMatchUserAndGroup), + TEST_DECL(test_GetUserConfMatchGroupAnd), + TEST_DECL(test_GetUserConfMatchSecondaryGroup), TEST_DECL(test_CAKeysFileDiffers), TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_GetUserAuthTypes), @@ -1443,6 +1625,7 @@ const TEST_CASE testCases[] = { TEST_DECL(test_CheckAuthKeysLine), #endif #ifndef _WIN32 + TEST_DECL(test_GetUserGroupNames), TEST_DECL(test_AuthReducePermissionsUser_ok), TEST_DECL(test_AuthReducePermissionsUser_gid_fail), TEST_DECL(test_AuthReducePermissionsUser_uid_fail),