Reject the KEX requests a client never receives - #1222
Conversation
This is just a follow-up PR to (wolfSSL#1221), that mirrors pretty much the same changes for the client side too.
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, aligns with the protocol message directionality, and is backed by targeted regression tests covering both the new client-side blocks and a “do not over-block” case.
Pull request overview
This PR extends the “reject peer-only KEX messages” hardening from #1221 to the client side, ensuring a malicious server cannot send client-only key exchange messages to trigger unnecessary or unsafe processing.
Changes:
- Added client-side message gating to always reject client-only KEX message IDs (30/32/34) when received.
- Added a regression test
TestClientOnlyKexMsgsBlockedvalidating these KEX messages are rejected on the client, while the expected server message (31) remains allowed in the correct state. - Expanded the existing server-side regression test to assert that message ID 32 remains allowed when expected (i.e., not over-blocked between blocked IDs).
File summaries
| File | Description |
|---|---|
| tests/regress.c | Adds client-side regression coverage for blocking client-only KEX messages and refines server-side coverage to ensure msg 32 remains permitted when expected. |
| src/internal.c | Updates client receive-side gating to reject client-only KEX messages (30/32/34) and clarifies the server-side comment for msg 31 aliasing. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Jenkins retest this please (invalid stream header) |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1222
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.
This is just a follow-up PR to (#1221), that mirrors pretty much the same changes for the client side too.
Addresses some post merge feedback received from @ejohnstown.