Skip to content

fix: validate keyboard-interactive prompts - #1199

Merged
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:sf20
Aug 28, 2026
Merged

fix: validate keyboard-interactive prompts#1199
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:sf20

Conversation

@ejohnstown

Copy link
Copy Markdown
Contributor

RFC 4256 section 3.3 forbids an empty prompt, and RFC 4251 section 5
allows only 0 or 1 in a boolean field. Validation runs before the
payload is sized, so the sizing and building passes agree.

  • reject a zero-length prompt or one with a NULL buffer
  • reject a prompt count with the arrays unset
  • normalize the outgoing echo byte to 0 or 1
  • cover the rejections in tests/api.c

Issue: F-10583, F-10584

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

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 tightens server-side keyboard-interactive (RFC 4256) handling by validating prompt metadata before packet sizing/building, preventing invalid INFO_REQUEST serialization and ensuring boolean fields are encoded correctly per RFC 4251.

Changes:

  • Add validation in SendUserAuthKeyboardRequest() to reject empty/NULL prompts and missing prompt arrays when promptCount > 0.
  • Normalize the outgoing per-prompt echo byte to a strict RFC 4251 boolean (0 or 1).
  • Add API-level tests covering the new rejection paths.

Reviewed changes

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

File Description
src/internal.c Adds prompt/array validation prior to packet sizing/building and normalizes the encoded echo boolean.
tests/api.c Adds regression tests to ensure invalid keyboard-interactive prompt setups are rejected.

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

Comment thread tests/api.c
@ejohnstown
ejohnstown force-pushed the sf20 branch 2 times, most recently from 1abdb6d to 4166b8c Compare August 27, 2026 20:51

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

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. Reported findings require changes before merge.

Comment thread tests/auth.c Outdated
RFC 4256 section 3.3 forbids an empty prompt, and RFC 4251 section 5
allows only 0 or 1 in a boolean field. Validation runs before the
payload is sized, so the sizing and building passes agree.

- reject a zero-length prompt or one with a NULL buffer
- reject a prompt count with the arrays unset
- reject a prompt over WOLFSSH_MAX_PROMPT_SZ, which the sizing pass
  would otherwise sum into a wrapped payload size
- normalize the outgoing echo byte to 0 or 1
- cover the rejections in tests/api.c, the echo byte in tests/auth.c

Issue: F-10583, F-10584
Comment thread tests/auth.c Outdated

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

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.

@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot dismissed their stale review August 27, 2026 22:36

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

@philljj
philljj self-requested a review August 28, 2026 22:25
@philljj philljj self-assigned this Aug 28, 2026
@philljj
philljj merged commit 5fb7be4 into wolfSSL:master Aug 28, 2026
166 checks passed
@ejohnstown
ejohnstown deleted the sf20 branch August 28, 2026 22:55
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