v4: AES, remove covert frames #29
Open
Conversation
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
There was a problem hiding this comment.
Pull request overview
This pull request revives version-based cipher selection in the v3 API, enabling switching between ChaCha20-Poly1305 (version 3) and AES-GCM (version 4+) for encryption. The PR also removes support for covert frames throughout the codebase. The main motivation is performance: AES-GCM with hardware acceleration (AES-NI) provides approximately 4x speedup compared to ChaCha20-Poly1305.
Changes:
- Adds
peerVersionparameter to v3 API functions for cipher selection - Implements
newAEAD()function to choose between ChaCha20-Poly1305 (v3) and AES-GCM (v4+) - Removes all covert stream functionality including
DialCovertStream(), related buffer management, and padding logic - Updates package-level API to default to version 4
- Adds test coverage for multiple versions (3, 4, 5, 255)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| mux.go | Updates version negotiation from 3 to 4, passes negotiated version to v3 layer |
| v3/handshake.go | Implements cipher selection logic based on version, adds newAEAD() function, fixes typo |
| v3/mux.go | Adds peerVersion parameter to handshake functions, removes covert stream methods and buffer management |
| v3/frame.go | Removes covert frame handling logic, updates constant names for generality, uses modern range syntax |
| v3/mux_test.go | Adds version selection test, removes covert stream tests and benchmarks, removes unused imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ChrisSchinnerl
requested review from
Alrighttt,
ChrisSchinnerl,
n8mgr and
peterjan
ChrisSchinnerl
previously approved these changes
Feb 6, 2026
peterjan
requested changes
Feb 6, 2026
Member
peterjan
left a comment
peterjan
left a comment
There was a problem hiding this comment.
README.md still mentions covert streams
n8mgr
previously approved these changes
Feb 6, 2026
Member
There was a problem hiding this comment.
It's a bit unfortunate that we once again have protocol v4, in v3/*.go, but it makes sense.
There's only ~5% performance loss on my Mac going with AES-256. Since it's fairly negligible, we have the key material, and it's an easy boost I'd probably lean towards 256. I'm good either way and we can always revisit it later.
ChrisSchinnerl
previously approved these changes
Feb 9, 2026
Alrighttt
previously approved these changes
Feb 9, 2026
Alrighttt
left a comment
Alrighttt
left a comment
There was a problem hiding this comment.
LGTM;
Tested this manually, and everything works as expected. The only nit I have is that we now have v4 code within the v3 directory.
Member
Author
|
Perhaps we should rename to |
lukechampine
dismissed stale reviews from Alrighttt and ChrisSchinnerl
via
a79e6ae
n8mgr
approved these changes
Feb 10, 2026
peterjan
approved these changes
Feb 10, 2026
Member
|
Some additional benchmarks from my Proxmox server. All benchmarks used 1 core but different CPU settings: The difference between the
|
ChrisSchinnerl
approved these changes
Feb 10, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Revives the
versionargument the v3 API, allowing the handshake to switch between AES and ChaCha. Benchmarks show a ~4x speedup (how fitting). Also dropped support for covert frames andDialStreamContext; they were previously removed from the package-level API, but not the underlyingv3/implementation.Implements #28