Harden KEX and userauth packet handling by ejohnstown · Pull Request #1049 · wolfSSL/wolfssh

ejohnstown · 2026-06-22T23:55:03Z

Addresses parts of issue #1047 and finding F-5690.

Validate peer KEX public keys — check the peer public value before deriving the shared secret: wc_DhCheckPubKey (DH), wc_ecc_check_key (ECDH and ECC+ML-KEM hybrid), client and server. DH rejects degenerate/small-order values per NIST SP 800-56A (stricter than RFC 4253's [1, p-1]); ECC rejects off-curve/bad-order points. Negative unit tests cover the DH and ECDH paths; the hybrid shares the same helper and is covered by kex.test. (F-5690, Several SSH (RFC 4252/4253/4254) Compliance Issues in wolfSSH #1047(1))
Simplify redundant KEX method flags — drop the redundant useCurve25519MlKem flag, deriving the hybrid from useCurve25519 && useEccMlKem, and merge the duplicate Curve25519 keygen branch.
Reject packets with too-little padding — enforce the RFC 4253 6 minimum of 4 padding bytes on receive, returning WS_BUFFER_E below MIN_PAD_LENGTH, with a negative unit test. (Several SSH (RFC 4252/4253/4254) Compliance Issues in wolfSSH #1047(5))
Reject password-change auth requests — fail userauth when the password-change flag is set, never invoking the callback with the current password, while still fully parsing the new-password field; negative unit test. Per RFC 4252 8. (Several SSH (RFC 4252/4253/4254) Compliance Issues in wolfSSH #1047(6))

Copilot

Pull request overview

This PR hardens wolfSSH’s transport and authentication handling by adding stricter validation of peer-provided key-exchange inputs and enforcing additional RFC-required parsing/validation rules, addressing parts of issue #1047 / finding F-5690.

Changes:

Add DH/ECDH peer public-key validation (including hybrid ML-KEM paths) before deriving shared secrets; simplify Curve25519+ML-KEM flag handling.
Enforce RFC 4253 minimum padding length on received packets and add a negative unit test.
Reject password-change userauth requests (per RFC 4252 §8) without invoking the userauth callback, while still consuming the full message; add a negative unit test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`src/internal.c`	Adds DH/ECDH peer key validation, enforces minimum padding on receive, rejects password-change auth, and simplifies Curve25519+ML-KEM flag routing.
`wolfssh/internal.h`	Removes redundant handshake flag and exposes additional `WOLFSSH_TEST_INTERNAL` wrappers for new negative KEX tests.
`tests/unit.c`	Adds unit tests for short padding rejection, password-change userauth rejection, and DH/ECDH invalid peer public-key handling.

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

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1049

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

Copilot

Pull request overview

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

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1049

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

Copilot

Pull request overview

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

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1049

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.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1049

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

- DH: add wc_DhCheckPubKey on client and server before wc_DhAgree; rejects degenerate/small-order values per NIST SP 800-56A (2 <= y <= p-2), stricter than RFC 4253 section 8's [1, p-1] - ECDH and ECC+ML-KEM hybrid: validate the peer point through a shared EccCheckPeerKey helper (import + wc_ecc_check_key) on client and server, so the plain and hybrid paths cannot diverge - add wolfSSH_TestKeyAgreeEcdh_server/client and wolfSSH_TestSetDhKexKey test hooks - add negative unit tests rejecting bad DH and off-curve ECDH peer keys on client and server; the hybrid paths share the same helper and are exercised by kex.test Issues: F-5690, wolfSSL#1047 (1)

- remove redundant useCurve25519MlKem HandshakeInfo flag - identify the Curve25519+ML-KEM hybrid as useCurve25519 && useEccMlKem - merge the duplicate Curve25519 keygen branch in SendKexDhInit - order the client KeyAgree dispatch to match the combined flags

- enforce RFC 4253 section 6 minimum of 4 padding bytes on receive - return WS_BUFFER_E when padding_length is below MIN_PAD_LENGTH - add negative unit test driving DoReceive with a short-padded packet Issue: wolfSSL#1047 (5)

- fail userauth when the request sets the password-change flag - do not invoke the userauth callback with the current password - parse the new-password field so the message is fully consumed - add negative unit test asserting USERAUTH_FAILURE and no callback Per RFC 4252 section 8, an expired password MUST NOT be used to authenticate; password changes remain unsupported. Issue: wolfSSL#1047 (6)

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1049

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.

ejohnstown requested review from Copilot and wolfSSL-Fenrir-bot June 22, 2026 23:55

Copilot started reviewing on behalf of ejohnstown June 22, 2026 23:55 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread src/internal.c Outdated

Comment thread tests/unit.c Outdated

Comment thread tests/unit.c Outdated

Comment thread src/internal.c

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

ejohnstown force-pushed the ecckey-check branch from 83cef4e to 81626c5 Compare June 23, 2026 00:22

ejohnstown requested review from Copilot and wolfSSL-Fenrir-bot June 23, 2026 00:22

Copilot started reviewing on behalf of ejohnstown June 23, 2026 00:23 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread tests/unit.c Outdated

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

ejohnstown force-pushed the ecckey-check branch from 81626c5 to d866f14 Compare June 23, 2026 00:38

ejohnstown requested review from Copilot and wolfSSL-Fenrir-bot June 23, 2026 00:46

Copilot started reviewing on behalf of ejohnstown June 23, 2026 00:47 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread src/internal.c

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

ejohnstown force-pushed the ecckey-check branch from d866f14 to 8aaf4fa Compare June 23, 2026 17:27

ejohnstown requested review from Copilot and wolfSSL-Fenrir-bot June 23, 2026 17:34

Copilot started reviewing on behalf of ejohnstown June 23, 2026 17:35 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread src/internal.c Outdated

Comment thread src/internal.c Outdated

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

ejohnstown force-pushed the ecckey-check branch from 8aaf4fa to 0f48ec5 Compare June 23, 2026 18:07

ejohnstown added 4 commits June 23, 2026 11:27

Reject packets with too-little padding

0f6c386

- enforce RFC 4253 section 6 minimum of 4 padding bytes on receive - return WS_BUFFER_E when padding_length is below MIN_PAD_LENGTH - add negative unit test driving DoReceive with a short-padded packet Issue: wolfSSL#1047 (5)

ejohnstown force-pushed the ecckey-check branch from 0f48ec5 to ca9c8b6 Compare June 23, 2026 18:28

ejohnstown requested a review from Copilot June 23, 2026 18:28

ejohnstown requested a review from wolfSSL-Fenrir-bot June 23, 2026 18:28

Copilot started reviewing on behalf of ejohnstown June 23, 2026 18:29 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

wolfSSL-Fenrir-bot reviewed Jun 23, 2026

View reviewed changes

Comment thread tests/unit.c

ejohnstown marked this pull request as ready for review June 23, 2026 18:57

ejohnstown assigned wolfSSL-Bot Jun 23, 2026

ejohnstown requested a review from padelsbach June 23, 2026 18:57

Uh oh!

Conversation

ejohnstown commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1049

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1049

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1049

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1049

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1049

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ejohnstown commented Jun 22, 2026 •

edited

Loading