From 58cacec0aae4e2cffa6937f353cace0a27b9d34e Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Wed, 8 Feb 2023 17:10:55 +0100 Subject: [PATCH 1/8] feat: add bootstrap banning client message --- massa-bootstrap/src/establisher.rs | 23 +------- massa-bootstrap/src/server.rs | 58 ++++++++++++++++++- massa-bootstrap/src/tests/mock_establisher.rs | 6 +- .../base_config/bootstrap_whitelist.json | 3 +- 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/massa-bootstrap/src/establisher.rs b/massa-bootstrap/src/establisher.rs index 661635bd136..38cdb811d3a 100644 --- a/massa-bootstrap/src/establisher.rs +++ b/massa-bootstrap/src/establisher.rs @@ -14,13 +14,8 @@ pub mod types { #[cfg(not(test))] /// Connection types pub mod types { - use crate::tools::normalize_ip; use massa_time::MassaTime; - use std::{ - collections::HashSet, - io, - net::{IpAddr, SocketAddr}, - }; + use std::{io, net::SocketAddr}; use tokio::{ net::{TcpListener, TcpStream}, time::timeout, @@ -40,23 +35,9 @@ pub mod types { impl DefaultListener { /// Accepts a new incoming connection from this listener. - pub async fn accept( - &mut self, - whitelist: &Option>, - blacklist: &Option>, - ) -> io::Result<(Duplex, SocketAddr)> { + pub async fn accept(&mut self) -> io::Result<(Duplex, SocketAddr)> { // accept let (sock, mut remote_addr) = self.0.accept().await?; - let ip = normalize_ip(remote_addr.ip()); - if let Some(blacklist) = blacklist && blacklist.contains(&ip) { - return Err(io::Error::new(io::ErrorKind::Other, "IP is blacklisted")); - } - if let Some(whitelist) = whitelist && !whitelist.contains(&ip) { - return Err(io::Error::new( - io::ErrorKind::Other, - "A whitelist exists and the IP is not whitelisted", - )); - } // normalize address remote_addr.set_ip(remote_addr.ip().to_canonical()); Ok((sock, remote_addr)) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 21c2f5c3b01..1d4fe804c8e 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -15,6 +15,7 @@ use massa_time::MassaTime; use parking_lot::RwLock; use std::{ collections::{hash_map, HashMap, HashSet}, + io, net::{IpAddr, SocketAddr}, path::PathBuf, sync::Arc, @@ -28,6 +29,7 @@ use crate::{ messages::{BootstrapClientMessage, BootstrapServerMessage}, server_binder::BootstrapServerBinder, tools::normalize_ip, + types::Duplex, BootstrapConfig, Establisher, }; @@ -187,11 +189,18 @@ impl BootstrapServer { } // listener - res_connection = listener.accept(&whitelist, &blacklist) => { + res_connection = listener.accept() => { let (dplx, remote_addr) = if res_connection.is_ok() { - res_connection.unwrap() + let (dplx, remote_addr) = res_connection.unwrap(); + // check whether incoming peer IP is allowed or return an error which is ignored + let res = self.is_ip_allowed(dplx, remote_addr, &whitelist, &blacklist).await; + if res.is_ok() { + res.unwrap() + } else { + continue + } } else { - continue; + continue }; if bootstrap_sessions.len() < self.bootstrap_config.max_simultaneous_bootstraps.try_into().map_err(|_| BootstrapError::GeneralError("Fail to convert u32 to usize".to_string()))? { @@ -300,6 +309,49 @@ impl BootstrapServer { Ok(()) } + + // whether the peer IP address is banned + async fn is_ip_allowed( + &self, + duplex: Duplex, + remote_address: SocketAddr, + whitelist: &Option>, + blacklist: &Option>, + ) -> io::Result<(Duplex, SocketAddr)> { + let ip = normalize_ip(remote_address.ip()); + // whether the peer IP address is blacklisted, send back an error message + if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { + let config = self.bootstrap_config.clone(); + let mut server = BootstrapServerBinder::new(duplex, self.keypair.clone(), config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); + let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { + error: format!("IP {} is blacklisted.", &ip) + })).await { + Err(_) => Err(std::io::Error::new(std::io::ErrorKind::TimedOut, format!("IP {} is blacklisted send timed out", &ip)).into()), + Ok(Err(e)) => Err(e), + Ok(Ok(_)) => Ok(()), + }; + + massa_trace!("bootstrap.lib.run.select.accept.refuse_blacklisted", {"remote_addr": remote_address}); + return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, format!("IP {} is blacklisted.", &ip))); + } + + // whether the peer IP address is not present in the whitelist, send back an error message + if let Some(ip_list) = &whitelist && !ip_list.contains(&ip){ + let config = self.bootstrap_config.clone(); + let mut server = BootstrapServerBinder::new(duplex, self.keypair.clone(), config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); + let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { + error: format!("A whitelist exists and the IP {} is not whitelisted.", &ip) + })).await { + Err(_) => Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, format!("A whitelist exists and the IP {} is not whitelisted timed out", &ip)).into()), + Ok(Err(e)) => Err(e), + Ok(Ok(_)) => Ok(()), + }; + + massa_trace!("bootstrap.lib.run.select.accept.refuse_not_whitelisted", {"remote_addr": remote_address}); + return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied,format!("A whitelist exists and the IP {} is not whitelisted.", &ip))); + } + Ok((duplex, remote_address)) + } } #[allow(clippy::too_many_arguments)] diff --git a/massa-bootstrap/src/tests/mock_establisher.rs b/massa-bootstrap/src/tests/mock_establisher.rs index f829ecba146..785fd3eabe3 100644 --- a/massa-bootstrap/src/tests/mock_establisher.rs +++ b/massa-bootstrap/src/tests/mock_establisher.rs @@ -36,11 +36,7 @@ pub struct MockListener { } impl MockListener { - pub async fn accept( - &mut self, - _whitelist: &Option>, - _blacklist: &Option>, - ) -> std::io::Result<(Duplex, SocketAddr)> { + pub async fn accept(&mut self) -> std::io::Result<(Duplex, SocketAddr)> { let (addr, sender) = self.connection_listener_rx.recv().await.ok_or_else(|| { io::Error::new( io::ErrorKind::Other, diff --git a/massa-node/base_config/bootstrap_whitelist.json b/massa-node/base_config/bootstrap_whitelist.json index 88b3f4bd461..c44dc44f37c 100644 --- a/massa-node/base_config/bootstrap_whitelist.json +++ b/massa-node/base_config/bootstrap_whitelist.json @@ -1,4 +1,3 @@ [ - "149.202.89.125", - "2001:41d0:a:7f7d::" + ] \ No newline at end of file From f874ed0953168aa588f0d99b6dcf67561ddc329d Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Wed, 8 Feb 2023 17:14:34 +0100 Subject: [PATCH 2/8] fix: reset whitelist config file --- massa-node/base_config/bootstrap_whitelist.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/massa-node/base_config/bootstrap_whitelist.json b/massa-node/base_config/bootstrap_whitelist.json index c44dc44f37c..88b3f4bd461 100644 --- a/massa-node/base_config/bootstrap_whitelist.json +++ b/massa-node/base_config/bootstrap_whitelist.json @@ -1,3 +1,4 @@ [ - + "149.202.89.125", + "2001:41d0:a:7f7d::" ] \ No newline at end of file From de7c22e86c9235e5d89df0b78071c9cd82a42b95 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Thu, 9 Feb 2023 13:57:44 +0100 Subject: [PATCH 3/8] fix: skip check whether IP is allowed --- massa-bootstrap/src/server.rs | 21 +++++++++++++------ massa-bootstrap/src/tests/mock_establisher.rs | 3 +-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 1d4fe804c8e..2e1ac703066 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -192,12 +192,21 @@ impl BootstrapServer { res_connection = listener.accept() => { let (dplx, remote_addr) = if res_connection.is_ok() { let (dplx, remote_addr) = res_connection.unwrap(); - // check whether incoming peer IP is allowed or return an error which is ignored - let res = self.is_ip_allowed(dplx, remote_addr, &whitelist, &blacklist).await; - if res.is_ok() { - res.unwrap() - } else { - continue + // we didn't test whether Ip is allowed + #[cfg(test)] + { + (dplx, remote_addr) + } + + #[cfg(not(test))] + { + // check whether incoming peer IP is allowed or return an error which is ignored + let res = self.is_ip_allowed(dplx, remote_addr, &whitelist, &blacklist).await; + if res.is_ok() { + res.unwrap() + } else { + continue + } } } else { continue diff --git a/massa-bootstrap/src/tests/mock_establisher.rs b/massa-bootstrap/src/tests/mock_establisher.rs index 785fd3eabe3..8d995eb5440 100644 --- a/massa-bootstrap/src/tests/mock_establisher.rs +++ b/massa-bootstrap/src/tests/mock_establisher.rs @@ -2,9 +2,8 @@ use massa_models::config::{CHANNEL_SIZE, MAX_DUPLEX_BUFFER_SIZE}; use massa_time::MassaTime; -use std::collections::HashSet; use std::io; -use std::net::{IpAddr, SocketAddr}; +use std::net::SocketAddr; use tokio::io::DuplexStream; use tokio::sync::{mpsc, oneshot}; use tokio::time::timeout; From 668a4986753faaa8008fb96241f6c1398f783a04 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Mon, 13 Feb 2023 14:37:17 +0100 Subject: [PATCH 4/8] refactor: improve server binder usage --- massa-bootstrap/src/server.rs | 87 +++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 2e1ac703066..042dfe8ce69 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -29,7 +29,6 @@ use crate::{ messages::{BootstrapClientMessage, BootstrapServerMessage}, server_binder::BootstrapServerBinder, tools::normalize_ip, - types::Duplex, BootstrapConfig, Establisher, }; @@ -190,18 +189,27 @@ impl BootstrapServer { // listener res_connection = listener.accept() => { - let (dplx, remote_addr) = if res_connection.is_ok() { - let (dplx, remote_addr) = res_connection.unwrap(); - // we didn't test whether Ip is allowed + let (mut server, remote_addr) = if res_connection.is_ok() { + let (dplx, remote_addr) = res_connection.unwrap(); + let server = BootstrapServerBinder::new( + dplx, + self.keypair.clone(), + self.bootstrap_config.max_bytes_read_write, + self.bootstrap_config.max_bootstrap_message_size, + self.bootstrap_config.thread_count, + self.bootstrap_config.max_datastore_key_length, + self.bootstrap_config.randomness_size_bytes, + self.bootstrap_config.consensus_bootstrap_part_size); + // we didn't test whether Ip is allowed #[cfg(test)] { - (dplx, remote_addr) + (server, remote_addr) } #[cfg(not(test))] { // check whether incoming peer IP is allowed or return an error which is ignored - let res = self.is_ip_allowed(dplx, remote_addr, &whitelist, &blacklist).await; + let res = self.is_ip_allowed(remote_addr, server, &whitelist, &blacklist).await; if res.is_ok() { res.unwrap() } else { @@ -212,7 +220,6 @@ impl BootstrapServer { continue }; if bootstrap_sessions.len() < self.bootstrap_config.max_simultaneous_bootstraps.try_into().map_err(|_| BootstrapError::GeneralError("Fail to convert u32 to usize".to_string()))? { - massa_trace!("bootstrap.lib.run.select.accept", {"remote_addr": remote_addr}); let now = Instant::now(); @@ -230,7 +237,6 @@ impl BootstrapServer { match self.ip_hist_map.entry(remote_addr.ip()) { hash_map::Entry::Occupied(mut occ) => { if now.duration_since(*occ.get()) <= per_ip_min_interval { - let mut server = BootstrapServerBinder::new(dplx, self.keypair.clone(), self.bootstrap_config.max_bytes_read_write, self.bootstrap_config.max_bootstrap_message_size, self.bootstrap_config.thread_count, self.bootstrap_config.max_datastore_key_length, self.bootstrap_config.randomness_size_bytes, self.bootstrap_config.consensus_bootstrap_part_size); let _ = match tokio::time::timeout(self.bootstrap_config.write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { error: format!("Your last bootstrap on this server was {} ago and you have to wait {} before retrying.", format_duration(occ.get().elapsed()), format_duration(per_ip_min_interval.saturating_sub(occ.get().elapsed()))) @@ -271,11 +277,9 @@ impl BootstrapServer { let data_execution = self.final_state.clone(); let consensus_command_sender = self.consensus_controller.clone(); let network_command_sender = self.network_command_sender.clone(); - let keypair = self.keypair.clone(); let config = self.bootstrap_config.clone(); bootstrap_sessions.push(async move { - let mut server = BootstrapServerBinder::new(dplx, keypair, config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); debug!("awaiting on bootstrap of peer {}", remote_addr); match tokio::time::timeout(config.bootstrap_timeout.into(), manage_bootstrap(&config, &mut server, data_execution, version, consensus_command_sender, network_command_sender)).await { Ok(mgmt) => match mgmt { @@ -300,7 +304,6 @@ impl BootstrapServer { massa_trace!("bootstrap.session.started", {"active_count": bootstrap_sessions.len()}); } else { let config = self.bootstrap_config.clone(); - let mut server = BootstrapServerBinder::new(dplx, self.keypair.clone(), config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { error: "Bootstrap failed because the bootstrap server currently has no slots available.".to_string() })).await { @@ -319,47 +322,53 @@ impl BootstrapServer { Ok(()) } + #[cfg(not(test))] // whether the peer IP address is banned async fn is_ip_allowed( &self, - duplex: Duplex, remote_address: SocketAddr, + mut server: BootstrapServerBinder, whitelist: &Option>, blacklist: &Option>, - ) -> io::Result<(Duplex, SocketAddr)> { + ) -> io::Result<(BootstrapServerBinder, SocketAddr)> { let ip = normalize_ip(remote_address.ip()); - // whether the peer IP address is blacklisted, send back an error message - if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { - let config = self.bootstrap_config.clone(); - let mut server = BootstrapServerBinder::new(duplex, self.keypair.clone(), config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); - let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { - error: format!("IP {} is blacklisted.", &ip) - })).await { - Err(_) => Err(std::io::Error::new(std::io::ErrorKind::TimedOut, format!("IP {} is blacklisted send timed out", &ip)).into()), - Ok(Err(e)) => Err(e), - Ok(Ok(_)) => Ok(()), - }; - + // whether the peer IP address is blacklisted + let not_allowed_msg = if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { massa_trace!("bootstrap.lib.run.select.accept.refuse_blacklisted", {"remote_addr": remote_address}); - return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, format!("IP {} is blacklisted.", &ip))); - } + Some(format!("IP {} is blacklisted", &ip)) + // whether the peer IP address is not present in the whitelist + } else if let Some(ip_list) = &whitelist && !ip_list.contains(&ip){ + massa_trace!("bootstrap.lib.run.select.accept.refuse_not_whitelisted", {"remote_addr": remote_address}); + Some(format!("A whitelist exists and the IP {} is not whitelisted", &ip)) + } else { + None + }; - // whether the peer IP address is not present in the whitelist, send back an error message - if let Some(ip_list) = &whitelist && !ip_list.contains(&ip){ - let config = self.bootstrap_config.clone(); - let mut server = BootstrapServerBinder::new(duplex, self.keypair.clone(), config.max_bytes_read_write, config.max_bootstrap_message_size, config.thread_count, config.max_datastore_key_length, config.randomness_size_bytes, config.consensus_bootstrap_part_size); - let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { - error: format!("A whitelist exists and the IP {} is not whitelisted.", &ip) - })).await { - Err(_) => Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, format!("A whitelist exists and the IP {} is not whitelisted timed out", &ip)).into()), + // whether the peer IP address is not allowed, send back an error message + if let Some(error_msg) = not_allowed_msg { + let _ = match tokio::time::timeout( + self.bootstrap_config.write_error_timeout.into(), + server.send(BootstrapServerMessage::BootstrapError { + error: error_msg.clone(), + }), + ) + .await + { + Err(_) => Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + format!("{} timed out", &error_msg), + ) + .into()), Ok(Err(e)) => Err(e), Ok(Ok(_)) => Ok(()), }; - - massa_trace!("bootstrap.lib.run.select.accept.refuse_not_whitelisted", {"remote_addr": remote_address}); - return Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied,format!("A whitelist exists and the IP {} is not whitelisted.", &ip))); + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + error_msg, + )); } - Ok((duplex, remote_address)) + + Ok((server, remote_address)) } } From 72fc6f503599d277aae24e46291f1d48d013e967 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Mon, 13 Feb 2023 18:15:56 +0100 Subject: [PATCH 5/8] refactor: switch to if let else version --- massa-bootstrap/src/server.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 042dfe8ce69..7c15202a7ec 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -189,8 +189,7 @@ impl BootstrapServer { // listener res_connection = listener.accept() => { - let (mut server, remote_addr) = if res_connection.is_ok() { - let (dplx, remote_addr) = res_connection.unwrap(); + let (mut server, remote_addr) = if let Ok((dplx, remote_addr)) = res_connection { let server = BootstrapServerBinder::new( dplx, self.keypair.clone(), @@ -200,25 +199,25 @@ impl BootstrapServer { self.bootstrap_config.max_datastore_key_length, self.bootstrap_config.randomness_size_bytes, self.bootstrap_config.consensus_bootstrap_part_size); - // we didn't test whether Ip is allowed + + // we didn't test whether Ip is allowed #[cfg(test)] { - (server, remote_addr) + (server, remote_addr) } #[cfg(not(test))] { - // check whether incoming peer IP is allowed or return an error which is ignored - let res = self.is_ip_allowed(remote_addr, server, &whitelist, &blacklist).await; - if res.is_ok() { - res.unwrap() - } else { - continue - } + // check whether incoming peer IP is allowed or return an error which is ignored + let Ok((server, remote_addr)) = self.is_ip_allowed(remote_addr, server, &whitelist, &blacklist).await else { + continue; + }; + (server, remote_addr) } } else { continue }; + if bootstrap_sessions.len() < self.bootstrap_config.max_simultaneous_bootstraps.try_into().map_err(|_| BootstrapError::GeneralError("Fail to convert u32 to usize".to_string()))? { massa_trace!("bootstrap.lib.run.select.accept", {"remote_addr": remote_addr}); let now = Instant::now(); From 9167f2bd7c6c8aaae5502e3f37b37eae7ac83281 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Mon, 13 Feb 2023 18:52:50 +0100 Subject: [PATCH 6/8] refactor: small format --- massa-bootstrap/src/server.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 7c15202a7ec..09b676e4960 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -189,16 +189,16 @@ impl BootstrapServer { // listener res_connection = listener.accept() => { - let (mut server, remote_addr) = if let Ok((dplx, remote_addr)) = res_connection { - let server = BootstrapServerBinder::new( - dplx, - self.keypair.clone(), - self.bootstrap_config.max_bytes_read_write, - self.bootstrap_config.max_bootstrap_message_size, - self.bootstrap_config.thread_count, - self.bootstrap_config.max_datastore_key_length, - self.bootstrap_config.randomness_size_bytes, - self.bootstrap_config.consensus_bootstrap_part_size); + let (mut server, remote_addr) = if let Ok((dplx, remote_addr)) = res_connection { + let server = BootstrapServerBinder::new( + dplx, + self.keypair.clone(), + self.bootstrap_config.max_bytes_read_write, + self.bootstrap_config.max_bootstrap_message_size, + self.bootstrap_config.thread_count, + self.bootstrap_config.max_datastore_key_length, + self.bootstrap_config.randomness_size_bytes, + self.bootstrap_config.consensus_bootstrap_part_size); // we didn't test whether Ip is allowed #[cfg(test)] From d552a3a9c383e179c81f3f0337b3b032f72f7955 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Mon, 13 Feb 2023 22:24:17 +0100 Subject: [PATCH 7/8] refactor: scope test macro --- massa-bootstrap/src/server.rs | 80 +++++++++++++++++------------------ 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 09b676e4960..4796e410169 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -200,20 +200,11 @@ impl BootstrapServer { self.bootstrap_config.randomness_size_bytes, self.bootstrap_config.consensus_bootstrap_part_size); - // we didn't test whether Ip is allowed - #[cfg(test)] - { - (server, remote_addr) - } - - #[cfg(not(test))] - { // check whether incoming peer IP is allowed or return an error which is ignored let Ok((server, remote_addr)) = self.is_ip_allowed(remote_addr, server, &whitelist, &blacklist).await else { continue; }; (server, remote_addr) - } } else { continue }; @@ -302,8 +293,7 @@ impl BootstrapServer { }); massa_trace!("bootstrap.session.started", {"active_count": bootstrap_sessions.len()}); } else { - let config = self.bootstrap_config.clone(); - let _ = match tokio::time::timeout(config.clone().write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { + let _ = match tokio::time::timeout(self.bootstrap_config.write_error_timeout.into(), server.send(BootstrapServerMessage::BootstrapError { error: "Bootstrap failed because the bootstrap server currently has no slots available.".to_string() })).await { Err(_) => Err(std::io::Error::new(std::io::ErrorKind::TimedOut, "bootstrap error no available slots send timed out").into()), @@ -321,53 +311,61 @@ impl BootstrapServer { Ok(()) } - #[cfg(not(test))] // whether the peer IP address is banned async fn is_ip_allowed( &self, - remote_address: SocketAddr, + remote_addr: SocketAddr, mut server: BootstrapServerBinder, whitelist: &Option>, blacklist: &Option>, ) -> io::Result<(BootstrapServerBinder, SocketAddr)> { - let ip = normalize_ip(remote_address.ip()); - // whether the peer IP address is blacklisted - let not_allowed_msg = if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { - massa_trace!("bootstrap.lib.run.select.accept.refuse_blacklisted", {"remote_addr": remote_address}); + // we didn't test whether Ip is allowed + #[cfg(test)] + { + Ok((server, remote_addr)) + } + // we didn't test whether Ip is allowed + #[cfg(not(test))] + { + let ip = normalize_ip(remote_addr.ip()); + // whether the peer IP address is blacklisted + let not_allowed_msg = if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { + massa_trace!("bootstrap.lib.run.select.accept.refuse_blacklisted", {"remote_addr": remote_addr}); Some(format!("IP {} is blacklisted", &ip)) // whether the peer IP address is not present in the whitelist } else if let Some(ip_list) = &whitelist && !ip_list.contains(&ip){ - massa_trace!("bootstrap.lib.run.select.accept.refuse_not_whitelisted", {"remote_addr": remote_address}); + massa_trace!("bootstrap.lib.run.select.accept.refuse_not_whitelisted", {"remote_addr": remote_addr}); Some(format!("A whitelist exists and the IP {} is not whitelisted", &ip)) } else { None }; - // whether the peer IP address is not allowed, send back an error message - if let Some(error_msg) = not_allowed_msg { - let _ = match tokio::time::timeout( - self.bootstrap_config.write_error_timeout.into(), - server.send(BootstrapServerMessage::BootstrapError { - error: error_msg.clone(), - }), - ) - .await - { - Err(_) => Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - format!("{} timed out", &error_msg), + // whether the peer IP address is not allowed, send back an error message + if let Some(error_msg) = not_allowed_msg { + let _ = match tokio::time::timeout( + self.bootstrap_config.write_error_timeout.into(), + server.send(BootstrapServerMessage::BootstrapError { + error: error_msg.clone(), + }), ) - .into()), - Ok(Err(e)) => Err(e), - Ok(Ok(_)) => Ok(()), - }; - return Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - error_msg, - )); - } + .await + { + Err(_) => Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + format!("{} timed out", &error_msg), + ) + .into()), + Ok(Err(e)) => Err(e), + Ok(Ok(_)) => Ok(()), + }; + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + error_msg, + )); + } - Ok((server, remote_address)) + Ok((server, remote_addr)) + } } } From ea95031493849ecb46d3753123af3fbe2c418bd6 Mon Sep 17 00:00:00 2001 From: Moncef AOUDIA Date: Tue, 14 Feb 2023 12:01:32 +0100 Subject: [PATCH 8/8] fix: silent the compiler even in test mode --- massa-bootstrap/src/server.rs | 74 ++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/massa-bootstrap/src/server.rs b/massa-bootstrap/src/server.rs index 4796e410169..78af8c6b770 100644 --- a/massa-bootstrap/src/server.rs +++ b/massa-bootstrap/src/server.rs @@ -311,6 +311,19 @@ impl BootstrapServer { Ok(()) } + #[cfg(test)] + // TODO we didn't test whether the peer IP address is banned + async fn is_ip_allowed( + &self, + remote_addr: SocketAddr, + server: BootstrapServerBinder, + _whitelist: &Option>, + _blacklist: &Option>, + ) -> io::Result<(BootstrapServerBinder, SocketAddr)> { + Ok((server, remote_addr)) + } + + #[cfg(not(test))] // whether the peer IP address is banned async fn is_ip_allowed( &self, @@ -319,17 +332,9 @@ impl BootstrapServer { whitelist: &Option>, blacklist: &Option>, ) -> io::Result<(BootstrapServerBinder, SocketAddr)> { - // we didn't test whether Ip is allowed - #[cfg(test)] - { - Ok((server, remote_addr)) - } - // we didn't test whether Ip is allowed - #[cfg(not(test))] - { - let ip = normalize_ip(remote_addr.ip()); - // whether the peer IP address is blacklisted - let not_allowed_msg = if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { + let ip = normalize_ip(remote_addr.ip()); + // whether the peer IP address is blacklisted + let not_allowed_msg = if let Some(ip_list) = &blacklist && ip_list.contains(&ip) { massa_trace!("bootstrap.lib.run.select.accept.refuse_blacklisted", {"remote_addr": remote_addr}); Some(format!("IP {} is blacklisted", &ip)) // whether the peer IP address is not present in the whitelist @@ -340,32 +345,31 @@ impl BootstrapServer { None }; - // whether the peer IP address is not allowed, send back an error message - if let Some(error_msg) = not_allowed_msg { - let _ = match tokio::time::timeout( - self.bootstrap_config.write_error_timeout.into(), - server.send(BootstrapServerMessage::BootstrapError { - error: error_msg.clone(), - }), - ) - .await - { - Err(_) => Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - format!("{} timed out", &error_msg), - ) - .into()), - Ok(Err(e)) => Err(e), - Ok(Ok(_)) => Ok(()), - }; - return Err(std::io::Error::new( + // whether the peer IP address is not allowed, send back an error message + if let Some(error_msg) = not_allowed_msg { + let _ = match tokio::time::timeout( + self.bootstrap_config.write_error_timeout.into(), + server.send(BootstrapServerMessage::BootstrapError { + error: error_msg.clone(), + }), + ) + .await + { + Err(_) => Err(std::io::Error::new( std::io::ErrorKind::PermissionDenied, - error_msg, - )); - } - - Ok((server, remote_addr)) + format!("{} timed out", &error_msg), + ) + .into()), + Ok(Err(e)) => Err(e), + Ok(Ok(_)) => Ok(()), + }; + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + error_msg, + )); } + + Ok((server, remote_addr)) } }