diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 9710c683c..162a0267b 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -1236,26 +1236,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; } diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index cac8585c7..e5f901bef 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -501,6 +501,104 @@ static int test_GetUserConfMatchOverride(void) 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; +} + /* 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 @@ -936,6 +1034,7 @@ const TEST_CASE testCases[] = { TEST_DECL(test_ParseConfigLine), TEST_DECL(test_ConfigCopy), TEST_DECL(test_GetUserConfMatchOverride), + TEST_DECL(test_GetUserConfMatchGroupAnd), TEST_DECL(test_CAKeysFileDiffers), TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_ConfigFree),