Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor android ipv6 fix #7849

Merged
merged 2 commits into from
Mar 21, 2025
Merged

Refactor android ipv6 fix #7849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2de990f
Refactor how runtime IP version is applied
Serock3 Mar 20, 2025
3de440f
Rename and change to tuple struct
Serock3 Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions mullvad-daemon/src/tunnel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{

use tokio::sync::Mutex;

use mullvad_relay_selector::{GetRelay, RelaySelector, RuntimeParameters, WireguardConfig};
use mullvad_relay_selector::{GetRelay, RelaySelector, RuntimeIpAvailability, WireguardConfig};
use mullvad_types::{
endpoint::MullvadWireguardEndpoint, location::GeoIpLocation, relay_list::Relay,
settings::TunnelOptions,
Expand Down Expand Up @@ -164,9 +164,10 @@ impl InnerParametersGenerator {
ipv6: bool,
) -> Result<TunnelParameters, Error> {
let data = self.device().await?;
let selected_relay = self
.relay_selector
.get_relay(retry_attempt as usize, RuntimeParameters::new(ipv4, ipv6))?;
let selected_relay = self.relay_selector.get_relay(
retry_attempt as usize,
RuntimeIpAvailability::new(ipv4, ipv6),
)?;

match selected_relay {
#[cfg(not(target_os = "android"))]
Expand Down
3 changes: 3 additions & 0 deletions mullvad-relay-selector/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ pub enum Error {

#[error("Invalid bridge settings")]
InvalidBridgeSettings(#[from] MissingCustomBridgeSettings),

#[error("The requested IP version is not available")]
IpVersionUnavailable,
}

/// Special type which only shows up in [`Error`]. This error variant signals that no valid
Expand Down
6 changes: 3 additions & 3 deletions mullvad-relay-selector/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod relay_selector;
pub use error::Error;
pub use relay_selector::{
detailer, matcher, matcher::filter_matching_relay_list, query, relays::WireguardConfig,
AdditionalRelayConstraints, AdditionalWireguardConstraints, GetRelay, IpAvailability,
RelaySelector, RuntimeParameters, SelectedBridge, SelectedObfuscator, SelectorConfig,
OPENVPN_RETRY_ORDER, WIREGUARD_RETRY_ORDER,
AdditionalRelayConstraints, AdditionalWireguardConstraints, GetRelay, RelaySelector,
RuntimeIpAvailability, SelectedBridge, SelectedObfuscator, SelectorConfig, OPENVPN_RETRY_ORDER,
WIREGUARD_RETRY_ORDER,
};
143 changes: 48 additions & 95 deletions mullvad-relay-selector/src/relay_selector/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ use talpid_types::{
net::{
obfuscation::ObfuscatorConfig,
proxy::{CustomProxy, Shadowsocks},
Endpoint, TransportProtocol, TunnelType,
Endpoint, IpVersion, TransportProtocol, TunnelType,
},
ErrorExt,
};

/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
/// prioritize on successive connection attempts. Note that these will *never* override user
/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
/// should prioritize on successive connection attempts. Note that these will *never* override user
/// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
///
/// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
Expand All @@ -80,8 +80,8 @@ pub static WIREGUARD_RETRY_ORDER: LazyLock<Vec<RelayQuery>> = LazyLock::new(|| {
]
});

/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
/// prioritize on successive connection attempts. Note that these will *never* override user
/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
/// should prioritize on successive connection attempts. Note that these will *never* override user
/// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
///
/// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
Expand Down Expand Up @@ -178,64 +178,26 @@ pub struct AdditionalWireguardConstraints {
pub quantum_resistant: QuantumResistantState,
}

/// Values which affect the choice of relay but are only known at runtime.
/// Whether IPv4 and IPv6 is available at runtime.
///
/// `None` means that no IP version is available, `Some(Any)` means that both are available,
#[derive(Clone, Debug)]
pub struct RuntimeParameters {
/// Whether IPv4, IPv6 or both is available
pub ip_availability: IpAvailability,
}

impl RuntimeParameters {
/// Return whether a given [query][`RelayQuery`] is valid given the current runtime parameters
pub fn compatible(&self, query: &RelayQuery) -> bool {
match query.wireguard_constraints().ip_version {
Constraint::Any => true,
Constraint::Only(talpid_types::net::IpVersion::V4) => self.ip_availability.has_ipv4(),
Constraint::Only(talpid_types::net::IpVersion::V6) => self.ip_availability.has_ipv6(),
}
}

pub fn new(ipv4: bool, ipv6: bool) -> RuntimeParameters {
if ipv4 && ipv6 {
RuntimeParameters {
ip_availability: IpAvailability::All,
}
} else if !ipv6 {
RuntimeParameters {
ip_availability: IpAvailability::Ipv4,
}
} else if !ipv4 {
RuntimeParameters {
ip_availability: IpAvailability::Ipv6,
}
} else {
panic!("Device is offline!")
}
pub struct RuntimeIpAvailability(Option<Constraint<IpVersion>>);

impl RuntimeIpAvailability {
pub fn new(ipv4: bool, ipv6: bool) -> RuntimeIpAvailability {
RuntimeIpAvailability(match (ipv4, ipv6) {
(true, true) => Some(Constraint::Any),
(false, true) => Some(Constraint::Only(IpVersion::V6)),
(true, false) => Some(Constraint::Only(IpVersion::V4)),
(false, false) => None,
})
}
}

impl Default for RuntimeParameters {
impl Default for RuntimeIpAvailability {
fn default() -> Self {
RuntimeParameters {
ip_availability: IpAvailability::Ipv4,
}
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum IpAvailability {
Ipv4,
Ipv6,
All,
}

impl IpAvailability {
fn has_ipv4(&self) -> bool {
self.clone() == IpAvailability::Ipv4 || self.clone() == IpAvailability::All
}

fn has_ipv6(&self) -> bool {
self.clone() == IpAvailability::Ipv6 || self.clone() == IpAvailability::All
RuntimeIpAvailability(Some(Constraint::Only(IpVersion::V4)))
}
}

Expand Down Expand Up @@ -588,7 +550,7 @@ impl RelaySelector {
pub fn get_relay(
&self,
retry_attempt: usize,
runtime_params: RuntimeParameters,
runtime_ip_availability: RuntimeIpAvailability,
) -> Result<GetRelay, Error> {
let config_guard = self.config.lock().unwrap();
let config = SpecializedSelectorConfig::from(&*config_guard);
Expand All @@ -604,12 +566,12 @@ impl RelaySelector {
TunnelType::Wireguard => self.get_relay_with_custom_params(
retry_attempt,
&WIREGUARD_RETRY_ORDER,
runtime_params,
runtime_ip_availability,
),
TunnelType::OpenVpn => self.get_relay_with_custom_params(
retry_attempt,
&OPENVPN_RETRY_ORDER,
runtime_params,
runtime_ip_availability,
),
}
}
Expand All @@ -622,7 +584,7 @@ impl RelaySelector {
&self,
retry_attempt: usize,
retry_order: &[RelayQuery],
runtime_params: RuntimeParameters,
runtime_ip_availability: RuntimeIpAvailability,
) -> Result<GetRelay, Error> {
let config_guard = self.config.lock().unwrap();
let config = SpecializedSelectorConfig::from(&*config_guard);
Expand All @@ -639,7 +601,7 @@ impl RelaySelector {
let query = Self::pick_and_merge_query(
retry_attempt,
retry_order,
runtime_params,
runtime_ip_availability,
&normal_config,
&relay_list,
)?;
Expand All @@ -665,23 +627,16 @@ impl RelaySelector {
fn pick_and_merge_query(
retry_attempt: usize,
retry_order: &[RelayQuery],
runtime_params: RuntimeParameters,
runtime_ip_availability: RuntimeIpAvailability,
user_config: &NormalSelectorConfig<'_>,
parsed_relays: &RelayList,
) -> Result<RelayQuery, Error> {
let user_query = RelayQuery::try_from(user_config.clone())?;
let mut user_query = RelayQuery::try_from(user_config.clone())?;
apply_ip_availability(runtime_ip_availability, &mut user_query)?;
log::trace!("Merging user preferences {user_query:?} with default retry strategy");
retry_order
.iter()
// Remove candidate queries based on runtime parameters before trying to merge user
// settings
.filter(|query| runtime_params.compatible(query))
// Check if the user query aligns with the runtime parameters so that if the user
// has selected an ip version that is not available it will return an error
.filter(|_query| runtime_params.compatible(&user_query))
.filter_map(|query| query.clone().intersection(user_query.clone()))
// Resolve query ip version set to Any based on runtime ip availability
.map(|query| resolve_valid_ip_version(&query, &runtime_params))
.filter(|query| Self::get_relay_inner(query, parsed_relays, user_config.custom_lists).is_ok())
.cycle() // If the above filters remove all relays, cycle will also return an empty iterator
.nth(retry_attempt)
Expand Down Expand Up @@ -723,10 +678,10 @@ impl RelaySelector {
parsed_relays: &RelayList,
custom_lists: &CustomListsSettings,
) -> Result<GetRelay, Error> {
// FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query that
// doesn't specify Wireguard as the desired tunnel type is not valid and will lead
// to unwanted behavior. This should be seen as a workaround, and it would be nicer
// to lift this invariant to be checked by the type system instead.
// FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query
// that doesn't specify Wireguard as the desired tunnel type is not valid and will
// lead to unwanted behavior. This should be seen as a workaround, and it would be
// nicer to lift this invariant to be checked by the type system instead.
let mut query = query.clone();
query.set_tunnel_protocol(TunnelType::Wireguard)?;
Self::get_wireguard_relay_inner(&query, custom_lists, parsed_relays)
Expand Down Expand Up @@ -1213,24 +1168,22 @@ impl RelaySelector {
}
}

/// If the selected ip version is Any we want to resolve that to an Only ip version if only
/// one ip version is available on the network. This is to avoid situations where in other parts
/// of the relay selector that Any is resolved to IPv4 and IPv4 is not available.
fn resolve_valid_ip_version(query: &RelayQuery, runtime_params: &RuntimeParameters) -> RelayQuery {
let mut wireguard_constraints = query.wireguard_constraints().clone();
if wireguard_constraints.ip_version.is_any() {
if runtime_params.ip_availability == IpAvailability::Ipv4 {
wireguard_constraints.ip_version = Constraint::Only(talpid_types::net::IpVersion::V4)
}
if runtime_params.ip_availability == IpAvailability::Ipv6 {
wireguard_constraints.ip_version = Constraint::Only(talpid_types::net::IpVersion::V6)
}
}
let mut ret = query.clone();
match ret.set_wireguard_constraints(wireguard_constraints) {
Ok(()) => ret,
_error => query.clone(),
}
fn apply_ip_availability(
runtime_ip_availability: RuntimeIpAvailability,
user_query: &mut RelayQuery,
) -> Result<(), Error> {
let wireguard_constraints = user_query
.wireguard_constraints()
.to_owned()
.intersection(WireguardRelayQuery {
ip_version: runtime_ip_availability
.0
.ok_or(Error::IpVersionUnavailable)?,
..Default::default()
})
.ok_or(Error::IpVersionUnavailable)?;
user_query.set_wireguard_constraints(wireguard_constraints)?;
Ok(())
}

#[derive(Clone)]
Expand Down
Loading
Loading