From 713b4436c5ee52ba5e5f253efad1f0b721cc3ce5 Mon Sep 17 00:00:00 2001 From: Dan Hepworth Date: Fri, 23 Oct 2026 13:36:36 -0400 Subject: [PATCH] net: exempt manual peers from the hybrid fallback, fix its state Found in review of #233. - Manual peers never fall back. An attacker on the path can make an endpoint's hybrid handshakes fail without any keys (damage the peer's confirmation, reset the connection, drop its traffic), and three failures put it in the fallback set, repeatably. Failures of MANUAL connections (addnode, -connect) no longer count toward a streak, and a manual connection always negotiates, even to an endpoint in the fallback set. Counters, rings, log lines and the local-fault warning still see them. Automatic connections keep the fallback. - The success set is capped at 1,000 endpoints, the least recently confirmed evicted first: feelers to advertised endpoints grew it without bound. A separate flag keeps "a handshake succeeded since startup" for the local-fault warning. - ObservePQ() no longer re-reads fNetworkActive. Events from the peer's bytes, a verified confirmation included, are always accounted; DisconnectNodes() passes the snapshot it closed connections with, so closes caused by switching the network off are never derived as abandoned or closed_after_switch. - A fallback window also ends once its length has passed on the steady clock, so a wall clock step back can't stretch it; the stats keep their wall clock times. - The local-fault warning counts only failures this node's own ML-KEM code can cause: ek_modulus, length, tag, and closed_after_switch with eof or reset. ek_length, not_decoy and timeout are the peer's or the path's. - getnetworkinfo counts connections_pq from the connection manager's node list, without CopyStats() and a cs_main lock per peer. Every node in the list is one getpeerinfo lists, so the result is the same. - A backward step of the mocked steady clock restarts load shedding's current second and quiet run, so shedding can't latch. - net_pq_tests restores CreateSock with an RAII guard in pq_config. - The rpc fuzz target mocks the steady clock too, as getpqtransportinfo now reads it. - The specification's downgrade note, the operator guide (fallback, local_fault, abandoned), the RPC delta reference and the release note say what the code does. The fallback functional tests use automatic connections. Co-Authored-By: Claude Opus 5.5 --- doc/design/pq-transport.md | 15 +- doc/integration/rpc-delta-reference.md | 3 +- doc/release-notes-240.md | 10 +- doc/user/network/pq-transport.md | 55 ++-- src/net.cpp | 147 +++++++--- src/net.h | 61 +++-- src/rpc/net.cpp | 30 +- src/test/fuzz/rpc.cpp | 2 + src/test/net_pq_tests.cpp | 361 +++++++++++++++++++++---- test/functional/p2p_v2_pq_fallback.py | 129 ++++++--- test/functional/rpc_pqtransport.py | 47 +++- 11 files changed, 658 insertions(+), 202 deletions(-) diff --git a/doc/design/pq-transport.md b/doc/design/pq-transport.md index fd07c84a82..5c8f9addc0 100644 --- a/doc/design/pq-transport.md +++ b/doc/design/pq-transport.md @@ -319,8 +319,9 @@ qbit reports each v2 connection's negotiation status as one of these values: - `off` when the switch is off, when a responder sheds its offer under load, or after a local key generation, key check or encapsulation fault that left the connection on plain v2; -- `fallback` for an outbound connection that runs plain v2 - because of earlier failures; inbound connections never fall back. +- `fallback` for an automatic outbound connection that runs + plain v2 because of earlier failures; manual and inbound connections never + fall back. There is no failed status. A connection that closes on any failure above or in key confirmation, local faults included, reports `pending` until it is @@ -342,7 +343,15 @@ removed. - **Downgrade.** Stripping an offer or an accept means forging packets under the ECDH keys, so it takes an active attacker who breaks ECDH in real time or sits in the middle, and an unauthenticated session can't stop the latter - anyway. A passive recorder can't downgrade a connection. + anyway. A passive recorder can't downgrade a connection. qbit's fallback + policy is weaker: an attacker on the network path, without any keys, can + make hybrid handshakes with an endpoint fail after the switch, by damaging + the peer's key confirmation in transit, resetting the connection or + dropping its traffic. Three such failures in a row put an automatic + outbound peer in the fallback set, so that new connections to it use ECDH + keys only, for 1 hour, then 4, then 24, and the attacker can repeat this. + Manual peers (`addnode`, `-connect`) are exempt: + their failures never count, and they always negotiate. - **Transcript binding.** Both keys, both contents and the network magic feed the derivation, so two sides that saw different negotiation bytes derive different keys and fail the confirmation instead of continuing. diff --git a/doc/integration/rpc-delta-reference.md b/doc/integration/rpc-delta-reference.md index 778e690624..1b704ea347 100644 --- a/doc/integration/rpc-delta-reference.md +++ b/doc/integration/rpc-delta-reference.md @@ -274,13 +274,14 @@ Important result fields: - `instance_id` and `since`: the process start the counters count from - `handshakes.inbound` and `handshakes.outbound`: event counters, such as `switched`, `legacy_peer`, `first_packet_failed` and, inbound only, `shed` - `load_shedding`: `active`, `threshold_per_s` and `since` -- `fallback_set` and `failure_streaks`: outbound endpoints that fall back to plain v2, or are on their way there +- `fallback_set` and `failure_streaks`: automatic outbound endpoints that fall back to plain v2, or are on their way there; manual connections (`addnode`, `-connect`) never fall back - `recent_failures.inbound` and `recent_failures.outbound`: the most recent non-success outcomes, with sequence numbers Integrator notes: - Every field is always present. The counters, rings and endpoint history are kept in memory only and start over when `instance_id` changes. - Counters count events, not exclusive outcomes: a switch whose key confirmation then fails counts `switched` and `first_packet_failed`. +- A `fallback_set` entry's `expires` is the latest its window ends: the window also ends once `window_seconds` have passed on the node's monotonic clock, so after a clock step back it can end earlier. - Endpoints contain peer addresses; treat the output as sensitive. ## Shared RPC Names With qbit-Specific Semantics diff --git a/doc/release-notes-240.md b/doc/release-notes-240.md index 86748bbe61..1ef12bcf55 100644 --- a/doc/release-notes-240.md +++ b/doc/release-notes-240.md @@ -32,10 +32,12 @@ handshake outcomes, load shedding and the fallback set. `getpeerinfo` gains `transport_pq` and `transport_pq_status`, `getnetworkinfo` gains `connections_pq`, `qbit-cli -netinfo` marks hybrid peers `2p`, and the log states the setting at -startup in a `v2 pq: … arith=… keccak=…` line. Outbound -connections to an address whose hybrid handshakes keep failing fall back to -plain v2 for a while, and a node under an inbound connection flood stops -offering hybrid keys until it subsides. +startup in a `v2 pq: … arith=… keccak=…` line. Automatic +outbound connections to an address whose hybrid handshakes keep failing fall +back to plain v2 for a while. Someone on the network path can cause such +failures, so peers added with `addnode` or `-connect` never fall back. A node +under an inbound connection flood stops offering hybrid keys until it +subsides. See the [operator guide](user/network/pq-transport.md) to try it, check it and troubleshoot it, and the [specification](design/pq-transport.md). (#240) diff --git a/doc/user/network/pq-transport.md b/doc/user/network/pq-transport.md index 9aa613c43a..1da209a2a6 100644 --- a/doc/user/network/pq-transport.md +++ b/doc/user/network/pq-transport.md @@ -191,7 +191,7 @@ verified, and `transport_pq_status`: | `pending` | Still negotiating. A connection that is closing after a hybrid failure also stays `pending` until it is removed: there is no failed status, and the failure is counted in `getpqtransportinfo`. A handshake that doesn't finish within 60 seconds (`-peertimeout`) is closed. | | `hybrid` | The session keys are hybrid. | | `legacy_peer` | The peer's handshake carried no hybrid offer or accept, so the connection keeps the ECDH keys, as before. Peers running v1.0.0, or with the feature off, show this. | -| `fallback` | Plain v2 to an address in the fallback set (outbound only). | +| `fallback` | Plain v2 to an address in the fallback set (automatic outbound connections only). | | `off` | Plain v2 because the feature is off on this node, the offer was shed under load, or a local ML-KEM fault happened before anything was committed. | | `v1` | An unencrypted v1 connection. | @@ -213,18 +213,30 @@ peers show `off` and `connections_pq` is 0. ## Fallback and Load Shedding -**Fallback.** If hybrid handshakes with the same outbound address fail three -times in a row, the node stops trying hybrid keys with that address for an hour, -and connects to it with plain v2 instead, so that a broken peer or a middlebox -that mangles the handshake can't keep the connection from working. The second -time the address falls back it stays out for 4 hours, and from then on for 24 -hours. Only failures the peer side can cause count: -`malformed_record`, `first_packet_failed` and -`closed_after_switch`. An address that completed a hybrid -handshake since startup never falls back, inbound connections never fall back, -and a restart forgets the whole history. `getpqtransportinfo` lists the -addresses in `fallback_set`, with `expires`, and -those on their way there in `failure_streaks`. +**Fallback.** If hybrid handshakes of automatic outbound connections with the +same address fail three times in a row, the node stops trying hybrid keys with +that address for an hour, and connects to it with plain v2 instead, so that a +broken peer or a middlebox that mangles the handshake can't keep the connection +from working. The second time the address falls back it stays out for 4 hours, +and from then on for 24 hours. Each window is also measured on the node's +monotonic clock, so setting the clock back can't lengthen it. Only failures the +peer side can cause count: `malformed_record`, +`first_packet_failed` and `closed_after_switch`. +An address that completed a hybrid handshake since startup never falls back +(the node remembers the 1,000 that did so most recently), inbound connections +never fall back, and a restart forgets the whole history. `getpqtransportinfo` +lists the addresses in `fallback_set`, with +`expires`, and those on their way there in +`failure_streaks`. + +**Manual peers never fall back.** Someone on the network path between you and +a peer can cause those failures without breaking any keys, by damaging or +dropping packets, and so keep an automatic peer on ECDH keys. Connections you +make yourself, with `addnode` or `-connect` (also +`-addnode`), always negotiate hybrid keys, even to an address in +the fallback set, and their failures never count toward it; they are still +counted, listed in `recent_failures` and logged. If hybrid keys with particular +peers matter to you, add them with `addnode`. **Load shedding.** An offer costs the responding node an ML-KEM key generation, and a decapsulation when the peer accepts. If more inbound v2 connections than @@ -255,8 +267,9 @@ It does not: `session_id` out of band detects that. - **Protect traffic to v1.0.0 nodes,** or to any peer that has the feature off: those connections use ECDH keys (v2) or no encryption (v1), as before. The - fallback, load shedding and local faults also leave some connections on ECDH - keys; `transport_pq` reports which. + fallback, which someone on the network path can trigger for automatic peers, + load shedding and local faults also leave some connections on ECDH keys; + `transport_pq` reports which. - **Hide where transactions come from.** Your peers see which transactions you announce, and when, as before, which can reveal that one started at your node. - **Hide that you use it.** Hybrid sessions can be told apart from plain v2 by @@ -301,13 +314,13 @@ details such as `role=`, `peer=` and the ML-KEM-1024 backends; counters are in | `was given 0 or a non-numeric value, so it was read as` | A double negative switched the feature on. | `-nov2pqtransport=0` or `nov2pqtransport=true`. | Use `v2pqtransport=0` to switch it off. | | `legacy_peer` (counter and status) | The peer sent no hybrid offer or accept (reason `no_features`), or contents qbit can't parse (`parse_error`). | The peer runs v1.0.0 or has the feature off. `parse_error`: another implementation or a bug. | Nothing for `no_features`; it is normal while the network upgrades. `parse_error` from many peers: open an issue. | | `v2 pq: load_shedding started threshold_per_s=…`, `v2 pq: load_shedding stopped`, counter `shed` | Load shedding started or stopped. | A connection flood, or many peers reconnecting at once. | Nothing; those connections continue as plain v2. If it stays on, check where inbound connections come from (`qbit-cli -netinfo 4`), and consider firewall rules or `-maxconnections`. | -| `v2 pq: malformed_record reason=…`, counter `malformed_record` | The peer's offer (`ek_length`, `ek_modulus`) or accept (`ct_length`) was malformed. The connection closes; outbound, it counts toward the fallback. | A non-conforming implementation, or tampering in transit. | One-off: nothing. From many peers: report it. | -| `v2 pq: first_packet_failed reason=…`, counter `first_packet_failed` | The peer's key confirmation failed (`length`, `tag` or `not_decoy`): the two sides derived different keys. The connection closes; outbound, it counts toward the fallback. | Tampering in transit, a peer bug, or this node's ML-KEM code (see `local_fault`). | One-off: nothing. From many peers: report it. | -| `v2 pq: closed_after_switch cause=…`, counter `closed_after_switch` | After this node switched to hybrid keys, the peer closed (`eof`), reset (`reset`) or timed out (`timeout`) before its key confirmation arrived. Counts toward the fallback. | The peer restarted, the network dropped, or the peer's check of our confirmation failed. | One-off: nothing. Often with one peer: ask its operator for their side's log. | +| `v2 pq: malformed_record reason=…`, counter `malformed_record` | The peer's offer (`ek_length`, `ek_modulus`) or accept (`ct_length`) was malformed. The connection closes; on an automatic outbound connection, it counts toward the fallback. | A non-conforming implementation, or tampering in transit. | One-off: nothing. From many peers: report it. | +| `v2 pq: first_packet_failed reason=…`, counter `first_packet_failed` | The peer's key confirmation failed (`length`, `tag` or `not_decoy`): the two sides derived different keys. The connection closes; on an automatic outbound connection, it counts toward the fallback. | Tampering in transit, a peer bug, or this node's ML-KEM code (see `local_fault`). | One-off: nothing. From many peers: report it. | +| `v2 pq: closed_after_switch cause=…`, counter `closed_after_switch` | After this node switched to hybrid keys, the peer closed (`eof`), reset (`reset`) or timed out (`timeout`) before its key confirmation arrived. On an automatic connection, it counts toward the fallback. | The peer restarted, the network dropped, or the peer's check of our confirmation failed. | One-off: nothing. Often with one peer: ask its operator for their side's log. | | `v2 pq: internal_error reason=…`, counter `internal_error` | A local ML-KEM or cipher-state error (`keygen`, `check_public_key`, `encaps`, `decaps`, `cipher_state`). Peer data never causes it. | A fault in this node's ML-KEM code or its CPU. | Report it. Try `-mlkemportable`. If a peer can trigger it, use `SECURITY.md`. | -| `v2 pq: fallback cause=… until=…`, counter `fallback` | An outbound address failed three times in a row, and gets plain v2 until the time given (UTC). | See the failure it names. | Usually nothing: the window ends by itself, and a restart clears it. Many addresses: see `local_fault`. | -| `v2 pq: local_fault` | Hybrid handshakes failed with 8 different outbound addresses and none succeeded since startup, so this node's own ML-KEM code may be at fault. | A miscompiled or misbehaving native ML-KEM path, or a broken CPU. | If the line names native code, restart with `-mlkemportable`, as it suggests. Report it either way. | -| `abandoned` (inbound) | An initiator read our offer and closed before sending its version packet. | Port scanners, peers giving up, flaky links. | Nothing at low rates. Many, together with `shed`: a flood. | +| `v2 pq: fallback cause=… until=…`, counter `fallback` | An address failed three times in a row on automatic outbound connections, and gets plain v2 on them until the time given (UTC). | See the failure it names. | Usually nothing: the window ends by itself, and a restart clears it. To keep hybrid keys with a peer, add it with `addnode`. Many addresses: see `local_fault`. | +| `v2 pq: local_fault` | Hybrid handshakes failed with 8 different outbound addresses, in ways this node's own ML-KEM code can cause (`ek_modulus`, `length`, `tag`, or `closed_after_switch` with `eof` or `reset`), and none succeeded since startup, so that code may be at fault. Failures only the peer can cause (`ek_length`, `not_decoy`) and `timeout` don't count. | A miscompiled or misbehaving native ML-KEM path, or a broken CPU. | If the line names native code, restart with `-mlkemportable`, as it suggests. Report it either way. | +| `abandoned` (inbound) | This node offered, and the connection closed before the initiator's version packet authenticated, whichever side closed it. The reason is the close cause, or what failed (`garbage_terminator`, `version_length`, `version_tag`); `local` is this node's own close, such as an eviction. Closes because the network was switched off, or at shutdown, aren't counted. | Port scanners, peers giving up, flaky links; with `local`, evictions when the inbound slots are full. | Nothing at low rates. Many, together with `shed`: a flood. | | `transport_pq_status` stays `pending` | The handshake is still running, or the connection is closing after a failure. | A slow link, or a failure counted above. | It ends within 60 seconds. Look for the peer's `peer_id` in `recent_failures`. | | `transport_pq_status` is `off` while `enabled` is true | This connection is plain v2. | The offer was shed, or a local fault (`internal_error`) before anything was committed. | Check `shed` and `internal_error`. | diff --git a/src/net.cpp b/src/net.cpp index 3cbe507887..70a8269345 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -527,9 +527,14 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo // Repeated hybrid failures to this endpoint put it in the fallback set: plain v2, without // parsing or validating the peer's offer. The next attempt, whenever the connection logic - // makes one, asks again. + // makes one, asks again. A manual connection (addnode, -connect) always negotiates, even + // to an endpoint an automatic connection put in the set: an attacker on the path can make + // handshakes fail without breaking a key, and pinning a peer keeps its hybrid keys. PQMode pq_mode{m_pq_mode}; - if (pq_mode == PQMode::NEGOTIATE && use_v2transport && IsPQFallback(pq_endpoint, Now())) pq_mode = PQMode::FALLBACK; + if (pq_mode == PQMode::NEGOTIATE && use_v2transport && conn_type != ConnectionType::MANUAL && + IsPQFallback(pq_endpoint, Now())) { + pq_mode = PQMode::FALLBACK; + } NetPermissionFlags permission_flags = NetPermissionFlags::None; std::vector whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector{}; @@ -2325,6 +2330,32 @@ std::string_view PQFailureReason(PQFailure failure) noexcept assert(false); } +/** Whether this node's own code can cause a negotiation failure: its key check rejecting the + * peer's key, keys that don't match (the confirmation's length or tag), or an internal error. A + * record of the wrong length, and a confirmation that verified under the hybrid keys but isn't a + * decoy, are the peer's alone. */ +bool PQFailureMayBeLocal(PQFailure failure) noexcept +{ + switch (failure) { + case PQFailure::EK_MODULUS: + case PQFailure::CONFIRM_LENGTH: + case PQFailure::CONFIRM_TAG: + case PQFailure::KEYGEN_INTERNAL: + case PQFailure::CHECK_EK_INTERNAL: + case PQFailure::ENCAPS_INTERNAL: + case PQFailure::DECAPS_INTERNAL: + case PQFailure::CIPHER_STATE_INTERNAL: + return true; + case PQFailure::EK_LENGTH: + case PQFailure::CT_LENGTH: + case PQFailure::CONFIRM_NOT_DECOY: + return false; + case PQFailure::NONE: + break; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + /** The reason word of a close cause. A node without one is finalized as LOCAL. */ std::string_view PQCloseCauseReason(NodeCloseCause cause) noexcept { @@ -2374,12 +2405,20 @@ std::chrono::seconds PQFallbackWindow(const PQEndpointHistory& history) noexcept return PQ_FALLBACK_WINDOWS[std::min(history.completed_windows, PQ_FALLBACK_WINDOWS.size() - 1)]; } +/** Whether an endpoint's fallback window is still running: it ends at expires, or once its length + * has passed on the steady clock, whichever comes first, so a wall clock step back can't stretch it. */ +bool PQFallbackActive(const PQEndpointHistory& history, NodeSeconds now, MockableSteadyClock::time_point steady_now) noexcept +{ + return history.expires && now < *history.expires && steady_now < *history.steady_expires; +} + /** End an expired fallback window: the streak restarts at 0, and the escalation level stays. */ -void ExpirePQFallback(PQEndpointHistory& history, NodeSeconds now) noexcept +void ExpirePQFallback(PQEndpointHistory& history, NodeSeconds now, MockableSteadyClock::time_point steady_now) noexcept { - if (!history.expires || now < *history.expires) return; + if (!history.expires || PQFallbackActive(history, now, steady_now)) return; history.entered.reset(); history.expires.reset(); + history.steady_expires.reset(); history.streak = 0; history.completed_windows = static_cast(std::min(history.completed_windows + 1, PQ_FALLBACK_WINDOWS.size())); } @@ -2400,7 +2439,7 @@ PQCounts LoadPQCounts(const PQCounters& counters) noexcept } } // namespace -void CConnman::ObservePQ(CNode& node, bool finalizing, bool record_outcomes) +void CConnman::ObservePQ(CNode& node, bool finalizing, bool attribute_close) { AssertLockNotHeld(m_pq_mutex); // The transport takes and releases its own lock. @@ -2408,12 +2447,11 @@ void CConnman::ObservePQ(CNode& node, bool finalizing, bool record_outcomes) // Nothing to account yet, as on every connection without the negotiation. if (!finalizing && !snapshot.switched && snapshot.legacy == PQLegacyReason::NONE && snapshot.failure == PQFailure::NONE) return; const NodeCloseCause cause{node.GetCloseCause()}; - const bool record{record_outcomes && fNetworkActive}; PQLogLines lines; { LOCK(m_pq_mutex); if (node.m_pq_finalized) return; - if (record) lines = AccountPQ(node, snapshot, cause, finalizing, Now()); + lines = AccountPQ(node, snapshot, cause, /*account_close=*/finalizing && attribute_close, Now()); if (finalizing) node.m_pq_finalized = true; } if (finalizing) node.m_transport->ClearPQSecrets(); @@ -2422,22 +2460,29 @@ void CConnman::ObservePQ(CNode& node, bool finalizing, bool record_outcomes) } CConnman::PQLogLines CConnman::AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, - bool finalizing, NodeSeconds now) + bool account_close, NodeSeconds now) { AssertLockHeld(m_pq_mutex); const bool inbound{node.IsInboundConn()}; PQLogLines lines; // Each event counts once per connection, however often it is observed. - const auto account{[&](PQOutcome outcome, std::string_view reason) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex) { + const auto account{[&](PQOutcome outcome, std::string_view reason, bool may_be_local = false) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex) { const uint16_t bit{static_cast(1U << static_cast(outcome))}; if (node.m_pq_accounted_events & bit) return; node.m_pq_accounted_events |= bit; if (std::atomic* counter{PQCounter(m_pq_counters[PQDirection(inbound)], outcome)}) ++*counter; if (outcome == PQOutcome::CONFIRMED) { - // A confirmed hybrid handshake protects an outbound endpoint until restart: no - // failure, however late, can count against it again. + // A confirmed hybrid handshake protects an outbound endpoint while it stays in the + // success set: no failure, however late, can count against it. if (!inbound) { - m_pq_successful_endpoints.insert(node.m_pq_endpoint); + m_pq_any_success = true; + // A new endpoint in a full set evicts the least recently confirmed one. + if (!m_pq_successful_endpoints.contains(node.m_pq_endpoint) && + m_pq_successful_endpoints.size() >= PQ_MAX_SUCCESSFUL_ENDPOINTS) { + m_pq_successful_endpoints.erase( + std::ranges::min_element(m_pq_successful_endpoints, {}, [](const auto& entry) { return entry.second; })); + } + m_pq_successful_endpoints.insert_or_assign(node.m_pq_endpoint, m_pq_success_confirmations++); m_pq_history.erase(node.m_pq_endpoint); } return; @@ -2456,7 +2501,7 @@ CConnman::PQLogLines CConnman::AccountPQ(CNode& node, const PQHandshake::Snapsho // Only failures the peer side caused count toward the fallback, and only outbound. if (!inbound && (outcome == PQOutcome::MALFORMED_RECORD || outcome == PQOutcome::FIRST_PACKET_FAILED || outcome == PQOutcome::CLOSED_AFTER_SWITCH)) { - if (const auto until{CountPQFailure(node, outcome, reason, now, lines.local_fault)}) { + if (const auto until{CountPQFailure(node, outcome, reason, may_be_local, now, lines.local_fault)}) { lines.outcomes.push_back({PQOutcome::FALLBACK, PQOutcomeString(outcome), until}); } } @@ -2468,13 +2513,17 @@ CConnman::PQLogLines CConnman::AccountPQ(CNode& node, const PQHandshake::Snapsho if (snapshot.legacy != PQLegacyReason::NONE) { account(PQOutcome::LEGACY_PEER, snapshot.legacy == PQLegacyReason::PARSE_ERROR ? "parse_error" : "no_features"); } - if (snapshot.failure != PQFailure::NONE) account(PQFailureOutcome(snapshot.failure), PQFailureReason(snapshot.failure)); - if (finalizing && snapshot.failure == PQFailure::NONE) { + if (snapshot.failure != PQFailure::NONE) { + account(PQFailureOutcome(snapshot.failure), PQFailureReason(snapshot.failure), PQFailureMayBeLocal(snapshot.failure)); + } + if (account_close && snapshot.failure == PQFailure::NONE) { // We switched, and the peer closed or stopped responding before its confirmation - // verified. Our own closes and send errors are not the peer's doing. + // verified. Our own closes and send errors are not the peer's doing. A peer that closes + // may have failed its check of our confirmation, as keys our own ML-KEM code got wrong + // would make it; one that stops responding has no such reason. if (!inbound && snapshot.switched && !snapshot.confirmed && (cause == NodeCloseCause::PEER_EOF || cause == NodeCloseCause::PEER_RESET || cause == NodeCloseCause::TIMEOUT)) { - account(PQOutcome::CLOSED_AFTER_SWITCH, PQCloseCauseReason(cause)); + account(PQOutcome::CLOSED_AFTER_SWITCH, PQCloseCauseReason(cause), /*may_be_local=*/cause != NodeCloseCause::TIMEOUT); } // Our offer was queued, but no authenticated version arrived before the close. An offer // that failed internally is internal_error only. @@ -2485,36 +2534,46 @@ CConnman::PQLogLines CConnman::AccountPQ(CNode& node, const PQHandshake::Snapsho return lines; } -std::optional CConnman::CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, NodeSeconds now, - bool& local_fault) +std::optional CConnman::CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, bool may_be_local, + NodeSeconds now, bool& local_fault) { AssertLockHeld(m_pq_mutex); const PQEndpointKey& endpoint{node.m_pq_endpoint}; if (m_pq_successful_endpoints.contains(endpoint)) return std::nullopt; - // Many endpoints failing, and none succeeding, points at this node's own ML-KEM code. - if (m_pq_failed_endpoints_for_warning.size() < PQ_LOCAL_FAULT_THRESHOLD) m_pq_failed_endpoints_for_warning.insert(endpoint); - if (!m_pq_local_fault_warned && m_pq_failed_endpoints_for_warning.size() >= PQ_LOCAL_FAULT_THRESHOLD && - m_pq_successful_endpoints.empty()) { - m_pq_local_fault_warned = true; - local_fault = true; + // Many endpoints failing in a way this node's own ML-KEM code can cause, and none succeeding, + // points at that code. Manual connections count here too: a node with only -connect peers + // still gets the warning. + if (may_be_local) { + if (m_pq_failed_endpoints_for_warning.size() < PQ_LOCAL_FAULT_THRESHOLD) m_pq_failed_endpoints_for_warning.insert(endpoint); + if (!m_pq_local_fault_warned && m_pq_failed_endpoints_for_warning.size() >= PQ_LOCAL_FAULT_THRESHOLD && !m_pq_any_success) { + m_pq_local_fault_warned = true; + local_fault = true; + } } + // An attacker on the path can make an endpoint's handshakes fail without breaking a key, by + // damaging the peer's confirmation, resetting the connection or dropping its traffic. A + // manual connection's failures never count toward the fallback, so a pinned peer keeps its + // hybrid keys; they are still counted, kept in the ring and logged. + if (node.m_conn_type == ConnectionType::MANUAL) return std::nullopt; + + const MockableSteadyClock::time_point steady_now{MockableSteadyClock::now()}; auto it{m_pq_history.find(endpoint)}; if (it == m_pq_history.end()) { if (m_pq_history.size() >= PQ_MAX_ENDPOINT_HISTORY) { // Spare active fallback windows: evict the oldest insertion outside one, and only when // every endpoint is in one, the window that ends first. - m_pq_history.erase(std::ranges::min_element(m_pq_history, {}, [now](const auto& entry) { + m_pq_history.erase(std::ranges::min_element(m_pq_history, {}, [now, steady_now](const auto& entry) { const PQEndpointHistory& history{entry.second}; - const bool active{history.expires && now < *history.expires}; + const bool active{PQFallbackActive(history, now, steady_now)}; return std::tuple{active, active ? *history.expires : NodeSeconds{}, history.insertion_order}; })); } it = m_pq_history.emplace(endpoint, PQEndpointHistory{.insertion_order = m_pq_history_insertions++}).first; } PQEndpointHistory& history{it->second}; - ExpirePQFallback(history, now); + ExpirePQFallback(history, now, steady_now); // A connection that was already running when the window began can't extend it. if (history.expires) return std::nullopt; ++history.streak; @@ -2523,8 +2582,10 @@ std::optional CConnman::CountPQFailure(const CNode& node, PQOutcome history.reason = reason; if (history.streak < PQ_FAILURE_THRESHOLD) return std::nullopt; + const std::chrono::seconds window{PQFallbackWindow(history)}; history.entered = now; - history.expires = now + PQFallbackWindow(history); + history.expires = now + window; + history.steady_expires = steady_now + window; ++m_pq_counters[PQDirection(/*inbound=*/false)].fallback; AddPQFailure({.sequence = 0, .time = now, @@ -2543,7 +2604,7 @@ bool CConnman::IsPQFallback(const PQEndpointKey& endpoint, NodeSeconds now) LOCK(m_pq_mutex); const auto it{m_pq_history.find(endpoint)}; if (it == m_pq_history.end()) return false; - ExpirePQFallback(it->second, now); + ExpirePQFallback(it->second, now, MockableSteadyClock::now()); return it->second.expires.has_value(); } @@ -2659,10 +2720,11 @@ PQTransportStats CConnman::GetPQTransportStats() const .load_shedding = load_shedding, }; const NodeSeconds now{Now()}; + const MockableSteadyClock::time_point steady_now{MockableSteadyClock::now()}; for (const auto& [endpoint, history] : m_pq_history) { if (history.expires) { // A window that expired ended the streak; nothing to report until the next failure. - if (now < *history.expires) { + if (PQFallbackActive(history, now, steady_now)) { stats.fallback_set.push_back({endpoint, history.cause, history.reason, history.streak, *history.entered, *history.expires}); } } else if (history.streak > 0) { @@ -2705,6 +2767,14 @@ bool CConnman::AllowPQOffer() noexcept bool CConnman::AdvancePQShedding(PQShedSeconds now) { AssertLockHeld(m_pq_shed_mutex); + if (now < m_pq_shed_second) { + // Only a test's mocked steady clock steps back (setmocktime 0 after a later time, with + // -test=mock_steady_clock). Start the current second, and the run of quiet seconds, over + // from now: otherwise the current second would never close, and shedding never end. + m_pq_shed_second = now; + m_pq_shed_attempts = 0; + m_pq_quiet_since.reset(); + } if (now > m_pq_shed_second) { // Close the current second, and the seconds without attempts since: a second is quiet // with fewer than half the threshold's attempts. @@ -2792,8 +2862,9 @@ void CConnman::DisconnectNodes() pnode->CloseSocketDisconnect(); // account the hybrid negotiation's close outcome and wipe its pending secrets - // before the disconnected pool keeps the node - ObservePQ(*pnode, /*finalizing=*/true); + // before the disconnected pool keeps the node; with the network off, as this pass + // saw it when it closed every connection, the close is ours + ObservePQ(*pnode, /*finalizing=*/true, /*attribute_close=*/network_active); // update connection count by network if (pnode->IsManualOrFullOutboundConn()) --m_network_conn_counts[pnode->addr.GetNetwork()]; @@ -4335,8 +4406,8 @@ void CConnman::StopNodes() LogDebug(BCLog::NET, "Stopping node, %s", pnode->DisconnectMsg(fLogIPs)); pnode->CloseSocketDisconnect(); // Wipe the hybrid negotiation's pending secrets before the node goes, as DisconnectNodes() - // does. A shutdown close is our own decision, so it records no outcome. - ObservePQ(*pnode, /*finalizing=*/true, /*record_outcomes=*/false); + // does. A shutdown close is our own decision, so no close outcome is derived from it. + ObservePQ(*pnode, /*finalizing=*/true, /*attribute_close=*/false); DeleteNode(pnode); } @@ -4455,6 +4526,12 @@ bool CConnman::AddedNodesContain(const CAddress& addr) const [&](const auto& p) { return p.m_added_node == addr_str || p.m_added_node == addr_port_str; })); } +size_t CConnman::GetPQNodeCount() const +{ + LOCK(m_nodes_mutex); + return std::ranges::count_if(m_nodes, [](const CNode* node) { return node->m_transport->GetInfo().transport_pq; }); +} + size_t CConnman::GetNodeCount(ConnectionDirection flags) const { LOCK(m_nodes_mutex); diff --git a/src/net.h b/src/net.h index 9840af6b44..993a7af4fb 100644 --- a/src/net.h +++ b/src/net.h @@ -887,8 +887,8 @@ struct PQFailureRing { /** At most this many entries in each failure ring. */ inline constexpr size_t PQ_FAILURE_RING_SIZE{256}; -/** Consecutive counted failures (outbound malformed_record, first_packet_failed and - * closed_after_switch) that put an endpoint in the fallback set. */ +/** Consecutive counted failures (malformed_record, first_packet_failed and closed_after_switch of + * automatic outbound connections) that put an endpoint in the fallback set. */ inline constexpr uint32_t PQ_FAILURE_THRESHOLD{3}; /** An endpoint's fallback windows: its first entry, its second, then every later one. */ inline constexpr std::array PQ_FALLBACK_WINDOWS{ @@ -896,7 +896,10 @@ inline constexpr std::array PQ_FALLBACK_WINDOWS{ /** At most this many endpoints in the fallback, streak and escalation history. Eviction spares * endpoints in an active fallback window while any other is left. */ inline constexpr size_t PQ_MAX_ENDPOINT_HISTORY{1000}; -/** Distinct endpoints with counted failures, and no success since startup, that suggest a local fault. */ +/** At most this many endpoints in the success set; the least recently confirmed is evicted first. */ +inline constexpr size_t PQ_MAX_SUCCESSFUL_ENDPOINTS{1000}; +/** Distinct endpoints with failures this node's own ML-KEM code can cause, and no success since + * startup, that suggest a local fault. */ inline constexpr size_t PQ_LOCAL_FAULT_THRESHOLD{8}; /** Inbound offers attempted within one second above which responders stop offering. R3's flood @@ -921,9 +924,11 @@ struct PQEndpointHistory { uint32_t streak{0}; //! Fallback windows that ended; saturates at PQ_FALLBACK_WINDOWS.size(). uint8_t completed_windows{0}; - //! The active fallback window, if any. + //! The active fallback window, if any. It ends at expires, or once its length has passed on the + //! steady clock (steady_expires), whichever comes first: a wall clock step back can't stretch it. std::optional entered{}; std::optional expires{}; + std::optional steady_expires{}; //! The latest counted failure. NodeSeconds last_failure{}; PQOutcome cause{PQOutcome::MALFORMED_RECORD}; @@ -1601,6 +1606,8 @@ class CConnman bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !m_pq_mutex); size_t GetNodeCount(ConnectionDirection) const; + /** The number of connections with hybrid session keys (transport_pq). */ + size_t GetPQNodeCount() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); std::map getNetLocalAddresses() const; uint32_t GetMappedAS(const CNetAddr& addr) const; void GetNodeStats(std::vector& vstats) const; @@ -1653,8 +1660,8 @@ class CConnman /** The hybrid post-quantum v2 transport configuration, as Init() received it. */ const PQTransportConfig& GetPQTransportConfig() const noexcept { return m_pq_config; } - /** Whether new outbound connections to endpoint run plain v2, because repeated hybrid - * failures put it in the fallback set. Ends a window that expired. */ + /** Whether new automatic outbound connections to endpoint run plain v2, because repeated + * hybrid failures put it in the fallback set. Ends a window that expired. */ bool IsPQFallback(const PQEndpointKey& endpoint, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); /** Get a unique deterministic randomizer. */ @@ -2088,30 +2095,35 @@ class CConnman * runs after the node received bytes, and once more, finalizing, before DisconnectNodes() * hands the node to the disconnected pool: that derives closed_after_switch and abandoned * from the close cause, marks the node finalized so no later observation counts anything, - * and wipes the transport's pending secrets. With the network off, every close is our own - * decision, so nothing is recorded, but finalization still wipes. StopNodes() finalizes the - * connections it deletes at shutdown the same way, with record_outcomes false: shutdown - * closes are our own too. + * and wipes the transport's pending secrets. attribute_close is false when this node closed + * the connection along with every other one: DisconnectNodes() with the network off, as the + * pass that closed them saw it, and StopNodes() at shutdown. Such a close is our own decision, + * so neither outcome is derived from it. The events the peer's bytes caused, a verified + * confirmation included, are accounted either way. * * The transport is snapshotted under its own lock, which is released before m_pq_mutex is * taken; nothing calls into the transport, socket, peer manager or node enumeration while * holding m_pq_mutex. */ - void ObservePQ(CNode& node, bool finalizing, bool record_outcomes = true) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); + void ObservePQ(CNode& node, bool finalizing, bool attribute_close = true) EXCLUSIVE_LOCKS_REQUIRED(!m_pq_mutex); /** Count each event of snapshot not yet accounted for node, once, add a ring entry for each - * non-success outcome, and update the endpoint history. Returns what to log. */ + * non-success outcome, and update the endpoint history. With account_close, also derive + * closed_after_switch and abandoned from the close cause. Returns what to log. */ PQLogLines AccountPQ(CNode& node, const PQHandshake::Snapshot& snapshot, NodeCloseCause cause, - bool finalizing, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + bool account_close, NodeSeconds now) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); /** - * Count an outbound connection's failure against its endpoint. The third consecutive one - * enters the fallback set, unless the endpoint completed a hybrid handshake since startup. - * A failure during an active window neither extends it nor enters again. Returns the end of - * a window it started, and sets local_fault when the local-fault warning is due. + * Count an outbound connection's failure against its endpoint, unless the endpoint is in the + * success set. A failure this node's own ML-KEM code can cause (may_be_local) counts toward + * the local-fault warning. A manual connection's failure never counts toward the fallback: + * an attacker on the path can make any handshake fail without breaking a key. Otherwise the + * third consecutive one enters the fallback set. A failure during an active window neither + * extends it nor enters again. Returns the end of a window it started, and sets local_fault + * when the local-fault warning is due. */ - std::optional CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, NodeSeconds now, - bool& local_fault) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); + std::optional CountPQFailure(const CNode& node, PQOutcome outcome, std::string_view reason, bool may_be_local, + NodeSeconds now, bool& local_fault) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); /** Add a non-success outcome to the ring of its direction. */ void AddPQFailure(PQFailureEntry entry) EXCLUSIVE_LOCKS_REQUIRED(m_pq_mutex); @@ -2140,10 +2152,13 @@ class CConnman //! Outbound endpoints with counted failures: streaks, fallback windows and escalation. std::map m_pq_history GUARDED_BY(m_pq_mutex); uint64_t m_pq_history_insertions GUARDED_BY(m_pq_mutex){0}; - //! Outbound endpoints that completed a hybrid handshake since startup. Never evicted; its - //! size is bounded by the distinct successful outbound endpoints. - std::set m_pq_successful_endpoints GUARDED_BY(m_pq_mutex); - //! Distinct endpoints with counted failures, up to PQ_LOCAL_FAULT_THRESHOLD. + //! Outbound endpoints that completed a hybrid handshake, each with the order of its latest + //! confirmation: at most PQ_MAX_SUCCESSFUL_ENDPOINTS, the least recently confirmed evicted first. + std::map m_pq_successful_endpoints GUARDED_BY(m_pq_mutex); + uint64_t m_pq_success_confirmations GUARDED_BY(m_pq_mutex){0}; + //! Whether an outbound hybrid handshake completed since startup; eviction from the set can't clear it. + bool m_pq_any_success GUARDED_BY(m_pq_mutex){false}; + //! Distinct endpoints with failures this node's own ML-KEM code can cause, up to PQ_LOCAL_FAULT_THRESHOLD. std::set m_pq_failed_endpoints_for_warning GUARDED_BY(m_pq_mutex); bool m_pq_local_fault_warned GUARDED_BY(m_pq_mutex){false}; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 5d3fb0eb4a..c62e7ebb2a 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -134,7 +134,7 @@ struct ListedPeer { CNodeStateStats statestats; }; -/** The peers getpeerinfo lists; getnetworkinfo counts over the same ones. */ +/** The peers getpeerinfo lists. */ static std::vector GetListedPeers(const CConnman& connman, const PeerManager& peerman) { std::vector vstats; @@ -863,13 +863,10 @@ static RPCHelpMan getnetworkinfo() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { NodeContext& node = EnsureAnyNodeContext(request.context); - // Counted before cs_main is taken, as getpeerinfo lists the peers. - int connections_pq{0}; - if (node.connman && node.peerman) { - for (const ListedPeer& peer : GetListedPeers(*node.connman, *node.peerman)) { - if (peer.stats.m_transport_pq) ++connections_pq; - } - } + // Counted before cs_main is taken, from the connection manager's own node list: every + // connection in it is one getpeerinfo lists, as the peer manager knows a node before the list + // has it, and until after the list dropped it. + const size_t connections_pq{node.connman ? node.connman->GetPQNodeCount() : 0}; LOCK(cs_main); UniValue obj(UniValue::VOBJ); obj.pushKV("version", CLIENT_VERSION); @@ -1520,9 +1517,14 @@ static RPCHelpMan getpqtransportinfo() "\n" "Counters count events, not exclusive outcomes: switched counts key installations, even if the key\n" "confirmation then fails; fallback counts entries into the fallback set, not plain v2 connections;\n" - "abandoned counts offers whose initiator sent no authenticated version packet before the connection\n" - "closed. Counters, rings and endpoint history are kept in memory only, and start over when\n" - "instance_id changes.\n" + "abandoned counts offers after which no authenticated version packet arrived before the connection\n" + "closed, whichever side closed it: reason local is this node's own close, such as an eviction (a\n" + "close because the network was switched off, or at shutdown, counts nothing). Counters, rings and\n" + "endpoint history are kept in memory only, and start over when instance_id changes.\n" + "\n" + "Only automatic outbound connections fall back. Manual ones (addnode, -connect) always negotiate,\n" + "and their failures never count toward a streak, as an attacker on the path can cause them without\n" + "breaking a key. Their outcomes are counted, kept and logged all the same.\n" "\n" "A connection that closes on a hybrid failure reports transport_pq_status \"pending\" in getpeerinfo\n" "until it is removed; its outcome is counted here.\n" @@ -1556,7 +1558,7 @@ static RPCHelpMan getpqtransportinfo() {RPCResult::Type::NUM, "threshold_per_s", "The threshold, in offers attempted per second"}, {RPCResult::Type::NUM_TIME, "since", "The " + UNIX_EPOCH_TIME + " the current shedding period started, or 0 when inactive"}, }}, - {RPCResult::Type::ARR, "fallback_set", "Endpoints that outbound connections reach with plain v2, after repeated hybrid failures", + {RPCResult::Type::ARR, "fallback_set", "Endpoints that automatic outbound connections reach with plain v2, after repeated hybrid failures", { {RPCResult::Type::OBJ, "", "", { @@ -1565,11 +1567,11 @@ static RPCHelpMan getpqtransportinfo() {RPCResult::Type::STR, "reason", "Its reason"}, {RPCResult::Type::NUM, "streak", "Consecutive counted failures"}, {RPCResult::Type::NUM_TIME, "entered", "The " + UNIX_EPOCH_TIME + " the endpoint entered the set"}, - {RPCResult::Type::NUM_TIME, "expires", "The " + UNIX_EPOCH_TIME + " the window ends"}, + {RPCResult::Type::NUM_TIME, "expires", "The " + UNIX_EPOCH_TIME + " the window ends, at the latest: it also ends once window_seconds have passed on the monotonic clock, so a clock step back can't lengthen it"}, {RPCResult::Type::NUM, "window_seconds", "The window: " + windows}, }}, }}, - {RPCResult::Type::ARR, "failure_streaks", "Outbound endpoints with counted failures, not in the fallback set", + {RPCResult::Type::ARR, "failure_streaks", "Endpoints of automatic outbound connections with counted failures, not in the fallback set", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 4ca52fdaae..1f3bad5228 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -413,6 +413,8 @@ FUZZ_TARGET(rpc, FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; bool good_data{true}; SetMockTime(ConsumeTime(fuzzed_data_provider)); + // getpqtransportinfo reads the steady clock, which bounds fallback windows. + MockableSteadyClock::SetMockTime(MockableSteadyClock::INITIAL_MOCK_TIME); const std::string rpc_command = g_limit_to_rpc_command.empty() ? fuzzed_data_provider.ConsumeRandomLengthString(64) : g_limit_to_rpc_command; const bool safe_for_fuzzing = std::find(RPC_COMMANDS_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_SAFE_FOR_FUZZING.end(); if (!safe_for_fuzzing) { diff --git a/src/test/net_pq_tests.cpp b/src/test/net_pq_tests.cpp index b7df2d0fe9..c766a7d1be 100644 --- a/src/test/net_pq_tests.cpp +++ b/src/test/net_pq_tests.cpp @@ -151,13 +151,13 @@ class RawPeer } }; -/** A record CompactSize(1 + payload size) || header || payload. */ -std::vector MakeRecord(uint8_t header, size_t payload_size) +/** A record CompactSize(1 + payload size) || header || payload, every payload byte fill. */ +std::vector MakeRecord(uint8_t header, size_t payload_size, std::byte fill = std::byte{0x42}) { DataStream stream; WriteCompactSize(stream, 1 + payload_size); stream << header; - stream.write(std::vector(payload_size, std::byte{0x42})); + stream.write(std::vector(payload_size, fill)); return {stream.begin(), stream.end()}; } @@ -380,6 +380,20 @@ struct PQNetSetup : public RegTestingSetup { /** Queue what peer has to send for the node. */ static void PeerToNode(const Link& link, Transport& peer) { Receive(link, PeerBytes(peer)); } + /** Hand to what from has to send, without a socket; to must accept all of it. */ + static void Deliver(Transport& from, Transport& to) + { + const std::vector bytes{PeerBytes(from)}; + std::span remaining{bytes}; + while (!remaining.empty()) { + BOOST_REQUIRE(to.ReceivedBytes(remaining)); + if (to.ReceivedMessageComplete()) { + bool reject{false}; + to.GetReceivedMessage({}, reject); + } + } + } + /** Feed peer what the node sent; false if peer failed. */ static bool NodeToPeer(const Link& link, Transport& peer) { @@ -445,19 +459,22 @@ struct PQNetSetup : public RegTestingSetup { } /** How a test connection ends. */ - enum class End { MALFORMED, CONFIRMATION, PEER_EOF, PEER_RESET, TIMEOUT, LOCAL, SEND_ERROR, LEGACY, SUCCESS }; + enum class End { MALFORMED, BAD_MODULUS, CONFIRMATION, PEER_EOF, PEER_RESET, TIMEOUT, LOCAL, SEND_ERROR, LEGACY, SUCCESS }; - /** One outbound connection to endpoint that ends as end; the connection manager finalizes it. */ - void Connect(const PQEndpointKey& endpoint, End end) + /** One outbound connection of conn_type to endpoint that ends as end; the connection manager + * finalizes it. */ + void Connect(const PQEndpointKey& endpoint, End end, ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY) { - Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + Link link{Add(conn_type, PQMode::NEGOTIATE, endpoint)}; switch (end) { - case End::MALFORMED: { // an offer of the wrong length + case End::MALFORMED: // an offer of the wrong length + case End::BAD_MODULUS: { // an offer whose key fails the modulus check: every coefficient is 4095 Pass(link); RawPeer peer{/*initiator=*/false}; peer.Initialize(NodeKey(link)); Receive(link, peer.Key()); - Receive(link, peer.TerminatorAndVersion(MakeRecord(PQ_MLKEM1024, 100))); + Receive(link, peer.TerminatorAndVersion(end == End::MALFORMED ? MakeRecord(PQ_MLKEM1024, 100) : + MakeRecord(PQ_MLKEM1024, mlkem::PUBLIC_KEY_BYTES, std::byte{0xff}))); Pass(link); break; } @@ -1273,7 +1290,7 @@ BOOST_AUTO_TEST_CASE(pq_shutdown_finalizes) { // Shutdown deletes the connections it still holds itself, without DisconnectNodes(). It // finalizes them first: no node is deleted holding hybrid secrets, but a shutdown close is - // our own decision, so nothing is recorded, as with the network off. + // our own decision, so no close outcome is recorded, as with the network off. Link inbound{Add(ConnectionType::INBOUND)}; // its offer is pending: decapsulation key and ECDH secret Receive(inbound, InitiatorKey()); Pass(inbound); @@ -1544,6 +1561,182 @@ BOOST_AUTO_TEST_CASE(pq_history_eviction_all_in_fallback) BOOST_CHECK(Streak(TestEndpoint(9, int{PQ_MAX_ENDPOINT_HISTORY}))); } +BOOST_AUTO_TEST_CASE(pq_manual_never_falls_back) +{ + // An attacker on the path can make hybrid handshakes fail without breaking a key: damage the + // peer's confirmation in transit, reset the connection or drop its traffic. A manual + // connection's failures never count toward the fallback, but are counted, kept in the ring + // and logged like any other. + const PQEndpointKey endpoint{TestEndpoint(12, 0)}; + const std::vector failures{End::CONFIRMATION, End::PEER_RESET, End::TIMEOUT, End::PEER_EOF, End::MALFORMED}; + { + const std::string text{strprintf("v2 pq: closed_after_switch cause=reset role=initiator conn_type=manual peer=%d%s", + m_next_id + 1, Backends())}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::INFO)}; + for (const End end : failures) Connect(endpoint, end, ConnectionType::MANUAL); + } + BOOST_CHECK(!Streak(endpoint)); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.first_packet_failed, 1U); + BOOST_CHECK_EQUAL(stats.outbound.closed_after_switch, 3U); + BOOST_CHECK_EQUAL(stats.outbound.malformed_record, 1U); + BOOST_CHECK_EQUAL(stats.outbound.fallback, 0U); + BOOST_REQUIRE_EQUAL(stats.outbound_failures.entries.size(), failures.size()); + for (const PQFailureEntry& entry : stats.outbound_failures.entries) { + BOOST_CHECK(entry.connection_type == ConnectionType::MANUAL); + BOOST_CHECK(entry.endpoint == endpoint); + } + // They still count toward the local-fault warning, so a node with only -connect peers gets it. + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), 1U); + + // The same failures on automatic connections put the endpoint in the fallback set. + for (uint32_t i{0}; i < PQ_FAILURE_THRESHOLD; ++i) Connect(endpoint, End::PEER_RESET); + BOOST_REQUIRE(Fallback(endpoint)); + // New automatic connections to it run plain v2, but a new manual one negotiates, and completes + // a hybrid handshake. + const auto create_sock{MockCreateSock()}; + const auto connect{[&](ConnectionType conn_type) { + std::unique_ptr node{m_connman.ConnectNodeOnly("10.12.0.1:8333", conn_type, /*use_v2transport=*/true)}; + BOOST_REQUIRE(node); + BOOST_REQUIRE(node->m_pq_endpoint == endpoint); + return node; + }}; + for (const ConnectionType conn_type : {ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::BLOCK_RELAY, ConnectionType::FEELER}) { + BOOST_CHECK(connect(conn_type)->m_transport->GetInfo().transport_pq_status == PQStatus::FALLBACK); + } + const std::unique_ptr manual{connect(ConnectionType::MANUAL)}; + BOOST_CHECK(manual->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); + V2Transport peer{Responder()}; + for (int round{0}; round < 4; ++round) { + Deliver(*manual->m_transport, peer); + Deliver(peer, *manual->m_transport); + } + BOOST_CHECK(manual->m_transport->GetInfo().transport_pq); + BOOST_CHECK(manual->m_transport->GetInfo().transport_pq_status == PQStatus::HYBRID); + // Its confirmation shows the endpoint works: the window ends for every connection. + m_connman.ObservePQPublic(*manual, /*finalizing=*/false); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(connect(ConnectionType::OUTBOUND_FULL_RELAY)->m_transport->GetInfo().transport_pq_status == PQStatus::PENDING); +} + +BOOST_AUTO_TEST_CASE(pq_success_set_bound) +{ + // An attacker who advertises many endpoints can get feelers to confirm hybrid handshakes with + // each. The success set keeps the PQ_MAX_SUCCESSFUL_ENDPOINTS most recently confirmed ones, + // and evicts the least recently confirmed first. + const int cap{int{PQ_MAX_SUCCESSFUL_ENDPOINTS}}; + for (int i{0}; i < cap; ++i) Connect(TestEndpoint(13, i), End::SUCCESS); + // Confirming again refreshes an endpoint, so the second one is now the least recent. + Connect(TestEndpoint(13, 0), End::SUCCESS); + Connect(TestEndpoint(13, cap), End::SUCCESS); + // The evicted endpoint's failures count again; every other stays protected. + Connect(TestEndpoint(13, 1), End::MALFORMED); + BOOST_CHECK(Streak(TestEndpoint(13, 1))); + for (const int i : {0, 2, 3, cap - 1, cap}) { + Connect(TestEndpoint(13, i), End::MALFORMED); + BOOST_CHECK(!Streak(TestEndpoint(13, i))); + } + // Each new endpoint evicts exactly one, the next least recent: the set stays at its bound. + Connect(TestEndpoint(13, cap + 1), End::SUCCESS); + Connect(TestEndpoint(13, 2), End::MALFORMED); + BOOST_CHECK(Streak(TestEndpoint(13, 2))); + Connect(TestEndpoint(13, 3), End::MALFORMED); + BOOST_CHECK(!Streak(TestEndpoint(13, 3))); + + // Eviction never forgets that a handshake succeeded since startup: eight endpoints failing in + // a way a local fault could cause don't warn. + LogLineCounter warnings{"v2 pq: local_fault"}; + for (int i{0}; i < 8; ++i) Connect(TestEndpoint(14, i), End::CONFIRMATION); + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), PQ_LOCAL_FAULT_THRESHOLD); + BOOST_CHECK_EQUAL(warnings.m_count, 0); +} + +BOOST_AUTO_TEST_CASE(pq_fallback_window_clock_step) +{ + // A wall clock step back (an NTP correction or a VM restore) can't stretch a fallback window: + // it also ends once its length has passed on the steady clock. The stats keep the wall clock + // times it entered with. + const PQEndpointKey endpoint{TestEndpoint(15, 0)}; + const NodeSeconds start{Now()}; + const auto at{[&](std::chrono::seconds wall, std::chrono::seconds steady) { + SetMockTime(TicksSinceEpoch(start + wall)); + MockableSteadyClock::SetMockTime(STEADY_START + steady); + }}; + for (uint32_t i{0}; i < PQ_FAILURE_THRESHOLD; ++i) Connect(endpoint, End::MALFORMED); + BOOST_REQUIRE(Fallback(endpoint)); + + // Half an hour in, the wall clock steps back two hours. + at(-90min, 30min); + BOOST_CHECK(m_connman.IsPQFallback(endpoint, Now())); + BOOST_REQUIRE(Fallback(endpoint)); + BOOST_CHECK(Fallback(endpoint)->entered == start); + BOOST_CHECK(Fallback(endpoint)->expires == start + 1h); + // An hour after it began on the steady clock, it ends, though the wall clock is still two + // hours from expires. + at(-60min - 1s, 1h - 1s); + BOOST_CHECK(Fallback(endpoint)); + BOOST_CHECK(m_connman.IsPQFallback(endpoint, Now())); + at(-60min, 1h); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + // It ended like any window: the streak restarts, and the next window is the second. + Connect(endpoint, End::MALFORMED); + BOOST_REQUIRE(Streak(endpoint)); + BOOST_CHECK_EQUAL(Streak(endpoint)->streak, 1U); + BOOST_CHECK(Streak(endpoint)->next_window == 4h); + + // A wall clock step forward may end a window early, which only fails toward hybrid keys. + for (uint32_t i{1}; i < PQ_FAILURE_THRESHOLD; ++i) Connect(endpoint, End::MALFORMED); + BOOST_REQUIRE(Fallback(endpoint)); + BOOST_CHECK(Fallback(endpoint)->expires == start - 60min + 4h); + at(-60min + 4h, 1h + 1s); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); +} + +BOOST_AUTO_TEST_CASE(pq_network_off_confirmation) +{ + // setnetworkactive false can land in the socket handler pass in which a peer's confirmation + // verifies. That confirmation is a success all the same: it ends the endpoint's streak and + // protects it, so later closed_after_switch failures can't put it in the fallback set. The + // closes the network switch then causes are ours, and count nothing. + const PQEndpointKey endpoint{TestEndpoint(10, 0)}; + for (uint32_t i{1}; i < PQ_FAILURE_THRESHOLD; ++i) Connect(endpoint, End::PEER_EOF); + BOOST_REQUIRE(Streak(endpoint)); + + Link inbound{Accept()}; // an offer that would count as abandoned if our close counted + Receive(inbound, InitiatorKey()); + Pass(inbound); + BOOST_REQUIRE(inbound.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT); + // Our confirmation reaches the peer, which switches and verifies it; its own is on the way. + Link link{Add(ConnectionType::OUTBOUND_FULL_RELAY, PQMode::NEGOTIATE, endpoint)}; + V2Transport peer{Responder()}; + Pass(link); + BOOST_REQUIRE(NodeToPeer(link, peer)); + PeerToNode(link, peer); + Pass(link); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().switched); + BOOST_REQUIRE(NodeToPeer(link, peer)); + m_connman.SetNetworkActive(false); + PeerToNode(link, peer); + Pass(link); + BOOST_REQUIRE(link.node->m_transport->GetPQSnapshot().confirmed); + const uint64_t abandoned{Stats().inbound.abandoned}; + Disconnect(); + m_connman.SetNetworkActive(true); + BOOST_CHECK(m_connman.TestNodes().empty()); + BOOST_CHECK_EQUAL(Stats().inbound.abandoned, abandoned); + + BOOST_CHECK(!Streak(endpoint)); + for (uint32_t i{0}; i < PQ_FAILURE_THRESHOLD; ++i) Connect(endpoint, End::PEER_EOF); + BOOST_CHECK(!Streak(endpoint)); + BOOST_CHECK(!Fallback(endpoint)); + BOOST_CHECK(!m_connman.IsPQFallback(endpoint, Now())); + BOOST_CHECK_EQUAL(Stats().outbound.closed_after_switch, 2U * PQ_FAILURE_THRESHOLD - 1); +} + namespace { /** The warning names the backends that ran, and -mlkemportable as the remedy while native code runs. */ std::string LocalFaultLine() @@ -1561,17 +1754,17 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_warning) { // Whatever backends this machine runs. LogLineCounter warnings{"v2 pq: local_fault"}; - for (int i{0}; i < 7; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + for (int i{0}; i < 7; ++i) Connect(TestEndpoint(7, i), End::CONFIRMATION); BOOST_CHECK_EQUAL(warnings.m_count, 0); { const std::string text{LocalFaultLine()}; DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; - Connect(TestEndpoint(7, 7), End::MALFORMED); + Connect(TestEndpoint(7, 7), End::CONFIRMATION); } BOOST_CHECK_EQUAL(warnings.m_count, 1); BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), PQ_LOCAL_FAULT_THRESHOLD); // It warns once, and behavior doesn't change. - for (int i{0}; i < 12; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + for (int i{0}; i < 12; ++i) Connect(TestEndpoint(7, i), End::CONFIRMATION); BOOST_CHECK_EQUAL(warnings.m_count, 1); BOOST_CHECK(m_connman.IsPQFallback(TestEndpoint(7, 0), Now()) == false); BOOST_REQUIRE(Streak(TestEndpoint(7, 0))); @@ -1592,7 +1785,7 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_warning_portable) LogLineCounter warnings{"v2 pq: local_fault"}; { DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; - for (int i{0}; i < 8; ++i) Connect(TestEndpoint(7, i), End::MALFORMED); + for (int i{0}; i < 8; ++i) Connect(TestEndpoint(7, i), End::CONFIRMATION); } BOOST_CHECK_EQUAL(warnings.m_count, 1); } @@ -1602,15 +1795,44 @@ BOOST_AUTO_TEST_CASE(pq_local_fault_after_success) // One success since startup rules out a local fault. LogLineCounter warnings{"v2 pq: local_fault"}; Connect(TestEndpoint(8, 0), End::SUCCESS); - for (int i{1}; i <= 12; ++i) Connect(TestEndpoint(8, i), End::MALFORMED); + for (int i{1}; i <= 12; ++i) Connect(TestEndpoint(8, i), End::CONFIRMATION); + BOOST_CHECK_EQUAL(warnings.m_count, 0); +} + +BOOST_AUTO_TEST_CASE(pq_local_fault_reasons) +{ + // Only failures this node's own ML-KEM code can cause count toward the warning. A record of + // the wrong length, and a peer that stops responding after the switch, are the peer's or the + // path's alone, however many endpoints fail that way: buggy or hostile peers can't raise it. + LogLineCounter warnings{"v2 pq: local_fault"}; + int index{0}; + for (int i{0}; i < 8; ++i) Connect(TestEndpoint(11, index++), End::MALFORMED); + for (int i{0}; i < 8; ++i) Connect(TestEndpoint(11, index++), End::TIMEOUT); + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), 0U); + // A key our check rejects, keys that don't match, and a peer that closes after the switch, as + // one whose check of our confirmation failed does, all count. + size_t counted{0}; + for (const End end : {End::BAD_MODULUS, End::CONFIRMATION, End::PEER_EOF, End::PEER_RESET}) { + Connect(TestEndpoint(11, index++), end); + BOOST_CHECK_EQUAL(m_connman.PQLocalFaultEndpoints(), ++counted); + } BOOST_CHECK_EQUAL(warnings.m_count, 0); + { + const std::string text{LocalFaultLine()}; + DebugLogHelper expect{text, AtLevel({text}, LineLevel::WARNING)}; + for (int i{0}; i < 4; ++i) Connect(TestEndpoint(11, index++), End::CONFIRMATION); + } + BOOST_CHECK_EQUAL(warnings.m_count, 1); + const PQTransportStats stats{Stats()}; + BOOST_CHECK_EQUAL(stats.outbound.malformed_record, 9U); + BOOST_CHECK_EQUAL(stats.outbound.closed_after_switch, 10U); } BOOST_AUTO_TEST_CASE(pq_inbound_success_does_not_protect) { - // Only outbound successes enter the success set, which is never evicted. An inbound peer's - // key is its address and source port, so letting inbound successes in would let a peer that - // reconnects grow the set without bound, and silence the local-fault warning. + // Only outbound successes enter the success set. An inbound peer's key is its address and + // source port, so letting inbound successes in would let a peer that reconnects churn the + // set, and silence the local-fault warning. const CService addr{LookupNumeric("10.7.0.1", 8333)}; Link inbound{Add(ConnectionType::INBOUND, PQMode::NEGOTIATE, std::nullopt, addr)}; V2Transport peer{Initiator()}; @@ -1620,10 +1842,10 @@ BOOST_AUTO_TEST_CASE(pq_inbound_success_does_not_protect) Disconnect(); LogLineCounter warnings{"v2 pq: local_fault"}; // An outbound failure to the same address and port still starts a streak... - Connect(addr, End::MALFORMED); + Connect(addr, End::CONFIRMATION); BOOST_CHECK(Streak(addr)); // ...and with no outbound success, eight failing endpoints still warn. - for (int i{1}; i < 8; ++i) Connect(TestEndpoint(9, i), End::MALFORMED); + for (int i{1}; i < 8; ++i) Connect(TestEndpoint(9, i), End::CONFIRMATION); BOOST_CHECK_EQUAL(warnings.m_count, 1); } @@ -1838,6 +2060,42 @@ BOOST_AUTO_TEST_CASE(pq_load_shedding_wall_clock_step) BOOST_CHECK(attempt()); } +BOOST_AUTO_TEST_CASE(pq_load_shedding_steady_clock_step_back) +{ + // Only a test's mocked steady clock steps back: setmocktime 0 after a later time, with + // -test=mock_steady_clock. The seconds start over from the new time, so attempts on either + // side of the step don't add up, and shedding still ends after the quiet period. + InitConnman(/*shed_threshold=*/5); + const auto attempt{[&] { + Link link{Accept()}; + Receive(link, InitiatorKey()); + Pass(link); + const bool offered{link.node->m_transport->GetPQSnapshot().offer == PQOfferState::SENT}; + link.node->RequestDisconnect(); + Disconnect(); + return offered; + }}; + for (int i{0}; i < 3; ++i) BOOST_CHECK(attempt()); + MockableSteadyClock::SetMockTime(STEADY_START - 3000s); + for (int i{0}; i < 3; ++i) BOOST_CHECK(attempt()); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK_EQUAL(Stats().inbound.shed, 0U); + + // A flood starts shedding; then the clock steps back again, and nothing more is attempted. + for (int i{0}; i < 3; ++i) attempt(); + BOOST_REQUIRE(Stats().load_shedding.active); + constexpr std::chrono::seconds back{100s}; + MockableSteadyClock::SetMockTime(back); + m_connman.UpdatePQSheddingPublic(); + MockableSteadyClock::SetMockTime(back + PQ_SHED_QUIET_PERIOD - 1s); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(Stats().load_shedding.active); + MockableSteadyClock::SetMockTime(back + PQ_SHED_QUIET_PERIOD); + m_connman.UpdatePQSheddingPublic(); + BOOST_CHECK(!Stats().load_shedding.active); + BOOST_CHECK(attempt()); +} + BOOST_AUTO_TEST_CASE(pq_load_shedding_socket_thread) { // The socket handler thread ends shedding on time when the flood stops entirely, without a @@ -1867,8 +2125,7 @@ BOOST_AUTO_TEST_CASE(pq_config) { // Init() takes the negotiation of new connections, inbound and outbound, from the // configuration: on only with both switches on. - const auto create_sock_orig{CreateSock}; - CreateSock = [](int, int, int) -> std::unique_ptr { return std::make_unique(); }; + const auto create_sock{MockCreateSock()}; for (const auto& [config, enabled] : std::vector>{ {{.v2_enabled = false, .pq_requested = true}, false}, {{.v2_enabled = true, .pq_requested = false}, false}, @@ -1899,7 +2156,6 @@ BOOST_AUTO_TEST_CASE(pq_config) BOOST_CHECK(ours.failure == PQFailure::CONFIRM_LENGTH || ours.failure == PQFailure::CONFIRM_TAG); BOOST_CHECK(inbound.node->fDisconnect); BOOST_CHECK(peer.GetPQSnapshot().switched); - CreateSock = create_sock_orig; } BOOST_AUTO_TEST_CASE(pq_rpc_status) @@ -2013,48 +2269,41 @@ BOOST_AUTO_TEST_CASE(pq_rpc_windows) BOOST_CHECK_EQUAL(next_window(), 86400); } -BOOST_AUTO_TEST_CASE(pq_rpc_listed_peers) +BOOST_AUTO_TEST_CASE(pq_rpc_connections_pq) { - // getnetworkinfo's connections_pq counts exactly the peers getpeerinfo lists: a hybrid - // connection the peer manager doesn't know yet is in the connection count, but not listed or - // counted as hybrid. + // getnetworkinfo's connections_pq counts the hybrid connections in the connection manager's + // node list, without per-peer statistics or cs_main, and matches the peers getpeerinfo lists + // with transport_pq true: as in production, the peer manager knows each node before the list + // has it. A connection that is still negotiating isn't counted. auto& connman{static_cast(*m_node.connman)}; - CNode* node{new CNode{/*id=*/100000, std::make_shared(), CAddress{LookupNumeric("10.8.0.1", 50000), NODE_NONE}, - /*nKeyedNetGroupIn=*/0, /*nLocalHostNonceIn=*/0, CAddress{}, /*addrNameIn=*/"", ConnectionType::INBOUND, - /*inbound_onion=*/false, CNodeOptions{.use_v2transport = true, .pq = {.mode = PQMode::NEGOTIATE}}}}; + std::vector nodes; + for (const auto& [id, addr] : std::vector>{{100000, "10.8.0.1"}, {100001, "10.8.0.2"}}) { + nodes.push_back(new CNode{id, std::make_shared(), CAddress{LookupNumeric(addr, 50000), NODE_NONE}, + /*nKeyedNetGroupIn=*/0, /*nLocalHostNonceIn=*/0, CAddress{}, /*addrNameIn=*/"", ConnectionType::INBOUND, + /*inbound_onion=*/false, CNodeOptions{.use_v2transport = true, .pq = {.mode = PQMode::NEGOTIATE}}}); + m_node.peerman->InitializeNode(*nodes.back(), NODE_NONE); + } V2Transport peer{Initiator()}; - const auto deliver{[](Transport& from, Transport& to) { - const std::vector bytes{PeerBytes(from)}; - std::span remaining{bytes}; - while (!remaining.empty()) { - BOOST_REQUIRE(to.ReceivedBytes(remaining)); - if (to.ReceivedMessageComplete()) { - bool reject{false}; - to.GetReceivedMessage({}, reject); - } - } - }}; for (int round{0}; round < 4; ++round) { - deliver(peer, *node->m_transport); - deliver(*node->m_transport, peer); + Deliver(peer, *nodes[0]->m_transport); + Deliver(*nodes[0]->m_transport, peer); } - BOOST_REQUIRE(node->m_transport->GetInfo().transport_pq); - connman.AddTestNode(*node); - - UniValue networkinfo{CallRPC(m_node, "getnetworkinfo")}; - BOOST_CHECK_EQUAL(networkinfo["connections"].getInt(), 1); - BOOST_CHECK_EQUAL(networkinfo["connections_pq"].getInt(), 0); - BOOST_CHECK(CallRPC(m_node, "getpeerinfo").empty()); + BOOST_REQUIRE(nodes[0]->m_transport->GetInfo().transport_pq); + for (CNode* node : nodes) connman.AddTestNode(*node); - m_node.peerman->InitializeNode(*node, NODE_NONE); - networkinfo = CallRPC(m_node, "getnetworkinfo"); + const UniValue networkinfo{CallRPC(m_node, "getnetworkinfo")}; + BOOST_CHECK_EQUAL(networkinfo["connections"].getInt(), 2); BOOST_CHECK_EQUAL(networkinfo["connections_pq"].getInt(), 1); const UniValue peers{CallRPC(m_node, "getpeerinfo")}; - BOOST_REQUIRE_EQUAL(peers.size(), 1U); - BOOST_CHECK(peers[0]["transport_pq"].get_bool()); - BOOST_CHECK_EQUAL(peers[0]["transport_pq_status"].get_str(), "hybrid"); + BOOST_REQUIRE_EQUAL(peers.size(), 2U); + std::map> listed; + for (size_t i{0}; i < peers.size(); ++i) { + listed[peers[i]["addr"].get_str()] = {peers[i]["transport_pq"].get_bool(), peers[i]["transport_pq_status"].get_str()}; + } + BOOST_CHECK(listed["10.8.0.1:50000"] == std::make_pair(true, std::string{"hybrid"})); + BOOST_CHECK(listed["10.8.0.2:50000"] == std::make_pair(false, std::string{"pending"})); - m_node.peerman->FinalizeNode(*node); + for (CNode* node : nodes) m_node.peerman->FinalizeNode(*node); connman.ClearTestNodes(); } diff --git a/test/functional/p2p_v2_pq_fallback.py b/test/functional/p2p_v2_pq_fallback.py index 805bdf82b8..6968403be0 100755 --- a/test/functional/p2p_v2_pq_fallback.py +++ b/test/functional/p2p_v2_pq_fallback.py @@ -5,20 +5,26 @@ """Test the bounded fallback to plain v2 after repeated hybrid post-quantum failures with one endpoint (#184, doc/design/pq-transport.md). -- test_three_failures: three explicit outbound attempts whose key confirmation fails put the - endpoint in the fallback set; the fourth connection is plain v2 with status fallback, and one - warning line says so. -- test_malformed_offer_fallback: against a responder that always sends a malformed offer, the - fourth connection succeeds, because a fallback connection neither parses nor validates offers. -- test_inbound_and_success_exclusion: inbound failures never lead to a fallback, and neither do - outbound failures after a hybrid success with the same endpoint. +Only automatic outbound connections fall back, so the attempts that count use addconnection +(outbound-full-relay). Manual connections (addnode onetry) never count toward a streak and never +fall back: an attacker on the path can make handshakes fail without breaking a key. + +- test_three_failures: three automatic attempts whose key confirmation fails put the endpoint in + the fallback set; the fourth connection is plain v2 with status fallback, and one warning line + says so. +- test_manual_exempt: a manual connection to that endpoint still negotiates hybrid keys, and its + failure is counted and logged but leaves the fallback set alone. - test_expiry_and_restart: the hybrid negotiation is tried again once the window expires (and the next window is longer), and a restart clears all of it under a new instance_id. +- test_malformed_offer_fallback: against a responder that always sends a malformed offer, manual + failures start no streak; after three automatic ones, the fourth automatic connection + succeeds, because a fallback connection neither parses nor validates offers, while a manual one + still sees the malformed offer. +- test_inbound_and_success_exclusion: inbound failures never lead to a fallback, and neither do + outbound failures after a hybrid success with the same endpoint. - test_endpoint_separation: through a SOCKS5 proxy, two hostnames (in any letter case, with or without the root dot), one hostname on two ports, and one address on two ports each keep their own history: a name-proxy destination is its hostname and port, never the proxy. - -addconnection can't open manual connections, so explicit outbound attempts use addnode onetry. """ import threading @@ -96,6 +102,7 @@ def setup_network(self): def run_test(self): self.test_three_failures() + self.test_manual_exempt() self.test_expiry_and_restart() self.test_malformed_offer_fallback() self.test_inbound_and_success_exclusion() @@ -105,9 +112,20 @@ def wait_no_peers(self, *nodes): for node in nodes: self.wait_until(lambda node=node: len(node.getpeerinfo()) == 0) - def connect_responder(self, node, peer, *, idx, target=None): - """Let a Python responder listen on its own port and make node open a manual connection to it - (to target, through a proxy that leads there, if given).""" + def connect_automatic(self, a, b): + """Open an automatic outbound connection from node a to node b, as the connection logic would, + and wait until both sides completed the version handshake (a pong arrived, as connect_nodes() + checks).""" + node_a, node_b = self.nodes[a], self.nodes[b] + node_a.addconnection(f"127.0.0.1:{p2p_port(b)}", "outbound-full-relay", True) + for node, inbound in [(node_a, False), (node_b, True)]: + self.wait_until(lambda node=node, inbound=inbound: any( + peer["inbound"] == inbound and peer["bytesrecv_per_msg"].get("pong", 0) >= 29 for peer in node.getpeerinfo())) + + def connect_responder(self, node, peer, *, idx, target=None, automatic=False): + """Let a Python responder listen on its own port and make node open a connection to it (to + target, through a proxy that leads there, if given): a manual one, or with automatic an + outbound-full-relay one.""" listening = threading.Event() peer.peer_accept_connection(connect_cb=lambda addr, port: listening.set(), connect_id=idx, net=node.chain, timeout_factor=node.timeout_factor, supports_v2_p2p=True, @@ -116,7 +134,11 @@ def connect_responder(self, node, peer, *, idx, target=None): # The RPC call comes from this thread, not the network thread's callback: the RPC connection is # not shared across threads. self.wait_until(listening.is_set) - node.addnode(node=target or f"127.0.0.1:{responder_port(idx)}", command="onetry") + target = target or f"127.0.0.1:{responder_port(idx)}" + if automatic: + node.addconnection(target, "outbound-full-relay", True) + else: + node.addnode(node=target, command="onetry") return responder_port(idx) def test_three_failures(self): @@ -126,13 +148,13 @@ def test_three_failures(self): target = f"127.0.0.1:{p2p_port(1)}" b_before = counters(node_b) for attempt in range(1, 4): - expected = ["v2 pq: switched role=initiator conn_type=manual peer="] + expected = ["v2 pq: switched role=initiator conn_type=outbound-full-relay peer="] unexpected = ["v2 pq: fallback"] if attempt == 3: expected.append("v2 pq: fallback cause=") unexpected = [] with node_a.assert_debug_log(expected, unexpected_msgs=unexpected): - node_a.addnode(node=target, command="onetry") + node_a.addconnection(target, "outbound-full-relay", True) self.wait_until(lambda: counted_failures(node_a) == attempt) self.wait_no_peers(node_a, node_b) a_counts = counters(node_a) @@ -159,12 +181,12 @@ def test_three_failures(self): warnings = [line for line in log if "v2 pq: fallback cause=" in line] assert_equal(len(warnings), 1) assert f"v2 pq: fallback cause={entry['cause']} until=" in warnings[0] - assert "role=initiator conn_type=manual" in warnings[0] + assert "role=initiator conn_type=outbound-full-relay" in warnings[0] self.log.info("The fourth connection is plain v2, with status fallback") before = counters(node_a) with node_a.assert_debug_log([], unexpected_msgs=["v2 pq: switched", "v2 pq: fallback"]): - self.connect_nodes(0, 1) + self.connect_automatic(0, 1) [outbound] = node_a.getpeerinfo() [inbound] = node_b.getpeerinfo() assert_equal((outbound["transport_protocol_type"], outbound["transport_pq"], outbound["transport_pq_status"]), @@ -175,16 +197,37 @@ def test_three_failures(self): assert_equal(counters(node_a), before) self.disconnect_nodes(0, 1) + def test_manual_exempt(self): + node_a, node_b = self.nodes + self.log.info("A manual connection to an endpoint in the fallback set still negotiates hybrid keys") + info = node_a.getpqtransportinfo() + [entry] = info["fallback_set"] + before = counters(node_a) + # Node A still damages its own shared secret, so the handshake fails: counted and logged, but + # the fallback set stays as it was. + with node_a.assert_debug_log(["v2 pq: switched role=initiator conn_type=manual peer="], + unexpected_msgs=["v2 pq: fallback cause="]): + node_a.addnode(node=f"127.0.0.1:{p2p_port(1)}", command="onetry") + self.wait_until(lambda: counted_failures(node_a) == sum(before[key] for key in COUNTED) + 1) + self.wait_no_peers(node_a, node_b) + after = node_a.getpqtransportinfo() + assert_equal((after["fallback_set"], after["failure_streaks"]), ([entry], [])) + assert_equal(counters(node_a)["outbound_switched"], before["outbound_switched"] + 1) + assert_equal(counters(node_a)["outbound_fallback"], before["outbound_fallback"]) + last = after["recent_failures"]["outbound"]["entries"][-1] + assert_equal((last["connection_type"], last["endpoint"]), ("manual", entry["endpoint"])) + def test_expiry_and_restart(self): node_a, node_b = self.nodes self.log.info("Once the window expires, the next connection tries the hybrid keys again") [entry] = node_a.getpqtransportinfo()["fallback_set"] + before = counted_failures(node_a) for node in self.nodes: node.setmocktime(entry["expires"]) - with node_a.assert_debug_log(["v2 pq: switched role=initiator conn_type=manual"], + with node_a.assert_debug_log(["v2 pq: switched role=initiator conn_type=outbound-full-relay"], unexpected_msgs=["v2 pq: fallback cause="]): - node_a.addnode(node=f"127.0.0.1:{p2p_port(1)}", command="onetry") - self.wait_until(lambda: counted_failures(node_a) == 4) + node_a.addconnection(f"127.0.0.1:{p2p_port(1)}", "outbound-full-relay", True) + self.wait_until(lambda: counted_failures(node_a) == before + 1) self.wait_no_peers(node_a, node_b) info = node_a.getpqtransportinfo() assert_equal(info["fallback_set"], []) @@ -204,30 +247,45 @@ def test_expiry_and_restart(self): assert_equal(len(info["instance_id"]), 64) assert_equal((info["fallback_set"], info["failure_streaks"]), ([], [])) assert_equal(set(counters(node_a).values()), {0}) - self.connect_nodes(0, 1) + self.connect_automatic(0, 1) assert_equal(node_a.getpeerinfo()[0]["transport_pq_status"], "hybrid") self.disconnect_nodes(0, 1) def test_malformed_offer_fallback(self): node_a = self.nodes[0] - self.log.info("Three malformed offers from one responder put it in the fallback set") + self.log.info("Malformed offers on manual connections are counted and logged, but start no streak") self.restart_node(0, extra_args=PQ_ON) port = None - for attempt in range(1, 4): + attempts = 0 + for _ in range(4): + attempts += 1 with node_a.assert_debug_log(["v2 pq: malformed_record reason=ek_modulus role=initiator conn_type=manual"]): peer = MalformedOfferPeer() port = self.connect_responder(node_a, peer, idx=1) - self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == attempt) + self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == attempts) + peer.wait_for_disconnect() + self.wait_no_peers(node_a) + info = node_a.getpqtransportinfo() + assert_equal((info["fallback_set"], info["failure_streaks"]), ([], [])) + assert_equal(len(info["recent_failures"]["outbound"]["entries"]), 4) + + self.log.info("Three malformed offers on automatic connections put the responder in the fallback set") + for _ in range(3): + attempts += 1 + with node_a.assert_debug_log(["v2 pq: malformed_record reason=ek_modulus role=initiator conn_type=outbound-full-relay"]): + peer = MalformedOfferPeer() + self.connect_responder(node_a, peer, idx=1, automatic=True) + self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == attempts) peer.wait_for_disconnect() self.wait_no_peers(node_a) [entry] = node_a.getpqtransportinfo()["fallback_set"] assert_equal(entry["endpoint"], address_endpoint("127.0.0.1", port)) assert_equal((entry["cause"], entry["reason"], entry["streak"]), ("malformed_record", "ek_modulus", 3)) - self.log.info("The fourth connection ignores the malformed offer and completes as plain v2") + self.log.info("The next automatic connection ignores the malformed offer and completes as plain v2") before = counters(node_a) peer = MalformedOfferPeer() - self.connect_responder(node_a, peer, idx=1) + self.connect_responder(node_a, peer, idx=1, automatic=True) peer.wait_for_connect() peer.wait_for_verack() peer.sync_with_ping() @@ -239,6 +297,15 @@ def test_malformed_offer_fallback(self): node_a.disconnect_p2ps() self.wait_no_peers(node_a) + self.log.info("A manual connection to it still negotiates, and so still sees the malformed offer") + with node_a.assert_debug_log(["v2 pq: malformed_record reason=ek_modulus role=initiator conn_type=manual"]): + peer = MalformedOfferPeer() + self.connect_responder(node_a, peer, idx=1) + self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == attempts + 1) + peer.wait_for_disconnect() + self.wait_no_peers(node_a) + assert_equal(node_a.getpqtransportinfo()["fallback_set"], [entry]) + def test_inbound_and_success_exclusion(self): node_a = self.nodes[0] self.log.info("Repeated inbound failures never lead to a fallback") @@ -259,7 +326,7 @@ def test_inbound_and_success_exclusion(self): self.log.info("Outbound failures after a hybrid success with the same endpoint never lead to a fallback") peer = P2PInterface() - self.connect_responder(node_a, peer, idx=2) + self.connect_responder(node_a, peer, idx=2, automatic=True) peer.wait_for_connect() peer.wait_for_verack() assert_equal(node_a.getpeerinfo()[0]["transport_pq_status"], "hybrid") @@ -267,7 +334,7 @@ def test_inbound_and_success_exclusion(self): self.wait_no_peers(node_a) for attempt in range(1, 5): peer = MalformedOfferPeer() - self.connect_responder(node_a, peer, idx=2) + self.connect_responder(node_a, peer, idx=2, automatic=True) self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == attempt) peer.wait_for_disconnect() self.wait_no_peers(node_a) @@ -302,7 +369,7 @@ def test_endpoint_separation(self): for i, (target, endpoint) in enumerate(attempts, start=1): self.log.info(f"- {target}") peer = MalformedOfferPeer() - self.connect_responder(node_a, peer, idx=3, target=target) + self.connect_responder(node_a, peer, idx=3, target=target, automatic=True) self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == i) peer.wait_for_disconnect() self.wait_no_peers(node_a) @@ -326,7 +393,7 @@ def test_endpoint_separation(self): self.log.info("Only the endpoint in the fallback set runs plain v2") before = counters(node_a) peer = MalformedOfferPeer() - self.connect_responder(node_a, peer, idx=3, target="alpha.pqtest.:9001") + self.connect_responder(node_a, peer, idx=3, target="alpha.pqtest.:9001", automatic=True) peer.wait_for_connect() peer.wait_for_verack() assert_equal(node_a.getpeerinfo()[0]["transport_pq_status"], "fallback") @@ -334,7 +401,7 @@ def test_endpoint_separation(self): self.wait_no_peers(node_a) assert_equal(counters(node_a), before) peer = MalformedOfferPeer() - self.connect_responder(node_a, peer, idx=3, target="beta.pqtest:9001") + self.connect_responder(node_a, peer, idx=3, target="beta.pqtest:9001", automatic=True) self.wait_until(lambda: counters(node_a)["outbound_malformed_record"] == len(attempts) + 1) peer.wait_for_disconnect() self.wait_no_peers(node_a) diff --git a/test/functional/rpc_pqtransport.py b/test/functional/rpc_pqtransport.py index 6603bccc1a..b56ee62046 100755 --- a/test/functional/rpc_pqtransport.py +++ b/test/functional/rpc_pqtransport.py @@ -253,9 +253,21 @@ def wait_for_no_peers(self, *nodes): for node in nodes: self.wait_until(lambda: node.getpeerinfo() == []) - def failing_connection(self, a, b, *, enters_fallback=False): + def connect_automatic(self, a, b): + """Open an automatic outbound connection from node a to node b, as the connection logic would, + and wait until both sides completed the version handshake (a pong arrived, as connect_nodes() + checks).""" + node_a, node_b = self.nodes[a], self.nodes[b] + node_a.addconnection(f"127.0.0.1:{p2p_port(b)}", "outbound-full-relay", True) + for node, inbound in [(node_a, False), (node_b, True)]: + self.wait_until(lambda node=node, inbound=inbound: any( + peer["inbound"] == inbound and peer["bytesrecv_per_msg"].get("pong", 0) >= 29 for peer in node.getpeerinfo())) + + def failing_connection(self, a, b, *, enters_fallback=False, automatic=False): """Connect node a to node b, whose hybrid handshake fails its key confirmation - (-test=pq_fail_first_packet on either side), and wait until both removed the peer. + (-test=pq_fail_first_packet on either side), and wait until both removed the peer. The + connection is a manual one, or with automatic an outbound-full-relay one: only those count + toward the fallback. The initiator's version packet and confirmation travel together, so the responder's check of that confirmation fails, and it closes before its own confirmation leaves: the @@ -264,7 +276,10 @@ def failing_connection(self, a, b, *, enters_fallback=False): failure completes a streak and the initiator also counts fallback.""" initiator, responder = self.nodes[a], self.nodes[b] before_initiator, before_responder = self.handshakes(initiator), self.handshakes(responder) - initiator.addnode(f"127.0.0.1:{p2p_port(b)}", "onetry") + if automatic: + initiator.addconnection(f"127.0.0.1:{p2p_port(b)}", "outbound-full-relay", True) + else: + initiator.addnode(f"127.0.0.1:{p2p_port(b)}", "onetry") self.wait_for_handshakes(responder, before_responder, {"inbound": {"switched": 1, "first_packet_failed": 1}}) outcomes = ["closed_after_switch", "first_packet_failed"] fallback = {"fallback": 1} if enters_fallback else {} @@ -471,9 +486,13 @@ def peer(self, node, *, inbound): assert_equal(len(peers), 1) return peers[0] - def check_peers(self, a, b, status_a, status_b, **kwargs): - """Connect node a to node b and check each side's transport_pq_status; then disconnect.""" - self.connect_nodes(a, b, **kwargs) + def check_peers(self, a, b, status_a, status_b, *, automatic=False, **kwargs): + """Connect node a to node b (with automatic, as an automatic outbound connection) and check + each side's transport_pq_status; then disconnect.""" + if automatic: + self.connect_automatic(a, b) + else: + self.connect_nodes(a, b, **kwargs) for node, status, inbound in [(self.nodes[a], status_a, False), (self.nodes[b], status_b, True)]: peer = self.peer(node, inbound=inbound) assert_equal((peer["transport_pq"], peer["transport_pq_status"]), (status == "hybrid", status)) @@ -524,7 +543,7 @@ def test_states(self): reasons = {"closed_after_switch": ["eof", "reset"], "first_packet_failed": ["length", "tag"]} outcomes = [] for streak in range(1, 4): - outcomes.append(self.failing_connection(0, 1, enters_fallback=streak == 3)) + outcomes.append(self.failing_connection(0, 1, enters_fallback=streak == 3, automatic=True)) info = node0.getpqtransportinfo() if streak < 3: assert_equal(info["fallback_set"], []) @@ -539,7 +558,7 @@ def test_states(self): # The fallback entry names the failure that completed the streak, on the same connection. assert_equal((entries[3]["reason"], entries[3]["peer_id"]), (outcomes[-1], entries[2]["peer_id"])) assert entries[0]["peer_id"] < entries[1]["peer_id"] < entries[2]["peer_id"] - expected_entries = [{"sequence": sequence, "time": now, "endpoint": endpoint, "direction": "outbound", "connection_type": "manual", + expected_entries = [{"sequence": sequence, "time": now, "endpoint": endpoint, "direction": "outbound", "connection_type": "outbound-full-relay", "peer_id": entry["peer_id"], "outcome": outcome, "reason": entry["reason"]} for sequence, (outcome, entry) in enumerate(zip(outcomes + ["fallback"], entries), start=1)] counts = zero_counts() @@ -554,8 +573,8 @@ def test_states(self): failure_streaks=[], recent_failures={"inbound": empty_ring(), "outbound": {"last_sequence": 4, "dropped": 0, "entries": expected_entries}}) - self.log.info("Test a connection to an endpoint in the fallback set runs plain v2: getpeerinfo fallback") - self.check_peers(0, 1, "fallback", "legacy_peer") + self.log.info("Test an automatic connection to an endpoint in the fallback set runs plain v2: getpeerinfo fallback") + self.check_peers(0, 1, "fallback", "legacy_peer", automatic=True) node0.setmocktime(0) def test_counts(self): @@ -661,11 +680,11 @@ def test_rings(self): self.restart_node(1, extra_args=PQ) def outbound_failure(self, dest): - """Node 0 connects to dest, which ends at node 1, whose confirmation check fails. Returns node 0's - outbound ring entry.""" + """Node 0 opens an automatic outbound connection to dest, which ends at node 1, whose confirmation + check fails. Returns node 0's outbound ring entry.""" node0, node1, _ = self.nodes before = self.handshakes(node0) - node0.addnode(dest, "onetry") + node0.addconnection(dest, "outbound-full-relay", True) self.wait_for_handshakes(node0, before, *[{"outbound": {"switched": 1, outcome: 1}} for outcome in ["closed_after_switch", "first_packet_failed"]]) self.wait_for_no_peers(node0, node1) @@ -688,7 +707,7 @@ def test_endpoints(self): self.restart_node(0, extra_args=PQ + [f"-proxy=127.0.0.1:{proxy_port}"]) endpoint = {"kind": "name_proxy", "network": "name_proxy", "address": "pqtest.example", "port": 8333} entry = self.outbound_failure("PQTest.Example.:8333") - assert_equal((entry["endpoint"], entry["connection_type"]), (endpoint, "manual")) + assert_equal((entry["endpoint"], entry["connection_type"]), (endpoint, "outbound-full-relay")) assert_equal([streak["endpoint"] for streak in node0.getpqtransportinfo()["failure_streaks"]], [endpoint]) proxy.stop()