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

[yosuke-wolfssl]

Problem

HandleMatch() built each Match node with wolfSSHD_ConfigCopy(*conf) — a copy of the preceding Match node rather than the global config. Since wolfSSHD_GetUserConf() returns the first matching node and stops, a user selected by a later block silently inherited every setting changed by earlier blocks that did not apply to them:

ForceCommand /bin/global
Match User alice
    ForceCommand /bin/alice
Match Group staff          # sets no ForceCommand
    PasswordAuthentication no

A staff user other than alice resolved to ForceCommand /bin/alice. This affected every copied field (forceCmd, chrootDir, authKeysFile, userCAKeysFile, permitRootLogin, …) and is usually fail-open.

A second defect in the same linkage: HandleMatch() linked the new node onto the parse cursor ((*conf)->next = newConf). An Included file's Match nodes are appended to the list but do not move the caller's cursor, so a later Match in the parent overwrote the only pointer to them — silently dropping those blocks and leaking them.

Fix (apps/wolfsshd/configuration.c)

• head pointer added to WOLFSSHD_CONFIG, set in wolfSSHD_ConfigNew() and propagated by wolfSSHD_ConfigCopy(). HandleMatch() now copies (*conf)->head, so each block carries the global defaults plus only its own overrides.
• Tail append: HandleMatch() walks from head to the end of the list before linking, instead of assuming the parse cursor is the tail. Included Match blocks survive and are no longer leaked. Match scope is deliberately unchanged — a directive after an Include still belongs to the global config, which is what OpenSSH does.
• wolfSSHD_ConfigNew() now stores its heap argument; the previous code discarded it, so allocations and frees could use different heaps.
• apps/wolfsshd/auth.c: the RequestAuthentication comment describing the old inheritance is now false — a user whose own Match never set TrustedUserCAKeys keeps the global value and is no longer rejected for certificate auth. The fail-closed branch for a genuinely different CA file is unchanged.

Closes f_8815.

Tests

test_GetUserConfMatchNoInherit and test_ConfigIncludeMatchChain in apps/wolfsshd/test/test_configuration.c. The latter also asserts that a directive placed after an Include resolves for an unmatched user, pinning the scope behavior. test_IncludeRecursionBound's inline file writing was factored into a shared WriteConfigFile() helper.

Verification

• make check: 11 passed, 0 failed; test_configuration 55 cases, 255 assertions, 0 failures.
• Negative controls: reverting the head copy, the tail walk, or the include scope each fails its corresponding test.
• Include/Match scope cross-checked against OpenSSH 10.2p1 using sshd -T.
• gcc-13 -Werror clean across 6 configs, plus a non-unix (-U__unix__ -U__APPLE__) -Wall -Wextra pass; ASan + UBSan clean.

[Copilot]

Pull request overview

Fixes wolfsshd Match inheritance and Include chaining so that (1) each Match block starts from global defaults rather than inheriting from the immediately preceding Match, and (2) included config files correctly advance the parse cursor, preventing later Match blocks from overwriting/dropping the include chain.

Changes:

• Add a head pointer to WOLFSSHD_CONFIG and build Match nodes from the global config (head) instead of the current node.
• Thread the parse cursor through ConfigLoad() / HandleInclude() via WOLFSSHD_CONFIG** so Include behaves like true textual inclusion and preserves appended Match chains.
• Add regression tests covering non-inheritance across Match blocks and include-chain preservation; update auth comment to reflect new behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
apps/wolfsshd/configuration.c	Reworks Match node creation and Include parsing to use global defaults and a shared cursor.
apps/wolfsshd/test/test_configuration.c	Adds focused regression tests and factors file-writing into a helper.
apps/wolfsshd/auth.c	Updates commentary in RequestAuthentication to match the new Match resolution semantics.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1153

Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 3
2 finding(s) posted as inline comments (see file-level comments below)

Low (1)

Global-only directives after an Include are silently swallowed by an included Match block

File: apps/wolfsshd/configuration.c:979
Function: HandleInclude
Category: Logic errors

HandleInclude() now advances the caller's cursor, so when an included file ends inside a Match block, later directives in the parent file are applied to that Match node. Port, HostKey, HostCertificate, Banner, and TrustedUserCAKeys are read only from the head node (wolfsshd.c:376-530, 2974) and are therefore dropped with no diagnostic.

Related known finding #8815 (similar but distinct): Both defects concern incorrect configuration state around Match nodes. However, the candidate is HandleInclude propagating an included file's parse cursor into its parent, while #8815 is HandleMatch copying the prior Match node; they have different faulting operations, root causes, and required patches.

Recommendation: Reject or log directives that only take effect on the global node when the parse cursor is a Match node.

---

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

🔵 [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.

[wolfSSL-Fenrir-bot]

🔵 [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.