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

Author: yosuke-wolfssl

apps/wolfsshd/configuration.c

@@ -1309,26 +1309,40 @@ WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf,
 {
     WOLFSSHD_CONFIG* ret;
     WOLFSSHD_CONFIG* current;
+    int matches;
 
     /* 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;
+            if (matches && current->groupAppliesTo != NULL) {
+                if (grp == NULL ||
+                        XSTRCMP(current->groupAppliesTo, grp) != 0) {
+                    matches = 0;
+                }
             }
         }
 
+        if (matches) {
+            ret = current;
+            break;
+        }
+
         current = current->next;
     }
 

apps/wolfsshd/test/test_configuration.c

@@ -571,7 +571,104 @@ 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;
+
+    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) {
+        match = wolfSSHD_GetUserConf(head, "alice", "admins", 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) {
+        match = wolfSSHD_GetUserConf(head, "alice", "users", 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) {
+        match = wolfSSHD_GetUserConf(head, "carol", "admins", NULL, NULL,
+                                     NULL, NULL, NULL);
+        if (match != head)
+            ret = WS_FATAL_ERROR;
+    }
+
+    /* alice with no resolved group must NOT get the combined block: a NULL
+     * group 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 grp == NULL. */
+    if (ret == WS_SUCCESS) {
+        match = wolfSSHD_GetUserConf(head, "alice", NULL, 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) {
+        match = wolfSSHD_GetUserConf(head, "bob", "anygroup", 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) {
+        match = wolfSSHD_GetUserConf(head, "anyuser", "staff", NULL, NULL,
+                                     NULL, NULL, NULL);
+        if (match != groupOnly)
+            ret = WS_FATAL_ERROR;
+    }
+
+    wolfSSHD_ConfigFree(head);
     return ret;
 }
 
@@ -1011,6 +1108,7 @@ const TEST_CASE testCases[] = {
     TEST_DECL(test_ConfigCopy),
     TEST_DECL(test_GetUserConfMatchOverride),
     TEST_DECL(test_MatchUnsupportedSelector),
+    TEST_DECL(test_GetUserConfMatchGroupAnd),
     TEST_DECL(test_CAKeysFileDiffers),
     TEST_DECL(test_IncludeRecursionBound),
     TEST_DECL(test_ConfigFree),