Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
+284 −71
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions apps/wolfsshd/auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2377,13 +2377,9 @@ 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
* starts from the global config, so a user whose own Match never set
* TrustedUserCAKeys keeps the global value and is not rejected here.
*/
static int RequestAuthentication(WS_UserAuthData* authData,
WOLFSSHD_AUTH* authCtx)
Expand Down
52 changes: 29 additions & 23 deletions apps/wolfsshd/configuration.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -112,7 +113,7 @@ struct WOLFSSHD_CONFIG {
#ifndef WOLFSSHD_MAX_INCLUDE_DEPTH
#define WOLFSSHD_MAX_INCLUDE_DEPTH 16
#endif
static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth);
static int ConfigLoad(WOLFSSHD_CONFIG** conf, const char* filename, int depth);

static int CountWhitespace(const char* in, int inSz, byte inv);
static int SetFileString(char** dst, const char* src, void* heap);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Include now exports Match scope to the caller, silently demoting later global directives · Privilege escalation in wolfsshd

Threading the parse cursor through ConfigLoad makes an included file that ends inside a Match block leave the cursor on that Match node, so every directive after the Include in the parent file is scoped to that block. Hardening lines such as PasswordAuthentication no, ForceCommand, ChrootDirectory and AuthorizedKeysFile then never reach the global node, and unmatched users fall back to the built-in defaults (passwordAuth/pubKeyAuth = 1, no chroot, no forced command) with no diagnostic.

Related known finding #8815 (similar but distinct): Both defects cause configuration directives to inherit an unintended Match scope. However, this finding is caused by HandleInclude/ConfigLoad exporting the parse cursor after an included file, while #8815 is caused by HandleMatch copying and chaining preceding Match nodes; they affect different operations and require separate patches.

Fix: Log a warning when an included file returns with the cursor on a Match node different from the one it was entered with.

wolfSSHD_ConfigFree(newConf);
Expand Down Expand Up @@ -718,22 +722,25 @@ static int HandlePort(WOLFSSHD_CONFIG* conf, const char* value)
}

/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
static int HandleInclude(WOLFSSHD_CONFIG **conf, const char *value, int depth)
{
const char *ptr;
const char *ptr2;
const char *postfix = NULL;
const char *prefix = NULL;
void *heap = NULL;
int prefixLen = 0;
int found = 0;
int ret = WS_SUCCESS;

/* No value, nothing to do */
if (!value || value[0] == '\0') {
if (conf == NULL || *conf == NULL || value == NULL || value[0] == '\0') {
ret = WS_BAD_ARGUMENT;
}

if (ret == WS_SUCCESS) {
heap = (*conf)->heap;

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] heap is set but never read on non-unix builds · Dead/unreachable code

Every read of the new heap local sits inside the __unix__/__APPLE__ wildcard block (lines 782-968). On other targets it is only written, producing -Wunused-but-set-variable; the #else arm voids postfix, prefix, and prefixLen but not heap, so -Werror builds break.

Fix: Add (void)heap; to the non-unix #else arm next to the existing (void)postfix; casts.


/* Ignore trailing whitespace */
ptr = value + WSTRLEN(value) - 1;
while (ptr != value) {
Expand Down Expand Up @@ -772,7 +779,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
struct dirent *dir;
WDIR d;
char *path = NULL;
char *filepath = (char*)WMALLOC(PATH_MAX, conf->heap, DYNTYPE_PATH);
char *filepath = (char*)WMALLOC(PATH_MAX, heap, DYNTYPE_PATH);

if (filepath == NULL) {
ret = WS_MEMORY_E;
Expand All @@ -788,8 +795,8 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
}

if (ptr2 != value) {
path = (char*)WMALLOC(ptr2 - value + 1,
conf->heap, DYNTYPE_PATH);
path = (char*)WMALLOC(ptr2 - value + 1, heap,
DYNTYPE_PATH);
if (path == NULL) {
ret = WS_MEMORY_E;
}
Expand All @@ -801,7 +808,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
}
}
else {
path = (char*)WMALLOC(2, conf->heap, DYNTYPE_PATH);
path = (char*)WMALLOC(2, heap, DYNTYPE_PATH);
if (path == NULL) {
ret = WS_MEMORY_E;
}
Expand All @@ -815,7 +822,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
}

if (ret == WS_SUCCESS) {
if (!WOPENDIR(NULL, conf->heap, &d, path)) {
if (!WOPENDIR(NULL, heap, &d, path)) {
word32 fileCount = 0, fileFilled = 0, i, j;
char** fileNames = NULL;

Expand All @@ -842,7 +849,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)

if (fileCount > 0) {
fileNames = (char**)WMALLOC(fileCount * sizeof(char*),
conf->heap, DYNTYPE_PATH);
heap, DYNTYPE_PATH);
if (fileNames == NULL) {
ret = WS_MEMORY_E;
}
Expand Down Expand Up @@ -874,7 +881,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
/* Duplicate the name; readdir() may reuse its
* dirent storage on the next call, so the
* pointer cannot be retained across the loop. */
char* nameCopy = WSTRDUP(dir->d_name, conf->heap,
char* nameCopy = WSTRDUP(dir->d_name, heap,
DYNTYPE_PATH);
if (nameCopy == NULL) {
ret = WS_MEMORY_E;
Expand Down Expand Up @@ -940,11 +947,11 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
* holds a valid pointer. */
for (i = 0; i < fileFilled; i++) {
if (fileNames[i] != NULL) {
WFREE(fileNames[i], conf->heap, DYNTYPE_PATH);
WFREE(fileNames[i], heap, DYNTYPE_PATH);
}
}
if (fileNames != NULL) {
WFREE(fileNames, conf->heap, DYNTYPE_PATH);
WFREE(fileNames, heap, DYNTYPE_PATH);
}
}
WCLOSEDIR(NULL, &d);
Expand All @@ -955,10 +962,10 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
}
}
if (path != NULL) {
WFREE(path, conf->heap, DYNTYPE_PATH);
WFREE(path, heap, DYNTYPE_PATH);
}
if (filepath != NULL) {
WFREE(filepath, conf->heap, DYNTYPE_PATH);
WFREE(filepath, heap, DYNTYPE_PATH);
}
#else
(void)postfix;
Expand Down Expand Up @@ -1161,9 +1168,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;
}
Expand Down Expand Up @@ -1294,7 +1300,7 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
ret = WS_SUCCESS;
break;
case OPT_INCLUDE:
ret = HandleInclude(*conf, value, depth);
ret = HandleInclude(conf, value, depth);
break;
case OPT_CHROOT_DIR:
ret = HandleChrootDir(*conf, value);
Expand Down Expand Up @@ -1415,20 +1421,21 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
*/
int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
{
return ConfigLoad(conf, filename, 0);
WOLFSSHD_CONFIG* current = conf;

return ConfigLoad(&current, filename, 0);
}


/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
static int ConfigLoad(WOLFSSHD_CONFIG** conf, const char* filename, int depth)
{
WFILE *f;
WOLFSSHD_CONFIG* currentConfig;
int ret = WS_SUCCESS;
char buf[MAX_LINE_SIZE];
const char* current;

if (conf == NULL || filename == NULL)
if (conf == NULL || *conf == NULL || filename == NULL)
return BAD_FUNC_ARG;

if (depth >= WOLFSSHD_MAX_INCLUDE_DEPTH) {
Expand All @@ -1446,7 +1453,6 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
wolfSSH_Log(WS_LOG_INFO, "[SSHD] parsing config file %s", filename);
depth++;

currentConfig = conf;
while ((current = XFGETS(buf, MAX_LINE_SIZE, f)) != NULL) {
int currentSz = (int)XSTRLEN(current);

Expand All @@ -1465,7 +1471,7 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
continue; /* commented out line */
}

ret = ParseConfigLine(&currentConfig, current, currentSz, depth);
ret = ParseConfigLine(conf, current, currentSz, depth);
if (ret != WS_SUCCESS) {
fprintf(stderr, "Unable to parse config line : %s\n", current);
break;
Expand Down
Loading
Loading