From 40eb3f266faea569038346e65acecaafd97144fb Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Tue, 10 Dec 2024 08:45:58 +0800 Subject: [PATCH] fix(p2p): log user agent on banning a peer (#5056) --- src/chain_sync/network_context.rs | 2 ++ src/libp2p/discovery.rs | 2 +- src/libp2p/peer_manager.rs | 24 ++++++++++++++--- src/libp2p/service.rs | 44 ++++++++++++++++++++++++++----- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/chain_sync/network_context.rs b/src/chain_sync/network_context.rs index d39c9e842796..6b5f1aa50c39 100644 --- a/src/chain_sync/network_context.rs +++ b/src/chain_sync/network_context.rs @@ -451,10 +451,12 @@ where // Internal libp2p error, score failure for peer and potentially disconnect match e { RequestResponseError::UnsupportedProtocols => { + // refactor this into Networkevent if user agent logging is critical here peer_manager .ban_peer_with_default_duration( peer_id, "ChainExchange protocol unsupported", + |_| None, ) .await; } diff --git a/src/libp2p/discovery.rs b/src/libp2p/discovery.rs index 2903ced87fe2..94a5d2d41aa6 100644 --- a/src/libp2p/discovery.rs +++ b/src/libp2p/discovery.rs @@ -226,7 +226,7 @@ pub struct DiscoveryBehaviour { /// Keeps hash set of peers connected. peers: HashSet, /// Keeps hash map of peers and their information. - peer_info: HashMap, + pub(crate) peer_info: HashMap, /// Number of connected peers to pause discovery on. target_peer_count: u64, /// Seed peers diff --git a/src/libp2p/peer_manager.rs b/src/libp2p/peer_manager.rs index 272a61834f0a..a55ac8538fff 100644 --- a/src/libp2p/peer_manager.rs +++ b/src/libp2p/peer_manager.rs @@ -286,15 +286,21 @@ impl PeerManager { peer: PeerId, reason: impl Into, duration: Option, + get_user_agent: impl Fn(&PeerId) -> Option, ) { if self.is_peer_protected(&peer) { return; } let mut locked = self.peer_ban_list.write().await; locked.insert(peer, duration.and_then(|d| Instant::now().checked_add(d))); + let user_agent = get_user_agent(&peer); if let Err(e) = self .peer_ops_tx - .send_async(PeerOperation::Ban(peer, reason.into())) + .send_async(PeerOperation::Ban { + peer, + user_agent, + reason: reason.into(), + }) .await { warn!("ban_peer err: {e}"); @@ -302,9 +308,15 @@ impl PeerManager { } /// Bans a peer with the default duration(`1h`) - pub async fn ban_peer_with_default_duration(&self, peer: PeerId, reason: impl Into) { + pub async fn ban_peer_with_default_duration( + &self, + peer: PeerId, + reason: impl Into, + get_user_agent: impl Fn(&PeerId) -> Option, + ) { const BAN_PEER_DURATION: Duration = Duration::from_secs(60 * 60); //1h - self.ban_peer(peer, reason, Some(BAN_PEER_DURATION)).await + self.ban_peer(peer, reason, Some(BAN_PEER_DURATION), get_user_agent) + .await } pub async fn peer_operation_event_loop_task(self: Arc) -> anyhow::Result<()> { @@ -385,6 +397,10 @@ fn log_time(info: &mut PeerInfo, dur: Duration) { } pub enum PeerOperation { - Ban(PeerId, String), + Ban { + peer: PeerId, + user_agent: Option, + reason: String, + }, Unban(PeerId), } diff --git a/src/libp2p/service.rs b/src/libp2p/service.rs index 50ae3bd8aabd..4d8589f030ee 100644 --- a/src/libp2p/service.rs +++ b/src/libp2p/service.rs @@ -40,7 +40,7 @@ use tracing::{debug, error, info, trace, warn}; use super::{ chain_exchange::{make_chain_exchange_response, ChainExchangeRequest, ChainExchangeResponse}, - discovery::DerivedDiscoveryBehaviourEvent, + discovery::{DerivedDiscoveryBehaviourEvent, PeerInfo}, ForestBehaviour, ForestBehaviourEvent, Libp2pConfig, }; use crate::libp2p::{ @@ -405,10 +405,15 @@ fn handle_peer_ops( ) { use PeerOperation::*; match peer_ops { - Ban(peer, reason) => { + Ban { + peer, + user_agent, + reason, + } => { // Do not ban bootstrap nodes if !bootstrap_peers.contains_key(&peer) { - debug!(%peer, %reason, "Banning peer"); + let user_agent = user_agent.unwrap_or_default(); + debug!(%peer, %user_agent, %reason, "Banning peer"); swarm.behaviour_mut().blocked_peers.block_peer(peer); } } @@ -572,6 +577,7 @@ async fn handle_network_message( } async fn handle_discovery_event( + peer_info_map: &HashMap, discovery_out: DiscoveryEvent, network_sender_out: &Sender, peer_manager: &PeerManager, @@ -596,13 +602,18 @@ async fn handle_discovery_event( let protocols = HashSet::from_iter(info.protocols.iter().map(|p| p.to_string())); if !protocols.contains(super::hello::HELLO_PROTOCOL_NAME) { peer_manager - .ban_peer_with_default_duration(*peer_id, "hello protocol unsupported") + .ban_peer_with_default_duration( + *peer_id, + "hello protocol unsupported", + |p| get_user_agent(peer_info_map, p), + ) .await; } else if !protocols.contains(super::chain_exchange::CHAIN_EXCHANGE_PROTOCOL_NAME) { peer_manager .ban_peer_with_default_duration( *peer_id, "chain exchange protocol unsupported", + |p| get_user_agent(peer_info_map, p), ) .await; } @@ -667,6 +678,7 @@ async fn handle_gossip_event( } async fn handle_hello_event( + peer_info_map: &HashMap, hello: &mut HelloBehaviour, event: request_response::Event, peer_manager: &PeerManager, @@ -698,6 +710,7 @@ async fn handle_hello_event( "Genesis hash mismatch: {} received, {genesis_cid} expected", request.genesis_cid ), + |p| get_user_agent(peer_info_map, p), ) .await; } else { @@ -741,7 +754,9 @@ async fn handle_hello_event( match error { request_response::OutboundFailure::UnsupportedProtocols => { peer_manager - .ban_peer_with_default_duration(peer, "Hello protocol unsupported") + .ban_peer_with_default_duration(peer, "Hello protocol unsupported", |p| { + get_user_agent(peer_info_map, p) + }) .await; } _ => { @@ -877,14 +892,22 @@ async fn handle_forest_behaviour_event( { match event { ForestBehaviourEvent::Discovery(discovery_out) => { - handle_discovery_event(discovery_out, network_sender_out, peer_manager).await + handle_discovery_event( + &swarm.behaviour().discovery.peer_info, + discovery_out, + network_sender_out, + peer_manager, + ) + .await } ForestBehaviourEvent::Gossipsub(e) => { handle_gossip_event(e, network_sender_out, pubsub_block_str, pubsub_msg_str).await } ForestBehaviourEvent::Hello(rr_event) => { + let behaviour_mut = swarm.behaviour_mut(); handle_hello_event( - &mut swarm.behaviour_mut().hello, + &behaviour_mut.discovery.peer_info, + &mut behaviour_mut.hello, rr_event, peer_manager, genesis_cid, @@ -922,3 +945,10 @@ async fn emit_event(sender: &Sender, event: NetworkEvent) { error!("Failed to emit event: Network channel receiver has been dropped"); } } + +fn get_user_agent(peer_info_map: &HashMap, peer: &PeerId) -> Option { + peer_info_map + .get(peer) + .and_then(|i| i.identify_info.as_ref()) + .map(|i| i.agent_version.clone()) +}