From 0106f682f7db8a97ca57854a638369971a78a074 Mon Sep 17 00:00:00 2001 From: John Smith Date: Thu, 4 Aug 2022 12:48:19 -0400 Subject: [PATCH] harden network errors --- veilid-core/src/network_manager/native/mod.rs | 17 ++--- .../network_manager/native/protocol/udp.rs | 33 +++++----- veilid-core/src/xx/network_result.rs | 65 ++++++++++++------- 3 files changed, 68 insertions(+), 47 deletions(-) diff --git a/veilid-core/src/network_manager/native/mod.rs b/veilid-core/src/network_manager/native/mod.rs index 9d976539..82629099 100644 --- a/veilid-core/src/network_manager/native/mod.rs +++ b/veilid-core/src/network_manager/native/mod.rs @@ -316,7 +316,7 @@ impl Network { let h = RawUdpProtocolHandler::new_unspecified_bound_handler(&peer_socket_addr) .await .wrap_err("create socket failure")?; - network_result_try!(h + let _ = network_result_try!(h .send_message(data, peer_socket_addr) .await .map(NetworkResult::Value) @@ -375,9 +375,10 @@ impl Network { let h = RawUdpProtocolHandler::new_unspecified_bound_handler(&peer_socket_addr) .await .wrap_err("create socket failure")?; - h.send_message(data, peer_socket_addr) + network_result_try!(h + .send_message(data, peer_socket_addr) .await - .wrap_err("send message failure")?; + .wrap_err("send message failure")?); self.network_manager() .stats_packet_sent(dial_info.to_ip_addr(), data_len as u64); @@ -449,10 +450,10 @@ impl Network { &peer_socket_addr, &descriptor.local().map(|sa| sa.to_socket_addr()), ) { - ph.clone() - .send_message(data, peer_socket_addr) + network_result_value_or_log!(debug ph.clone() + .send_message(data.clone(), peer_socket_addr) .await - .wrap_err("sending data to existing conection")?; + .wrap_err("sending data to existing conection")? => { return Ok(Some(data)); } ); // Network accounting self.network_manager() @@ -506,7 +507,7 @@ impl Network { Some(ph) => ph, None => bail!("no appropriate UDP protocol handler for dial_info"), }; - network_result_try!(ph + let _ = network_result_try!(ph .send_message(data, peer_socket_addr) .await .into_network_result() @@ -532,7 +533,7 @@ impl Network { self.network_manager() .stats_packet_sent(dial_info.to_ip_addr(), data_len as u64); - Ok(NetworkResult::Value(())) + Ok(NetworkResult::value(())) } ///////////////////////////////////////////////////////////////// diff --git a/veilid-core/src/network_manager/native/protocol/udp.rs b/veilid-core/src/network_manager/native/protocol/udp.rs index 95804099..165144c0 100644 --- a/veilid-core/src/network_manager/native/protocol/udp.rs +++ b/veilid-core/src/network_manager/native/protocol/udp.rs @@ -14,21 +14,12 @@ impl RawUdpProtocolHandler { #[instrument(level = "trace", err, skip(self, data), fields(data.len = data.len(), ret.len, ret.from))] pub async fn recv_message(&self, data: &mut [u8]) -> io::Result<(usize, ConnectionDescriptor)> { let (size, remote_addr) = loop { - match self.socket.recv_from(data).await { - Ok((size, remote_addr)) => { - if size > MAX_MESSAGE_SIZE { - bail_io_error_other!("received too large UDP message"); - } - break (size, remote_addr); - } - Err(e) => { - if e.kind() == io::ErrorKind::ConnectionReset { - // Ignore icmp - } else { - return Err(e); - } - } + let (size, remote_addr) = network_result_value_or_log!(debug self.socket.recv_from(data).await.into_network_result()? => continue); + if size > MAX_MESSAGE_SIZE { + log_net!(debug "{}({}) at {}@{}:{}", "Invalid message".green(), "received too large UDP message", file!(), line!(), column!()); + continue; } + break (size, remote_addr); }; let peer_addr = PeerAddress::new( @@ -47,17 +38,25 @@ impl RawUdpProtocolHandler { } #[instrument(level = "trace", err, skip(self, data), fields(data.len = data.len(), ret.len, ret.from))] - pub async fn send_message(&self, data: Vec, socket_addr: SocketAddr) -> io::Result<()> { + pub async fn send_message( + &self, + data: Vec, + socket_addr: SocketAddr, + ) -> io::Result> { if data.len() > MAX_MESSAGE_SIZE { bail_io_error_other!("sending too large UDP message"); } - let len = self.socket.send_to(&data, socket_addr).await?; + let len = network_result_try!(self + .socket + .send_to(&data, socket_addr) + .await + .into_network_result()?); if len != data.len() { bail_io_error_other!("UDP partial send") } - Ok(()) + Ok(NetworkResult::value(())) } #[instrument(level = "trace", err)] diff --git a/veilid-core/src/xx/network_result.rs b/veilid-core/src/xx/network_result.rs index d2eb4233..a924c180 100644 --- a/veilid-core/src/xx/network_result.rs +++ b/veilid-core/src/xx/network_result.rs @@ -38,13 +38,20 @@ impl IoNetworkResultExt for io::Result { _ => Err(e), }, #[cfg(not(feature = "io_error_more"))] - Err(e) => match e.kind() { - io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), - io::ErrorKind::ConnectionAborted - | io::ErrorKind::ConnectionRefused - | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), - _ => Err(e), - }, + Err(e) => { + if let Some(os_err) = e.raw_os_error() { + if os_err == libc::EHOSTUNREACH || os_err == libc::ENETUNREACH { + return Ok(NetworkResult::NoConnection(e)); + } + } + match e.kind() { + io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), + io::ErrorKind::ConnectionAborted + | io::ErrorKind::ConnectionRefused + | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), + _ => Err(e), + } + } } } } @@ -85,13 +92,20 @@ impl FoldedNetworkResultExt for io::Result> { _ => Err(e), }, #[cfg(not(feature = "io_error_more"))] - Err(e) => match e.kind() { - io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), - io::ErrorKind::ConnectionAborted - | io::ErrorKind::ConnectionRefused - | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), - _ => Err(e), - }, + Err(e) => { + if let Some(os_err) = e.raw_os_error() { + if os_err == libc::EHOSTUNREACH || os_err == libc::ENETUNREACH { + return Ok(NetworkResult::NoConnection(e)); + } + } + match e.kind() { + io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), + io::ErrorKind::ConnectionAborted + | io::ErrorKind::ConnectionRefused + | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), + _ => Err(e), + } + } } } } @@ -111,13 +125,20 @@ impl FoldedNetworkResultExt for io::Result> { _ => Err(e), }, #[cfg(not(feature = "io_error_more"))] - Err(e) => match e.kind() { - io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), - io::ErrorKind::ConnectionAborted - | io::ErrorKind::ConnectionRefused - | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), - _ => Err(e), - }, + Err(e) => { + if let Some(os_err) = e.raw_os_error() { + if os_err == libc::EHOSTUNREACH || os_err == libc::ENETUNREACH { + return Ok(NetworkResult::NoConnection(e)); + } + } + match e.kind() { + io::ErrorKind::TimedOut => Ok(NetworkResult::Timeout), + io::ErrorKind::ConnectionAborted + | io::ErrorKind::ConnectionRefused + | io::ErrorKind::ConnectionReset => Ok(NetworkResult::NoConnection(e)), + _ => Err(e), + } + } } } } @@ -270,7 +291,7 @@ macro_rules! network_result_value_or_log { $f } NetworkResult::InvalidMessage(s) => { - log_net!($level "{}({}) at {}@{}:{}", "No connection".green(), s, file!(), line!(), column!()); + log_net!($level "{}({}) at {}@{}:{}", "Invalid message".green(), s, file!(), line!(), column!()); $f } NetworkResult::Value(v) => v,