diff --git a/node/src/net/manager.rs b/node/src/net/manager.rs index 9ab99f8b0..96b6e83cf 100644 --- a/node/src/net/manager.rs +++ b/node/src/net/manager.rs @@ -42,6 +42,20 @@ const MAX_OUTBOUND_BLOCK_RELAY: usize = 2; /// since the connection was made. const ADDR_FETCH_TIMEOUT_SECS: u64 = 10 * 30; const BAN_THRESHOLD: u32 = 100; + +/// What becomes of a peer whose misbehaviour reached [`BAN_THRESHOLD`]; see +/// [`PeerManager::misbehaviour_action`]. +#[derive(Debug)] +enum MisbehaviourAction { + /// Neither disconnected nor punished. Names the kind of peer, as Core's + /// log line does. + Keep(&'static str), + /// Disconnected; its address is not banned. + Disconnect, + /// Disconnected, and the address it names banned for `-bantime`. + Ban(crate::net::ban::BanTarget), +} + /// Keepalive cadence: how often each peer is sent a `ping` when none is /// outstanding. Bitcoin Core's `PING_INTERVAL` (net_processing.h). /// @@ -516,6 +530,10 @@ fn log_received(id: PeerId, msg: &NetworkMessage) { #[path = "manager_p2pbounds_tests.rs"] mod p2pbounds_tests; +#[cfg(test)] +#[path = "manager_peerpunish_tests.rs"] +mod peerpunish_tests; + /// How a transaction is announced to a peer: `MSG_WTX` carrying the wtxid /// to a peer that negotiated BIP 339 wtxid relay, the txid to the rest. fn tx_announcement(wtxid_relay: bool, txid: bitcoin::Txid, wtxid: bitcoin::Wtxid) -> Inventory { @@ -2949,6 +2967,7 @@ impl PeerManager { let (msg_tx, msg_rx) = mpsc::channel::(256); let mut info = PeerInfo::new(id, addr, Direction::Inbound); info.permissions = perms; + info.inbound_onion = inbound_onion; // `getpeerinfo`'s `addrbind`: which of our listeners this peer // reached us on. Distinct from `-bind` config, since a node may // listen on several addresses. @@ -3668,53 +3687,107 @@ impl PeerManager { self.ban_list.read().is_banned(&addr.ip(), now) } - /// Add ban score to a peer. If the score exceeds BAN_THRESHOLD, the peer - /// is disconnected, removed, and its address is banned. + /// Add ban score to a peer. Once the score reaches [`BAN_THRESHOLD`] the + /// peer has misbehaved (Core's `Misbehaving`), and it is dealt with as + /// [`Self::misbehaviour_action`] says. fn add_ban_score(&self, id: PeerId, score: u32, reason: &str) { let mut peers = self.peers.write(); - let (should_ban, ban_addr) = if let Some(handle) = peers.get_mut(&id) { - // NoBan peers (-whitelist/-whitebind) are never banned or - // disconnected for misbehavior. - if handle.info.permissions.noban { - tracing::debug!(id, addr = %handle.info.addr, reason, "Skipping ban score for noban peer"); + let Some(handle) = peers.get_mut(&id) else { + return; + }; + handle.info.ban_score = handle.info.ban_score.saturating_add(score); + if handle.info.ban_score < BAN_THRESHOLD { + tracing::debug!(id, score = handle.info.ban_score, reason, "Increased ban score"); + return; + } + let ban = match Self::misbehaviour_action(&handle.info) { + MisbehaviourAction::Keep(kind) => { + // Core clears `m_should_discourage` and does nothing else, so + // the next misbehaviour is logged again. + handle.info.ban_score = 0; + tracing::warn!(id, reason, "Not punishing {kind} peer {id}!"); return; } - handle.info.ban_score += score; - if handle.info.ban_score >= BAN_THRESHOLD { - tracing::warn!(id, addr = %handle.info.addr, score = handle.info.ban_score, reason, "Banning peer"); - (true, Some(handle.info.addr.ip())) - } else { - tracing::debug!(id, score = handle.info.ban_score, reason, "Increased ban score"); - (false, None) - } - } else { - (false, None) - }; - if should_ban { - // Signal, for the same reason `disconnect_by_id` does: dropping - // the handle closes the peer task's `msg_rx` only when the last - // sender goes, and a `getcfilters` stream task can hold a clone. - // Without the signal a banned peer stayed connected -- and kept - // feeding the node -- until that task drained. - if let Some(handle) = peers.remove(&id) { - handle.disconnect.notify_one(); + MisbehaviourAction::Disconnect => { + let kind = if handle.info.inbound_onion { "inbound onion" } else { "local" }; + tracing::debug!(id, reason, "Warning: disconnecting but not banning {kind} peer {id}!"); + None } - if let Some(addr) = ban_addr { - drop(peers); // release peers lock before acquiring ban_list lock - let now = crate::time::now_secs(); - let duration = self.ban_duration_secs.load(Ordering::Relaxed); - let target = crate::net::ban::BanTarget::Net( - ipnet::IpNet::from(addr), + MisbehaviourAction::Ban(target) => { + tracing::warn!( + id, + addr = %handle.info.addr_string(), + score = handle.info.ban_score, + reason, + "Banning peer" ); - // Best-effort: misbehaviour bans are fire-and-forget; if the - // entry already exists (e.g. repeated misbehaviour) the add - // fails silently. - let _ = self.ban_list.write().add(&target, now, now + duration); - self.flush_banlist(); + Some(target) } + }; + // Signal, for the same reason `disconnect_by_id` does: dropping + // the handle closes the peer task's `msg_rx` only when the last + // sender goes, and a `getcfilters` stream task can hold a clone. + // Without the signal a banned peer stayed connected -- and kept + // feeding the node -- until that task drained. + if let Some(handle) = peers.remove(&id) { + handle.disconnect.notify_one(); + } + if let Some(target) = ban { + drop(peers); // release peers lock before acquiring ban_list lock + let now = crate::time::now_secs(); + let duration = self.ban_duration_secs.load(Ordering::Relaxed); + // Best-effort: misbehaviour bans are fire-and-forget; if the + // entry already exists (e.g. repeated misbehaviour) the add + // fails silently. + let _ = self.ban_list.write().add(&target, now, now + duration); + self.flush_banlist(); } } + /// What Core's `MaybeDiscourageAndDisconnect` (`net_processing.cpp`) does + /// with a peer that has misbehaved, in this order: + /// - a `noban` peer is not punished; + /// - nor is a manual connection (`addnode`, `-addnode`, `-connect`); + /// - a peer on a local address is disconnected, but its address is not + /// punished, "since that would discourage all peers on the same local + /// address". Every inbound onion peer arrives from the address Tor + /// dialled from, usually loopback, as does every local integration. + /// satd treats an inbound onion peer this way whatever that address + /// is (see below); + /// - anyone else is disconnected and its address punished. Core + /// discourages it; satd bans it for `-bantime`. + /// + /// satd went straight to the last step for every peer but `noban` ones. + /// One misbehaving inbound onion peer banned 127.0.0.1, which refused + /// every onion peer and unwhitelisted local client until the ban expired, + /// across restarts, and a manual peer was banned and then skipped by the + /// redial loop. + fn misbehaviour_action(info: &PeerInfo) -> MisbehaviourAction { + if info.permissions.noban { + return MisbehaviourAction::Keep("noban"); + } + if info.conn_type == ConnType::Manual { + return MisbehaviourAction::Keep("manually connected"); + } + // An outbound onion peer's `addr` is the `0.0.0.0` placeholder all + // onion peers share. Core's address for it is the onion host, which + // is not a local one, and is what it punishes. + if let Some(host) = &info.onion_host { + return MisbehaviourAction::Ban(crate::net::ban::BanTarget::Onion(host.clone())); + } + // Core tests the address alone (`pnode.addr.IsLocal()`), and names + // an inbound onion peer only in its log line: Tor on this host dials + // from loopback. With Tor on another host (`-bind==onion`) + // every onion peer arrives from that host's address. Core discourages + // it, and still admits it while inbound slots are free; satd's ban + // would refuse every onion peer for `-bantime`. So an inbound onion + // peer is disconnected without a ban wherever Tor dials from. + if crate::net::is_local(info.addr.ip()) || info.inbound_onion { + return MisbehaviourAction::Disconnect; + } + MisbehaviourAction::Ban(crate::net::ban::BanTarget::Net(ipnet::IpNet::from(info.addr.ip()))) + } + /// Run the main event loop. Returns when shutdown signal is received. pub async fn run(self: &Arc) { let mut event_rx = self.event_rx.lock().await; diff --git a/node/src/net/manager_peerpunish_tests.rs b/node/src/net/manager_peerpunish_tests.rs new file mode 100644 index 000000000..265daa2f7 --- /dev/null +++ b/node/src/net/manager_peerpunish_tests.rs @@ -0,0 +1,269 @@ +//! What happens to a peer whose misbehaviour reaches the threshold: Core's +//! `MaybeDiscourageAndDisconnect` (`net_processing.cpp`), against a live +//! `PeerManager`. + +use super::*; +use crate::net::flow::InFlight; + +fn mk_pm() -> (Arc, tempfile::TempDir) { + use crate::chain::state::AssumeValid; + use crate::storage::db::InMemoryStore; + use crate::storage::flatfile::FlatFileManager; + use crate::validation::script::NoopVerifier; + + let dir = tempfile::TempDir::new().unwrap(); + let store = Box::new(InMemoryStore::new()); + let flat_files = FlatFileManager::new(&dir.path().join("blocks")).unwrap(); + let chain_state = Arc::new( + ChainState::new( + store, + flat_files, + Network::Regtest, + Box::new(NoopVerifier), + AssumeValid::Disabled, + 450, + 4, + Default::default(), + Default::default(), + Default::default(), + ) + .unwrap(), + ); + let mempool = Arc::new(Mempool::new(1_000_000, 0)); + let fee_estimator = Arc::new(FeeEstimator::new()); + let (_shutdown_tx, shutdown_rx) = tokio::sync::watch::channel(false); + let pm = PeerManager::new(chain_state, mempool, fee_estimator, Network::Regtest, shutdown_rx); + (pm, dir) +} + +/// A connected peer at `addr`. +fn peer(id: PeerId, addr: &str, direction: Direction) -> PeerInfo { + let mut info = PeerInfo::new(id, addr.parse().unwrap(), direction); + info.state = PeerState::Connected; + info +} + +fn add_peer(pm: &PeerManager, info: PeerInfo) -> mpsc::Receiver { + let (tx, rx) = mpsc::channel::(8); + pm.peers.write().insert( + info.id, + PeerHandle { + info, + msg_tx: tx.into(), + disconnect: Arc::new(tokio::sync::Notify::new()), + flow: Arc::new(crate::net::flow::PeerFlow::new()), + last_getheaders_sent: None, + last_mempool_served: None, + fee_filter_sent: None, + stats: PeerStats::new(NetTotals::new()), + }, + ); + rx +} + +/// What became of a peer after it misbehaved. +#[derive(Debug, PartialEq, Eq)] +enum Outcome { + /// Still connected, nothing banned. + Kept, + /// Disconnected, nothing banned. + Disconnected, + /// Disconnected, and a ban entry added. + Banned, +} + +fn outcome(pm: &PeerManager, id: PeerId) -> Outcome { + let connected = pm.peers.read().contains_key(&id); + let banned = !pm.list_banned().is_empty(); + match (connected, banned) { + (true, false) => Outcome::Kept, + (false, false) => Outcome::Disconnected, + (false, true) => Outcome::Banned, + (true, true) => panic!("peer {id} is still connected, but something was banned"), + } +} + +/// Core's ladder, rung by rung. A `noban` peer and a manual connection are +/// never punished ("Not punishing noban peer", "Not punishing manually +/// connected peer"), whatever their address. A peer on a local address is +/// disconnected but its address is not discouraged, "since that would +/// discourage all peers on the same local address": every inbound onion peer +/// and every local integration shares 127.0.0.1. Anyone else is disconnected +/// and its address punished (satd bans it for `-bantime`). +#[test] +fn misbehaviour_follows_cores_ladder() { + use crate::net::permissions::NetPermissions; + let noban = |mut info: PeerInfo| { + info.permissions = NetPermissions { noban: true, ..NetPermissions::NONE }; + info + }; + let manual = |mut info: PeerInfo| { + info.conn_type = ConnType::Manual; + info + }; + let onion = |mut info: PeerInfo| { + info.inbound_onion = true; + info + }; + let cases: Vec<(&str, PeerInfo, Outcome)> = vec![ + ("an inbound peer", peer(1, "10.62.0.1:50000", Direction::Inbound), Outcome::Banned), + ("an outbound peer", peer(1, "10.62.0.2:8333", Direction::Outbound), Outcome::Banned), + // Private, but not local: Core's `IsLocal` is loopback and 0.0.0.0/8. + ("a peer on a private network", peer(1, "192.168.7.5:8333", Direction::Inbound), Outcome::Banned), + ("a noban peer", noban(peer(1, "10.62.0.3:50000", Direction::Inbound)), Outcome::Kept), + ("a manual peer", manual(peer(1, "10.62.0.4:8333", Direction::Outbound)), Outcome::Kept), + // Manual comes before local in the ladder. + ("a manual peer on loopback", manual(peer(1, "127.0.0.1:8333", Direction::Outbound)), Outcome::Kept), + ("a peer on 127.0.0.1", peer(1, "127.0.0.1:50000", Direction::Inbound), Outcome::Disconnected), + ("a peer elsewhere in 127.0.0.0/8", peer(1, "127.5.6.7:50000", Direction::Inbound), Outcome::Disconnected), + ("a peer on ::1", peer(1, "[::1]:50000", Direction::Inbound), Outcome::Disconnected), + // What a dual-stack listener reports for an IPv4 loopback peer. + ( + "a peer on ::ffff:127.0.0.1", + peer(1, "[::ffff:127.0.0.1]:50000", Direction::Inbound), + Outcome::Disconnected, + ), + ("an inbound onion peer", onion(peer(1, "127.0.0.1:50000", Direction::Inbound)), Outcome::Disconnected), + // Tor on another host (`-bind=:=onion`): the + // address is the Tor host's, shared by every onion peer, so it is not + // banned either. See `misbehaviour_action`. + ( + "an inbound onion peer forwarded from another host", + onion(peer(1, "192.168.1.20:50000", Direction::Inbound)), + Outcome::Disconnected, + ), + ("an outbound peer on loopback", peer(1, "127.0.0.1:8333", Direction::Outbound), Outcome::Disconnected), + ]; + for (case, info, expected) in cases { + let (pm, _dir) = mk_pm(); + let _rx = add_peer(&pm, info); + pm.add_ban_score(1, BAN_THRESHOLD, "test"); + assert_eq!(outcome(&pm, 1), expected, "{case}"); + } +} + +/// An outbound onion peer's socket address is the `0.0.0.0` placeholder all +/// onion peers share, not its address. Core punishes the onion address +/// itself, which is not a local one; satd bans that host, and leaves the +/// placeholder alone. +#[test] +fn a_misbehaving_outbound_onion_peer_is_banned_by_its_onion_host() { + let host = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"; + let (pm, _dir) = mk_pm(); + let mut info = peer(1, "0.0.0.0:8333", Direction::Outbound); + info.onion_host = Some(host.to_string()); + let _rx = add_peer(&pm, info); + pm.add_ban_score(1, BAN_THRESHOLD, "test"); + + assert!(!pm.peers.read().contains_key(&1), "the peer is disconnected"); + let now = crate::time::now_secs(); + assert!(pm.ban_list.read().is_onion_banned(host, now), "its onion host is banned"); + assert!( + !pm.is_addr_banned(&"0.0.0.0:8333".parse().unwrap()), + "the shared placeholder is not a peer's address" + ); +} + +/// Neither a `noban` peer nor a manual one is punished however often it +/// misbehaves, and a score that stays under the threshold punishes no one. +#[test] +fn kept_peers_stay_however_often_they_misbehave() { + use crate::net::permissions::NetPermissions; + for case in ["noban", "manual"] { + let (pm, _dir) = mk_pm(); + let mut info = peer(1, "10.62.0.5:8333", Direction::Outbound); + match case { + "noban" => info.permissions = NetPermissions { noban: true, ..NetPermissions::NONE }, + _ => info.conn_type = ConnType::Manual, + } + let _rx = add_peer(&pm, info); + for _ in 0..5 { + pm.add_ban_score(1, BAN_THRESHOLD, "test"); + } + pm.add_ban_score(1, 1, "test"); + assert_eq!(outcome(&pm, 1), Outcome::Kept, "{case}"); + } + + // The must-succeed half: an ordinary peer is punished once its score + // reaches the threshold, and not before. + let (pm, _dir) = mk_pm(); + let _rx = add_peer(&pm, peer(1, "10.62.0.6:8333", Direction::Outbound)); + pm.add_ban_score(1, BAN_THRESHOLD - 1, "test"); + assert_eq!(outcome(&pm, 1), Outcome::Kept, "under the threshold"); + pm.add_ban_score(1, 1, "test"); + assert_eq!(outcome(&pm, 1), Outcome::Banned, "at the threshold"); +} + +/// The case that matters most, through the message handler: a `getblocktxn` +/// naming an index past the end of the block is `Misbehaving` in Core +/// (`SendBlockTransactions`, "getblocktxn with out-of-bounds tx indices"). +/// From an inbound onion peer it disconnects that peer and bans nothing, so +/// the next onion peer still gets in; from an ordinary peer the address is +/// banned as before. +#[test] +fn an_out_of_range_getblocktxn_from_an_inbound_onion_peer_bans_nothing() { + let (pm, _dir) = mk_pm(); + let mempool = Mempool::new(1_000_000, 0); + let block = crate::mining::miner::build_block_to_script( + &pm.chain_state, + &mempool, + bitcoin::ScriptBuf::new_op_return([0x2au8; 4]), + None, + ) + .expect("mine a regtest block"); + pm.chain_state.accept_block(&block).expect("accept the block"); + let request = || { + NetworkMessage::GetBlockTxn(bitcoin::p2p::message_compact_blocks::GetBlockTxn { + txs_request: bitcoin::bip152::BlockTransactionsRequest { + block_hash: block.block_hash(), + indexes: vec![5], + }, + }) + }; + + let mut onion = peer(1, "127.0.0.1:50000", Direction::Inbound); + onion.inbound_onion = true; + let _rx = add_peer(&pm, onion); + pm.handle_message(1, request(), InFlight::adopt(None)); + assert!(!pm.peers.read().contains_key(&1), "the onion peer is disconnected"); + assert!(pm.list_banned().is_empty(), "and nothing is banned"); + assert!( + !pm.is_addr_banned(&"127.0.0.1:50001".parse().unwrap()), + "the next onion peer is not refused" + ); + + let ordinary: SocketAddr = "10.62.0.7:50000".parse().unwrap(); + let _rx = add_peer(&pm, peer(2, "10.62.0.7:50000", Direction::Inbound)); + pm.handle_message(2, request(), InFlight::adopt(None)); + assert!(!pm.peers.read().contains_key(&2), "the ordinary peer is disconnected"); + assert!(pm.is_addr_banned(&ordinary), "and its address banned"); +} + +/// Core's `ConnectedThroughNetwork`: a peer that came in on the onion +/// listener is on the onion network, whatever its socket address, and +/// `getpeerinfo.network` says so (`net.cpp`, `rpc/net.cpp`). +#[test] +fn getpeerinfo_reports_an_inbound_onion_peer_as_onion() { + let stats = PeerStats::new(NetTotals::new()); + let mut info = peer(1, "127.0.0.1:50000", Direction::Inbound); + assert_eq!(info.to_rpc_json(&stats)["network"], "not_publicly_routable"); + info.inbound_onion = true; + assert_eq!(info.to_rpc_json(&stats)["network"], "onion"); +} + +/// The accept path records which listener a peer came in on. +#[tokio::test] +async fn a_peer_accepted_on_the_onion_listener_is_marked_inbound_onion() { + use crate::net::permissions::NetPermissions; + for inbound_onion in [false, true] { + let (pm, _dir) = mk_pm(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let dialer = tokio::net::TcpStream::connect(listener.local_addr().unwrap()); + let (dialed, accepted) = tokio::join!(dialer, listener.accept()); + let (_dialed, (accepted, peer_addr)) = (dialed.unwrap(), accepted.unwrap()); + pm.accept_inbound_with_perms(accepted, peer_addr, NetPermissions::NONE, inbound_onion); + let peers = pm.peers.read(); + let marked: Vec = peers.values().map(|h| h.info.inbound_onion).collect(); + assert_eq!(marked, vec![inbound_onion], "accepted with inbound_onion = {inbound_onion}"); + } +} diff --git a/node/src/net/mod.rs b/node/src/net/mod.rs index ad9694624..75062f31a 100644 --- a/node/src/net/mod.rs +++ b/node/src/net/mod.rs @@ -106,6 +106,16 @@ pub fn is_routable(ip: std::net::IpAddr) -> bool { } } +/// Bitcoin Core's `CNetAddr::IsLocal` (`netaddress.cpp`): IPv4 127.0.0.0/8 +/// or 0.0.0.0/8, or IPv6 `::1`. An IPv4-mapped IPv6 address is judged as the +/// IPv4 address it carries, as Core's `CNetAddr` holds it. +pub fn is_local(ip: std::net::IpAddr) -> bool { + match ip.to_canonical() { + std::net::IpAddr::V4(v4) => matches!(v4.octets()[0], 0 | 127), + std::net::IpAddr::V6(v6) => v6 == std::net::Ipv6Addr::LOCALHOST, + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/node/src/net/peer.rs b/node/src/net/peer.rs index 9cae1f83e..f92ce064e 100644 --- a/node/src/net/peer.rs +++ b/node/src/net/peer.rs @@ -276,6 +276,11 @@ pub struct PeerInfo { /// dedup, getpeerinfo, and addrman can distinguish onion peers. `None` /// for clearnet and inbound peers. pub onion_host: Option, + /// The peer came in on the listener a Tor hidden service forwards to + /// (`-bind=…=onion`): Core's `CNode::m_inbound_onion`. Its socket address + /// is the one Tor dialled from, usually loopback, and says nothing about + /// the peer. + pub inbound_onion: bool, /// The local socket address this connection is bound to /// (`getsockname()`), behind `getpeerinfo`'s `addrbind`. `None` until the /// socket exists, and for onion peers, whose local end belongs to the @@ -330,6 +335,7 @@ impl PeerInfo { permissions: crate::net::permissions::NetPermissions::NONE, bind_addr: None, onion_host: None, + inbound_onion: false, conn_type: match direction { Direction::Inbound => ConnType::Inbound, Direction::Outbound => ConnType::OutboundFullRelay, @@ -422,9 +428,10 @@ impl PeerInfo { .as_secs(); let addr_str = self.addr_string(); - // Derive the peer's network from its address. - let network = if let Some(ref onion) = self.onion_host { - let _ = onion; // suppress unused + // Derive the peer's network from its address. A peer that came in on + // the onion listener is on the onion network whatever address Tor + // dialled from: Core's `ConnectedThroughNetwork` (`net.cpp`). + let network = if self.onion_host.is_some() || self.inbound_onion { "onion" } else if !crate::net::is_routable(self.addr.ip()) { "not_publicly_routable" diff --git a/satd/tests/compact_blocks.rs b/satd/tests/compact_blocks.rs index cd888bf31..9c5b4e191 100644 --- a/satd/tests/compact_blocks.rs +++ b/satd/tests/compact_blocks.rs @@ -607,7 +607,9 @@ fn blocktxn_with_wrong_count_is_penalised() { transactions: BlockTransactions { block_hash: hash, transactions: vec![b.txdata[1].clone()] }, })); assert!(peer.wait_closed(Duration::from_secs(15)), "a mismatched blocktxn must disconnect the peer"); - poll_until(|| banned_count(&node) == 1, test_timeout(10), "and ban it"); + // The peer is on 127.0.0.1: Core disconnects a local peer without + // punishing its address, which every local peer shares. + assert_eq!(banned_count(&node), 0, "a local peer's address is not banned"); } fn getblocktxn(hash: BlockHash, indexes: Vec) -> NetworkMessage { @@ -665,7 +667,8 @@ fn getblocktxn_out_of_range_index_is_penalised() { // holds only its coinbase. peer.send(getblocktxn(best_hash(&node), vec![5])); assert!(peer.wait_closed(Duration::from_secs(15)), "an out-of-range getblocktxn must disconnect"); - poll_until(|| banned_count(&node) == 1, test_timeout(10), "and ban the peer"); + // As above: a local peer is disconnected, its address not banned. + assert_eq!(banned_count(&node), 0, "a local peer's address is not banned"); } /// Core allows at most three peers to hold a partial reconstruction of the diff --git a/satd/tests/p2p_limits.rs b/satd/tests/p2p_limits.rs index 83edd2c67..80bcea57a 100644 --- a/satd/tests/p2p_limits.rs +++ b/satd/tests/p2p_limits.rs @@ -171,8 +171,8 @@ fn an_inv_over_max_inv_sz_ends_the_connection() { let mut peer = Peer::connect(port); peer.send(NetworkMessage::Inv(vec![entry; 50_000])); assert!(peer.alive(), "50,000 entries are within the limit"); - // Misbehaviour bans the address, and every peer here is 127.0.0.1, so - // the refusal goes last. + // Misbehaviour ends the connection (a peer on 127.0.0.1 is disconnected, + // not banned), so the refusal goes last. peer.send(NetworkMessage::Inv(vec![entry; 50_001])); assert!(peer.closed_within(Duration::from_secs(30)), "50,001 entries end the connection"); node.stop(); diff --git a/satd/tests/p2p_punish.rs b/satd/tests/p2p_punish.rs new file mode 100644 index 000000000..5484ff8f7 --- /dev/null +++ b/satd/tests/p2p_punish.rs @@ -0,0 +1,261 @@ +//! What a regtest node does with a peer that misbehaves, by where the peer +//! connected from: Bitcoin Core's `MaybeDiscourageAndDisconnect` +//! (`net_processing.cpp`), exercised over raw connections. + +mod common; + +use bitcoin::consensus::{deserialize, serialize}; +use bitcoin::p2p::message::{NetworkMessage, RawNetworkMessage}; +use bitcoin::p2p::message_blockdata::Inventory; +use bitcoin::p2p::message_network::VersionMessage; +use bitcoin::p2p::{Address, Magic, ServiceFlags}; +use bitcoin::BlockHash; +use common::{TestNode, find_available_port, poll_until, test_timeout}; +use serde_json::json; +use std::io::{self, Read, Write}; +use std::net::{SocketAddr, TcpListener, TcpStream}; +use std::time::{Duration, Instant}; + +const ADDR: &str = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqdku202"; + +/// A raw peer, read on the test's own thread. +struct Peer { + stream: TcpStream, +} + +impl Peer { + /// Connect to `port` and complete the handshake, or report why not. + fn connect(port: u16) -> io::Result { + let addr: SocketAddr = format!("127.0.0.1:{port}").parse().unwrap(); + let deadline = Instant::now() + Duration::from_secs(30); + let stream = loop { + match TcpStream::connect_timeout(&addr, Duration::from_secs(2)) { + Ok(s) => break s, + Err(_) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(100)), + Err(e) => return Err(e), + } + }; + Self::handshake(stream) + } + + /// Accept the connection the node dials to `listener`: an outbound peer + /// of the node. + fn accept(listener: &TcpListener) -> Self { + listener.set_nonblocking(true).unwrap(); + let deadline = Instant::now() + Duration::from_secs(30); + let stream = loop { + match listener.accept() { + Ok((s, _)) => break s, + Err(e) if e.kind() == io::ErrorKind::WouldBlock && Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(20)) + } + Err(e) => panic!("the node never dialled us: {e}"), + } + }; + stream.set_nonblocking(false).unwrap(); + Self::handshake(stream).expect("handshake with the dialling node") + } + + fn handshake(stream: TcpStream) -> io::Result { + stream.set_nodelay(true).unwrap(); + stream.set_write_timeout(Some(Duration::from_secs(30))).unwrap(); + let mut peer = Peer { stream }; + peer.send(NetworkMessage::Version(version())); + let (mut got_version, mut got_verack) = (false, false); + while !(got_version && got_verack) { + match peer.recv(Duration::from_secs(30))? { + NetworkMessage::Version(_) => { + got_version = true; + peer.send(NetworkMessage::Verack); + } + NetworkMessage::Verack => got_verack = true, + _ => {} + } + } + Ok(peer) + } + + fn send(&mut self, msg: NetworkMessage) { + // The node may close the socket part way through. + let _ = self.stream.write_all(&serialize(&RawNetworkMessage::new(Magic::REGTEST, msg))); + } + + fn recv(&mut self, timeout: Duration) -> io::Result { + self.stream.set_read_timeout(Some(timeout)).unwrap(); + let mut header = [0u8; 24]; + self.stream.read_exact(&mut header)?; + let len = u32::from_le_bytes(header[16..20].try_into().unwrap()) as usize; + let mut buf = header.to_vec(); + buf.resize(24 + len, 0); + self.stream.read_exact(&mut buf[24..])?; + let raw: RawNetworkMessage = + deserialize(&buf).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?; + Ok(raw.into_payload()) + } + + /// Answer pings until the node sends a message `pred` accepts; `None` if + /// the connection ends or nothing arrives in time. + fn recv_until(&mut self, timeout: Duration, pred: impl Fn(&NetworkMessage) -> bool) -> Option { + let deadline = Instant::now() + timeout; + loop { + let left = deadline.saturating_duration_since(Instant::now()).max(Duration::from_millis(1)); + match self.recv(left) { + Ok(NetworkMessage::Ping(n)) => self.send(NetworkMessage::Pong(n)), + Ok(m) if pred(&m) => return Some(m), + Ok(_) => {} + Err(_) => return None, + } + } + } + + /// The node closes the connection within `timeout`. + fn closed_within(&mut self, timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + loop { + let left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + return false; + } + match self.recv(left) { + Ok(NetworkMessage::Ping(n)) => self.send(NetworkMessage::Pong(n)), + Ok(_) => {} + Err(e) if matches!(e.kind(), io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut) => return false, + Err(_) => return true, + } + } + } + + /// The node still serves this peer: a `getdata` sent now, after whatever + /// was sent before it, is answered with the block. The node handles one + /// peer's messages in order, so the answer also shows that nothing sent + /// earlier ended the connection. + fn served(&mut self, block: BlockHash) -> bool { + self.send(NetworkMessage::GetData(vec![Inventory::WitnessBlock(block)])); + self.recv_until(Duration::from_secs(30), |m| matches!(m, NetworkMessage::Block(b) if b.block_hash() == block)) + .is_some() + } +} + +fn version() -> VersionMessage { + let services = ServiceFlags::NETWORK | ServiceFlags::WITNESS; + let zero: SocketAddr = "0.0.0.0:0".parse().unwrap(); + VersionMessage { + version: 70016, + services, + timestamp: 1_760_000_000, + receiver: Address::new(&zero, ServiceFlags::NONE), + sender: Address::new(&zero, services), + nonce: rand_nonce(), + user_agent: "/p2p-punish-test/".into(), + start_height: 0, + relay: true, + } +} + +/// A distinct version nonce per connection, so the node's self-connection +/// check never mistakes one raw peer for another. +fn rand_nonce() -> u64 { + use std::hash::{BuildHasher, Hasher}; + static COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let mut h = std::collections::hash_map::RandomState::new().build_hasher(); + h.write_u64(COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed)); + h.finish() +} + +fn best_hash(node: &TestNode) -> BlockHash { + node.rpc_ok("getbestblockhash", vec![]).as_str().unwrap().parse().unwrap() +} + +/// `getblocktxn` for an index past the end of `block`: Core's +/// "getblocktxn with out-of-bounds tx indices", `Misbehaving`. +fn out_of_range_getblocktxn(block: BlockHash) -> NetworkMessage { + NetworkMessage::GetBlockTxn(bitcoin::p2p::message_compact_blocks::GetBlockTxn { + txs_request: bitcoin::bip152::BlockTransactionsRequest { block_hash: block, indexes: vec![5] }, + }) +} + +fn banned(node: &TestNode) -> Vec { + node.rpc_ok("listbanned", vec![]).as_array().unwrap().clone() +} + +/// `getpeerinfo` entries for peers that reached the node on `port`. +fn peers_on(node: &TestNode, port: u16) -> Vec { + let suffix = format!(":{port}"); + node.rpc_ok("getpeerinfo", vec![]) + .as_array() + .unwrap() + .iter() + .filter(|p| p["addrbind"].as_str().is_some_and(|b| b.ends_with(&suffix))) + .cloned() + .collect() +} + +/// Core disconnects a misbehaving peer on a local address without punishing +/// the address, since that would punish every peer on it. Every inbound +/// onion peer arrives from the address Tor dialled from, 127.0.0.1 here, as +/// does every local integration. satd banned 127.0.0.1 for `-bantime`, so one +/// such peer shut every other one out until the ban expired, across restarts. +/// +/// Each listener in turn: the peer misbehaves and is disconnected, nothing +/// is banned, and a second peer on the same listener is let in and served. +#[test] +fn a_misbehaving_onion_or_local_peer_is_disconnected_without_a_ban() { + let p2p = find_available_port(); + let onion = find_available_port(); + let mut node = TestNode::start(&[ + &format!("--port={p2p}"), + &format!("--bind=127.0.0.1:{p2p}"), + &format!("--bind=127.0.0.1:{onion}=onion"), + ]); + node.mine_blocks(1, ADDR); + let tip = best_hash(&node); + + for (listener, port, network) in [("onion", onion, "onion"), ("clearnet", p2p, "not_publicly_routable")] { + let mut peer = Peer::connect(port).unwrap_or_else(|e| panic!("{listener}: first peer: {e}")); + poll_until(|| peers_on(&node, port).len() == 1, test_timeout(20), "the peer is listed"); + // Core's `ConnectedThroughNetwork`: an inbound onion peer is on the + // onion network whatever its socket address. + assert_eq!(peers_on(&node, port)[0]["network"], json!(network), "{listener}: getpeerinfo.network"); + + peer.send(out_of_range_getblocktxn(tip)); + assert!(peer.closed_within(test_timeout(30)), "{listener}: the misbehaving peer is disconnected"); + assert_eq!(banned(&node), Vec::::new(), "{listener}: nothing is banned"); + + let mut next = Peer::connect(port) + .unwrap_or_else(|e| panic!("{listener}: a second peer on the same listener is refused: {e}")); + assert!(next.served(tip), "{listener}: the second peer is served"); + } + node.stop(); +} + +/// Core never punishes a manual connection (`addnode`, `-addnode`, +/// `-connect`): it stays connected and its address is not punished. satd +/// banned it like any other peer, and the redial loop then skipped the +/// banned address, so a `-connect` node could be left with no peers. +#[test] +fn a_misbehaving_manual_peer_stays_connected() { + let mut node = TestNode::start(&["--v2transport=0"]); + node.mine_blocks(1, ADDR); + let tip = best_hash(&node); + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let target = listener.local_addr().unwrap().to_string(); + node.rpc_ok("addnode", vec![json!(target), json!("onetry")]); + let mut peer = Peer::accept(&listener); + poll_until( + || { + node.rpc_ok("getpeerinfo", vec![]) + .as_array() + .unwrap() + .iter() + .any(|p| p["addr"] == json!(target) && p["connection_type"] == json!("manual")) + }, + test_timeout(20), + "the manual peer is listed", + ); + + peer.send(out_of_range_getblocktxn(tip)); + assert!(peer.served(tip), "the manual peer is still connected and served"); + assert_eq!(banned(&node), Vec::::new(), "nothing is banned"); + node.stop(); +}