Harden connect token history and restart handling

[gafferongames]

Problem

The Rust server retained the pre-1.4.5 connect-token behavior: a token could be reused from the original address after disconnect, token history evicted its oldest entry under load, and server restarts accepted tokens whose keys may have been used before the restart.

Fix

• Track token history entries as pending or consumed, retaining the token HMAC, source address, expiry, and creation time.
• Allow only same-address retransmits while pending; consume the entry when its encryption mapping installs a client; refuse consumed tokens from every address.
• Reuse history slots only after token expiry and refuse new tokens when all entries are unexpired.
• Add ServerConfig and Server::new_with_config for the backend's maximum token lifetime, and reject pre-start tokens before private-token decryption on every server start.
• Sync STANDARD.md with the current netcode 1.02 normative document, which now requires these restart and history rules.
• Add unit, packet, and live UDP regressions plus README documentation.

No packet layout or wire bytes changed.

Validation

• cargo fmt --check
• cargo test
• cargo test --release
• cargo build --all-targets
• cargo clippy --all-targets -- -D warnings
• RUSTDOCFLAGS='-D warnings' cargo doc --no-deps
• cargo deny check
• rustup run 1.85 cargo check
• NETCODE_C_SERVER=... NETCODE_C_CLIENT=... cargo test --test c_interop -- --ignored --test-threads=1 --nocapture against C 1.4.8

Fixes #24

[gafferongames]

Security seat second eyes at exact head 2404f58 (Alex; this repairs the defect I confirmed in security#39 — the pre-1.4.5 token history, netcode.rs#24).

Verified: the three-piece C 1.4.8 contract is present and correct.

1. Pending/consumed with consume at install: ConnectTokenEntryResult::Accepted/Refused/HistoryFull; pending admits only the same address, consumed admits nothing forever. consume_connect_token_entry fires at client install via the encryption mapping's connect_token_entry_index (carried through add_encryption_mapping), so the response path consumes the entry the request created.
2. No live eviction: history-full returns HistoryFull (refuse); only Free or expired (expire_timestamp <= current_timestamp) slots are reclaimed — and since the packet read refuses expired tokens before FindOrAdd, the reclaim only fires on dead entries.
3. Restart lifetime guard: min_connect_token_expire_timestamp = unix_timestamp().saturating_add(max_connect_token_lifetime) at every start (saturating — no u64 overflow), zeroed at stop, enforced in read_packet_with_min_connect_token_expire_timestamp before the private token decrypt. This is what makes the stop-time table reset safe.

The old same-address-forever test contract is properly inverted: the suite now pins consumed-then-refused and history-full-refusal (56 unit + 10 integration, all green at this head on this bench). The legacy read_packet wrapper defaults the floor to zero so clients and existing callers are unaffected.

This closes netcode.rs#24 / security#39. Consistent shape-for-shape with netcode.go#29 and netcode.cs#9 — the three family fixes of this advisory are now verified identical in contract.

[rowan-claude]

Read (Fable) at 2404f58

Verdict: APPROVE.

Cold read against C netcode v1.4.8 (47a156b) netcode.c and its STANDARD.md; the PR's STANDARD.md is byte-identical to the C copy (diff exit 0, 0 lines). No HIGH, no MEDIUM. Three LOW notes.

Contract, hunk by hunk

C contract (netcode.c v1.4.8)	Rust (this head)
3986-3989: match only state != FREE && memcmp(mac); last match wins	server.rs:298 entry.state != Free && &entry.mac == mac
3991-3996: first slot that is FREE || expire_timestamp <= current_timestamp	server.rs:301-306 same predicate, <=
4001-4002: no free slot -> HISTORY_FULL (never evicts)	server.rs:312-314 HistoryFull
4007-4012: new token -> PENDING, time, expiry, address, mac	server.rs:315-321
4021-4027: PENDING + same address admits; anything else REFUSED	server.rs:324-330
4030-4036: consume = state CONSUMED, time untouched	server.rs:335-337
3779/3795: mapping carries the entry index	server.rs:141,155 connect_token_entry_index
4796-4800: consume at install, from the mapping's index	server.rs:1011-1015 in connect_client
4364: min_expire = now + max_connect_token_lifetime at start	server.rs:492-493 unix_timestamp().saturating_add(..)
1942: if (expire < min) return NULL, before private decrypt	packet.rs:432-435 <, before nonce/decrypt
4195-4196: <= 0 takes default 30	server.rs:452-456; lib.rs:130 DEFAULT_MAX_CONNECT_TOKEN_LIFETIME = 30
4298 / 4587: history reset at create and stop	server.rs:447 / 521
4668-4686: request order whitelist, addr, id, history(FULL, REFUSED), then full-server Denied	server.rs process_connection_request: same order

Lifetime guard direction: configured max shorter than the backend's longest issued lifetime lowers min, so a pre-start token with the longer lifetime passes; that is the unsafe direction. The ServerConfig doc says "the longest lifetime ... issued by the backend", which is the right instruction. Before start() min is 0 but receive_packets drops everything while !running (server.rs:704), so it is unreachable.

History bounds: fixed MAX_CLIENTS * 8 slots, no growth; a consumed entry is only reused once its own token expiry <= now, never while live.

Evidence

• cargo test: 56 unit, 10 integration, 1 doc, all ok; cargo fmt --check exit 0; cargo clippy --all-targets -- -D warnings clean.
• Red proof, each hunk reverted alone, then restored (tree clean at 2404f58):
  • consume call removed: connect_token_cannot_be_reused_after_disconnect ... FAILED / left: Connected right: Connected (tests/client_server.rs:208)
  • guard removed: packet::tests::connection_request_rejects_token_before_server_start ... FAILED (src/packet.rs:664)
  • free-slot predicate widened to any slot: connect_token_entries_enforce_single_use_and_bounded_history ... FAILED / left: Accepted(0) right: HistoryFull (src/server.rs:1325)
• C interop, built v1.4.8 here (cmake), cargo test --test c_interop -- --ignored --test-threads=1: c_client_connects_to_rust_server ... ok, rust_client_connects_to_c_server ... ok.

LOW (notes, no change required for merge)

1. tests/c_interop.rs is unchanged and connects once per direction; it does not exercise reuse. A C-client-reconnects-with-the-same-token case against the Rust server would pin the contract on real C bytes. Follow-up.
2. README.md:"configure it when creating the server" should say the value must be at least the longest lifetime the backend issues; shorter is the unsafe direction, longer only delays acceptance after start. One sentence.
3. Shared with C, not a port defect: the mapping keeps connect_token_entry_index past the entry's expiry (timeout -1 mappings never expire), so a late connection response can consume a slot the history has since reused for another token. C 4796-4800 has the identical shape; if it is worth closing it belongs upstream first.

[rowan-claude]

Read (Opus) at 2404f58

Verdict: APPROVE.

Cold read against C netcode at v1.4.8 (built and run here). Whole diff read, C contract quoted beside each Rust line, regression proven red by reverting the fix. No HIGH, no MEDIUM.

The contract, line by line

• Pending/consumed history. C netcode.c:3922-4027: ENTRY_FREE/PENDING/CONSUMED, "a pending entry admits the address that created it, and nothing else. a consumed entry admits nothing." Rust src/server.rs:246-333: ConnectTokenEntryState{Free,Pending,Consumed}, and the matching arm returns Accepted only for state == Pending && address == Some(address), else Refused. Same shape, same result set (Accepted/Refused/HistoryFull vs index/-1/-2).
• No live eviction. C: "A history whose entries all hold unexpired connect tokens refuses a new connect token instead of evicting one" — free slot is state == FREE || expire_timestamp <= current_timestamp (netcode.c:3993-3995), else HISTORY_FULL. Rust src/server.rs:296-306, 309-321: identical predicate, HistoryFull when free_index is None. The old oldest_index/oldest_time eviction is gone.
• Consume at install. C netcode.c:4796-4800 consumes in netcode_server_connect_client via netcode_encryption_manager_get_connect_token_entry_index. Rust src/server.rs:1011-1015 does exactly that from EncryptionEntry::connect_token_entry_index (plumbed at src/server.rs:78, 141, 155, 903). remove_encryption_mapping (src/server.rs:175) writes EncryptionEntry::new(), so the index cannot go stale; stop() (src/server.rs:520-521) resets both the history and the encryption manager, so a restart cannot consume a fresh entry through an old mapping.
• Lifetime/restart guard, and its direction. C netcode.c:4364: min = time(NULL) + config.max_connect_token_lifetime at start, checked in the packet read at netcode.c:1942 as if ( packet_connect_token_expire_timestamp < min ) return NULL;, placed after the <= current_timestamp expiry check and before netcode_decrypt_connect_token_private. Rust src/server.rs:492-493 (unix_timestamp().saturating_add(...), set in start(), zeroed in stop()) and src/packet.rs:432-435 — same < comparison, same direction (earlier expiry refused), same position: after expire_timestamp <= current_timestamp at src/packet.rs:427, before the nonce read and decrypt at src/packet.rs:437+.
• Configured maximum. DEFAULT_MAX_CONNECT_TOKEN_LIFETIME = 30 (src/lib.rs:130) equals C netcode.h:95; <= 0 falls back to the default (src/server.rs:452-456) exactly as netcode.c:4195-4196. README documents that the deployment must configure its own lifetime.
• Order of checks in process_connection_request (src/server.rs:844-887) — whitelist, address-already-connected, id-already-connected, history, server-full — matches netcode.c:4640-4698 step for step, including that a Denied-because-full still leaves a pending entry.
• STANDARD.md is byte-identical to mas-bandwidth/netcode STANDARD.md at v1.4.8 (diff clean). The new Connect Token History section is normative there, not invented here.

Evidence

• cargo test: 56 unit + 10 integration + 1 doc green, 2 interop ignored.
• Red proof. Deleting only the consume block at src/server.rs:1011-1015, nothing else, then cargo test --test client_server connect_token_cannot_be_reused_after_disconnect:
  thread 'connect_token_cannot_be_reused_after_disconnect' panicked at tests/client_server.rs:208:9: assertion 'left != right' failed left: Connected right: Connected
  Restored; green again. The integration test actually pins the advisory, it does not merely pass.
• C interop. Built mas-bandwidth/netcode at v1.4.8 and ran NETCODE_C_SERVER=... NETCODE_C_CLIENT=... cargo test --test c_interop -- --ignored --test-threads=1: 2 passed. Rust client ↔ C 1.4.8 server and C 1.4.8 client ↔ Rust server both hold past the token timeout, so the restart guard does not refuse a legitimate C-issued token (C examples issue a 30s lifetime against a 30s configured maximum).
• History bounds: MAX_CONNECT_TOKEN_ENTRIES = MAX_CLIENTS * 8 as in C; free_index and the consume index are both slot indices into a Vec of that fixed length, refilled by reset_connect_token_entries; no unchecked arithmetic on the hostile path.

Findings (all LOW, none blocking)

• LOW — ServerConfig is a bare public struct (src/server.rs:52-60) and README teaches struct-literal construction. The next config field is then a breaking change; C grows netcode_server_config_t freely because callers go through netcode_default_server_config. Consider #[non_exhaustive] plus ServerConfig::default()-and-mutate in the README example, before this ships.
• LOW — ConnectTokenEntry::time is #[allow(dead_code)] (src/server.rs:263-264). It is normative (STANDARD.md: "An entry's time is set when the entry is created, and is never refreshed") and a unit test asserts it, so keeping it is right; say that in the comment rather than leaving a bare allow, or a later cleanup will delete it.
• LOW — interop startup is a fixed sleep(500ms) (tests/c_interop.rs:63). My first run of both interop tests failed with connection request timed out against freshly built C binaries and both passed on immediate rerun. Poll until the C process is listening instead of sleeping a constant, or CI will flake.
• LOW — v6 address equality is stricter than C. entries[index].address == Some(address) (src/server.rs:325) compares flowinfo and scope_id; netcode_address_equal compares type, address and port only. Fail-closed, and identical in practice for a peer whose socket address is stable, so no action beyond knowing it.