Repository navigation
Wipe decrypted traffic from transport and channel buffers - #1323
Open
ejohnstown wants to merge 3 commits into
Open
ejohnstown wants to merge 3 commits into
ejohnstown wants to merge 3 commits into
Conversation
The channel read paths wipe the bytes they hand to the application, and GrowBuffer() wipes the stale copy its in-place compaction leaves above the new length. _UpdateChannelWindow() compacts through GrowBuffer() instead of its own WMEMMOVE, so it gets the same wipe. - _BufferTake() serves wolfSSH_stream_read(), _ChannelRead() and _ChannelReadExt() - tests: short reads, a compacting read, a drained stderr chunk in the inline buffer, and GrowBuffer() compaction Issue: F-11677, F-8848, F-13438, F-10593
DoReceive() wipes a packet that fails to decrypt, authenticate or pass the length checks before it returns. Under CBC the wipe waits until DiscardPacket() has run its MAC, then covers a fixed MAX_PACKET_SZ, so it adds nothing that depends on the decrypted length. - BundlePacket() wipes the aborted packet it rewinds over, for every sender - tests: reject and discard paths check the packet is gone; the BundlePacket() failure test checks the output buffer tail Issue: F-14437, F-14438
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1323
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 3 of 3 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
GenerateKey() wipes its wc_HashAlg after wc_HashFree(). The SHA-2 free functions already zero it, but SHA-1 and hardware hash ports can leave the last input block, the derived key, in the stack context. Issue: F-15670
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1323
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decrypted traffic is wiped once it is no longer needed, instead of waiting for
wolfSSH_free(): channel data as the application reads it, and transport packets that are dropped on an error.GrowBuffer()compaction wipes the stale copy it leaves behind, which_UpdateChannelWindow()now uses tooDoReceive()wipes a packet that fails decryption, its MAC or the length checks; under CBC the wipe follows the discard, at a fixed sizeBundlePacket()wipes an aborted outgoing packet for every senderIssue: F-11677, F-8848, F-13438, F-10593, F-14437, F-14438