Skip to content

OFP NonceTracker does O(n) retain() on every handshake — DoS amplifier under flood; also runs before HMAC verify #3880

Description

@houko

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:

  1. 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).

  2. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/securitySecurity systems and auditingbugSomething isn't workinghas-prA pull request has been linked to this issueneeds-triageAuto-applied when the issue title/body matched no area label — needs maintainer reviewsecuritySecurity issuesseverity/mediumBug or limitation with workaround / partial impact

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions