Treat combined Match User/Group blocks as a conjunction in wolfsshd

yosuke-wolfssl · ejohnstown · commit 281e4bc19ff1 · 2026-06-23T14:57:09.000-07:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
