p2p: don't ban local, inbound-onion or manual peers for misbehaviour (Core parity)

[bkeroack]

Problem

When a peer's misbehaviour score reached 100, add_ban_score (node/src/net/manager.rs) exempted only noban peers. Every other peer was disconnected and its IP banned for -bantime (24 h by default), and the ban was written to banlist.json.

Bitcoin Core handles a misbehaving peer in MaybeDiscourageAndDisconnect (src/net_processing.cpp:5083-5120), in this order:

1. a noban peer is not punished ("Not punishing noban peer", :5094-5098);
2. a manual connection is not punished either ("Not punishing manually connected peer", :5100-5104);
3. a peer on a local address (CNetAddr::IsLocal, src/netaddress.cpp:398-412: IPv4 127.0.0.0/8 or 0.0.0.0/8, IPv6 ::1) is disconnected, but its address is not discouraged, "since that would discourage all peers on the same local address" (:5106-5113). Its log line names the peer "inbound onion" or "local";
4. anyone else is disconnected and its address discouraged (:5115-5119).

satd skipped steps 2 and 3, which had these effects:

• Tor hands every inbound onion connection to the onion listener from the address it dials from, usually 127.0.0.1. One misbehaving inbound onion peer therefore banned 127.0.0.1, and the accept path (is_addr_banned before a peer is admitted) refused every onion peer, and every local client without noban, until the ban expired. The ban was kept across restarts.
• A manual peer (addnode, -addnode, -connect) was banned like any other, and the redial loop then skipped the banned address.

getpeerinfo also reported an inbound onion peer as not_publicly_routable (its loopback address). Core reports it as onion: ConnectedThroughNetwork returns NET_ONION for m_inbound_onion (src/net.cpp:602-605, :619; src/rpc/net.cpp:235).

The out-of-range getblocktxn that scores 100 matches Core: SendBlockTransactions calls Misbehaving for "getblocktxn with out-of-bounds tx indices" (src/net_processing.cpp:2598-2605). That trigger is unchanged. Only what happens to the peer afterwards changes.

Fix

• PeerManager::misbehaviour_action applies Core's ladder when the score reaches the threshold: noban → keep; manual → keep; local address (net::is_local, Core's IsLocal, with an IPv4-mapped address judged as the IPv4 address it carries) or an inbound onion peer → disconnect without a ban; otherwise → disconnect and ban for -bantime as before. add_ban_score acts on the result. When a peer is kept, its score resets to zero and a warning is logged, as Core clears m_should_discourage and logs Not punishing … peer N!.
• PeerInfo::inbound_onion (Core's m_inbound_onion) is set for peers accepted on the -bind=…=onion listener. It names the kind of peer in the disconnect log line, and getpeerinfo reports these peers with network: "onion".
• An outbound onion peer's socket address is the 0.0.0.0 placeholder that every onion peer shares. That placeholder IsLocal would count as local, and banning it said nothing about the peer. Core judges and punishes the peer by its onion address, which is not local, so satd now bans the onion host (BanTarget::Onion).

Tests

Unit tests, net::manager::peerpunish_tests (new file node/src/net/manager_peerpunish_tests.rs):

• misbehaviour_follows_cores_ladder: one case per rung. Ordinary inbound, outbound and private-network (192.168.x) peers are banned (the must-succeed half). noban, manual, and manual-on-loopback peers are kept. 127.0.0.1, 127.5.6.7, ::1, ::ffff:127.0.0.1, an inbound onion peer (on loopback, and forwarded from a LAN address) and an outbound peer on loopback are disconnected without a ban.
• a_misbehaving_outbound_onion_peer_is_banned_by_its_onion_host
• kept_peers_stay_however_often_they_misbehave: repeated misbehaviour from noban and manual peers, plus an ordinary peer kept at 99 and banned at 100.
• an_out_of_range_getblocktxn_from_an_inbound_onion_peer_bans_nothing: through handle_message. The ordinary peer is still banned.
• getpeerinfo_reports_an_inbound_onion_peer_as_onion
• a_peer_accepted_on_the_onion_listener_is_marked_inbound_onion: a real accept, with and without the onion flag.

Integration tests (satd/tests/p2p_punish.rs, new target, run against a live regtest node):

• a_misbehaving_onion_or_local_peer_is_disconnected_without_a_ban: the node listens on 127.0.0.1:P and on 127.0.0.1:O=onion. For each listener, a peer sends an out-of-range getblocktxn and is disconnected, listbanned stays empty, and a second peer on the same listener is admitted and served. getpeerinfo.network is onion on the onion listener.
• a_misbehaving_manual_peer_stays_connected: after addnode … onetry to a raw listener, the same message leaves the manual peer connected and served, and nothing is banned.

Existing tests edited in place: compact_blocks::blocktxn_with_wrong_count_is_penalised and getblocktxn_out_of_range_index_is_penalised still require the disconnect. They now require that nothing is banned, because their raw peer is on 127.0.0.1. One comment in p2p_limits.rs was updated for the same reason.

Fails without the fix: yes.

• All 6 unit tests were run on the unfixed tree, with only the new inbound_onion field added so they compile, and each failed on its own assertion. Examples: misbehaviour_follows_cores_ladder "a manual peer: left: Banned, right: Kept"; kept_peers_stay_however_often_they_misbehave "manual: left: Banned, right: Kept"; an_out_of_range_getblocktxn_from_an_inbound_onion_peer_bans_nothing "and nothing is banned"; a_misbehaving_outbound_onion_peer_is_banned_by_its_onion_host "its onion host is banned"; getpeerinfo_reports_an_inbound_onion_peer_as_onion "not_publicly_routable" vs "onion"; a_peer_accepted_on_the_onion_listener_is_marked_inbound_onion "[false]" vs "[true]".
• Both integration tests failed on the unfixed tree: a_misbehaving_manual_peer_stays_connected with "the manual peer is still connected and served", and a_misbehaving_onion_or_local_peer_is_disconnected_without_a_ban with "onion: getpeerinfo.network".
• The inbound-onion case added in review ("an inbound onion peer forwarded from another host") failed before its fix with left: Banned, right: Disconnected.
• Perturbation. I removed each guard in a single build: the manual rung, the local rung, the outbound-onion rung, the accept path's inbound_onion assignment, and the inbound_onion term in getpeerinfo.network. All six unit tests failed on their assertions. misbehaviour_follows_cores_ladder and kept_peers_stay_however_often_they_misbehave failed on "manual". an_out_of_range_getblocktxn_from_an_inbound_onion_peer_bans_nothing failed on "and nothing is banned", and the other three failed as listed above. With only the local rung removed, a_misbehaving_onion_or_local_peer_is_disconnected_without_a_ban failed on "onion: nothing is banned".

Notes

• Ordinary peers are still banned, not discouraged. Core keeps discouraged addresses in an in-memory filter, admits them while inbound slots are free (src/net.cpp:1810-1816), and does not list them in listbanned. satd keeps its persistent -bantime ban for the last rung. Changing that would change a default and what listbanned and banlist.json contain, so it is out of scope here.
• Score model. satd still accumulates partial scores (for example 10 for a rejected block and 1 to 10 for rejected transactions) up to 100. Core now treats every Misbehaving as reaching the threshold. Only what happens at the threshold changes here. A noban peer now accumulates a score like any other, logs Not punishing noban peer N! at warn level when the score reaches 100 (as Core logs), and its score resets. Before, every increment was skipped with a debug line.
• One deliberate step past Core: inbound onion peers are never banned. Core's third rung tests the address alone (pnode.addr.IsLocal(), src/net_processing.cpp:5106), and m_inbound_onion only picks the word in its log line. With Tor on the same host the two agree. With Tor on another host (-bind=<lan address>=onion), every onion peer arrives from that host's address. Core discourages that address, and a discouraged address is still admitted while inbound slots are free. satd's equivalent would be a -bantime ban that refuses every onion peer. So satd disconnects an inbound onion peer without a ban whatever address it arrives from (is_local(addr) || inbound_onion), as the rung is described for this change. Removing || info.inbound_onion in misbehaviour_action restores Core's exact test.
• Onion dials and onion address gossip do not consult the ban list, so a banned onion host can still be dialled again. This was already true of setban <host>.onion, and of the 0.0.0.0/32 ban this used to add for an outbound onion peer. It is unchanged here and left for the dial-path work. Core skips a banned or discouraged address only for automatic dials (OpenNetworkConnection, src/net.cpp:3023-3027).
• In Core's last rung, DisconnectNode(pnode.addr) also disconnects every other peer on the same address (src/net.cpp:3839-3842). satd still disconnects only the peer that misbehaved, and the ban refuses the others when they next connect. Unchanged here.

Release notes

satd now treats a misbehaving peer the way Bitcoin Core does. Peers with the noban permission and manual connections (addnode, -addnode, -connect) are never disconnected or banned for misbehaviour. A peer on a local address, and every peer that connects through the node's Tor hidden service, is disconnected but its address is not banned. Before this change, one such peer's misbehaviour could ban 127.0.0.1 for -bantime (24 hours by default, kept across restarts), which refused all onion and local P2P clients without noban for that time. getpeerinfo now reports peers that came in through the hidden service with network: "onion".

🤖 Generated with Claude Code