Skip to content

fix: clear userauth state on bad service - #1201

Open
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:sf22
Open

fix: clear userauth state on bad service#1201
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:sf22

Conversation

@ejohnstown

Copy link
Copy Markdown
Contributor

A USERAUTH_REQUEST naming a service other than "ssh-connection" is
refused before method dispatch, so it drops the per-method state there
too. A following USERAUTH_INFO_RESPONSE is no longer run against a
keyboard-interactive exchange the server has already failed.

  • reset ssh->authId and clear ssh->kbSetupPending on the refusal
  • leave ssh->kbAuth alone; clients free those buffers by promptCount
  • add wolfSSH_TestDoUserAuthInfoResponse() for the tests
  • cover the refusal and the stale INFO_RESPONSE in tests/unit.c

Issue: F-10575

Copilot AI lite review requested due to automatic review settings August 27, 2026 19:58

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.

Pull request overview

This PR fixes a server-side USERAUTH edge case where a USERAUTH_REQUEST with an invalid service name (not "ssh-connection") was refused before method dispatch but could leave keyboard-interactive state (e.g., authId, kbSetupPending) in a stale/incorrect state, allowing a subsequent USERAUTH_INFO_RESPONSE to be processed against an exchange the server already failed.

Changes:

  • Clear ssh->authId (and ssh->kbSetupPending when enabled) when rejecting USERAUTH due to an invalid service name.
  • Expose a new internal test hook wolfSSH_TestDoUserAuthInfoResponse() (keyboard-interactive only) for unit testing.
  • Add a regression unit test to ensure bad-service refusal clears keyboard-interactive state and rejects stale INFO_RESPONSE without invoking the userauth callback.

Reviewed changes

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

File Description
src/internal.c Clears userauth/keyboard-interactive state on invalid service name; adds test wrapper for DoUserAuthInfoResponse().
wolfssh/internal.h Declares the new internal test entry point for USERAUTH_INFO_RESPONSE handling (keyboard-interactive only).
tests/unit.c Adds a regression test that validates the state reset on bad service and ensures stale INFO_RESPONSE doesn’t reach the auth callback.

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

Comment thread src/internal.c
A USERAUTH_REQUEST naming a service other than "ssh-connection" is
refused before method dispatch, so it drops the per-method state there
too. A following USERAUTH_INFO_RESPONSE is no longer run against a
keyboard-interactive exchange the server has already failed.

- reset ssh->authId and clear ssh->kbSetupPending on the refusal
- charge the abandoned exchange, as the dispatch path does
- leave ssh->kbAuth alone; clients free those buffers by promptCount
- add wolfSSH_TestDoUserAuthInfoResponse() for the tests
- cover the refusal and the stale INFO_RESPONSE in tests/unit.c

Issue: F-10575

@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 #1201

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

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