From 7effe94ed0029bf132819392413cabd4310759e9 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Tue, 19 Mar 2024 10:04:17 +0100 Subject: [PATCH] Add Error types for `detailer.rs` --- mullvad-relay-selector/src/error.rs | 21 +++--- mullvad-relay-selector/src/lib.rs | 1 + .../src/relay_selector/detailer.rs | 66 ++++++++++++------- .../src/relay_selector/mod.rs | 14 ++-- 4 files changed, 64 insertions(+), 38 deletions(-) diff --git a/mullvad-relay-selector/src/error.rs b/mullvad-relay-selector/src/error.rs index fdc3cdc01238..f988be1b8931 100644 --- a/mullvad-relay-selector/src/error.rs +++ b/mullvad-relay-selector/src/error.rs @@ -3,7 +3,7 @@ use mullvad_types::{relay_constraints::MissingCustomBridgeSettings, relay_list::Relay}; -use crate::WireguardConfig; +use crate::{detailer, WireguardConfig}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -22,8 +22,11 @@ pub enum Error { #[error("No obfuscators matching current constraints")] NoObfuscator, - #[error("No endpoint could be constructed for relay {0:?}")] - NoEndpoint(EndpointError), + #[error("No endpoint could be constructed due to {} for relay {:?}", .internal, .relay)] + NoEndpoint { + internal: detailer::Error, + relay: EndpointErrorDetails, + }, #[error("Failure in serialization of the relay list")] Serialize(#[from] serde_json::Error), @@ -35,7 +38,7 @@ pub enum Error { /// Special type which only shows up in [`Error`]. This error variant signals that no valid /// endpoint could be constructed from the selected relay. #[derive(Debug)] -pub enum EndpointError { +pub enum EndpointErrorDetails { /// No valid Wireguard endpoint could be constructed from this [`WireguardConfig`]. /// /// # Note @@ -48,16 +51,16 @@ pub enum EndpointError { OpenVpn(Box), } -impl EndpointError { +impl EndpointErrorDetails { /// Helper function for constructing an [`Error::NoEndpoint`] from `relay`. /// Takes care of boxing the [`WireguardConfig`] for you! - pub(crate) fn from_wireguard(relay: WireguardConfig) -> Error { - Error::NoEndpoint(EndpointError::Wireguard(Box::new(relay))) + pub(crate) fn from_wireguard(relay: WireguardConfig) -> Self { + EndpointErrorDetails::Wireguard(Box::new(relay)) } /// Helper function for constructing an [`Error::NoEndpoint`] from `relay`. /// Takes care of boxing the [`Relay`] for you! - pub(crate) fn from_openvpn(relay: Relay) -> Error { - Error::NoEndpoint(EndpointError::OpenVpn(Box::new(relay))) + pub(crate) fn from_openvpn(relay: Relay) -> Self { + EndpointErrorDetails::OpenVpn(Box::new(relay)) } } diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs index 5d170e6c7b54..17b781519a5d 100644 --- a/mullvad-relay-selector/src/lib.rs +++ b/mullvad-relay-selector/src/lib.rs @@ -8,6 +8,7 @@ mod relay_selector; // Re-exports pub use error::Error; +pub use relay_selector::detailer; pub use relay_selector::{ query, GetRelay, RelaySelector, SelectedBridge, SelectedObfuscator, SelectorConfig, WireguardConfig, RETRY_ORDER, diff --git a/mullvad-relay-selector/src/relay_selector/detailer.rs b/mullvad-relay-selector/src/relay_selector/detailer.rs index 31bf900887c3..e26dd30afff1 100644 --- a/mullvad-relay-selector/src/relay_selector/detailer.rs +++ b/mullvad-relay-selector/src/relay_selector/detailer.rs @@ -27,6 +27,20 @@ use super::{ WireguardConfig, }; +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("No OpenVPN endpoint could be derived")] + NoOpenVPNEndpoint, + #[error("No bridge endpoint could be derived")] + NoBridgeEndpoint, + #[error("The selected relay does not support IPv6")] + NoIPv6(Box), + #[error("Invalid port argument: port {0} is not in any valid Wireguard port range")] + PortNotInRange(u16), + #[error("Port selection algorithm is broken")] + PortSelectionAlgorithm, +} + /// Constructs a [`MullvadWireguardEndpoint`] with details for how to connect to a Wireguard relay. /// /// # Returns @@ -37,7 +51,7 @@ pub fn wireguard_endpoint( query: &WireguardRelayQuery, data: &WireguardEndpointData, relay: &WireguardConfig, -) -> Option { +) -> Result { match relay { WireguardConfig::Singlehop { exit } => wireguard_singlehop_endpoint(query, data, exit), WireguardConfig::Multihop { exit, entry } => { @@ -51,7 +65,7 @@ fn wireguard_singlehop_endpoint( query: &WireguardRelayQuery, data: &WireguardEndpointData, exit: &Relay, -) -> Option { +) -> Result { let endpoint = { let host = get_address_for_wireguard_relay(query, exit)?; let port = get_port_for_wireguard_relay(query, data)?; @@ -64,7 +78,7 @@ fn wireguard_singlehop_endpoint( // This will be filled in later, not the relay selector's problem psk: None, }; - Some(MullvadWireguardEndpoint { + Ok(MullvadWireguardEndpoint { peer: peer_config, exit_peer: None, ipv4_gateway: data.ipv4_gateway, @@ -82,7 +96,7 @@ fn wireguard_multihop_endpoint( data: &WireguardEndpointData, exit: &Relay, entry: &Relay, -) -> Option { +) -> Result { let exit_endpoint = { let ip = exit.ipv4_addr_in; // The port that the exit relay listens for incoming connections from entry @@ -119,7 +133,7 @@ fn wireguard_multihop_endpoint( psk: None, }; - Some(MullvadWireguardEndpoint { + Ok(MullvadWireguardEndpoint { peer: entry, exit_peer: Some(exit), ipv4_gateway: data.ipv4_gateway, @@ -128,10 +142,16 @@ fn wireguard_multihop_endpoint( } /// Get the correct IP address for the given relay. -fn get_address_for_wireguard_relay(query: &WireguardRelayQuery, relay: &Relay) -> Option { +fn get_address_for_wireguard_relay( + query: &WireguardRelayQuery, + relay: &Relay, +) -> Result { match query.ip_version { - Constraint::Any | Constraint::Only(IpVersion::V4) => Some(relay.ipv4_addr_in.into()), - Constraint::Only(IpVersion::V6) => relay.ipv6_addr_in.map(|addr| addr.into()), + Constraint::Any | Constraint::Only(IpVersion::V4) => Ok(relay.ipv4_addr_in.into()), + Constraint::Only(IpVersion::V6) => relay + .ipv6_addr_in + .map(|addr| addr.into()) + .ok_or(Error::NoIPv6(Box::new(relay.clone()))), } } @@ -139,24 +159,18 @@ fn get_address_for_wireguard_relay(query: &WireguardRelayQuery, relay: &Relay) - fn get_port_for_wireguard_relay( query: &WireguardRelayQuery, data: &WireguardEndpointData, -) -> Option { +) -> Result { match query.port { - Constraint::Any => { - let random_port = select_random_port(&data.port_ranges); - if random_port.is_none() { - log::error!("Port selection algorithm is broken!"); - } - random_port - } + Constraint::Any => select_random_port(&data.port_ranges), Constraint::Only(port) => { if data .port_ranges .iter() .any(|range| (range.0 <= port && port <= range.1)) { - Some(port) + Ok(port) } else { - None + Err(Error::PortNotInRange(port)) } } } @@ -174,13 +188,13 @@ fn get_port_for_wireguard_relay( /// # Returns /// - `Option`: A randomly selected port number within the given ranges, or `None` if /// the input is empty or the total number of available ports is zero. -fn select_random_port(port_ranges: &[(u16, u16)]) -> Option { +fn select_random_port(port_ranges: &[(u16, u16)]) -> Result { use rand::Rng; let get_port_amount = |range: &(u16, u16)| -> u64 { (1 + range.1 - range.0) as u64 }; let port_amount: u64 = port_ranges.iter().map(get_port_amount).sum(); if port_amount < 1 { - return None; + return Err(Error::PortSelectionAlgorithm); } let mut port_index = rand::thread_rng().gen_range(0..port_amount); @@ -188,11 +202,11 @@ fn select_random_port(port_ranges: &[(u16, u16)]) -> Option { for range in port_ranges.iter() { let ports_in_range = get_port_amount(range); if port_index < ports_in_range { - return Some(port_index as u16 + range.0); + return Ok(port_index as u16 + range.0); } port_index -= ports_in_range; } - None + Err(Error::PortSelectionAlgorithm) } /// Constructs an [`Endpoint`] with details for how to connect to an OpenVPN relay. @@ -206,7 +220,7 @@ pub fn openvpn_endpoint( query: &OpenVpnRelayQuery, data: &OpenVpnEndpointData, relay: &Relay, -) -> Option { +) -> Result { // If `bridge_mode` is true, this function may only return endpoints which use TCP, not UDP. if BridgeQuery::should_use_bridge(&query.bridge_settings) { openvpn_bridge_endpoint(&query.port, data, relay) @@ -220,13 +234,14 @@ fn openvpn_singlehop_endpoint( port_constraint: &Constraint, data: &OpenVpnEndpointData, exit: &Relay, -) -> Option { +) -> Result { use rand::seq::IteratorRandom; data.ports .iter() .filter(|&endpoint| compatible_openvpn_port_combo(port_constraint, endpoint)) .choose(&mut rand::thread_rng()) .map(|endpoint| Endpoint::new(exit.ipv4_addr_in, endpoint.port, endpoint.protocol)) + .ok_or(Error::NoOpenVPNEndpoint) } /// Configure an endpoint that will be used together with a bridge. @@ -238,7 +253,7 @@ fn openvpn_bridge_endpoint( port_constraint: &Constraint, data: &OpenVpnEndpointData, exit: &Relay, -) -> Option { +) -> Result { use rand::seq::IteratorRandom; data.ports .iter() @@ -246,6 +261,7 @@ fn openvpn_bridge_endpoint( .filter(|endpoint| compatible_openvpn_port_combo(port_constraint, endpoint)) .choose(&mut rand::thread_rng()) .map(|endpoint| Endpoint::new(exit.ipv4_addr_in, endpoint.port, endpoint.protocol)) + .ok_or(Error::NoBridgeEndpoint) } /// Returns true if `port_constraint` can be used to connect to `endpoint`. diff --git a/mullvad-relay-selector/src/relay_selector/mod.rs b/mullvad-relay-selector/src/relay_selector/mod.rs index 62dcdf26c3d0..f8583ab4adfa 100644 --- a/mullvad-relay-selector/src/relay_selector/mod.rs +++ b/mullvad-relay-selector/src/relay_selector/mod.rs @@ -1,6 +1,6 @@ //! The implementation of the relay selector. -mod detailer; +pub mod detailer; mod helpers; mod matcher; mod parsed_relays; @@ -37,7 +37,7 @@ use talpid_types::{ ErrorExt, }; -use crate::error::{EndpointError, Error}; +use crate::error::{EndpointErrorDetails, Error}; use self::{ detailer::{openvpn_endpoint, wireguard_endpoint}, @@ -638,7 +638,10 @@ impl RelaySelector { &parsed_relays.parsed_list().wireguard, relay, ) - .ok_or(EndpointError::from_wireguard(relay.clone())) + .map_err(|internal| Error::NoEndpoint { + internal, + relay: EndpointErrorDetails::from_wireguard(relay.clone()), + }) } fn get_wireguard_obfuscator( @@ -709,7 +712,10 @@ impl RelaySelector { &parsed_relays.parsed_list().openvpn, relay, ) - .ok_or(EndpointError::from_openvpn(relay.clone())) + .map_err(|internal| Error::NoEndpoint { + internal, + relay: EndpointErrorDetails::from_openvpn(relay.clone()), + }) } /// Selects a suitable bridge based on the specified settings, relay information, and transport protocol.