internal, tests: start no highwater rekey after an inbound packet fails

[yosuke-wolfssl]

Problem

wolfSSH_worker() flushes queued output whatever DoReceive() returned (deliberate since #1217: an idle receive and a hard failure both return WS_FATAL_ERROR). The flush goes through wolfSSH_SendPacket(), which runs the transmit HighwaterCheck(), and the default callback calls wolfSSH_TriggerKeyExchange(). So when the mark comes due on a pass whose inbound packet failed its MAC or decrypt check, the library builds and sends a new KEXINIT and sets isKeying on a session whose input can no longer be read.

Fixing only the worker would not be enough. The check stays armed until it fires, so the KEXINIT would move to the next send, typically the CHANNEL_EOF/CLOSE that wolfSSH_shutdown() sends during teardown.

Separately, the wolfSSH_SendPacket() comment said transport failures record their code in ssh->error, which is not true of what the highwater callback returns.

Fix (src/internal.c)

The receive paths that reject the peer's bytes set highwaterFlag and msgHighwaterFlag, so no highwater callback fires for the rest of the session. HighwaterCheck() already skips a mark whose flag is set, and only DoNewKeys() clears the flags. No NEWKEYS can be processed after such a failure: the rejected packet is never consumed, so every retry fails on it again.

Function	Check that now sets the flags
DoReceive()	first-block Decrypt(), packet_length too large, not block aligned, body Decrypt(), VerifyMac(), DecryptAead()
DoPacket()	padding_length below MIN_PAD_LENGTH, or leaving no room for the message id

The length and alignment checks are included because, under CTR, a flipped bit in an encrypted packet_length fails them before VerifyMac() runs. DoPacket() errors that consume the packet are not covered: the input stays in sync and a rekey is still legitimate. The worker and SendPacketFlush() are unchanged. The wolfSSH_SendPacket() comment now covers flush failures only, and says HighwaterCheck() writes nothing to ssh->error but its callback may.

Behavior change: before, a mark crossed after an integrity failure sent a KEXINIT that could never complete, and SendChannelData() then refused with WS_REKEYING for the rest of the session. Now it doesn't, so an app that ignores the worker's WS_FATAL_ERROR + WS_VERIFY_MAC_E and keeps sending can go past the mark under the same key. ssh.h already classes that pair as an error, not a transient. A highwater callback used only for notification is also no longer called after such a failure.

Tests

• test_HighwaterQuietAfterBadPacket(): an idle worker pass fires the callback; a bad-MAC pass still flushes the queued output but fires nothing, and neither does the next wolfSSH_SendIgnore().
• test_DoReceive_RejectsPaddingUnderflow() covers the one reachable check no existing test reached.
• One test per latch site asserts that both flags are set: the existing DoReceive rejection tests and test_WorkerHardRecvErrorOutranksFlush(), plus the new underflow test.

Verification

• Negative controls: removing the flag writes at any single peer-reachable site fails that site's test. Only the two Decrypt() checks have no test, because a peer cannot make them fail: by then sizes are block-aligned, so only an internal or hardware crypto error reaches them.
• unit.test 172 passed / 0 failed. regress, testsuite, api and the scp, sshclient, get-put and sftp scripts pass.
• Builds clean with gcc-13 -Werror across 6 configs.

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation consistently covers unrecoverable receive paths and includes focused regression tests.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents highwater-triggered rekeying after unrecoverable inbound packet validation failures.

Changes:

• Disables byte/message highwater callbacks after framing, decryption, or integrity failures.
• Clarifies highwater flag and send-error semantics.
• Adds regression coverage for malformed packets and post-failure sends.

File	Description
src/​internal.c	Suppresses highwater callbacks after fatal packet rejection.
wolfssh/​internal.h	Documents expanded highwater flag semantics.
tests/​unit.c	Tests malformed packets and callback suppression.

---

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1279

Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 2 of 3 in-scope changed file(s) opened by the reviewer; not opened: 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