Skip to content

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

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5827
Open

wolfsshd: expand AuthorizedKeysFile %u/%h/%% tokens per user#1064
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5827

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

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

Problem

ResolveAuthKeysPath copied an absolute AuthorizedKeysFile pattern into the
resolved path verbatim, with no token substitution (a TODO marked the gap).
A common OpenSSH-migration config such as:

AuthorizedKeysFile /etc/ssh/keys/%u

resolved to the literal /etc/ssh/keys/%u for every user. If a file existed
at that un-substituted path, its keys were accepted for any valid system user,
collapsing per-user key isolation.

Addressed by f_5827.

Fix

  • Add ExpandAuthKeysTokens() — expands %u (user), %h (home dir), and %%
    (literal %) into a bounds-checked buffer. Unrecognized tokens and
    over-length expansions fail closed (WS_FATAL_ERROR) rather than reaching
    the filesystem.
  • ResolveAuthKeysPath() now takes the connecting user and expands the pattern
    before the absolute/relative decision, so both branches honor substitution and
    each user resolves to a distinct path.
  • Thread the username through SearchForPubKey()CheckPublicKeyUnix (Unix)
    and CheckPublicKeyWIN (Windows). Both call sites already held it; no new
    getpwnam/lookup is introduced and no shared mutable state is added.

Token semantics

Token Expands to
%u connecting user
%h user home dir
%% literal %

A recognized token with no value (e.g. %u with a NULL user) and any
unrecognized token fail closed. A trailing lone % is treated as a literal.

Platform notes

Absolute-path detection is /-rooted on POSIX; on Windows it additionally
recognizes a \-root or a drive-letter (X:) root, guarded by #ifdef _WIN32
so a POSIX relative pattern beginning with \ or <letter>: is not misread.

Testing

  • New test_ResolveAuthKeysPath unit test (13 scenarios): per-user %u
    (alice vs bob), %h, %%, relative, default-NULL fallback, trailing %,
    unrecognized token, NULL-user fail-closed, expansion over-length, relative
    home-dir over-length, and platform-aware drive-letter/backslash handling.
  • MAX_PATH_SZ moved to configuration.h so the test buffer tracks the real
    contract; ResolveAuthKeysPath exposed to the tests via WOLFSSHD_STATIC.
  • Verified under ASan + UBSan (clean), with negative controls confirming the
    tests catch both the verbatim-copy regression and a removed bounds check.
  • Native Windows MSVC build (VS Build Tools 2022, x64): auth.c /
    configuration.c compile with no new warnings; default test set
    (api-test, unit-test, testsuite) passes.

Files changed

  • apps/wolfsshd/auth.c
  • apps/wolfsshd/auth.h
  • apps/wolfsshd/configuration.h
  • apps/wolfsshd/test/test_configuration.c

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 26, 2026
Copilot AI review requested due to automatic review settings June 26, 2026 06:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1064

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants