wolfsshd: base Match blocks on the global config and keep included ones

pull/1163/head
Yosuke Shimizu 2026-08-10 14:52:34 +09:00 committed by Paul Adelsbach
parent ddd9c1a761
commit 6579f59236
3 changed files with 275 additions and 59 deletions

View File

@ -2459,13 +2459,11 @@ static void DoFakePasswordCheck(WS_UserAuthData* authData)
* CA-only branch below fails closed when a Match block sets a CA file that
* differs from the global one.
*
* Note: the comparison is against the *resolved* per-user value. Match nodes
* are built by copying the preceding config node (see HandleMatch in
* configuration.c), so with multiple Match blocks a user can inherit a
* TrustedUserCAKeys set by an earlier block even though that user's own Match
* never set it. Such a user is also rejected for certificate auth, which is
* consistent with the fail-closed intent: the resolved CA still differs from
* the global store the chain was verified against.
* Note: the comparison is against the *resolved* per-user value. A Match node
* snapshots the global config as it stood when the Match line was parsed, so a
* user whose own Match never set TrustedUserCAKeys keeps the global value and
* is not rejected. A Match parsed before the global directive snapshots no CA
* file and still fails closed.
*/
static int RequestAuthentication(WS_UserAuthData* authData,
WOLFSSHD_AUTH* authCtx)

View File

@ -95,6 +95,7 @@ struct WOLFSSHD_CONFIG {
char* pidFile;
char* authorizedUPNDomains; /* allowlist of UPN realms for cert auth */
WOLFSSHD_CONFIG* next; /* next config in list */
WOLFSSHD_CONFIG* head; /* global config the Match nodes branch from */
long loginTimer;
word16 port;
byte usePrivilegeSeparation:2;
@ -107,8 +108,8 @@ struct WOLFSSHD_CONFIG {
};
/* Maximum depth of nested Include directives. Bounds the recursion
* through wolfSSHD_ConfigLoad -> ParseConfigLine -> HandleConfigOption
* -> HandleInclude -> wolfSSHD_ConfigLoad. */
* through ConfigLoad -> ParseConfigLine -> HandleConfigOption
* -> HandleInclude -> ConfigLoad. */
#ifndef WOLFSSHD_MAX_INCLUDE_DEPTH
#define WOLFSSHD_MAX_INCLUDE_DEPTH 16
#endif
@ -238,6 +239,8 @@ WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap)
WMEMSET(ret, 0, sizeof(WOLFSSHD_CONFIG));
/* default values */
ret->heap = heap;
ret->head = ret;
ret->port = 22;
ret->passwordAuth = 1;
ret->pubKeyAuth = 1;
@ -348,6 +351,7 @@ static WOLFSSHD_CONFIG* wolfSSHD_ConfigCopy(WOLFSSHD_CONFIG* conf)
newConf->permitEmptyPasswords = conf->permitEmptyPasswords;
newConf->authKeysFileSet = conf->authKeysFileSet;
newConf->strictModes = conf->strictModes;
newConf->head = conf->head;
}
else {
wolfSSHD_ConfigFree(newConf);
@ -729,7 +733,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
int ret = WS_SUCCESS;
/* No value, nothing to do */
if (!value || value[0] == '\0') {
if (conf == NULL || value == NULL || value[0] == '\0') {
ret = WS_BAD_ARGUMENT;
}
@ -1142,6 +1146,7 @@ static int CheckMatchSelectors(const char* value)
static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
{
WOLFSSHD_CONFIG* newConf = NULL;
WOLFSSHD_CONFIG* tail;
int ret = WS_SUCCESS;
if (conf == NULL || *conf == NULL || value == NULL) {
@ -1161,9 +1166,8 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
}
}
/* create new configure for altered options specific to the match */
if (ret == WS_SUCCESS) {
newConf = wolfSSHD_ConfigCopy(*conf);
newConf = wolfSSHD_ConfigCopy((*conf)->head);
if (newConf == NULL) {
ret = WS_MEMORY_E;
}
@ -1182,14 +1186,16 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
newConf = NULL;
}
/* update current config being processed */
/* Link the node at the end of the list. An included file leaves its Match
* nodes on the list but not on the caller's cursor, so the cursor is not
* always the tail. */
if (ret == WS_SUCCESS) {
(*conf)->next = newConf;
(*conf) = newConf;
}
else {
/* newConf was allocated but not linked into the list; free it */
wolfSSHD_ConfigFree(newConf);
tail = (*conf)->head;
while (tail->next != NULL) {
tail = tail->next;
}
tail->next = newConf;
(*conf) = newConf;
}
(void)valueSz;

View File

@ -1152,61 +1152,59 @@ static int test_GetUserConfMatchRepeatedKeyword(void)
return ret;
}
/* writes 'contents' to the file 'path', creating or truncating it.
* Returns WS_SUCCESS on success. */
static int WriteConfigFile(const char* path, const char* contents)
{
WFILE* f = WBADFILE;
word32 sz, wr;
int ret = WS_SUCCESS;
int cl;
if (WFOPEN(NULL, &f, path, "w") != 0 || f == WBADFILE) {
Log(" Could not create %s.\n", path);
return WS_FATAL_ERROR;
}
sz = (word32)WSTRLEN(contents);
wr = (word32)WFWRITE(NULL, contents, sizeof(char), sz, f);
cl = WFCLOSE(NULL, f);
/* both can fail from one I/O error, report the write first */
if (sz != wr) {
Log(" Could not write %s.\n", path);
ret = WS_FATAL_ERROR;
}
else if (cl != 0) {
Log(" Could not close %s.\n", path);
ret = WS_FATAL_ERROR;
}
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
* config on the same WOLFSSHD_CONFIG still succeeds. */
static int test_IncludeRecursionBound(void)
{
int ret = WS_SUCCESS;
int ret;
WOLFSSHD_CONFIG* conf = NULL;
WFILE* f = WBADFILE;
const char* loopPath = "./include_loop.conf";
const char* normalPath = "./include_normal.conf";
const char* loopContents = "Include ./include_loop.conf\n";
const char* normalContents = "Port 22\n";
word32 sz, wr;
int cl;
if (WFOPEN(NULL, &f, loopPath, "w") != 0 || f == WBADFILE) {
Log(" Could not create %s.\n", loopPath);
return WS_FATAL_ERROR;
ret = WriteConfigFile(loopPath, loopContents);
if (ret == WS_SUCCESS) {
ret = WriteConfigFile(normalPath, normalContents);
}
sz = (word32)WSTRLEN(loopContents);
wr = (word32)WFWRITE(NULL, loopContents, sizeof(char), sz, f);
cl = WFCLOSE(NULL, f);
f = WBADFILE;
if (sz != wr) {
Log(" Could not write %s.\n", loopPath);
(void)WREMOVE(NULL, loopPath);
return WS_FATAL_ERROR;
}
if (cl != 0) {
Log(" Could not close %s.\n", loopPath);
(void)WREMOVE(NULL, loopPath);
return WS_FATAL_ERROR;
}
if (WFOPEN(NULL, &f, normalPath, "w") != 0 || f == WBADFILE) {
(void)WREMOVE(NULL, loopPath);
Log(" Could not create %s.\n", normalPath);
return WS_FATAL_ERROR;
}
sz = (word32)WSTRLEN(normalContents);
wr = (word32)WFWRITE(NULL, normalContents, sizeof(char), sz, f);
cl = WFCLOSE(NULL, f);
f = WBADFILE;
if (sz != wr) {
Log(" Could not write %s.\n", normalPath);
if (ret != WS_SUCCESS) {
(void)WREMOVE(NULL, loopPath);
(void)WREMOVE(NULL, normalPath);
return WS_FATAL_ERROR;
}
if (cl != 0) {
Log(" Could not close %s.\n", normalPath);
(void)WREMOVE(NULL, loopPath);
(void)WREMOVE(NULL, normalPath);
return WS_FATAL_ERROR;
return ret;
}
conf = wolfSSHD_ConfigNew(NULL);
@ -1242,6 +1240,218 @@ static int test_IncludeRecursionBound(void)
return ret;
}
/* Each Match block is built from the global config, not from the Match block
* before it. A user selected by a later block must not pick up settings that
* an earlier, non-matching block changed. */
static int test_GetUserConfMatchNoInherit(void)
{
int ret = WS_SUCCESS;
WOLFSSHD_CONFIG* head;
WOLFSSHD_CONFIG* conf;
WOLFSSHD_CONFIG* aliceConf = NULL;
WOLFSSHD_CONFIG* staffConf = NULL;
WOLFSSHD_CONFIG* match = NULL;
const char* cmd;
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)
if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/global");
if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords yes");
/* alice's block overrides several of the global settings */
if (ret == WS_SUCCESS) ret = PCL("Match User alice");
if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/alice");
if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/alice_keys");
if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords no");
if (ret == WS_SUCCESS) aliceConf = conf;
/* the staff block sets one option, everything else must resolve to the
* global value rather than to alice's */
if (ret == WS_SUCCESS) ret = PCL("Match Group staff");
if (ret == WS_SUCCESS) ret = PCL("PubkeyAuthentication no");
if (ret == WS_SUCCESS) staffConf = conf;
#undef PCL
if (ret == WS_SUCCESS) {
Log(" Testing scenario: staff user does not inherit alice.");
grps[0] = "staff";
match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL,
NULL, NULL, NULL);
if (match != staffConf)
ret = WS_FATAL_ERROR;
if (ret == WS_SUCCESS) {
cmd = wolfSSHD_ConfigGetForcedCmd(match);
if (cmd == NULL || XSTRCMP(cmd, "/bin/global") != 0)
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS &&
wolfSSHD_ConfigGetAuthKeysFileSet(match) != 0) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS &&
wolfSSHD_ConfigGetPermitEmptyPw(match) != 1) {
ret = WS_FATAL_ERROR;
}
/* the staff block's own override still applies */
if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPubKeyAuth(match) != 0) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS) {
Log(" PASSED.\n");
}
else {
Log(" FAILED.\n");
}
}
if (ret == WS_SUCCESS) {
Log(" Testing scenario: alice keeps her own overrides.");
grps[0] = "users";
match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL,
NULL, NULL, NULL);
if (match != aliceConf)
ret = WS_FATAL_ERROR;
if (ret == WS_SUCCESS) {
cmd = wolfSSHD_ConfigGetForcedCmd(match);
if (cmd == NULL || XSTRCMP(cmd, "/bin/alice") != 0)
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS &&
wolfSSHD_ConfigGetAuthKeysFileSet(match) != 1) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS &&
wolfSSHD_ConfigGetPermitEmptyPw(match) != 0) {
ret = WS_FATAL_ERROR;
}
/* alice is not in staff, so she keeps the global pubkey setting */
if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPubKeyAuth(match) != 1) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS) {
Log(" PASSED.\n");
}
else {
Log(" FAILED.\n");
}
}
wolfSSHD_ConfigFree(head);
return ret;
}
/* A Match block inside an Include'd file must survive a later Match block in
* the including file, and the include must not export its Match scope: a
* directive after the Include belongs to the global config, the way OpenSSH
* resets Match scope at the end of an included file. */
static int test_ConfigIncludeMatchChain(void)
{
int ret;
WOLFSSHD_CONFIG* head = NULL;
WOLFSSHD_CONFIG* match;
const char* cmd;
const char* incPath = "./include_match.conf";
const char* topPath = "./include_match_top.conf";
const char* incContents =
"Match User alice\n"
"ForceCommand /bin/alice\n";
const char* topContents =
"ForceCommand /bin/global\n"
"Include ./include_match.conf\n"
"PermitEmptyPasswords yes\n"
"Match User bob\n"
"ForceCommand /bin/bob\n";
ret = WriteConfigFile(incPath, incContents);
if (ret == WS_SUCCESS) {
ret = WriteConfigFile(topPath, topContents);
}
if (ret != WS_SUCCESS) {
(void)WREMOVE(NULL, incPath);
(void)WREMOVE(NULL, topPath);
return ret;
}
head = wolfSSHD_ConfigNew(NULL);
if (head == NULL) {
ret = WS_MEMORY_E;
}
if (ret == WS_SUCCESS) {
ret = wolfSSHD_ConfigLoad(head, topPath);
}
if (ret == WS_SUCCESS) {
Log(" Testing scenario: included Match block still applies.");
match = wolfSSHD_GetUserConf(head, "alice", NULL, 0, NULL, NULL,
NULL, NULL, NULL);
cmd = wolfSSHD_ConfigGetForcedCmd(match);
if (match == head || cmd == NULL ||
XSTRCMP(cmd, "/bin/alice") != 0) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS) {
Log(" PASSED.\n");
}
else {
Log(" FAILED.\n");
}
}
if (ret == WS_SUCCESS) {
Log(" Testing scenario: outer Match block still applies.");
match = wolfSSHD_GetUserConf(head, "bob", NULL, 0, NULL, NULL,
NULL, NULL, NULL);
cmd = wolfSSHD_ConfigGetForcedCmd(match);
if (match == head || cmd == NULL || XSTRCMP(cmd, "/bin/bob") != 0) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS) {
Log(" PASSED.\n");
}
else {
Log(" FAILED.\n");
}
}
if (ret == WS_SUCCESS) {
Log(" Testing scenario: unmatched user gets the global config.");
match = wolfSSHD_GetUserConf(head, "carol", NULL, 0, NULL, NULL,
NULL, NULL, NULL);
cmd = wolfSSHD_ConfigGetForcedCmd(match);
if (match != head || cmd == NULL ||
XSTRCMP(cmd, "/bin/global") != 0) {
ret = WS_FATAL_ERROR;
}
/* the include ends inside a Match block, so this proves the directive
* after it was not swallowed by that block */
if (ret == WS_SUCCESS &&
wolfSSHD_ConfigGetPermitEmptyPw(match) != 1) {
ret = WS_FATAL_ERROR;
}
if (ret == WS_SUCCESS) {
Log(" PASSED.\n");
}
else {
Log(" FAILED.\n");
}
}
wolfSSHD_ConfigFree(head);
(void)WREMOVE(NULL, incPath);
(void)WREMOVE(NULL, topPath);
return ret;
}
/* The public wolfSSHD_ConfigSetAuthKeysFile setter must mark the authorized
* keys file as explicitly configured, otherwise certificate public-key logins
* skip the authorized-keys check and rely on CA validation alone. */
@ -5952,6 +6162,8 @@ const TEST_CASE testCases[] = {
TEST_DECL(test_ConfigParseAuthorizedUPNDomains),
TEST_DECL(test_MatchUPNToUser),
TEST_DECL(test_IncludeRecursionBound),
TEST_DECL(test_GetUserConfMatchNoInherit),
TEST_DECL(test_ConfigIncludeMatchChain),
TEST_DECL(test_GetUserAuthTypes),
TEST_DECL(test_DefaultUserAuthTypesNullArgs),
TEST_DECL(test_ConfigSetAuthKeysFile),