Skip to content

wolfsshd: honor Match-block auth restrictions in RequestAuthentication#1003

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

wolfsshd: honor Match-block auth restrictions in RequestAuthentication#1003
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4793

Conversation

@yosuke-wolfssl
Copy link
Copy Markdown
Contributor

@yosuke-wolfssl yosuke-wolfssl commented Jun 5, 2026

This fixes auth-bypass where Match-block authentication restrictions were silently ignored. RequestAuthentication and DoCheckUser read authentication settings from the global root config node (authCtx->conf), which is assigned once in wolfSSHD_AuthCreateUser and never refreshed per-user. As a result, a restriction such as:
Match User alice
PasswordAuthentication no
was correctly advertised by DefaultUserAuthTypes (pubkey-only) but not enforced — the server still accepted alice's password attempt. The same stale-pointer flaw also bypassed per-user PermitRootLogin.

Changes

  • RequestAuthentication: resolve the per-user config once via wolfSSHD_AuthGetUserConf and use it for all five settings that were reading the global node — PasswordAuthentication, PermitEmptyPasswords, AuthorizedKeysFile (set flag), and the two TrustedUserCAKeys reads. This mirrors the correct pattern already used in DefaultUserAuthTypes.
  • DoCheckUser: the root-login check now resolves the per-user config so a Match block override of PermitRootLogin is honored instead of only consulting the global node. Scoped to the root branch.
  • Both new lookups fail closed: if the per-user config cannot be resolved (e.g. the user's primary group is unresolvable and getgrgid fails inside wolfSSHD_AuthGetUserConf), authentication is denied rather than falling back to the permissive global node. This is safe because such a user cannot complete a session anyway — session setup already rejects an unresolvable user config with WS_FATAL_ERROR. Comments explain this reasoning for future maintainers.
  • New tests to exercise the new code path

Addressed by f_4793 and f_4943

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 5, 2026
Copilot AI review requested due to automatic review settings June 5, 2026 02:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an authentication enforcement bypass in wolfsshd where per-user Match block restrictions were being advertised but not enforced during authentication, by ensuring RequestAuthentication and the root-login check resolve and use the per-user configuration node.

Changes:

  • RequestAuthentication now resolves usrConf via wolfSSHD_AuthGetUserConf() and consults it for auth-related settings (e.g., password auth, empty password policy, authorized keys file flag, CA keys file).
  • DoCheckUser now honors Match overrides of PermitRootLogin by resolving the root user’s effective config before allowing root login.
  • Adds a unit test verifying Match User ... overrides produce a distinct per-user config node with tightened auth settings, while non-matching users fall back to the global node.

Reviewed changes

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

File Description
apps/wolfsshd/test/test_configuration.c Adds a regression test validating Match overrides are returned by user-config resolution and differ from the global node.
apps/wolfsshd/auth.c Uses per-user config resolution in authentication and root-login gating to enforce Match-scoped auth restrictions and fail closed on resolution failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/wolfsshd/auth.c
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1003

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

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

Comment thread apps/wolfsshd/auth.c
Comment thread apps/wolfsshd/auth.c
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1003

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

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

Comment thread apps/wolfsshd/auth.c
Comment thread apps/wolfsshd/auth.c
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1003

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

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

Comment thread apps/wolfsshd/auth.c
Comment thread apps/wolfsshd/auth.c
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1003

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.

4 participants