Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(p2p): log user agent on banning a peer #5056

Merged
merged 2 commits into from
Dec 10, 2024
Merged

fix(p2p): log user agent on banning a peer #5056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a7260c
fix(p2p): log user agent on banning a peer
hanabi1224 Dec 9, 2024
26a82dd
Merge branch 'main' into hm/log-user-agent
hanabi1224 Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/chain_sync/network_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have a tracking issue, otherwise it's unlikely anyone will pick it up when browsing the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5064

peer_manager
.ban_peer_with_default_duration(
peer_id,
"ChainExchange protocol unsupported",
|_| None,
)
.await;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libp2p/discovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub struct DiscoveryBehaviour {
/// Keeps hash set of peers connected.
peers: HashSet<PeerId>,
/// Keeps hash map of peers and their information.
peer_info: HashMap<PeerId, PeerInfo>,
pub(crate) peer_info: HashMap<PeerId, PeerInfo>,
/// Number of connected peers to pause discovery on.
target_peer_count: u64,
/// Seed peers
Expand Down
24 changes: 20 additions & 4 deletions src/libp2p/peer_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,25 +286,37 @@ impl PeerManager {
peer: PeerId,
reason: impl Into<String>,
duration: Option<Duration>,
get_user_agent: impl Fn(&PeerId) -> Option<String>,
) {
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}");
}
}

/// Bans a peer with the default duration(`1h`)
pub async fn ban_peer_with_default_duration(&self, peer: PeerId, reason: impl Into<String>) {
pub async fn ban_peer_with_default_duration(
&self,
peer: PeerId,
reason: impl Into<String>,
get_user_agent: impl Fn(&PeerId) -> Option<String>,
) {
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<Self>) -> anyhow::Result<()> {
Expand Down Expand Up @@ -385,6 +397,10 @@ fn log_time(info: &mut PeerInfo, dur: Duration) {
}

pub enum PeerOperation {
Ban(PeerId, String),
Ban {
peer: PeerId,
user_agent: Option<String>,
reason: String,
},
Unban(PeerId),
}
44 changes: 37 additions & 7 deletions src/libp2p/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -572,6 +577,7 @@ async fn handle_network_message(
}

async fn handle_discovery_event(
peer_info_map: &HashMap<PeerId, PeerInfo>,
discovery_out: DiscoveryEvent,
network_sender_out: &Sender<NetworkEvent>,
peer_manager: &PeerManager,
Expand All @@ -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;
}
Expand Down Expand Up @@ -667,6 +678,7 @@ async fn handle_gossip_event(
}

async fn handle_hello_event(
peer_info_map: &HashMap<PeerId, PeerInfo>,
hello: &mut HelloBehaviour,
event: request_response::Event<HelloRequest, HelloResponse, HelloResponse>,
peer_manager: &PeerManager,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
_ => {
Expand Down Expand Up @@ -877,14 +892,22 @@ async fn handle_forest_behaviour_event<DB>(
{
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,
Expand Down Expand Up @@ -922,3 +945,10 @@ async fn emit_event(sender: &Sender<NetworkEvent>, event: NetworkEvent) {
error!("Failed to emit event: Network channel receiver has been dropped");
}
}

fn get_user_agent(peer_info_map: &HashMap<PeerId, PeerInfo>, peer: &PeerId) -> Option<String> {
peer_info_map
.get(peer)
.and_then(|i| i.identify_info.as_ref())
.map(|i| i.agent_version.clone())
}
Loading