diff options
| -rw-r--r-- | android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt | 1 | ||||
| -rw-r--r-- | mullvad-cli/src/cmds/bridge.rs | 2 | ||||
| -rw-r--r-- | mullvad-cli/src/cmds/relay.rs | 6 | ||||
| -rw-r--r-- | mullvad-cli/src/cmds/relay_constraints.rs | 2 | ||||
| -rw-r--r-- | mullvad-daemon/src/migrations/v6.rs | 303 | ||||
| -rw-r--r-- | mullvad-daemon/src/settings.rs | 4 | ||||
| -rw-r--r-- | mullvad-management-interface/src/types/conversions/relay_constraints.rs | 4 | ||||
| -rw-r--r-- | mullvad-relay-selector/src/lib.rs | 4 | ||||
| -rw-r--r-- | mullvad-types/src/relay_constraints.rs | 64 | ||||
| -rw-r--r-- | mullvad-types/src/settings/mod.rs | 16 |
10 files changed, 338 insertions, 68 deletions
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt index 5efca2648c..c6142694b9 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt @@ -6,7 +6,6 @@ import net.mullvad.mullvadvpn.ipc.EventDispatcher import net.mullvad.mullvadvpn.ipc.Request import net.mullvad.mullvadvpn.model.Constraint import net.mullvad.mullvadvpn.model.GeographicLocationConstraint -import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.PortRange import net.mullvad.mullvadvpn.model.RelayConstraints import net.mullvad.mullvadvpn.model.RelaySettings diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs index e43fce1b51..3cc1a95b37 100644 --- a/mullvad-cli/src/cmds/bridge.rs +++ b/mullvad-cli/src/cmds/bridge.rs @@ -168,7 +168,7 @@ impl Bridge { } else { Constraint::from(location) }; - let location = location.map(|location| LocationConstraint::Location { location }); + let location = location.map(LocationConstraint::Location); Self::update_bridge_settings(&mut rpc, Some(location), None, None).await } SetCommands::CustomList { custom_list_name } => { diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs index 1bb395c4e5..83f4ad4fdd 100644 --- a/mullvad-cli/src/cmds/relay.rs +++ b/mullvad-cli/src/cmds/relay.rs @@ -450,7 +450,7 @@ impl Relay { // The country field is assumed to be hostname due to CLI argument parsing find_relay_by_hostname(&countries, &location_constraint_args.country) { - Constraint::Only(LocationConstraint::Location { location: relay }) + Constraint::Only(LocationConstraint::Location(relay)) } else { let location_constraint: Constraint<GeographicLocationConstraint> = Constraint::from(location_constraint_args); @@ -468,7 +468,7 @@ impl Relay { } } } - location_constraint.map(|location| LocationConstraint::Location { location }) + location_constraint.map(LocationConstraint::Location) }; Self::update_constraints(RelaySettingsUpdate::Normal(RelayConstraintsUpdate { @@ -576,7 +576,7 @@ impl Relay { // The country field is assumed to be hostname due to CLI argument parsing wireguard_constraints.entry_location = if let Some(relay) = find_relay_by_hostname(&countries, &entry.country) { - Constraint::Only(LocationConstraint::Location { location: relay }) + Constraint::Only(LocationConstraint::Location(relay)) } else { Constraint::from(entry) }; diff --git a/mullvad-cli/src/cmds/relay_constraints.rs b/mullvad-cli/src/cmds/relay_constraints.rs index dc79e79f0b..626754edcb 100644 --- a/mullvad-cli/src/cmds/relay_constraints.rs +++ b/mullvad-cli/src/cmds/relay_constraints.rs @@ -49,6 +49,6 @@ impl From<LocationArgs> for Constraint<LocationConstraint> { } _ => unreachable!("invalid location arguments"), }; - Constraint::Only(LocationConstraint::Location { location }) + Constraint::Only(LocationConstraint::Location(location)) } } diff --git a/mullvad-daemon/src/migrations/v6.rs b/mullvad-daemon/src/migrations/v6.rs index 54dc2268e4..3a842923f0 100644 --- a/mullvad-daemon/src/migrations/v6.rs +++ b/mullvad-daemon/src/migrations/v6.rs @@ -36,16 +36,55 @@ pub fn migrate(settings: &mut serde_json::Value) -> Result<()> { migrate_udp2tcp_port_443(settings); - // TODO - // log::info!("Migrating settings format to V7"); + migrate_location_constraint(settings)?; - // Note: Not incrementing the version number yet, since this migration is still open - // for future modification. - // settings["settings_version"] = serde_json::json!(SettingsVersion::V7); + log::info!("Migrating settings format to V7"); + + settings["settings_version"] = serde_json::json!(SettingsVersion::V7); + + Ok(()) +} + +fn migrate_location_constraint(settings: &mut serde_json::Value) -> Result<()> { + if let Some(location) = settings + .get_mut("relay_settings") + .and_then(|relay_settings| relay_settings.get_mut("normal")) + .and_then(|normal_relay_settings| normal_relay_settings.get_mut("location")) + { + wrap_location(location)?; + } + + if let Some(location) = settings + .get_mut("relay_settings") + .and_then(|relay_settings| relay_settings.get_mut("normal")) + .and_then(|normal_relay_settings| normal_relay_settings.get_mut("wireguard_constraints")) + .and_then(|normal_relay_settings| normal_relay_settings.get_mut("entry_location")) + { + wrap_location(location)?; + } + + if let Some(location) = settings + .get_mut("bridge_settings") + .and_then(|relay_settings| relay_settings.get_mut("normal")) + .and_then(|normal_relay_settings| normal_relay_settings.get_mut("location")) + { + wrap_location(location)?; + } Ok(()) } +fn wrap_location(location: &mut serde_json::Value) -> Result<()> { + if let Some(only) = location.get_mut("only") { + only["location"] = only.clone(); + let only = only.as_object_mut().ok_or(Error::InvalidSettingsContent)?; + only.remove("country"); + only.remove("city"); + only.remove("hostname"); + } + Ok(()) +} + fn migrate_pq_setting(settings: &mut serde_json::Value) -> Result<()> { if let Some(tunnel_options) = settings .get_mut("tunnel_options") @@ -89,7 +128,7 @@ fn version_matches(settings: &mut serde_json::Value) -> bool { #[cfg(test)] mod test { - use super::{migrate, migrate_pq_setting, version_matches}; + use super::{migrate, migrate_location_constraint, migrate_pq_setting, version_matches}; pub const V6_SETTINGS: &str = r#" { @@ -175,7 +214,9 @@ mod test { "normal": { "location": { "only": { - "country": "se" + "location": { + "country": "se" + } } }, "tunnel_protocol": "any", @@ -241,7 +282,7 @@ mod test { } } }, - "settings_version": 6 + "settings_version": 7 } "#; @@ -256,6 +297,252 @@ mod test { assert_eq!(&old_settings, &new_settings); } + /// For relay settings + /// location: { only: { country : "se" } } should be replaced with + /// location: { only: { location: { country: "se" } } } + #[test] + fn test_from_relay_settings_location_constraint_country() { + let mut migrated_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "country": "se" + } + } + } + } + } + "#, + ) + .unwrap(); + migrate_location_constraint(&mut migrated_settings).unwrap(); + + let expected_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "location": { + "country": "se" + } + } + } + } + } + } + "#, + ) + .unwrap(); + + assert_eq!(migrated_settings, expected_settings); + } + + /// For relay settings + /// location: { only: { country : "se", city: "got" } } should be replaced with + /// location: { only: { location: { country: "se", city: "got" } } } + #[test] + fn test_from_relay_settings_location_constraint_city() { + let mut migrated_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "country": "se", + "city": "got" + } + } + } + } + } + "#, + ) + .unwrap(); + migrate_location_constraint(&mut migrated_settings).unwrap(); + + let expected_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "location": { + "country": "se", + "city": "got" + } + } + } + } + } + } + "#, + ) + .unwrap(); + + assert_eq!(migrated_settings, expected_settings); + } + + /// For relay settings + /// location: { only: { country : "se", city: "got", hostname: "se-got-wg-001" } } should be + /// replaced with location: { only: { location: { country: "se", city: "got", hostname: + /// "se-got-wg-001" } } } + #[test] + fn test_from_relay_settings_location_constraint_hostname() { + let mut migrated_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + "#, + ) + .unwrap(); + migrate_location_constraint(&mut migrated_settings).unwrap(); + + let expected_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "location": { + "only": { + "location": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + } + "#, + ) + .unwrap(); + + assert_eq!(migrated_settings, expected_settings); + } + + /// For bridge settings + /// location: { only: { country : "se", city: "got", hostname: "se-got-wg-001" } } should be + /// replaced with location: { only: { location: { country: "se", city: "got", hostname: + /// "se-got-wg-001" } } } + #[test] + fn test_from_bridge_location_constraint_hostname() { + let mut migrated_settings: serde_json::Value = serde_json::from_str( + r#" + { + "bridge_settings": { + "normal": { + "location": { + "only": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + "#, + ) + .unwrap(); + migrate_location_constraint(&mut migrated_settings).unwrap(); + + let expected_settings: serde_json::Value = serde_json::from_str( + r#" + { + "bridge_settings": { + "normal": { + "location": { + "only": { + "location": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + } + "#, + ) + .unwrap(); + + assert_eq!(migrated_settings, expected_settings); + } + + /// For wireguard constraints + /// location: { only: { country : "se", city: "got", hostname: "se-got-wg-001" } } should be + /// replaced with location: { only: { location: { country: "se", city: "got", hostname: + /// "se-got-wg-001" } } } + #[test] + fn test_from_wireguard_constraint_location_constraint_hostname() { + let mut migrated_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "wireguard_constraints": { + "entry_location": { + "only": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + } + "#, + ) + .unwrap(); + migrate_location_constraint(&mut migrated_settings).unwrap(); + + let expected_settings: serde_json::Value = serde_json::from_str( + r#" + { + "relay_settings": { + "normal": { + "wireguard_constraints": { + "entry_location": { + "only": { + "location": { + "country": "se", + "city": "got", + "hostname": "se-got-wg-001" + } + } + } + } + } + } + } + "#, + ) + .unwrap(); + + assert_eq!(migrated_settings, expected_settings); + } + /// use_pq_safe_psk=false should be replaced with quantum_resistant=null #[test] fn test_from_pq_safe_psk_false() { diff --git a/mullvad-daemon/src/settings.rs b/mullvad-daemon/src/settings.rs index ee3f355598..794b0381ca 100644 --- a/mullvad-daemon/src/settings.rs +++ b/mullvad-daemon/src/settings.rs @@ -378,9 +378,7 @@ mod test { "location": { "only": { "location": { - "location": { - "country": "gb" - } + "country": "gb" } } }, diff --git a/mullvad-management-interface/src/types/conversions/relay_constraints.rs b/mullvad-management-interface/src/types/conversions/relay_constraints.rs index 221c2140b2..e987ca102b 100644 --- a/mullvad-management-interface/src/types/conversions/relay_constraints.rs +++ b/mullvad-management-interface/src/types/conversions/relay_constraints.rs @@ -488,7 +488,7 @@ impl From<mullvad_types::relay_constraints::LocationConstraint> for proto::Locat fn from(location: mullvad_types::relay_constraints::LocationConstraint) -> Self { use mullvad_types::relay_constraints::LocationConstraint; match location { - LocationConstraint::Location { location } => Self { + LocationConstraint::Location(location) => Self { r#type: Some(proto::location_constraint::Type::Location( proto::RelayLocation::from(location), )), @@ -515,7 +515,7 @@ impl TryFrom<proto::LocationConstraint> match location { Constraint::Any => Ok(Constraint::Any), Constraint::Only(location) => { - Ok(Constraint::Only(LocationConstraint::Location { location })) + Ok(Constraint::Only(LocationConstraint::Location(location))) } } } diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs index 9fd6e3c6da..e62c63f3db 100644 --- a/mullvad-relay-selector/src/lib.rs +++ b/mullvad-relay-selector/src/lib.rs @@ -1564,7 +1564,7 @@ mod test { BridgeState::Off, attempt, TunnelType::OpenVpn, - &CustomListsSettings::default() + &CustomListsSettings::default(), ); assert_eq!( preferred.tunnel_protocol, @@ -1575,7 +1575,7 @@ mod test { BridgeState::Off, attempt, TunnelType::OpenVpn, - &CustomListsSettings::default() + &CustomListsSettings::default(), ) { Ok(result) if matches!(result.endpoint, MullvadEndpoint::OpenVpn(_)) => (), _ => panic!("OpenVPN endpoint was not selected"), diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs index 18ab4eef03..ab78ceca70 100644 --- a/mullvad-types/src/relay_constraints.rs +++ b/mullvad-types/src/relay_constraints.rs @@ -235,27 +235,19 @@ impl RelaySettings { } } -#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] #[cfg_attr(target_os = "android", derive(IntoJava, FromJava))] #[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] pub enum LocationConstraint { - Location { - location: GeographicLocationConstraint, - }, - CustomList { - list_id: Id, - }, + Location(GeographicLocationConstraint), + CustomList { list_id: Id }, } #[derive(Debug, Clone)] pub enum ResolvedLocationConstraint { - Location { - location: GeographicLocationConstraint, - }, - Locations { - locations: Vec<GeographicLocationConstraint>, - }, + Location(GeographicLocationConstraint), + Locations(Vec<GeographicLocationConstraint>), } impl ResolvedLocationConstraint { @@ -265,18 +257,14 @@ impl ResolvedLocationConstraint { ) -> Constraint<ResolvedLocationConstraint> { match location { Constraint::Any => Constraint::Any, - Constraint::Only(LocationConstraint::Location { location }) => { - Constraint::Only(Self::Location { location }) + Constraint::Only(LocationConstraint::Location(location)) => { + Constraint::Only(Self::Location(location)) } Constraint::Only(LocationConstraint::CustomList { list_id }) => custom_lists .custom_lists .iter() .find(|custom_list| custom_list.id == list_id) - .map(|custom_list| { - Constraint::Only(Self::Locations { - locations: custom_list.locations.clone(), - }) - }) + .map(|custom_list| Constraint::Only(Self::Locations(custom_list.locations.clone()))) .unwrap_or(Constraint::Any), } } @@ -284,7 +272,7 @@ impl ResolvedLocationConstraint { impl From<GeographicLocationConstraint> for LocationConstraint { fn from(location: GeographicLocationConstraint) -> Self { - Self::Location { location } + Self::Location(location) } } @@ -292,27 +280,23 @@ impl Set<Constraint<ResolvedLocationConstraint>> for Constraint<ResolvedLocation fn is_subset(&self, other: &Self) -> bool { match self { Constraint::Any => other.is_any(), - Constraint::Only(ResolvedLocationConstraint::Location { location }) => match other { + Constraint::Only(ResolvedLocationConstraint::Location(location)) => match other { Constraint::Any => true, - Constraint::Only(ResolvedLocationConstraint::Location { - location: other_location, - }) => location.is_subset(other_location), - Constraint::Only(ResolvedLocationConstraint::Locations { - locations: other_locations, - }) => other_locations - .iter() - .any(|other_location| location.is_subset(other_location)), + Constraint::Only(ResolvedLocationConstraint::Location(other_location)) => { + location.is_subset(other_location) + } + Constraint::Only(ResolvedLocationConstraint::Locations(other_locations)) => { + other_locations + .iter() + .any(|other_location| location.is_subset(other_location)) + } }, - Constraint::Only(ResolvedLocationConstraint::Locations { locations }) => match other { + Constraint::Only(ResolvedLocationConstraint::Locations(locations)) => match other { Constraint::Any => true, - Constraint::Only(ResolvedLocationConstraint::Location { - location: other_location, - }) => locations + Constraint::Only(ResolvedLocationConstraint::Location(other_location)) => locations .iter() .all(|location| location.is_subset(other_location)), - Constraint::Only(ResolvedLocationConstraint::Locations { - locations: other_locations, - }) => { + Constraint::Only(ResolvedLocationConstraint::Locations(other_locations)) => { for location in locations { if !other_locations .iter() @@ -332,10 +316,10 @@ impl Constraint<ResolvedLocationConstraint> { pub fn matches_with_opts(&self, relay: &Relay, ignore_include_in_country: bool) -> bool { match self { Constraint::Any => true, - Constraint::Only(ResolvedLocationConstraint::Location { location }) => { + Constraint::Only(ResolvedLocationConstraint::Location(location)) => { location.matches_with_opts(relay, ignore_include_in_country) } - Constraint::Only(ResolvedLocationConstraint::Locations { locations }) => locations + Constraint::Only(ResolvedLocationConstraint::Locations(locations)) => locations .iter() .any(|loc| loc.matches_with_opts(relay, ignore_include_in_country)), } @@ -345,7 +329,7 @@ impl Constraint<ResolvedLocationConstraint> { impl LocationConstraint { fn format(&self, f: &mut String, custom_lists: &CustomListsSettings) -> Result<(), fmt::Error> { match self { - Self::Location { location } => writeln!(f, "location - {location}"), + Self::Location(location) => writeln!(f, "location - {location}"), Self::CustomList { list_id } => match custom_lists .custom_lists .iter() diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs index 5af6ecfe08..12f5d831e1 100644 --- a/mullvad-types/src/settings/mod.rs +++ b/mullvad-types/src/settings/mod.rs @@ -21,7 +21,7 @@ mod dns; /// latest version that exists in `SettingsVersion`. /// This should be bumped when a new version is introduced along with a migration /// being added to `mullvad-daemon`. -pub const CURRENT_SETTINGS_VERSION: SettingsVersion = SettingsVersion::V6; +pub const CURRENT_SETTINGS_VERSION: SettingsVersion = SettingsVersion::V7; #[derive(Debug, PartialEq, Eq, PartialOrd, Clone, Copy)] #[repr(u32)] @@ -31,6 +31,7 @@ pub enum SettingsVersion { V4 = 4, V5 = 5, V6 = 6, + V7 = 7, } impl<'de> Deserialize<'de> for SettingsVersion { @@ -44,6 +45,7 @@ impl<'de> Deserialize<'de> for SettingsVersion { v if v == SettingsVersion::V4 as u32 => Ok(SettingsVersion::V4), v if v == SettingsVersion::V5 as u32 => Ok(SettingsVersion::V5), v if v == SettingsVersion::V6 as u32 => Ok(SettingsVersion::V6), + v if v == SettingsVersion::V7 as u32 => Ok(SettingsVersion::V7), v => Err(serde::de::Error::custom(format!( "{v} is not a valid SettingsVersion" ))), @@ -121,13 +123,13 @@ impl Default for Settings { fn default() -> Self { Settings { relay_settings: RelaySettings::Normal(RelayConstraints { - location: Constraint::Only(LocationConstraint::Location { - location: GeographicLocationConstraint::Country("se".to_owned()), - }), + location: Constraint::Only(LocationConstraint::Location( + GeographicLocationConstraint::Country("se".to_owned()), + )), wireguard_constraints: WireguardConstraints { - entry_location: Constraint::Only(LocationConstraint::Location { - location: GeographicLocationConstraint::Country("se".to_owned()), - }), + entry_location: Constraint::Only(LocationConstraint::Location( + GeographicLocationConstraint::Country("se".to_owned()), + )), ..Default::default() }, ..Default::default() |