Re-seed server global packet sequence on start, not just create#159
Merged
Conversation
Global packets (connection challenge, connection denied) encrypt with the same per-connect-token server-to-client key as per-client packets, whose sequences start at zero. The global sequence therefore starts at 1ULL << 63 so the AEAD nonces stay disjoint under the shared key. That seed was only applied in netcode_server_create, and netcode_server_stop reset the global sequence to zero. A server that was stopped and started again sent challenge packets from global sequence 0, 1, 2 ... overlapping the per-client sequence space: the same key and nonce encrypting two different plaintexts, which breaks the AEAD security guarantees. Seed the global sequence in netcode_server_start as well. No wire format change. Pinned by test_server_restart_global_sequence. Found while porting netcode to Rust (mas-bandwidth/netcode.rs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds finding 3: the server global packet sequence must be re-seeded to 1 << 63 on every start, not just in create, or a stopped-and-restarted server reuses AEAD nonces between global and per-client packets under the shared per-token server-to-client key. Fixed in dc21b70. Ports that copied the create/stop behavior should check their restart path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
The bug
Global packets (connection challenge, connection denied) are encrypted with the same per-connect-token server-to-client key as per-client packets, whose sequence numbers start at zero. To keep the AEAD nonces disjoint under that shared key, the server's global sequence starts at
1ULL << 63— but that seed was only applied innetcode_server_create, andnetcode_server_stopresets the global sequence to zero.A server object that is stopped and started again therefore sends challenge packets from global sequence 0, 1, 2 … — inside the per-client sequence space. Once a client connects, the server encrypts two different plaintexts (a challenge packet and a keep-alive/payload packet) with the same key and the same nonce, which voids the AEAD guarantees for those packets.
The fix
Seed
global_sequence = 1ULL << 63innetcode_server_startas well, so the invariant holds for every running lifetime of the server object. No wire format change — the sequence seed is server-local behavior.test_server_restart_global_sequence(44 tests pass).🤖 Generated with Claude Code