diff --git a/iroh-net/src/bin/derper.rs b/iroh-net/src/bin/derper.rs index 50836e42f4..26554e056a 100644 --- a/iroh-net/src/bin/derper.rs +++ b/iroh-net/src/bin/derper.rs @@ -13,7 +13,7 @@ use std::{ use anyhow::{anyhow, bail, Context as _, Result}; use clap::Parser; use futures::{Future, StreamExt}; -use http::response::Builder as ResponseBuilder; +use http::{response::Builder as ResponseBuilder, HeaderMap}; use hyper::body::Incoming; use hyper::{Method, Request, Response, StatusCode}; use iroh_metrics::inc; @@ -89,12 +89,13 @@ impl CertMode { tokio::spawn( async move { - loop { - match state.next().await.unwrap() { + while let Some(event) = state.next().await { + match event { Ok(ok) => debug!("acme event: {:?}", ok), Err(err) => error!("error: {:?}", err), } } + debug!("event stream finished"); } .instrument(info_span!("acme")), ); @@ -125,7 +126,8 @@ impl CertMode { } fn escape_hostname(hostname: &str) -> Cow<'_, str> { - let unsafe_hostname_characters = regex::Regex::new(r"[^a-zA-Z0-9-\.]").unwrap(); + let unsafe_hostname_characters = + regex::Regex::new(r"[^a-zA-Z0-9-\.]").expect("regex manually checked"); unsafe_hostname_characters.replace_all(hostname, "") } @@ -249,7 +251,7 @@ impl Default for Config { fn default() -> Self { Self { secret_key: SecretKey::generate(), - addr: "[::]:443".parse().unwrap(), + addr: (Ipv6Addr::UNSPECIFIED, 443).into(), stun_port: DEFAULT_DERP_STUN_PORT, hostname: NA_DERP_HOSTNAME.into(), enable_stun: true, @@ -444,7 +446,10 @@ async fn run( tls_config.cert_dir.unwrap_or_else(|| PathBuf::from(".")), ) .await?; - let headers: Vec<(&str, &str)> = TLS_HEADERS.into(); + let mut headers = HeaderMap::new(); + for (name, value) in TLS_HEADERS.iter() { + headers.insert(*name, value.parse()?); + } ( Some(DerpTlsConfig { config, acceptor }), headers, @@ -453,7 +458,7 @@ async fn run( .unwrap_or(DEFAULT_CAPTIVE_PORTAL_PORT), ) } else { - (None, Vec::new(), 0) + (None, HeaderMap::new(), 0) }; let mut builder = DerpServerBuilder::new(addr) @@ -592,8 +597,8 @@ impl hyper::service::Service> for CaptivePortalService { let r = Response::builder() .status(StatusCode::NOT_FOUND) .body(NOTFOUND.into()) - .unwrap(); - Box::pin(async move { Ok(r) }) + .map_err(|err| Box::new(err) as HyperError); + Box::pin(async move { r }) } } } @@ -603,23 +608,21 @@ fn derp_disabled_handler( _r: Request, response: ResponseBuilder, ) -> HyperResult> { - Ok(response + response .status(StatusCode::NOT_FOUND) .body(DERP_DISABLED.into()) - .unwrap()) + .map_err(|err| Box::new(err) as HyperError) } fn root_handler( _r: Request, response: ResponseBuilder, ) -> HyperResult> { - let response = response + response .status(StatusCode::OK) .header("Content-Type", "text/html; charset=utf-8") .body(INDEX.into()) - .unwrap(); - - Ok(response) + .map_err(|err| Box::new(err) as HyperError) } /// HTTP latency queries @@ -627,23 +630,21 @@ fn probe_handler( _r: Request, response: ResponseBuilder, ) -> HyperResult> { - let response = response + response .status(StatusCode::OK) .header("Access-Control-Allow-Origin", "*") .body(body_empty()) - .unwrap(); - - Ok(response) + .map_err(|err| Box::new(err) as HyperError) } fn robots_handler( _r: Request, response: ResponseBuilder, ) -> HyperResult> { - Ok(response + response .status(StatusCode::OK) .body(ROBOTS_TXT.into()) - .unwrap()) + .map_err(|err| Box::new(err) as HyperError) } /// For captive portal detection. @@ -666,10 +667,10 @@ fn serve_no_content_handler( } } - Ok(response + response .status(StatusCode::NO_CONTENT) .body(body_empty()) - .unwrap()) + .map_err(|err| Box::new(err) as HyperError) } fn is_challenge_char(c: char) -> bool { @@ -714,14 +715,20 @@ async fn server_stun_listener(sock: UdpSocket) { } match tokio::task::spawn_blocking(move || stun::parse_binding_request(&pkt)) .await - .unwrap() { - Ok(txid) => { + Ok(Ok(txid)) => { debug!(%src_addr, %txid, "STUN: received binding request"); - let res = - tokio::task::spawn_blocking(move || stun::response(txid, src_addr)) - .await - .unwrap(); + let res = match tokio::task::spawn_blocking(move || { + stun::response(txid, src_addr) + }) + .await + { + Ok(res) => res, + Err(err) => { + error!("JoinError: {err:#}"); + return; + } + }; match sock.send_to(&res, src_addr).await { Ok(len) => { if len != res.len() { @@ -743,10 +750,11 @@ async fn server_stun_listener(sock: UdpSocket) { } } } - Err(err) => { + Ok(Err(err)) => { inc!(StunMetrics, bad_requests); warn!(%src_addr, "STUN: invalid binding request: {:?}", err); } + Err(err) => error!("JoinError parsing STUN binding: {err:#}"), } }); } diff --git a/iroh-net/src/defaults.rs b/iroh-net/src/defaults.rs index 0ec337154c..251b85dd10 100644 --- a/iroh-net/src/defaults.rs +++ b/iroh-net/src/defaults.rs @@ -21,7 +21,9 @@ pub fn default_derp_map() -> DerpMap { /// Get the default [`DerpNode`] for NA. pub fn default_na_derp_node() -> DerpNode { // The default NA derper run by number0. - let url: Url = format!("https://{NA_DERP_HOSTNAME}").parse().unwrap(); + let url: Url = format!("https://{NA_DERP_HOSTNAME}") + .parse() + .expect("default url"); DerpNode { url: url.into(), stun_only: false, @@ -32,7 +34,9 @@ pub fn default_na_derp_node() -> DerpNode { /// Get the default [`DerpNode`] for EU. pub fn default_eu_derp_node() -> DerpNode { // The default EU derper run by number0. - let url: Url = format!("https://{EU_DERP_HOSTNAME}").parse().unwrap(); + let url: Url = format!("https://{EU_DERP_HOSTNAME}") + .parse() + .expect("default_url"); DerpNode { url: url.into(), stun_only: false, diff --git a/iroh-net/src/derp/client_conn.rs b/iroh-net/src/derp/client_conn.rs index 7bc21c4047..4c9cfe3962 100644 --- a/iroh-net/src/derp/client_conn.rs +++ b/iroh-net/src/derp/client_conn.rs @@ -428,7 +428,10 @@ where async fn send_packet(&mut self, packet: Packet) -> Result<()> { let src_key = packet.src; let content = packet.bytes; - inc_by!(Metrics, bytes_sent, content.len().try_into().unwrap()); + + if let Ok(len) = content.len().try_into() { + inc_by!(Metrics, bytes_sent, len); + } write_frame( &mut self.io, Frame::RecvPacket { src_key, content }, diff --git a/iroh-net/src/derp/codec.rs b/iroh-net/src/derp/codec.rs index 5766d5d7e0..4329df1cea 100644 --- a/iroh-net/src/derp/codec.rs +++ b/iroh-net/src/derp/codec.rs @@ -506,8 +506,8 @@ impl Frame { "invalid restarting frame length: {}", content.len() ); - let reconnect_in = u32::from_be_bytes(content[..4].try_into().unwrap()); - let try_for = u32::from_be_bytes(content[4..].try_into().unwrap()); + let reconnect_in = u32::from_be_bytes(content[..4].try_into()?); + let try_for = u32::from_be_bytes(content[4..].try_into()?); Self::Restarting { reconnect_in, try_for, @@ -555,9 +555,18 @@ impl Decoder for DerpCodec { return Ok(None); } - // Can't use the `get_` Buf api, as that advances the buffer - let frame_type: FrameType = src[0].into(); - let frame_len = u32::from_be_bytes(src[1..5].try_into().unwrap()) as usize; + // Can't use the `Buf::get_*` APIs, as that advances the buffer. + let Some(frame_type) = src.first().map(|b| FrameType::from(*b)) else { + return Ok(None); // Not enough bytes + }; + let Some(frame_len) = src + .get(1..5) + .and_then(|s| TryInto::<[u8; 4]>::try_into(s).ok()) + .map(u32::from_be_bytes) + .map(|l| l as usize) + else { + return Ok(None); // Not enough bytes + }; if frame_len > MAX_FRAME_SIZE { anyhow::bail!("Frame of length {} is too large.", frame_len); diff --git a/iroh-net/src/derp/http/client.rs b/iroh-net/src/derp/http/client.rs index 5df569e12b..eefcc14947 100644 --- a/iroh-net/src/derp/http/client.rs +++ b/iroh-net/src/derp/http/client.rs @@ -78,9 +78,12 @@ pub enum ClientError { /// No local addresses exist #[error("no local addr: {0}")] NoLocalAddr(String), - /// There was http [`hyper::Error`] + /// There was http server [`hyper::Error`] #[error("http connection error")] Hyper(#[from] hyper::Error), + /// There was an http error [`http::Error`]. + #[error("http error")] + Http(#[from] http::Error), /// There was an unexpected status code #[error("unexpected status code: expected {0}, got {1}")] UnexpectedStatusCode(hyper::StatusCode, hyper::StatusCode), @@ -751,7 +754,7 @@ impl Actor { } /// Sends the HTTP upgrade request to the derper. - async fn start_upgrade(io: T) -> Result, hyper::Error> + async fn start_upgrade(io: T) -> Result, ClientError> where T: AsyncRead + AsyncWrite + Send + Unpin + 'static, { @@ -774,9 +777,8 @@ impl Actor { let req = Request::builder() .uri("/derp") .header(UPGRADE, super::HTTP_UPGRADE_PROTOCOL) - .body(http_body_util::Empty::::new()) - .unwrap(); - request_sender.send_request(req).await + .body(http_body_util::Empty::::new())?; + request_sender.send_request(req).await.map_err(From::from) } async fn note_preferred(&mut self, is_preferred: bool) { diff --git a/iroh-net/src/derp/http/server.rs b/iroh-net/src/derp/http/server.rs index 7435b5a435..e50c9e6f00 100644 --- a/iroh-net/src/derp/http/server.rs +++ b/iroh-net/src/derp/http/server.rs @@ -36,7 +36,6 @@ type HyperHandler = Box< + Sync + 'static, >; -type Headers = Vec<(&'static str, &'static str)>; /// Creates a new [`BytesBody`] with no content. fn body_empty() -> BytesBody { @@ -188,7 +187,7 @@ pub struct ServerBuilder { #[debug("{}", derp_override.as_ref().map_or("None", |_| "Some(Box, ResponseBuilder) -> Result + Send + Sync + 'static>)"))] derp_override: Option, /// Headers to use for HTTP or HTTPS messages. - headers: Headers, + headers: HeaderMap, /// 404 not found response /// /// When `None`, a default is provided. @@ -208,7 +207,7 @@ impl ServerBuilder { handlers: Default::default(), derp_endpoint: "/derp", derp_override: None, - headers: Vec::new(), + headers: HeaderMap::new(), not_found_fn: None, } } @@ -271,8 +270,10 @@ impl ServerBuilder { } /// Add http headers. - pub fn headers(mut self, headers: Headers) -> Self { - self.headers = headers; + pub fn headers(mut self, headers: HeaderMap) -> Self { + for (k, v) in headers.iter() { + self.headers.insert(k.clone(), v.clone()); + } self } @@ -281,20 +282,12 @@ impl ServerBuilder { ensure!(self.secret_key.is_some() || self.derp_override.is_some(), "Must provide a `SecretKey` for the derp server OR pass in an override function for the 'derp' endpoint"); let (derp_handler, derp_server, mesh_clients) = if let Some(secret_key) = self.secret_key { let server = crate::derp::server::Server::new(secret_key.clone(), self.mesh_key); - let header_map: HeaderMap = HeaderMap::from_iter( - self.headers - .iter() - .map(|(k, v)| (k.parse().unwrap(), v.parse().unwrap())), - ); - let packet_fwd = server.packet_forwarder_handler(); - let mesh_clients = if let Some(mesh_addrs) = self.mesh_derpers { ensure!( self.mesh_key.is_some(), "Must provide a `MeshKey` when trying to join a mesh network." ); - let mesh_key = self.mesh_key.expect("checked"); Some(MeshClients::new( mesh_key, secret_key, mesh_addrs, packet_fwd, @@ -302,15 +295,17 @@ impl ServerBuilder { } else { None }; - ( - DerpHandler::ConnHandler(server.client_conn_handler(header_map)), + DerpHandler::ConnHandler(server.client_conn_handler(self.headers.clone())), Some(server), mesh_clients, ) } else { ( - DerpHandler::Override(self.derp_override.unwrap()), + DerpHandler::Override( + self.derp_override + .context("no derp handler override but also no secret key")?, + ), None, None, ) @@ -320,10 +315,10 @@ impl ServerBuilder { Some(f) => f, None => Box::new(move |_req: Request, mut res: ResponseBuilder| { for (k, v) in h.iter() { - res = res.header(*k, *v); + res = res.header(k.clone(), v.clone()); } let body = body_full("Not Found"); - let r = res.status(StatusCode::NOT_FOUND).body(body).unwrap(); + let r = res.status(StatusCode::NOT_FOUND).body(body)?; HyperResult::Ok(r) }), }; @@ -444,7 +439,7 @@ where async move { { - let mut res = builder.body(body_empty()).unwrap(); + let mut res = builder.body(body_empty()).expect("valid body"); // Send a 400 to any request that doesn't have an `Upgrade` header. if !req.headers().contains_key(UPGRADE) { @@ -538,7 +533,7 @@ struct Inner { #[debug("Box Result> + Send + Sync + 'static>")] pub not_found_fn: HyperHandler, pub handlers: Handlers, - pub headers: Headers, + pub headers: HeaderMap, } /// Action to take when a connection is made at the derp endpoint.` @@ -562,7 +557,7 @@ impl Inner { fn default_response(&self) -> ResponseBuilder { let mut response = Response::builder(); for (key, value) in self.headers.iter() { - response = response.header(*key, *value); + response = response.header(key.clone(), value.clone()); } response } @@ -584,7 +579,7 @@ impl DerpService { derp_handler: DerpHandler, derp_endpoint: &'static str, not_found_fn: HyperHandler, - headers: Headers, + headers: HeaderMap, ) -> Self { Self(Arc::new(Inner { derp_handler, diff --git a/iroh-net/src/derp/server.rs b/iroh-net/src/derp/server.rs index f783254da9..56870246fc 100644 --- a/iroh-net/src/derp/server.rs +++ b/iroh-net/src/derp/server.rs @@ -596,12 +596,8 @@ fn init_meta_cert(server_key: &PublicKey) -> Vec { rcgen::CertificateParams::new([format!("derpkey{}", hex::encode(server_key.as_bytes()))]); params.serial_number = Some((PROTOCOL_VERSION as u64).into()); // Windows requires not_after and not_before set: - params.not_after = time::OffsetDateTime::now_utc() - .checked_add(30 * time::Duration::DAY) - .unwrap(); - params.not_before = time::OffsetDateTime::now_utc() - .checked_sub(30 * time::Duration::DAY) - .unwrap(); + params.not_after = time::OffsetDateTime::now_utc().saturating_add(30 * time::Duration::DAY); + params.not_before = time::OffsetDateTime::now_utc().saturating_sub(30 * time::Duration::DAY); rcgen::Certificate::from_params(params) .expect("fixed inputs") diff --git a/iroh-net/src/derp/types.rs b/iroh-net/src/derp/types.rs index 62694dc746..56596322fb 100644 --- a/iroh-net/src/derp/types.rs +++ b/iroh-net/src/derp/types.rs @@ -1,6 +1,6 @@ use std::num::NonZeroU32; -use anyhow::{bail, ensure, Result}; +use anyhow::{bail, Context, Result}; use bytes::Bytes; use postcard::experimental::max_size::MaxSize; use serde::{Deserialize, Serialize}; @@ -25,8 +25,10 @@ impl RateLimiter { if bytes_per_second == 0 || bytes_burst == 0 { return Ok(None); } - let bytes_per_second = NonZeroU32::new(u32::try_from(bytes_per_second)?).unwrap(); - let bytes_burst = NonZeroU32::new(u32::try_from(bytes_burst)?).unwrap(); + let bytes_per_second = NonZeroU32::new(u32::try_from(bytes_per_second)?) + .context("bytes_per_second not non-zero")?; + let bytes_burst = + NonZeroU32::new(u32::try_from(bytes_burst)?).context("bytes_burst not non-zero")?; Ok(Some(Self { inner: governor::RateLimiter::direct( governor::Quota::per_second(bytes_per_second).allow_burst(bytes_burst), @@ -35,8 +37,7 @@ impl RateLimiter { } pub(crate) fn check_n(&self, n: usize) -> Result<()> { - ensure!(n != 0); - let n = NonZeroU32::new(u32::try_from(n)?).unwrap(); + let n = NonZeroU32::new(u32::try_from(n)?).context("n not non-zero")?; match self.inner.check_n(n) { Ok(_) => Ok(()), Err(_) => bail!("batch cannot go through"), diff --git a/iroh-net/src/disco.rs b/iroh-net/src/disco.rs index 3fb87e5cec..dfd4b42dee 100644 --- a/iroh-net/src/disco.rs +++ b/iroh-net/src/disco.rs @@ -23,7 +23,7 @@ use std::{ net::{IpAddr, SocketAddr}, }; -use anyhow::{anyhow, bail, ensure, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; use url::Url; use crate::{derp::DerpUrl, key, net::ip::to_canonical}; @@ -193,7 +193,7 @@ impl Ping { ensure!(ver == V0, "invalid version"); // Deliberately lax on longer-than-expected messages, for future compatibility. ensure!(p.len() >= PING_LEN, "message too short"); - let tx_id: [u8; TX_LEN] = p[..TX_LEN].try_into().unwrap(); + let tx_id: [u8; TX_LEN] = p[..TX_LEN].try_into().expect("length checked"); let raw_key = &p[TX_LEN..TX_LEN + key::PUBLIC_KEY_LENGTH]; let node_key = PublicKey::try_from(raw_key)?; let tx_id = stun::TransactionId::from(tx_id); @@ -217,8 +217,8 @@ fn send_addr_from_bytes(p: &[u8]) -> Result { ensure!(p.len() > 2, "too short"); match p[0] { 0u8 => { - ensure!(p.len() - 1 == EP_LENGTH, "invalid length"); - let addr = socket_addr_from_bytes(&p[1..]); + let bytes: [u8; EP_LENGTH] = p[1..].try_into().context("invalid length")?; + let addr = socket_addr_from_bytes(bytes); Ok(SendAddr::Udp(addr)) } 1u8 => { @@ -248,11 +248,11 @@ fn send_addr_to_vec(addr: &SendAddr) -> Vec { } // Assumes p.len() == EP_LENGTH -fn socket_addr_from_bytes(p: &[u8]) -> SocketAddr { - debug_assert_eq!(p.len(), EP_LENGTH); +fn socket_addr_from_bytes(p: [u8; EP_LENGTH]) -> SocketAddr { + debug_assert_eq!(EP_LENGTH, 16 + 2); - let raw_src_ip: [u8; 16] = p[..16].try_into().unwrap(); - let raw_port: [u8; 2] = p[16..].try_into().unwrap(); + let raw_src_ip: [u8; 16] = p[..16].try_into().expect("array long enough"); + let raw_port: [u8; 2] = p[16..].try_into().expect("array long enough"); let src_ip = to_canonical(IpAddr::from(raw_src_ip)); let src_port = u16::from_le_bytes(raw_port); @@ -275,8 +275,7 @@ fn socket_addr_as_bytes(addr: &SocketAddr) -> [u8; EP_LENGTH] { impl Pong { fn from_bytes(ver: u8, p: &[u8]) -> Result { ensure!(ver == V0, "invalid version"); - ensure!(p.len() >= TX_LEN, "message too short"); - let tx_id: [u8; TX_LEN] = p[..TX_LEN].try_into().unwrap(); + let tx_id: [u8; TX_LEN] = p[..TX_LEN].try_into().context("message too short")?; let tx_id = stun::TransactionId::from(tx_id); let src = send_addr_from_bytes(&p[TX_LEN..])?; @@ -305,7 +304,8 @@ impl CallMeMaybe { }; for chunk in p.chunks_exact(EP_LENGTH) { - let src = socket_addr_from_bytes(chunk); + let bytes: [u8; EP_LENGTH] = chunk.try_into().context("chunk must match")?; + let src = socket_addr_from_bytes(bytes); m.my_numbers.push(src); } diff --git a/iroh-net/src/key.rs b/iroh-net/src/key.rs index e501d7c5a8..ab5bb4d4b1 100644 --- a/iroh-net/src/key.rs +++ b/iroh-net/src/key.rs @@ -53,7 +53,7 @@ static KEY_CACHE: OnceCell>> = OnceCell::ne fn lock_key_cache() -> std::sync::MutexGuard<'static, TtlCache<[u8; 32], CryptoKeys>> { let mutex = KEY_CACHE.get_or_init(|| Mutex::new(TtlCache::new(KEY_CACHE_CAPACITY))); - mutex.lock().unwrap() + mutex.lock().expect("not poisoned") } /// Get or create the crypto keys, and project something out of them. diff --git a/iroh-net/src/key/encryption.rs b/iroh-net/src/key/encryption.rs index af6283ae15..ceb370783f 100644 --- a/iroh-net/src/key/encryption.rs +++ b/iroh-net/src/key/encryption.rs @@ -3,7 +3,7 @@ use std::fmt::Debug; use aead::Buffer; -use anyhow::{anyhow, ensure, Result}; +use anyhow::{anyhow, ensure, Context, Result}; pub(crate) const NONCE_LEN: usize = 24; @@ -47,7 +47,9 @@ impl SharedSecret { ensure!(buffer.len() > NONCE_LEN, "too short"); let offset = buffer.len() - NONCE_LEN; - let nonce: [u8; NONCE_LEN] = buffer.as_ref()[offset..].try_into().unwrap(); + let nonce: [u8; NONCE_LEN] = buffer.as_ref()[offset..] + .try_into() + .context("nonce wrong length")?; buffer.truncate(offset); self.0 diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index b9dde00f72..4c096e5b80 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -251,14 +251,14 @@ impl Inner { /// /// If `None`, then we are not connected to any derp region. fn my_derp(&self) -> Option { - self.my_derp.read().unwrap().clone() + self.my_derp.read().expect("not poisoned").clone() } /// Sets the derp node with the best latency. /// /// If we are not connected to any derp nodes, set this to `None`. fn set_my_derp(&self, my_derp: Option) { - *self.my_derp.write().unwrap() = my_derp; + *self.my_derp.write().expect("not poisoned") = my_derp; } fn is_closing(&self) -> bool { @@ -275,7 +275,7 @@ impl Inner { /// Get the cached version of the Ipv4 and Ipv6 addrs of the current connection. fn local_addr(&self) -> (SocketAddr, Option) { - *self.local_addrs.read().unwrap() + *self.local_addrs.read().expect("not poisoned") } fn normalized_local_addr(&self) -> io::Result { let (v4, v6) = self.local_addr(); @@ -449,14 +449,14 @@ impl Inner { } fn conn_for_addr(&self, addr: SocketAddr) -> io::Result<&RebindingUdpConn> { - if addr.is_ipv6() && self.pconn6.is_none() { - return Err(io::Error::new(io::ErrorKind::Other, "no IPv6 connection")); - } - Ok(if addr.is_ipv6() { - self.pconn6.as_ref().unwrap() - } else { - &self.pconn4 - }) + let sock = match addr { + SocketAddr::V4(_) => &self.pconn4, + SocketAddr::V6(_) => self + .pconn6 + .as_ref() + .ok_or(io::Error::new(io::ErrorKind::Other, "no IPv6 connection"))?, + }; + Ok(sock) } #[instrument(skip_all, fields(me = %self.me))] @@ -1341,8 +1341,8 @@ impl MagicSock { .actor_sender .send(ActorMessage::SetPreferredPort(port, s)) .await - .unwrap(); - r.await.unwrap(); + .ok(); + r.await.ok(); } /// Returns the DERP node with the best latency. @@ -1403,8 +1403,17 @@ impl MagicSock { .actor_sender .send(ActorMessage::RebindAll(s)) .await - .unwrap(); - r.await.unwrap(); + .map_err(|err| { + error!("actor_sender gone, rebind_all failed"); + err + }) + .ok(); + r.await + .map_err(|err| { + error!("actor failed to respond to rebind_all, stuff is probably messy"); + err + }) + .ok(); } /// Reference to optional discovery service @@ -1569,7 +1578,7 @@ impl AsyncUdpSocket for MagicSock { } fn local_addr(&self) -> io::Result { - match &*self.inner.local_addrs.read().unwrap() { + match &*self.inner.local_addrs.read().expect("not poisoned") { (ipv4, None) => { // Pretend to be IPv6, because our QuinnMappedAddrs // need to be IPv6. @@ -2325,7 +2334,7 @@ impl Actor { } let ipv4_addr = self.pconn4.local_addr()?; - *self.inner.local_addrs.write().unwrap() = (ipv4_addr, ipv6_addr); + *self.inner.local_addrs.write().expect("not poisoned") = (ipv4_addr, ipv6_addr); self.update_net_info("sockets rebound").await; diff --git a/iroh-net/src/magicsock/derp_actor.rs b/iroh-net/src/magicsock/derp_actor.rs index ed2af9eda0..20bc18bf28 100644 --- a/iroh-net/src/magicsock/derp_actor.rs +++ b/iroh-net/src/magicsock/derp_actor.rs @@ -205,8 +205,11 @@ impl ActiveDerp { // reset self.backoff.reset(); let now = Instant::now(); - if self.last_packet_time.is_none() - || self.last_packet_time.as_ref().unwrap().elapsed() > Duration::from_secs(5) + if self + .last_packet_time + .as_ref() + .map(|t| t.elapsed() > Duration::from_secs(5)) + .unwrap_or(true) { self.last_packet_time = Some(now); } @@ -220,8 +223,11 @@ impl ActiveDerp { trace!(len=%data.len(), "received msg"); // If this is a new sender we hadn't seen before, remember it and // register a route for this peer. - if self.last_packet_src.is_none() - || &source != self.last_packet_src.as_ref().unwrap() + if self + .last_packet_src + .as_ref() + .map(|p| *p != source) + .unwrap_or(true) { // avoid map lookup w/ high throughput single peer self.last_packet_src = Some(source); diff --git a/iroh-net/src/net/interfaces/bsd.rs b/iroh-net/src/net/interfaces/bsd.rs index 7b0fe8f9d5..9f18e09aa1 100644 --- a/iroh-net/src/net/interfaces/bsd.rs +++ b/iroh-net/src/net/interfaces/bsd.rs @@ -93,13 +93,12 @@ fn is_default_gateway(rm: &RouteMessage) -> bool { return false; } - let dst = rm.addrs.get(libc::RTAX_DST as usize); - let netmask = rm.addrs.get(libc::RTAX_NETMASK as usize); - if dst.is_none() || netmask.is_none() { + let Some(dst) = rm.addrs.get(libc::RTAX_DST as usize) else { return false; - } - let dst = dst.unwrap(); - let netmask = netmask.unwrap(); + }; + let Some(netmask) = rm.addrs.get(libc::RTAX_NETMASK as usize) else { + return false; + }; match (dst, netmask) { (Addr::Inet4 { ip: dst }, Addr::Inet4 { ip: netmask }) => { @@ -211,6 +210,28 @@ pub enum WireMessage { InterfaceMulticastAddr(InterfaceMulticastAddrMessage), } +/// Safely convert a some bytes from a slice into a u16. +fn u16_from_ne_range( + data: &[u8], + range: impl std::slice::SliceIndex<[u8], Output = [u8]>, +) -> Result { + data.get(range) + .and_then(|s| TryInto::<[u8; 2]>::try_into(s).ok()) + .map(u16::from_ne_bytes) + .ok_or(RouteError::MessageTooShort) +} + +/// Safely convert some bytes from a slice into a u32. +fn u32_from_ne_range( + data: &[u8], + range: impl std::slice::SliceIndex<[u8], Output = [u8]>, +) -> Result { + data.get(range) + .and_then(|s| TryInto::<[u8; 4]>::try_into(s).ok()) + .map(u32::from_ne_bytes) + .ok_or(RouteError::MessageTooShort) +} + impl WireFormat { #[cfg(any( target_os = "freebsd", @@ -224,27 +245,26 @@ impl WireFormat { if data.len() < self.body_off { return Err(RouteError::MessageTooShort); } - let l = u16::from_ne_bytes(data[..2].try_into().unwrap()); + let l = u16_from_ne_range(data, ..2)?; if data.len() < l as usize { return Err(RouteError::InvalidMessage); } - let addrs = parse_addrs( - u32::from_ne_bytes(data[12..16].try_into().unwrap()) as _, - parse_kernel_inet_addr, - &data[self.body_off..], - )?; + let attrs: i32 = u32_from_ne_range(data, 12..16)? + .try_into() + .map_err(|_| RouteError::InvalidMessage)?; + let addrs = parse_addrs(attrs, parse_kernel_inet_addr, &data[self.body_off..])?; let mut m = RouteMessage { version: data[2] as _, r#type: data[3] as _, - flags: u32::from_ne_bytes(data[8..12].try_into().unwrap()), - index: u16::from_ne_bytes(data[4..6].try_into().unwrap()), - id: u32::from_ne_bytes(data[16..20].try_into().unwrap()) as _, - seq: u32::from_ne_bytes(data[20..24].try_into().unwrap()), + flags: u32_from_ne_range(data, 8..12)?, + index: u16_from_ne_range(data, 4..6)?, + id: u32_from_ne_range(data, 16..20)? as _, + seq: u32_from_ne_range(data, 20..24)?, ext_off: self.ext_off, error: None, addrs, }; - let errno = u32::from_ne_bytes(data[28..32].try_into().unwrap()); + let errno = u32_from_ne_range(data, 28..32)?; if errno != 0 { m.error = Some(std::io::Error::from_raw_os_error(errno as _)); } @@ -255,12 +275,12 @@ impl WireFormat { if data.len() < self.body_off { return Err(RouteError::MessageTooShort); } - let l = u16::from_ne_bytes(data[..2].try_into().unwrap()); + let l = u16_from_ne_range(data, 0..2)?; if data.len() < l as usize { return Err(RouteError::InvalidMessage); } - let attrs = u32::from_ne_bytes(data[4..8].try_into().unwrap()); + let attrs = u32_from_ne_range(data, 4..8)?; if attrs as libc::c_int & libc::RTA_IFP == 0 { return Ok(None); } @@ -269,8 +289,8 @@ impl WireFormat { let m = InterfaceMessage { version: data[2] as _, r#type: data[3] as _, - flags: u32::from_ne_bytes(data[8..12].try_into().unwrap()) as _, - index: u16::from_ne_bytes(data[12..14].try_into().unwrap()) as _, + flags: u32_from_ne_range(data, 8..12)? as _, + index: u16_from_ne_range(data, 12..14)? as _, ext_off: self.ext_off, addr_rtax_ifp: addr, name, @@ -282,18 +302,18 @@ impl WireFormat { if data.len() < self.body_off { return Err(RouteError::MessageTooShort); } - let l = u16::from_ne_bytes(data[..2].try_into().unwrap()); + let l = u16_from_ne_range(data, ..2)?; if data.len() < l as usize { return Err(RouteError::InvalidMessage); } #[cfg(target_arch = "netbsd")] - let index = u16::from_ne_bytes(data[16..18].try_into().unwrap()); + let index = u16_from_ne_range(data, 16..18)?; #[cfg(not(target_arch = "netbsd"))] - let index = u16::from_ne_bytes(data[12..14].try_into().unwrap()); + let index = u16_from_ne_range(data, 12..14)?; let addrs = parse_addrs( - u32::from_ne_bytes(data[4..8].try_into().unwrap()) as _, + u32_from_ne_range(data, 4..8)? as _, parse_kernel_inet_addr, &data[self.body_off..], )?; @@ -301,7 +321,7 @@ impl WireFormat { let m = InterfaceAddrMessage { version: data[2] as _, r#type: data[3] as _, - flags: u32::from_ne_bytes(data[8..12].try_into().unwrap()) as _, + flags: u32_from_ne_range(data, 8..12)? as _, index: index as _, addrs, }; @@ -311,20 +331,20 @@ impl WireFormat { if data.len() < self.body_off { return Err(RouteError::MessageTooShort); } - let l = u16::from_ne_bytes(data[..2].try_into().unwrap()); + let l = u16_from_ne_range(data, ..2)?; if data.len() < l as usize { return Err(RouteError::InvalidMessage); } let addrs = parse_addrs( - u32::from_ne_bytes(data[4..8].try_into().unwrap()) as _, + u32_from_ne_range(data, 4..8)? as _, parse_kernel_inet_addr, &data[self.body_off..], )?; let m = InterfaceMulticastAddrMessage { version: data[2] as _, r#type: data[3] as _, - flags: u32::from_ne_bytes(data[8..12].try_into().unwrap()) as _, - index: u16::from_ne_bytes(data[12..14].try_into().unwrap()) as _, + flags: u32_from_ne_range(data, 8..12)? as _, + index: u16_from_ne_range(data, 12..14)? as _, addrs, }; Ok(Some(WireMessage::InterfaceMulticastAddr(m))) @@ -368,7 +388,7 @@ pub fn parse_rib(typ: RIBType, data: &[u8]) -> Result, RouteErr while b.len() > 4 { nmsgs += 1; - let l = u16::from_ne_bytes(b[..2].try_into().unwrap()); + let l = u16_from_ne_range(b, ..2)?; if l == 0 { return Err(RouteError::InvalidMessage); } @@ -809,8 +829,11 @@ fn parse_inet_addr(af: i32, b: &[u8]) -> Result { return Err(RouteError::InvalidAddress); } - let mut zone = u32::from_ne_bytes(b[24..28].try_into().unwrap()); - let mut oc: [u8; 16] = b[8..24].try_into().unwrap(); + let mut zone = u32_from_ne_range(b, 24..28)?; + let mut oc: [u8; 16] = b + .get(8..24) + .and_then(|s| TryInto::<[u8; 16]>::try_into(s).ok()) + .ok_or(RouteError::InvalidMessage)?; if oc[0] == 0xfe && oc[1] & 0xc0 == 0x80 || oc[0] == 0xff && (oc[1] & 0x0f == 0x01 || oc[1] & 0x0f == 0x02) { @@ -818,7 +841,12 @@ fn parse_inet_addr(af: i32, b: &[u8]) -> Result { // embeds the interface index in the // interface-local or link-local address as // the kernel-internal form. - let id = u16::from_be_bytes(oc[2..4].try_into().unwrap()) as u32; + // NOTE: This is the only place in which uses big-endian. Is that right? + let id = oc + .get(2..4) + .and_then(|s| TryInto::<[u8; 2]>::try_into(s).ok()) + .map(u16::from_be_bytes) + .ok_or(RouteError::InvalidMessage)? as u32; if id != 0 { zone = id; oc[2] = 0; @@ -878,7 +906,10 @@ fn parse_kernel_inet_addr(af: i32, b: &[u8]) -> Result<(i32, Addr), RouteError> const OFF6: usize = 8; // offset of in6_addr let addr = if b[0] as usize == SIZEOF_SOCKADDR_INET6 { - let octets: [u8; 16] = b[OFF6..OFF6 + 16].try_into().unwrap(); + let octets: [u8; 16] = b + .get(OFF6..OFF6 + 16) + .and_then(|s| TryInto::try_into(s).ok()) + .ok_or(RouteError::InvalidMessage)?; let ip = Ipv6Addr::from(octets); Addr::Inet6 { ip, zone: 0 } } else if af == libc::AF_INET6 { @@ -891,7 +922,10 @@ fn parse_kernel_inet_addr(af: i32, b: &[u8]) -> Result<(i32, Addr), RouteError> let ip = Ipv6Addr::from(octets); Addr::Inet6 { ip, zone: 0 } } else if b[0] as usize == SIZEOF_SOCKADDR_INET { - let octets: [u8; 4] = b[OFF4..OFF4 + 4].try_into().unwrap(); + let octets: [u8; 4] = b + .get(OFF4..OFF4 + 4) + .and_then(|s| TryInto::try_into(s).ok()) + .ok_or(RouteError::InvalidMessage)?; let ip = Ipv4Addr::from(octets); Addr::Inet4 { ip } } else { @@ -916,7 +950,7 @@ fn parse_link_addr(b: &[u8]) -> Result { let (_, mut a) = parse_kernel_link_addr(libc::AF_LINK, &b[4..])?; if let Addr::Link { index, .. } = &mut a { - *index = u16::from_ne_bytes(b[2..4].try_into().unwrap()) as _; + *index = u16_from_ne_range(b, 2..4)? as _; } Ok(a) diff --git a/iroh-net/src/net/netmon/linux.rs b/iroh-net/src/net/netmon/linux.rs index 942f8240eb..0cdc1a81ce 100644 --- a/iroh-net/src/net/netmon/linux.rs +++ b/iroh-net/src/net/netmon/linux.rs @@ -108,12 +108,12 @@ impl RouteMonitor { .unwrap_or_default(); if let Some(dst) = get_nla!(msg, route::Nla::Destination) { let dst_addr = match dst.len() { - 4 => Some(IpAddr::from( - TryInto::<[u8; 4]>::try_into(&dst[..]).unwrap(), - )), - 16 => Some(IpAddr::from( - TryInto::<[u8; 16]>::try_into(&dst[..]).unwrap(), - )), + 4 => TryInto::<[u8; 4]>::try_into(&dst[..]) + .ok() + .map(IpAddr::from), + 16 => TryInto::<[u8; 16]>::try_into(&dst[..]) + .ok() + .map(IpAddr::from), _ => None, }; if let Some(dst_addr) = dst_addr { diff --git a/iroh-net/src/netcheck.rs b/iroh-net/src/netcheck.rs index 1af6919ce2..4feaaff4af 100644 --- a/iroh-net/src/netcheck.rs +++ b/iroh-net/src/netcheck.rs @@ -647,14 +647,15 @@ impl Actor { let mut best_any = Duration::default(); let mut old_derp_cur_latency = Duration::default(); { - for (url, d) in r.derp_latency.iter() { + for (url, duration) in r.derp_latency.iter() { if Some(url) == prev_derp.as_ref() { - old_derp_cur_latency = d; + old_derp_cur_latency = duration; } - let best = best_recent.get(url).unwrap(); - if r.preferred_derp.is_none() || best < best_any { - best_any = best; - r.preferred_derp.replace(url.clone()); + if let Some(best) = best_recent.get(url) { + if r.preferred_derp.is_none() || best < best_any { + best_any = best; + r.preferred_derp.replace(url.clone()); + } } } diff --git a/iroh-net/src/netcheck/reportgen/hairpin.rs b/iroh-net/src/netcheck/reportgen/hairpin.rs index 2c21661e4f..e43ec5541e 100644 --- a/iroh-net/src/netcheck/reportgen/hairpin.rs +++ b/iroh-net/src/netcheck/reportgen/hairpin.rs @@ -12,7 +12,7 @@ //! Note it will only perform a single hairpin check before shutting down. Any further //! requests to it will fail which is intentional. -use std::net::SocketAddr; +use std::net::{Ipv4Addr, SocketAddr}; use std::time::Duration; use anyhow::{bail, Context, Result}; @@ -164,7 +164,7 @@ impl Actor { // documentation-only IPv4 range is enough to set up the mapping. // So do that for now. In the future we might want to classify networks // that do and don't require this separately. But for now help it. - let documentation_ip: SocketAddr = "203.0.113.1:12345".parse().unwrap(); + let documentation_ip = SocketAddr::from((Ipv4Addr::new(203, 0, 113, 1), 12345)); socket .send_to( diff --git a/iroh-net/src/stun.rs b/iroh-net/src/stun.rs index 35ccf65af9..9d1a98e939 100644 --- a/iroh-net/src/stun.rs +++ b/iroh-net/src/stun.rs @@ -91,8 +91,12 @@ pub fn parse_binding_request(b: &[u8]) -> Result { // TODO: Tailscale sets the software to tailscale, we should check if we want to do this too. - let attrs = msg.attributes(); - if attrs.is_empty() || !attrs.last().unwrap().is_fingerprint() { + if msg + .attributes() + .last() + .map(|attr| !attr.is_fingerprint()) + .unwrap_or_default() + { return Err(Error::NoFingerprint); } diff --git a/iroh-net/src/ticket/node.rs b/iroh-net/src/ticket/node.rs index a5894509de..9e8c2298c6 100644 --- a/iroh-net/src/ticket/node.rs +++ b/iroh-net/src/ticket/node.rs @@ -91,13 +91,13 @@ mod tests { use iroh_test::{assert_eq_hex, hexdump::parse_hexdump}; use crate::key::{PublicKey, SecretKey}; - use std::net::SocketAddr; + use std::net::{Ipv4Addr, SocketAddr}; use super::*; fn make_ticket() -> NodeTicket { let peer = SecretKey::generate().public(); - let addr = SocketAddr::from_str("127.0.0.1:1234").unwrap(); + let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, 1234)); let derp_url = None; NodeTicket { node: NodeAddr::from_parts(peer, derp_url, vec![addr]), diff --git a/iroh-net/src/tls/certificate.rs b/iroh-net/src/tls/certificate.rs index a9a6e5b327..b26e6d87a9 100644 --- a/iroh-net/src/tls/certificate.rs +++ b/iroh-net/src/tls/certificate.rs @@ -186,10 +186,14 @@ fn make_libp2p_extension( }; let public_key = identity_secret_key.public(); + let public_key_ref = OctetStringRef::new(&public_key.as_bytes()[..]) + .map_err(|_| rcgen::RcgenError::CouldNotParseKeyPair)?; let signature = signature.to_bytes(); + let signature_ref = + OctetStringRef::new(&signature).map_err(|_| rcgen::RcgenError::CouldNotParseCertificate)?; let key = SignedKey { - public_key: OctetStringRef::new(&public_key.as_bytes()[..]).unwrap(), - signature: OctetStringRef::new(&signature).unwrap(), + public_key: public_key_ref, + signature: signature_ref, }; let mut extension_content = Vec::new();