Repository navigation
Discard a rejected CBC packet before failing - #1281
ejohnstown wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The discard path masks local allocation failures as packet-rejection errors.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds constant-work rejection handling for malformed AES-CBC packets to reduce timing leakage.
Changes:
- Introduces resumable rejected-packet discard state.
- Preserves immediate rejection for non-CBC modes.
- Adds CBC discard and CTR regression tests.
| File | Description |
|---|---|
wolfssh/internal.h |
Adds discard state and metadata. |
src/internal.c |
Implements CBC packet discard handling. |
tests/unit.c |
Tests discard behavior and immediate CTR failure. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1281
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 1 of 3 in-scope changed file(s) opened by the reviewer; not opened: tests/unit.c, wolfssh/internal.h
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
Under AES-CBC, DoReceive() no longer fails as soon as the decrypted length or the MAC is rejected. It reads the packet out to MAX_PACKET_SZ bytes and runs the MAC over the rest first, as OpenSSH does, so neither the failure point nor the MAC work reveals the length. - the length checks and the MAC check all go through RejectPacket() - a PROCESS_DISCARD state resumes the discard after a want-read; a transport failure mid-discard reports the rejection - CTR, AEAD and unencrypted packets still fail immediately - the discard compiles out under WOLFSSH_NO_AES_CBC - tests/unit.c covers each rejection with short reads, a peer close, and a nonzero start offset Issue: F-12606, F-8807, F-11668
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1281
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
Under AES-CBC, a packet rejected on its length or its MAC no longer fails at once. DoReceive() reads it out to MAX_PACKET_SZ bytes and runs the MAC over the rest first, as OpenSSH does, so neither the failure point nor the MAC work reveals the length.