Skip to content

wolfsshd: expand AuthorizedKeysFile %u/%h/%% tokens per user #1064

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
+262 −30
Open

wolfsshd: expand AuthorizedKeysFile %u/%h/%% tokens per user #1064

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
161 changes: 131 additions & 30 deletions apps/wolfsshd/auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ struct WOLFSSHD_AUTH {
#define MAX_LINE_SZ 900
#endif
#endif
#ifndef MAX_PATH_SZ
#define MAX_PATH_SZ 80
#endif

#if 0
/* this could potentially be useful in a deeply embedded future port */
Expand Down Expand Up @@ -505,15 +502,122 @@ static int CheckPasswordUnix(const char* usr, const byte* pw, word32 pwSz, WOLFS

static const char authKeysDefault[] = ".ssh/authorized_keys";

/* Resolve the authorized keys file path for a user. The pattern is the user's
* configured AuthorizedKeysFile (resolved per request from the per-user config)
* and is passed in explicitly rather than read from shared state so concurrent
* authentications (e.g. Windows threaded mode) cannot race on it. A NULL or
* empty pattern falls back to the default authorized_keys location. */
static int ResolveAuthKeysPath(const char* homeDir, const char* pattern,
/* Expand AuthorizedKeysFile tokens (%% literal, %h home dir, %u user name)
* from pattern into out. Unrecognized tokens fail closed so a per-user pattern
* cannot collapse to one shared path. Returns WS_SUCCESS or a negative error. */
static int ExpandAuthKeysTokens(const char* pattern, const char* homeDir,
const char* user, char* out, word32 outSz)
{
int ret = WS_SUCCESS;
word32 outIdx = 0;
word32 i = 0;
word32 patSz;
word32 insSz;
const char* ins;
char lit[2];

if (pattern == NULL || out == NULL || outSz == 0) {
ret = WS_BAD_ARGUMENT;
}

if (ret == WS_SUCCESS) {
patSz = (word32)WSTRLEN(pattern);
lit[1] = '\0';

while (ret == WS_SUCCESS && i < patSz) {
ins = NULL;

if (pattern[i] == '%' && (i + 1) < patSz) {
switch (pattern[i + 1]) {
case '%':
lit[0] = '%';
ins = lit;
break;
case 'h':
ins = homeDir;
break;
case 'u':
ins = user;
break;
default:
wolfSSH_Log(WS_LOG_ERROR,
"[SSHD] Unsupported AuthorizedKeysFile token");
ret = WS_FATAL_ERROR;
break;
}
/* token recognized but its value is unavailable */
if (ret == WS_SUCCESS && ins == NULL) {
wolfSSH_Log(WS_LOG_ERROR,
"[SSHD] No value for AuthorizedKeysFile token");
ret = WS_FATAL_ERROR;
}
i += 2;
}
else {
/* literal character (including a trailing lone '%') */
lit[0] = pattern[i];
ins = lit;
i += 1;
}

if (ret == WS_SUCCESS) {
insSz = (word32)WSTRLEN(ins);
/* leave room for the terminating null */
if (outIdx + insSz >= outSz) {
wolfSSH_Log(WS_LOG_ERROR,
"[SSHD] Path for key file larger than max allowed");
ret = WS_FATAL_ERROR;
}
else {
WMEMCPY(out + outIdx, ins, insSz);
outIdx += insSz;
}
}
}
}

if (ret == WS_SUCCESS) {
out[outIdx] = '\0';
}

return ret;
}

/* True for a fully qualified path. POSIX roots only at '/'; Windows also roots
* at a '\' or a drive letter followed by a separator ("X:\"). A bare "X:" is
* drive-relative, not absolute, so it is left to resolve under the home dir. */
static int IsAbsoluteAuthKeysPath(const char* path)
{
int ret = 0;

if (path != NULL) {
if (path[0] == '/') {
ret = 1;
}
#ifdef _WIN32
else if (path[0] == '\\') {
ret = 1;
}
else if (((path[0] >= 'A' && path[0] <= 'Z') ||

@aidangarske aidangarske Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟠 [High] Windows drive-relative AuthorizedKeysFile paths are treated as absolute
🚫 BLOCK bug

The PR adds Windows absolute-path detection, but it treats any <letter>: prefix as absolute. On Windows, paths like C:keys\alice or C:alice are drive-relative, not fully qualified. Because ResolveAuthKeysPath() skips the home-directory join when IsAbsoluteAuthKeysPath() returns true (copying the path unchanged for SearchForPubKey to open), an AuthorizedKeysFile C:keys\%u pattern is opened relative to the daemon process's current directory on drive C instead of under the user's home directory. Data flow: config/public setter -> ExpandAuthKeysTokens expands %u -> IsAbsoluteAuthKeysPath returns true solely because path[1] == ':' -> ResolveAuthKeysPath copies unchanged -> SearchForPubKey opens it. Under a misconfigured Windows deployment using X:relative syntax, an attacker with write access to the relevant current-directory tree can influence which authorized_keys file is loaded. This is introduced by the new _WIN32 branch and is not covered by the new Windows test vectors, which only check C:\keys\%u. Severity views differ across modes: review rates this High (BLOCK), security rates the practical impact Low (requires Windows, a non-default/mistyped config, and write access to the current-directory tree); the stricter High is kept.

Recommendation: Require a slash or backslash after the drive colon for fully qualified Windows paths (path[1] == ':' && (path[2] == '\\' || path[2] == '/')), and add Windows test vectors for C:%u and C:keys\%u that expect home-relative resolution (or rejection).

@yosuke-wolfssl yosuke-wolfssl Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed

(path[0] >= 'a' && path[0] <= 'z')) && path[1] == ':' &&
(path[2] == '\\' || path[2] == '/')) {
ret = 1;
}
#endif
}

return ret;
}

/* Resolve the authorized keys file path for a user. The pattern is passed in
* explicitly so concurrent authentications cannot race on it, and its tokens
* are expanded so each user resolves to a distinct path. */
WOLFSSHD_STATIC int ResolveAuthKeysPath(const char* homeDir,
const char* pattern, const char* user,
char* resolved)
{
int ret = WS_SUCCESS;
char expanded[MAX_PATH_SZ];
char* idx;
int homeDirSz;
const char* suffix = authKeysDefault;
Expand All @@ -524,24 +628,19 @@ static int ResolveAuthKeysPath(const char* homeDir, const char* pattern,

if (ret == WS_SUCCESS) {
if (pattern != NULL && *pattern != 0) {
/* TODO: token substitutions (e.g. %h) */
if (*pattern == '/') {
/* Absolute path is used as-is. Error out rather than
* silently truncate when it does not fit, mirroring the
* relative-path branch below. */
if (WSTRLEN(pattern) >= MAX_PATH_SZ) {
wolfSSH_Log(WS_LOG_ERROR,
"[SSHD] Path for key file larger than max allowed");
ret = WS_FATAL_ERROR;
}
else {
WSTRNCPY(resolved, pattern, MAX_PATH_SZ - 1);
resolved[MAX_PATH_SZ - 1] = '\0';
}
ret = ExpandAuthKeysTokens(pattern, homeDir, user, expanded,
(word32)sizeof(expanded));
if (ret != WS_SUCCESS) {
wolfSSH_Log(WS_LOG_ERROR,
"[SSHD] Failed to expand AuthorizedKeysFile pattern");
}
/* expanded is NUL-terminated and shorter than MAX_PATH_SZ */
else if (IsAbsoluteAuthKeysPath(expanded)) {
WMEMCPY(resolved, expanded, WSTRLEN(expanded) + 1);
return ret;
}
else {
suffix = pattern;
suffix = expanded;
}
}
}
Expand All @@ -559,8 +658,8 @@ static int ResolveAuthKeysPath(const char* homeDir, const char* pattern,
XMEMCPY(idx, homeDir, homeDirSz);
idx += homeDirSz;
*(idx++) = '/';
/* Intentionally copying the null term from suffix. */
XMEMCPY(idx, suffix, WSTRLEN(suffix));
/* the bound check above leaves room for suffix and its null term */
XMEMCPY(idx, suffix, WSTRLEN(suffix) + 1);
}
}

Expand Down Expand Up @@ -784,6 +883,7 @@ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid,
}

static int SearchForPubKey(const char* path, const char* authKeysFile,
const char* user,
const WS_UserAuthData_PublicKey* pubKeyCtx,
WUID_T uid, int strictModes)
{
Expand All @@ -797,7 +897,7 @@ static int SearchForPubKey(const char* path, const char* authKeysFile,
int rc = 0;

WMEMSET(authKeysPath, 0, sizeof(authKeysPath));
rc = ResolveAuthKeysPath(path, authKeysFile, authKeysPath);
rc = ResolveAuthKeysPath(path, authKeysFile, user, authKeysPath);
if (rc != WS_SUCCESS) {
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to resolve authorized keys"
" file path.");
Expand Down Expand Up @@ -980,8 +1080,9 @@ static int CheckPublicKeyUnix(const char* name,
}

if (ret == WSSHD_AUTH_SUCCESS) {
ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, pubKeyCtx,
pwInfo->pw_uid, wolfSSHD_ConfigGetStrictModes(authCtx->conf));
ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, name,
pubKeyCtx, pwInfo->pw_uid,
wolfSSHD_ConfigGetStrictModes(authCtx->conf));
}
}

Expand Down Expand Up @@ -1325,7 +1426,7 @@ static int CheckPublicKeyWIN(const char* usr,
if (ret == WSSHD_AUTH_SUCCESS) {
r[rSz-1] = L'\0';

ret = SearchForPubKey(r, authorizedKeysFile, pubKeyCtx, 0,
ret = SearchForPubKey(r, authorizedKeysFile, usr, pubKeyCtx, 0,
wolfSSHD_ConfigGetStrictModes(authCtx->conf));
if (ret != WSSHD_AUTH_SUCCESS) {
wolfSSH_Log(WS_LOG_ERROR,
Expand Down
2 changes: 2 additions & 0 deletions apps/wolfsshd/auth.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ int CheckPasswordHashUnix(const char* input, char* stored);
#endif
int CheckAuthKeysLine(char* line, word32 lineSz, const byte* key,
word32 keySz);
int ResolveAuthKeysPath(const char* homeDir, const char* pattern,
const char* user, char* resolved);
int CAKeysFileDiffers(const char* a, const char* b);
int wolfSSHD_GetUserAuthTypes(const WOLFSSHD_CONFIG* usrConf);
#endif
Expand Down
4 changes: 4 additions & 0 deletions apps/wolfsshd/configuration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ typedef struct WOLFSSHD_CONFIG WOLFSSHD_CONFIG;
#define WOLFSSHD_PRIV_SANDBOX 1
#define WOLFSSHD_PRIV_OFF 2

#ifndef MAX_PATH_SZ
#define MAX_PATH_SZ 80
#endif

WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap);
void wolfSSHD_ConfigFree(WOLFSSHD_CONFIG* conf);
int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename);
Expand Down
125 changes: 125 additions & 0 deletions apps/wolfsshd/test/test_configuration.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1900,6 +1900,130 @@ static int test_OpenSecureFile(void)
}
#endif /* !_WIN32 */

/* Verify AuthorizedKeysFile token substitution so an absolute pattern with %u
* resolves to a per-user path instead of the same literal string for every
* user. */
static int test_ResolveAuthKeysPath(void)
{
int ret = WS_SUCCESS;
int rc;
word32 i;
char resolved[MAX_PATH_SZ];
char longPat[MAX_PATH_SZ + 16];
char longHome[MAX_PATH_SZ];
static const struct {
const char* home;
const char* pattern;
const char* user;
int expectRet;
const char* expect;
} vectors[] = {
/* absolute pattern with %u resolves to a distinct path per user */
{ "/home/alice", "/etc/ssh/keys/%u", "alice", WS_SUCCESS,
"/etc/ssh/keys/alice" },
{ "/home/bob", "/etc/ssh/keys/%u", "bob", WS_SUCCESS,
"/etc/ssh/keys/bob" },
/* %h expands to the home directory */
{ "/home/alice", "%h/.ssh/authorized_keys", "alice", WS_SUCCESS,
"/home/alice/.ssh/authorized_keys" },
/* %% is a literal percent */
{ "/home/alice", "/keys/100%%/%u", "alice", WS_SUCCESS,
"/keys/100%/alice" },
/* relative pattern is taken under the home directory */
{ "/home/alice", "keys/%u", "alice", WS_SUCCESS,
"/home/alice/keys/alice" },
#ifdef _WIN32
/* drive-letter and backslash roots are absolute on Windows */
{ "/home/alice", "C:\\keys\\%u", "alice", WS_SUCCESS,
"C:\\keys\\alice" },
{ "/home/alice", "\\keys\\%u", "alice", WS_SUCCESS,
"\\keys\\alice" },
#else
/* on POSIX they are relative, taken under the home directory */
{ "/home/alice", "C:\\keys\\%u", "alice", WS_SUCCESS,
"/home/alice/C:\\keys\\alice" },
{ "/home/alice", "\\keys\\%u", "alice", WS_SUCCESS,
"/home/alice/\\keys\\alice" },
#endif
/* a bare "X:" is drive-relative, not absolute, on either platform and
* so resolves under the home directory */
{ "/home/alice", "C:keys\\%u", "alice", WS_SUCCESS,
"/home/alice/C:keys\\alice" },
{ "/home/alice", "C:%u", "alice", WS_SUCCESS,
"/home/alice/C:alice" },
/* NULL pattern falls back to the default location */
{ "/home/alice", NULL, "alice", WS_SUCCESS,
"/home/alice/.ssh/authorized_keys" },
/* a trailing lone '%' is treated as a literal */
{ "/home/alice", "/etc/keys/%u%", "alice", WS_SUCCESS,
"/etc/keys/alice%" },
/* unrecognized token fails closed */
{ "/home/alice", "/etc/keys/%q", "alice", WS_FATAL_ERROR, NULL },
/* recognized token with no available value (NULL user) fails closed */
{ "/home/alice", "/etc/keys/%u", NULL, WS_FATAL_ERROR, NULL },
};

for (i = 0; ret == WS_SUCCESS && i < sizeof(vectors) / sizeof(vectors[0]);
i++) {
Log(" Testing scenario: pattern \"%s\" user \"%s\".",
vectors[i].pattern != NULL ? vectors[i].pattern : "(null)",
vectors[i].user != NULL ? vectors[i].user : "(null)");
/* non-zero fill so a missing null terminator cannot pass unnoticed */
WMEMSET(resolved, 0xA5, sizeof(resolved));
rc = ResolveAuthKeysPath(vectors[i].home, vectors[i].pattern,
vectors[i].user, resolved);
if (rc != vectors[i].expectRet) {
Log(" FAILED (rc=%d).\n", rc);
ret = WS_FATAL_ERROR;
}
else if (vectors[i].expect != NULL &&
WSTRCMP(resolved, vectors[i].expect) != 0) {
Log(" FAILED (got \"%s\").\n", resolved);
ret = WS_FATAL_ERROR;
}
else {
Log(" PASSED.\n");
}
}

/* an expansion that exceeds MAX_PATH_SZ fails closed */
if (ret == WS_SUCCESS) {
Log(" Testing scenario: over-length pattern is rejected.");
WMEMSET(longPat, 'a', sizeof(longPat) - 1);
longPat[0] = '/';
longPat[sizeof(longPat) - 1] = '\0';
WMEMSET(resolved, 0, sizeof(resolved));
rc = ResolveAuthKeysPath("/home/alice", longPat, "alice", resolved);
if (rc == WS_FATAL_ERROR) {
Log(" PASSED.\n");
}
else {
Log(" FAILED (rc=%d).\n", rc);
ret = WS_FATAL_ERROR;
}
}

/* a relative pattern under a long home directory fails closed */
if (ret == WS_SUCCESS) {
Log(" Testing scenario: relative pattern under long home is "
"rejected.");
WMEMSET(longHome, 'a', sizeof(longHome) - 1);
longHome[0] = '/';
longHome[sizeof(longHome) - 1] = '\0';
WMEMSET(resolved, 0, sizeof(resolved));
rc = ResolveAuthKeysPath(longHome, "keys/%u", "alice", resolved);
if (rc == WS_FATAL_ERROR) {
Log(" PASSED.\n");
}
else {
Log(" FAILED (rc=%d).\n", rc);
ret = WS_FATAL_ERROR;
}
}

return ret;
}

const TEST_CASE testCases[] = {
TEST_DECL(test_ConfigDefaults),
TEST_DECL(test_ParseConfigLine),
Expand All @@ -1917,6 +2041,7 @@ const TEST_CASE testCases[] = {
TEST_DECL(test_IncludeRecursionBound),
TEST_DECL(test_GetUserAuthTypes),
TEST_DECL(test_ConfigSetAuthKeysFile),
TEST_DECL(test_ResolveAuthKeysPath),
TEST_DECL(test_ConfigFree),
#ifndef _WIN32
TEST_DECL(test_OpenSecureFile),
Expand Down
Loading