diff --git a/chain/network-primitives/src/blacklist.rs b/chain/network-primitives/src/blacklist.rs index 078d857b428..5ed5fd5cf70 100644 --- a/chain/network-primitives/src/blacklist.rs +++ b/chain/network-primitives/src/blacklist.rs @@ -16,10 +16,12 @@ impl Blacklist { /// - `blacklist` - list of strings in one of the following format: /// - "IP" - for example 127.0.0.1 - if only IP is provided we will block all ports /// - "IP:PORT - for example 127.0.0.1:2134 - pub fn from_iter(blacklist: impl IntoIterator) -> Self { + pub fn from_iter + std::fmt::Display>( + blacklist: impl IntoIterator, + ) -> Self { let mut result = Self::default(); for addr in blacklist { - if result.add(&addr).is_err() { + if result.add(addr.as_ref()).is_err() { tracing::warn!(target: "network", "{}: invalid blacklist pattern, ignoring", addr); } } diff --git a/chain/network-primitives/src/config.rs b/chain/network-primitives/src/config.rs index a391282f3bb..afec463262a 100644 --- a/chain/network-primitives/src/config.rs +++ b/chain/network-primitives/src/config.rs @@ -1,4 +1,3 @@ -use crate::blacklist::Blacklist; use crate::network_protocol::PeerInfo; use crate::types::ROUTED_MESSAGE_TTL; use near_crypto::{KeyType, PublicKey, SecretKey}; @@ -57,7 +56,7 @@ pub struct NetworkConfig { pub push_info_period: Duration, /// Peers on blacklist by IP:Port. /// Nodes will not accept or try to establish connection to such peers. - pub blacklist: Blacklist, + pub blacklist: Vec, /// Flag to disable outbound connections. When this flag is active, nodes will not try to /// establish connection with other nodes, but will accept incoming connection if other requirements /// are satisfied. @@ -97,7 +96,7 @@ impl NetworkConfig { max_routes_to_store: 1, highest_peer_horizon: 5, push_info_period: Duration::from_millis(100), - blacklist: Blacklist::default(), + blacklist: vec![], outbound_disabled: false, archive: false, } diff --git a/chain/network-primitives/src/types.rs b/chain/network-primitives/src/types.rs index ea34734f442..07b5933c32b 100644 --- a/chain/network-primitives/src/types.rs +++ b/chain/network-primitives/src/types.rs @@ -225,6 +225,7 @@ pub enum ReasonForBan { EpochSyncNoResponse = 11, EpochSyncInvalidResponse = 12, EpochSyncInvalidFinalizationResponse = 13, + Blacklisted = 14, } /// Banning signal sent from Peer instance to PeerManager diff --git a/chain/network/src/peer_manager/peer_manager_actor.rs b/chain/network/src/peer_manager/peer_manager_actor.rs index d80457a21f5..d9ed2f18867 100644 --- a/chain/network/src/peer_manager/peer_manager_actor.rs +++ b/chain/network/src/peer_manager/peer_manager_actor.rs @@ -30,7 +30,7 @@ use near_network_primitives::types::{ RawRoutedMessage, ReasonForBan, RoutedMessage, RoutedMessageBody, RoutedMessageFrom, StateResponseInfo, }; -use near_network_primitives::types::{EdgeState, PartialEdgeInfo}; +use near_network_primitives::types::{Blacklist, EdgeState, PartialEdgeInfo}; use near_performance_metrics::framed_write::FramedWrite; use near_performance_metrics_macros::perf; use near_primitives::checked_feature; @@ -268,7 +268,11 @@ impl PeerManagerActor { view_client_addr: Recipient, routing_table_addr: Addr, ) -> Result> { - let peer_store = PeerStore::new(store.clone(), &config.boot_nodes)?; + let peer_store = PeerStore::new( + store.clone(), + &config.boot_nodes, + Blacklist::from_iter(config.blacklist.iter()), + )?; debug!(target: "network", len = peer_store.len(), boot_nodes = config.boot_nodes.len(), "Found known peers"); debug!(target: "network", blacklist = ?config.blacklist, "Blacklist"); @@ -1977,7 +1981,7 @@ impl PeerManagerActor { let _d = delay_detector::DelayDetector::new(|| "consolidate".into()); // Check if this is a blacklisted peer. - if (msg.peer_info.addr.as_ref()).map_or(true, |addr| self.config.blacklist.contains(addr)) { + if (msg.peer_info.addr.as_ref()).map_or(true, |addr| self.peer_store.is_blacklisted(addr)) { debug!(target: "network", peer_info = ?msg.peer_info, "Dropping connection from blacklisted peer or unknown address"); return RegisterPeerResponse::Reject; } diff --git a/chain/network/src/peer_manager/peer_store.rs b/chain/network/src/peer_manager/peer_store.rs index 78116486f61..7de1c3fcdff 100644 --- a/chain/network/src/peer_manager/peer_store.rs +++ b/chain/network/src/peer_manager/peer_store.rs @@ -1,6 +1,6 @@ use borsh::{BorshDeserialize, BorshSerialize}; use near_network_primitives::types::{ - KnownPeerState, KnownPeerStatus, NetworkConfig, PeerInfo, ReasonForBan, + Blacklist, KnownPeerState, KnownPeerStatus, NetworkConfig, PeerInfo, ReasonForBan, }; use near_primitives::network::PeerId; use near_primitives::time::{Clock, Utc}; @@ -13,7 +13,7 @@ use std::collections::HashMap; use std::error::Error; use std::net::SocketAddr; use std::ops::Not; -use tracing::{debug, error}; +use tracing::{debug, error, info}; /// Level of trust we have about a new (PeerId, Addr) pair. #[derive(Eq, PartialEq, Debug, Clone)] @@ -49,12 +49,14 @@ pub struct PeerStore { // It can happens that some peers don't have known address, so // they will not be present in this list, otherwise they will be present. addr_peers: HashMap, + blacklist: Blacklist, } impl PeerStore { pub(crate) fn new( store: Store, boot_nodes: &[PeerInfo], + blacklist: Blacklist, ) -> Result> { // A mapping from `PeerId` to `KnownPeerState`. let mut peerid_2_state = HashMap::default(); @@ -92,16 +94,34 @@ impl PeerStore { for (key, value) in store.iter(ColPeers) { let peer_id: PeerId = PeerId::try_from_slice(key.as_ref())?; let peer_state: KnownPeerState = KnownPeerState::try_from_slice(value.as_ref())?; + // Mark loaded node last seen to now, to avoid deleting them as soon as they are loaded. + let last_seen = now; + + // If it’s already banned, keep it banned. If it’s blacklisted, ban + // it. Otherwise, it’s not connected. + let status = if peer_state.status.is_banned() { + peer_state.status + } else { + let is_blacklisted = peer_state + .peer_info + .addr + .as_ref() + .map_or(false, |addr| blacklist.contains(addr)); + if is_blacklisted { + info!(target: "network", "Banning {:?} because address is blacklisted", + peer_state.peer_info); + KnownPeerStatus::Banned(ReasonForBan::Blacklisted, now) + } else { + KnownPeerStatus::NotConnected + } + }; let peer_state = KnownPeerState { peer_info: peer_state.peer_info, first_seen: peer_state.first_seen, - last_seen: now, - status: match peer_state.status { - banned_status @ KnownPeerStatus::Banned(_, _) => banned_status, - _ => KnownPeerStatus::NotConnected, - }, + last_seen, + status, }; match peerid_2_state.entry(peer_id) { @@ -126,7 +146,11 @@ impl PeerStore { } } } - Ok(PeerStore { store, peer_states: peerid_2_state, addr_peers: addr_2_peer }) + Ok(PeerStore { store, peer_states: peerid_2_state, addr_peers: addr_2_peer, blacklist }) + } + + pub fn is_blacklisted(&self, addr: &SocketAddr) -> bool { + self.blacklist.contains(addr) } pub(crate) fn len(&self) -> usize { @@ -314,10 +338,7 @@ impl PeerStore { Ok(()) } - /// Add list of peers into store. - /// When verified is true is because we establish direct connection with such peer and know - /// for sure its identity. If we receive a list of peers from another node in the network - /// by default all of them are unverified. + /// Adds a peer into the store with given trust level. fn add_peer( &mut self, peer_info: PeerInfo, @@ -367,22 +388,43 @@ impl PeerStore { Ok(()) } + /// Adds indirect peers into the store. + /// + /// Indirect peers are ones we’ve received from other peers and thus we + /// don’t know if their identity is correct. pub(crate) fn add_indirect_peers( &mut self, peers: impl Iterator, ) -> Result<(), Box> { + let mut total: usize = 0; + let mut blacklisted: usize = 0; for peer_info in peers { - self.add_peer(peer_info, TrustLevel::Indirect)?; + total += 1; + let is_blacklisted = + peer_info.addr.as_ref().map_or(false, |addr| self.blacklist.contains(addr)); + if is_blacklisted { + blacklisted += 1; + } else { + self.add_peer(peer_info, TrustLevel::Indirect)?; + } + } + if blacklisted != 0 { + info!(target: "network", "Ignored {} blacklisted peers out of {} indirect peer(s)", + blacklisted, total); } Ok(()) } + /// Adds a peer into the store with given trust level. To add indirect + /// peers, use [`add_indirect_peers`] instead. pub(crate) fn add_trusted_peer( &mut self, peer_info: PeerInfo, trust_level: TrustLevel, ) -> Result<(), Box> { - self.add_peer(peer_info, trust_level) + debug_assert_ne!(TrustLevel::Indirect, trust_level); + self.add_peer(peer_info, trust_level)?; + Ok(()) } } @@ -391,7 +433,7 @@ pub fn iter_peers_from_store(store: Store, f: F) where F: Fn((&PeerId, &KnownPeerState)), { - let peer_store = PeerStore::new(store, &[]).unwrap(); + let peer_store = PeerStore::new(store, &[], Default::default()).unwrap(); peer_store.iter().for_each(f); } @@ -400,6 +442,7 @@ mod test { use near_crypto::{KeyType, SecretKey}; use near_store::create_store; use near_store::test_utils::create_test_store; + use std::collections::HashSet; use std::net::{Ipv4Addr, SocketAddrV4}; use super::*; @@ -432,14 +475,15 @@ mod test { let boot_nodes = vec![peer_info_a, peer_info_to_ban.clone()]; { let store = create_store(tmp_dir.path()); - let mut peer_store = PeerStore::new(store, &boot_nodes).unwrap(); + let mut peer_store = PeerStore::new(store, &boot_nodes, Default::default()).unwrap(); assert_eq!(peer_store.healthy_peers(3).len(), 2); peer_store.peer_ban(&peer_info_to_ban.id, ReasonForBan::Abusive).unwrap(); assert_eq!(peer_store.healthy_peers(3).len(), 1); } { let store_new = create_store(tmp_dir.path()); - let peer_store_new = PeerStore::new(store_new, &boot_nodes).unwrap(); + let peer_store_new = + PeerStore::new(store_new, &boot_nodes, Default::default()).unwrap(); assert_eq!(peer_store_new.healthy_peers(3).len(), 1); } } @@ -452,7 +496,7 @@ mod test { let boot_nodes = vec![peer_info_a, peer_info_to_ban]; { let store = create_store(tmp_dir.path()); - let peer_store = PeerStore::new(store, &boot_nodes).unwrap(); + let peer_store = PeerStore::new(store, &boot_nodes, Default::default()).unwrap(); assert!(peer_store.unconnected_peer(|_| false).is_some()); assert!(peer_store.unconnected_peer(|_| true).is_none()); } @@ -500,7 +544,7 @@ mod test { #[test] fn handle_peer_id_change() { let store = create_test_store(); - let mut peer_store = PeerStore::new(store, &[]).unwrap(); + let mut peer_store = PeerStore::new(store, &[], Default::default()).unwrap(); let peers_id = (0..2).map(|ix| get_peer_id(format!("node{}", ix))).collect::>(); let addr = get_addr(0); @@ -523,7 +567,7 @@ mod test { #[test] fn dont_handle_address_change() { let store = create_test_store(); - let mut peer_store = PeerStore::new(store, &[]).unwrap(); + let mut peer_store = PeerStore::new(store, &[], Default::default()).unwrap(); let peers_id = (0..1).map(|ix| get_peer_id(format!("node{}", ix))).collect::>(); let addrs = (0..2).map(get_addr).collect::>(); @@ -541,7 +585,7 @@ mod test { #[test] fn check_add_peers_overriding() { let store = create_test_store(); - let mut peer_store = PeerStore::new(store.clone(), &[]).unwrap(); + let mut peer_store = PeerStore::new(store.clone(), &[], Default::default()).unwrap(); // Five peers: A, B, C, D, X, T let peers_id = (0..6).map(|ix| get_peer_id(format!("node{}", ix))).collect::>(); @@ -616,8 +660,77 @@ mod test { assert!(check_integrity(&peer_store)); // Check we are able to recover from store previous signed connection - let peer_store_2 = PeerStore::new(store, &[]).unwrap(); + let peer_store_2 = PeerStore::new(store, &[], Default::default()).unwrap(); assert!(check_exist(&peer_store_2, &peers_id[0], Some((addrs[0], TrustLevel::Indirect)))); assert!(check_integrity(&peer_store_2)); } + + #[test] + fn check_ignore_blacklisted_peers() { + fn assert_peers(peer_store: &PeerStore, expected: &[&PeerId], banned: &[&PeerId]) { + let expected: HashSet<&PeerId> = HashSet::from_iter(expected.iter().cloned()); + let got = HashSet::from_iter(peer_store.peer_states.keys()); + assert_eq!(expected, got); + + let expected: HashSet<&PeerId> = HashSet::from_iter(banned.iter().cloned()); + let got = + HashSet::from_iter(peer_store.peer_states.iter().filter_map(|(key, value)| { + if value.status.is_banned() { + Some(key) + } else { + None + } + })); + assert_eq!(expected, got); + } + + let ids = (0..6).map(|ix| get_peer_id(format!("node{}", ix))).collect::>(); + let store = create_test_store(); + + // Populate store with three peers. + { + let mut peer_store = PeerStore::new(store.clone(), &[], Default::default()).unwrap(); + peer_store + .add_indirect_peers( + [ + get_peer_info(ids[0].clone(), None), + get_peer_info(ids[1].clone(), Some(get_addr(1))), + get_peer_info(ids[2].clone(), Some(get_addr(2))), + ] + .into_iter(), + ) + .unwrap(); + assert_peers(&peer_store, &[&ids[0], &ids[1], &ids[2]], &[]); + } + + // Peers without address aren’t saved but make sure the rest are read + // correctly. + { + let peer_store = PeerStore::new(store.clone(), &[], Default::default()).unwrap(); + assert_peers(&peer_store, &[&ids[1], &ids[2]], &[]); + } + + // Blacklist one of the existing peers and one new peer. + { + let blacklist = Blacklist::from_iter( + ["127.0.0.1:2".to_string(), "127.0.0.1:5".to_string()].into_iter(), + ); + let mut peer_store = PeerStore::new(store.clone(), &[], blacklist).unwrap(); + // Peer 127.0.0.1:2 is there but is banned. + assert_peers(&peer_store, &[&ids[1], &ids[2]], &[&ids[2]]); + + peer_store + .add_indirect_peers( + [ + get_peer_info(ids[3].clone(), None), + get_peer_info(ids[4].clone(), Some(get_addr(4))), + get_peer_info(ids[5].clone(), Some(get_addr(5))), + ] + .into_iter(), + ) + .unwrap(); + // Peer 127.0.0.1:5 is ignored and never added. + assert_peers(&peer_store, &[&ids[1], &ids[2], &ids[3], &ids[4]], &[&ids[2]]); + } + } } diff --git a/integration-tests/src/tests/network/runner.rs b/integration-tests/src/tests/network/runner.rs index cde2ffd4db3..d614d44990d 100644 --- a/integration-tests/src/tests/network/runner.rs +++ b/integration-tests/src/tests/network/runner.rs @@ -29,7 +29,7 @@ use near_network::types::{NetworkRequests, NetworkResponses}; use near_network::types::{PeerManagerMessageRequest, PeerManagerMessageResponse}; use near_network::PeerManagerActor; use near_network_primitives::types::{ - Blacklist, NetworkConfig, OutboundTcpConnect, PeerInfo, ROUTED_MESSAGE_TTL, + NetworkConfig, OutboundTcpConnect, PeerInfo, ROUTED_MESSAGE_TTL, }; use near_primitives::network::PeerId; use near_primitives::types::{AccountId, ValidatorId}; @@ -627,13 +627,17 @@ impl Runner { .collect(), ); - let blacklist = Blacklist::from_iter(test_config.blacklist.iter().map(|x| { - if let Some(x) = x { - format!("127.0.0.1:{}", ports[*x]) - } else { - "127.0.0.1".to_string() - } - })); + let blacklist = test_config + .blacklist + .iter() + .map(|x| { + if let Some(x) = x { + format!("127.0.0.1:{}", ports[*x]) + } else { + "127.0.0.1".to_string() + } + }) + .collect(); let mut network_config = NetworkConfig::from_seed(accounts_id[node_id].as_ref(), ports[node_id]); diff --git a/nearcore/src/config.rs b/nearcore/src/config.rs index bfd0a32880f..59dec878021 100644 --- a/nearcore/src/config.rs +++ b/nearcore/src/config.rs @@ -25,7 +25,7 @@ use near_crypto::{InMemorySigner, KeyFile, KeyType, PublicKey, Signer}; #[cfg(feature = "json_rpc")] use near_jsonrpc::RpcConfig; use near_network::test_utils::open_port; -use near_network_primitives::types::{Blacklist, NetworkConfig, ROUTED_MESSAGE_TTL}; +use near_network_primitives::types::{NetworkConfig, ROUTED_MESSAGE_TTL}; use near_primitives::account::{AccessKey, Account}; use near_primitives::hash::CryptoHash; #[cfg(test)] @@ -739,7 +739,7 @@ impl NearConfig { max_routes_to_store: MAX_ROUTES_TO_STORE, highest_peer_horizon: HIGHEST_PEER_HORIZON, push_info_period: Duration::from_millis(100), - blacklist: Blacklist::from_iter(config.network.blacklist), + blacklist: config.network.blacklist, outbound_disabled: false, archive: config.archive, },