Note (verified 2026-04-28):
- Accurate:
check_and_record does run before HMAC verification in the incoming handshake handler (peer.rs ~551–552 vs ~564–570). This means unauthenticated connections can trigger the nonce tracker's GC sweep and consume tracker capacity before proving knowledge of the shared secret. This is a real ordering bug.
- Accurate:
DashMap::retain is called on every handshake inside check_and_record, which walks all shards under write locks. With many concurrent handshakes and a large map, this can serialize access.
- Inaccurate/Overstated: The issue states
check_and_record is called at line 552 and HMAC verify at line 565 — these match the current code. However, the issue says "anyone-with-the-port can trigger the sweep without proving knowledge of the shared_secret." In practice, shared_secret is required for OFP (config.shared_secret is checked non-empty at startup), so the attack surface is any TCP connection to the OFP port, not the broader internet (unless the port is exposed). The claim about "100k entries" and O(n) serialization is a valid worst-case scenario, but the default max_entries cap limits the map size, so GC overhead is bounded at max_entries entries. The issue does not mention that the initiator side (connect_to_peer, ~line 431) also calls check_and_record before HMAC verify, which has the same ordering issue.
Location
crates/librefang-wire/src/peer.rs lines ~62–90 (NonceTracker::check_and_record)
crates/librefang-wire/src/peer.rs lines ~551–570 (incoming handshake handler — nonce check before HMAC verify)
crates/librefang-wire/src/peer.rs lines ~430–440 (outgoing connect_to_peer — same ordering issue)
Problem
// peer.rs ~62–90 — called on every handshake
pub fn check_and_record(&self, nonce: &str) -> Result<(), String> {
// GC: walks all DashMap shards under write lock
self.seen.retain(|_, ts| now.duration_since(*ts) < self.window);
// cap check + insert ...
}
Two issues:
-
Ordering bug: check_and_record runs at line ~552, before HMAC verification at line ~564. Any TCP connection to the OFP port can trigger the nonce tracker's GC sweep and fill tracker capacity without proving knowledge of shared_secret. The same ordering bug exists in the connect_to_peer path (~line 431).
-
GC on every insert: DashMap::retain walks every shard under write locks on each handshake. Under concurrent handshake load with a large tracked-nonce set (bounded by max_entries), concurrent handshakes serialize behind the retain sweep.
Impact
- An attacker with TCP access to the OFP port can repeatedly send Handshake messages with random nonces, triggering GC sweeps and eventually filling
max_entries, causing legitimate handshakes to be rejected with "Nonce tracker at capacity."
- Under flood, concurrent legitimate handshakes block behind the write-locked GC sweep.
Fix
- Move
check_and_record to after HMAC verification so unauthenticated connections cannot drive the sweep or consume capacity
- Run GC on a background timer (e.g. every 30 s) rather than inline on every insert, so the insert path is O(1)
- Or use a bounded ring buffer +
HashSet for amortized O(1) GC
Note (verified 2026-04-28):
check_and_recorddoes run before HMAC verification in the incoming handshake handler (peer.rs ~551–552 vs ~564–570). This means unauthenticated connections can trigger the nonce tracker's GC sweep and consume tracker capacity before proving knowledge of the shared secret. This is a real ordering bug.DashMap::retainis called on every handshake insidecheck_and_record, which walks all shards under write locks. With many concurrent handshakes and a large map, this can serialize access.check_and_recordis called at line 552 and HMAC verify at line 565 — these match the current code. However, the issue says "anyone-with-the-port can trigger the sweep without proving knowledge of the shared_secret." In practice,shared_secretis required for OFP (config.shared_secretis checked non-empty at startup), so the attack surface is any TCP connection to the OFP port, not the broader internet (unless the port is exposed). The claim about "100k entries" and O(n) serialization is a valid worst-case scenario, but the defaultmax_entriescap limits the map size, so GC overhead is bounded atmax_entriesentries. The issue does not mention that the initiator side (connect_to_peer, ~line 431) also callscheck_and_recordbefore HMAC verify, which has the same ordering issue.Location
crates/librefang-wire/src/peer.rslines ~62–90 (NonceTracker::check_and_record)crates/librefang-wire/src/peer.rslines ~551–570 (incoming handshake handler — nonce check before HMAC verify)crates/librefang-wire/src/peer.rslines ~430–440 (outgoingconnect_to_peer— same ordering issue)Problem
Two issues:
Ordering bug:
check_and_recordruns at line ~552, before HMAC verification at line ~564. Any TCP connection to the OFP port can trigger the nonce tracker's GC sweep and fill tracker capacity without proving knowledge ofshared_secret. The same ordering bug exists in theconnect_to_peerpath (~line 431).GC on every insert:
DashMap::retainwalks every shard under write locks on each handshake. Under concurrent handshake load with a large tracked-nonce set (bounded bymax_entries), concurrent handshakes serialize behind the retain sweep.Impact
max_entries, causing legitimate handshakes to be rejected with "Nonce tracker at capacity."Fix
check_and_recordto after HMAC verification so unauthenticated connections cannot drive the sweep or consume capacityHashSetfor amortized O(1) GC