Merge branch 'refactor-openvpn-proto'

10 files changed, 304 insertions, 268 deletions

diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts
index 24594c5e4d..c52c2370da 100644
--- a/gui/src/main/daemon-rpc.ts
+++ b/gui/src/main/daemon-rpc.ts
@@ -1137,18 +1137,21 @@ function convertFromKeygenEvent(data: grpcTypes.KeygenEvent): KeygenEvent {
 function convertFromOpenVpnConstraints(
   constraints: grpcTypes.OpenvpnConstraints,
 ): IOpenVpnConstraints {
-  const port = convertFromConstraint(constraints.getPort());
-  let protocol: Constraint<RelayProtocol> = 'any';
-  switch (constraints.getProtocol()?.getProtocol()) {
-    case grpcTypes.TransportProtocol.TCP:
-      protocol = { only: 'tcp' };
-      break;
-    case grpcTypes.TransportProtocol.UDP:
-      protocol = { only: 'udp' };
-      break;
+  const transportPort = convertFromConstraint(constraints.getPort());
+  if (transportPort !== 'any' && 'only' in transportPort) {
+    const port = convertFromConstraint(transportPort.only.getPort());
+    let protocol: Constraint<RelayProtocol> = 'any';
+    switch (transportPort.only.getProtocol()) {
+      case grpcTypes.TransportProtocol.TCP:
+        protocol = { only: 'tcp' };
+        break;
+      case grpcTypes.TransportProtocol.UDP:
+        protocol = { only: 'udp' };
+        break;
+    }
+    return { port, protocol };
   }
-
-  return { port, protocol };
+  return { port: 'any', protocol: 'any' };
 }
 
 function convertFromWireguardConstraints(
@@ -1241,15 +1244,15 @@ function convertToOpenVpnConstraints(
 ): grpcTypes.OpenvpnConstraints | undefined {
   const openvpnConstraints = new grpcTypes.OpenvpnConstraints();
   if (constraints) {
-    const port = liftConstraint(constraints.port);
-    if (port) {
-      openvpnConstraints.setPort(port);
-    }
     const protocol = liftConstraint(constraints.protocol);
     if (protocol) {
-      const transportConstraint = new grpcTypes.TransportProtocolConstraint();
-      transportConstraint.setProtocol(convertToTransportProtocol(protocol));
-      openvpnConstraints.setProtocol(transportConstraint);
+      const portConstraints = new grpcTypes.TransportPort();
+      const port = liftConstraint(constraints.port);
+      if (port) {
+        portConstraints.setPort(port);
+      }
+      portConstraints.setProtocol(convertToTransportProtocol(protocol));
+      openvpnConstraints.setPort(portConstraints);
     }
     return openvpnConstraints;
   }
diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs
index c3d990403e..80ee09a6c9 100644
--- a/mullvad-cli/src/cmds/relay.rs
+++ b/mullvad-cli/src/cmds/relay.rs
@@ -13,8 +13,7 @@ use mullvad_management_interface::types::{
     relay_settings, relay_settings_update, ConnectionConfig, CustomRelaySettings, IpVersion,
     IpVersionConstraint, NormalRelaySettingsUpdate, OpenvpnConstraints, ProviderUpdate,
     RelayListCountry, RelayLocation, RelaySettingsUpdate, TransportPort, TransportProtocol,
-    TransportProtocolConstraint, TunnelType, TunnelTypeConstraint, TunnelTypeUpdate,
-    WireguardConstraints,
+    TunnelType, TunnelTypeConstraint, TunnelTypeUpdate, WireguardConstraints,
 };
 use mullvad_types::relay_constraints::Constraint;
 use talpid_types::net::all_of_the_internet;
@@ -149,13 +148,15 @@ impl Command for Relay {
                                     .arg(
                                         clap::Arg::with_name("port")
                                             .help("Port to use. Either 'any' or a specific port")
-                                            .required(true)
+                                            .long("port")
+                                            .default_value("any"),
                                     )
                                     .arg(
                                         clap::Arg::with_name("transport protocol")
+                                            .help("Transport protocol")
                                             .long("protocol")
-                                            .default_value("any")
-                                            .possible_values(&["any", "udp", "tcp"]),
+                                            .possible_values(&["any", "udp", "tcp"])
+                                            .default_value("any"),
                                     )
                             )
                             .subcommand(
@@ -165,17 +166,15 @@ impl Command for Relay {
                                         clap::Arg::with_name("port")
                                             .help("Port to use. Either 'any' or a specific port")
                                             .long("port")
-                                            .takes_value(true)
-                                            .requires("transport protocol"),
+                                            .default_value("any"),
                                     )
                                     .arg(
                                         clap::Arg::with_name("transport protocol")
                                             .help("Transport protocol. If TCP is selected, traffic is \
                                                    sent over TCP using a udp-over-tcp proxy")
                                             .long("protocol")
-                                            .takes_value(true)
-                                            .possible_values(&["udp", "tcp"])
-                                            .requires("port"),
+                                            .possible_values(&["any", "udp", "tcp"])
+                                            .default_value("any"),
                                     )
                                     .arg(
                                         clap::Arg::with_name("ip version")
@@ -516,20 +515,11 @@ impl Relay {
     }
 
     async fn set_openvpn_constraints(&self, matches: &clap::ArgMatches<'_>) -> Result<()> {
-        let port = parse_port_constraint(matches.value_of("port").unwrap())?;
-        let protocol = parse_protocol_constraint(matches.value_of("transport protocol").unwrap());
-
+        let port = parse_transport_port(matches)?;
         self.update_constraints(RelaySettingsUpdate {
             r#type: Some(relay_settings_update::Type::Normal(
                 NormalRelaySettingsUpdate {
-                    openvpn_constraints: Some(OpenvpnConstraints {
-                        port: port.unwrap_or(0) as u32,
-                        protocol: protocol
-                            .option()
-                            .map(|protocol| TransportProtocolConstraint {
-                                protocol: protocol as i32,
-                            }),
-                    }),
+                    openvpn_constraints: Some(OpenvpnConstraints { port }),
                     ..Default::default()
                 },
             )),
@@ -538,19 +528,7 @@ impl Relay {
     }
 
     async fn set_wireguard_constraints(&self, matches: &clap::ArgMatches<'_>) -> Result<()> {
-        let protocol = matches.value_of("transport protocol").map(parse_protocol);
-        let port = match matches.value_of("port").map(parse_port_constraint) {
-            None => None,
-            Some(Err(error)) => return Err(error),
-            Some(Ok(Constraint::Any)) => Some(TransportPort {
-                protocol: protocol.unwrap() as i32,
-                port: 0,
-            }),
-            Some(Ok(Constraint::Only(port))) => Some(TransportPort {
-                protocol: protocol.unwrap() as i32,
-                port: u32::from(port),
-            }),
-        };
+        let port = parse_transport_port(matches)?;
         let ip_version = parse_ip_version_constraint(matches.value_of("ip version").unwrap());
         let entry_location =
             parse_entry_location_constraint(matches.values_of("entry location").unwrap());
@@ -722,26 +700,12 @@ impl Relay {
         }
     }
 
-    fn format_port(port: u32) -> String {
-        if port != 0 {
-            format!("port {}", port)
-        } else {
-            "any port".to_string()
-        }
-    }
-
     fn format_openvpn_constraints(constraints: Option<&OpenvpnConstraints>) -> String {
         if let Some(constraints) = constraints {
-            format!(
-                "{} over {}",
-                Self::format_port(constraints.port),
-                Self::format_transport_protocol(
-                    constraints
-                        .protocol
-                        .clone()
-                        .map(|protocol| TransportProtocol::from_i32(protocol.protocol).unwrap())
-                )
-            )
+            let ovpn_constraints =
+                mullvad_types::relay_constraints::OpenVpnConstraints::try_from(constraints)
+                    .unwrap();
+            format!("{}", ovpn_constraints)
         } else {
             "any port over any transport protocol".to_string()
         }
@@ -805,19 +769,11 @@ fn parse_port_constraint(raw_port: &str) -> Result<Constraint<u16>> {
     }
 }
 
-/// Parses a protocol constraint string. Can be infallible because the possible values are limited
-/// with clap.
-fn parse_protocol_constraint(raw_protocol: &str) -> Constraint<TransportProtocol> {
+fn parse_protocol(raw_protocol: &str) -> Constraint<TransportProtocol> {
     match raw_protocol {
         "any" => Constraint::Any,
-        protocol => Constraint::Only(parse_protocol(protocol)),
-    }
-}
-
-fn parse_protocol(raw_protocol: &str) -> TransportProtocol {
-    match raw_protocol {
-        "udp" => TransportProtocol::Udp,
-        "tcp" => TransportProtocol::Tcp,
+        "udp" => Constraint::Only(TransportProtocol::Udp),
+        "tcp" => Constraint::Only(TransportProtocol::Tcp),
         _ => unreachable!(),
     }
 }
@@ -846,3 +802,22 @@ fn parse_entry_location_constraint<'a, T: Iterator<Item = &'a str>>(
         location.next(),
     ))
 }
+
+fn parse_transport_port(matches: &clap::ArgMatches<'_>) -> Result<Option<TransportPort>> {
+    let port = parse_port_constraint(matches.value_of("port").unwrap())?;
+    let protocol = parse_protocol(matches.value_of("transport protocol").unwrap());
+    match (port, protocol) {
+        (Constraint::Any, Constraint::Any) => Ok(None),
+        (Constraint::Any, Constraint::Only(protocol)) => Ok(Some(TransportPort {
+            protocol: protocol as i32,
+            ..TransportPort::default()
+        })),
+        (Constraint::Only(port), Constraint::Only(protocol)) => Ok(Some(TransportPort {
+            protocol: protocol as i32,
+            port: u32::from(port),
+        })),
+        (Constraint::Only(_), Constraint::Any) => Err(Error::InvalidCommand(
+            "a transport protocol must be given to select a specific port",
+        )),
+    }
+}
diff --git a/mullvad-daemon/src/relays.rs b/mullvad-daemon/src/relays.rs
index 8417926c3d..ad1a038817 100644
--- a/mullvad-daemon/src/relays.rs
+++ b/mullvad-daemon/src/relays.rs
@@ -389,12 +389,13 @@ impl RelaySelector {
             // If no tunnel protocol is selected, use preferred constraints
             Constraint::Any => {
                 if original_constraints.openvpn_constraints.port.is_any()
-                    && original_constraints.openvpn_constraints.protocol.is_any()
                     || bridge_state == BridgeState::On
                 {
                     relay_constraints.openvpn_constraints = OpenVpnConstraints {
-                        port: preferred_port,
-                        protocol: Constraint::Only(preferred_protocol),
+                        port: Constraint::Only(TransportPort {
+                            protocol: preferred_protocol,
+                            port: preferred_port,
+                        }),
                     };
                 } else {
                     relay_constraints.openvpn_constraints =
@@ -414,15 +415,19 @@ impl RelaySelector {
             Constraint::Only(TunnelType::OpenVpn) => {
                 let openvpn_constraints = &mut relay_constraints.openvpn_constraints;
                 *openvpn_constraints = original_constraints.openvpn_constraints;
-                if bridge_state == BridgeState::On && openvpn_constraints.protocol.is_any() {
+                if bridge_state == BridgeState::On && openvpn_constraints.port.is_any() {
                     // FIXME: This is temporary while talpid-core only supports TCP proxies
-                    openvpn_constraints.protocol = Constraint::Only(TransportProtocol::Tcp);
-                } else if openvpn_constraints.port.is_any() && openvpn_constraints.protocol.is_any()
-                {
+                    openvpn_constraints.port = Constraint::Only(TransportPort {
+                        protocol: TransportProtocol::Tcp,
+                        port: Constraint::Any,
+                    });
+                } else if openvpn_constraints.port.is_any() {
                     let (preferred_port, preferred_protocol) =
                         Self::preferred_openvpn_constraints(retry_attempt);
-                    openvpn_constraints.port = preferred_port;
-                    openvpn_constraints.protocol = Constraint::Only(preferred_protocol);
+                    openvpn_constraints.port = Constraint::Only(TransportPort {
+                        protocol: preferred_protocol,
+                        port: preferred_port,
+                    });
                 }
             }
             Constraint::Only(TunnelType::Wireguard) => {
diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index 489b8982d3..aa59458979 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -313,18 +313,13 @@ message TunnelTypeUpdate {
 	TunnelTypeConstraint tunnel_type = 2;
 }
 
-message TransportProtocolConstraint {
-	TransportProtocol protocol = 1;
-}
-
 message TransportPort {
 	TransportProtocol protocol = 1;
 	uint32 port = 2;
 }
 
 message OpenvpnConstraints {
-	uint32 port = 1;
-	TransportProtocolConstraint protocol = 2;
+	TransportPort port = 1;
 }
 
 enum IpVersion {
diff --git a/mullvad-management-interface/src/types.rs b/mullvad-management-interface/src/types.rs
index 390560fe49..38e5031df1 100644
--- a/mullvad-management-interface/src/types.rs
+++ b/mullvad-management-interface/src/types.rs
@@ -294,14 +294,6 @@ impl From<talpid_types::net::TransportProtocol> for TransportProtocol {
     }
 }
 
-impl From<TransportProtocol> for TransportProtocolConstraint {
-    fn from(protocol: TransportProtocol) -> Self {
-        Self {
-            protocol: i32::from(protocol),
-        }
-    }
-}
-
 impl From<talpid_types::net::IpVersion> for IpVersion {
     fn from(version: talpid_types::net::IpVersion) -> Self {
         match version {
@@ -518,14 +510,11 @@ impl From<mullvad_types::relay_constraints::RelaySettings> for RelaySettings {
                     }),
 
                     openvpn_constraints: Some(OpenvpnConstraints {
-                        port: u32::from(constraints.openvpn_constraints.port.unwrap_or(0)),
-                        protocol: constraints
+                        port: constraints
                             .openvpn_constraints
-                            .protocol
-                            .as_ref()
+                            .port
                             .option()
-                            .map(|protocol| TransportProtocol::from(*protocol))
-                            .map(TransportProtocolConstraint::from),
+                            .map(TransportPort::from),
                     }),
                 })
             }
@@ -728,6 +717,23 @@ impl TryFrom<&WireguardConstraints> for mullvad_types::relay_constraints::Wiregu
     }
 }
 
+impl TryFrom<&OpenvpnConstraints> for mullvad_types::relay_constraints::OpenVpnConstraints {
+    type Error = FromProtobufTypeError;
+
+    fn try_from(
+        constraints: &OpenvpnConstraints,
+    ) -> Result<mullvad_types::relay_constraints::OpenVpnConstraints, Self::Error> {
+        use mullvad_types::relay_constraints as mullvad_constraints;
+
+        Ok(mullvad_constraints::OpenVpnConstraints {
+            port: Constraint::from(match &constraints.port {
+                Some(port) => Some(mullvad_constraints::TransportPort::try_from(port.clone())?),
+                None => None,
+            }),
+        })
+    }
+}
+
 impl TryFrom<RelaySettingsUpdate> for mullvad_types::relay_constraints::RelaySettingsUpdate {
     type Error = FromProtobufTypeError;
 
@@ -912,11 +918,11 @@ impl TryFrom<RelaySettingsUpdate> for mullvad_types::relay_constraints::RelaySet
                     None
                 };
 
-                let openvpn_transport_protocol =
+                let openvpn_transport_port =
                     if let Some(ref constraints) = settings.openvpn_constraints {
-                        match &constraints.protocol {
-                            Some(constraint) => {
-                                Some(try_transport_protocol_from_i32(constraint.protocol)?)
+                        match &constraints.port {
+                            Some(port) => {
+                                Some(mullvad_constraints::TransportPort::try_from(port.clone())?)
                             }
                             None => None,
                         }
@@ -986,14 +992,9 @@ impl TryFrom<RelaySettingsUpdate> for mullvad_types::relay_constraints::RelaySet
                                 ),
                             }
                         }),
-                        openvpn_constraints: settings.openvpn_constraints.map(|constraints| {
+                        openvpn_constraints: settings.openvpn_constraints.map(|_constraints| {
                             mullvad_constraints::OpenVpnConstraints {
-                                port: if constraints.port != 0 {
-                                    Constraint::Only(constraints.port as u16)
-                                } else {
-                                    Constraint::Any
-                                },
-                                protocol: Constraint::from(openvpn_transport_protocol),
+                                port: Constraint::from(openvpn_transport_port),
                             }
                         }),
                     },
diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs
index fdefb25fce..d1d3a87f71 100644
--- a/mullvad-types/src/relay_constraints.rs
+++ b/mullvad-types/src/relay_constraints.rs
@@ -71,6 +71,10 @@ impl<T: fmt::Debug + Clone + Eq + PartialEq> Constraint<T> {
         }
     }
 
+    pub fn is_only(&self) -> bool {
+        !self.is_any()
+    }
+
     pub fn as_ref(&self) -> Constraint<&T> {
         match self {
             Constraint::Any => Constraint::Any,
@@ -176,13 +180,12 @@ impl RelaySettings {
                 if constraints.tunnel_protocol == Constraint::Only(TunnelType::Wireguard) {
                     constraints.tunnel_protocol = Constraint::Any;
                 }
-                if constraints.openvpn_constraints.protocol
-                    == Constraint::Only(TransportProtocol::Udp)
+                if let Constraint::Only(TransportPort {
+                    protocol: TransportProtocol::Udp,
+                    ..
+                }) = constraints.openvpn_constraints.port
                 {
-                    constraints.openvpn_constraints = OpenVpnConstraints {
-                        protocol: Constraint::Any,
-                        port: Constraint::Any,
-                    }
+                    constraints.openvpn_constraints.port = Constraint::Any;
                 }
             }
             RelaySettings::CustomTunnelEndpoint(config) => {
@@ -454,27 +457,36 @@ pub struct TransportPort {
 /// [`Constraint`]s applicable to OpenVPN relay servers.
 #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Deserialize, Serialize)]
 pub struct OpenVpnConstraints {
-    pub port: Constraint<u16>,
-    pub protocol: Constraint<TransportProtocol>,
+    pub port: Constraint<TransportPort>,
 }
 
 impl fmt::Display for OpenVpnConstraints {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
         match self.port {
-            Constraint::Any => write!(f, "any port")?,
-            Constraint::Only(port) => write!(f, "port {}", port)?,
-        }
-        write!(f, " over ")?;
-        match self.protocol {
-            Constraint::Any => write!(f, "any protocol"),
-            Constraint::Only(protocol) => write!(f, "{}", protocol),
+            Constraint::Any => write!(f, "any port"),
+            Constraint::Only(port) => {
+                match port.port {
+                    Constraint::Any => write!(f, "any port")?,
+                    Constraint::Only(port) => write!(f, "port {}", port)?,
+                }
+                write!(f, " over {}", port.protocol)
+            }
         }
     }
 }
 
 impl Match<OpenVpnEndpointData> for OpenVpnConstraints {
     fn matches(&self, endpoint: &OpenVpnEndpointData) -> bool {
-        self.port.matches_eq(&endpoint.port) && self.protocol.matches_eq(&endpoint.protocol)
+        match self.port {
+            Constraint::Any => true,
+            Constraint::Only(transport_port) => {
+                transport_port.protocol == endpoint.protocol
+                    && match transport_port.port {
+                        Constraint::Any => true,
+                        Constraint::Only(port) => port == endpoint.port,
+                    }
+            }
+        }
     }
 }
 
@@ -613,7 +625,11 @@ impl RelaySettingsUpdate {
                 if let Some(Constraint::Only(TunnelType::Wireguard)) = &update.tunnel_protocol {
                     false
                 } else if let Some(constraints) = &update.openvpn_constraints {
-                    if let Constraint::Only(TransportProtocol::Udp) = &constraints.protocol {
+                    if let Constraint::Only(TransportPort {
+                        protocol: TransportProtocol::Udp,
+                        ..
+                    }) = &constraints.port
+                    {
                         false
                     } else {
                         true
diff --git a/mullvad-types/src/settings/migrations/v1.rs b/mullvad-types/src/settings/migrations/v1.rs
index 6f7152fec6..8e9ed04b89 100644
--- a/mullvad-types/src/settings/migrations/v1.rs
+++ b/mullvad-types/src/settings/migrations/v1.rs
@@ -1,12 +1,5 @@
-use super::{Error, Result};
-use crate::{
-    custom_tunnel::CustomTunnelEndpoint,
-    relay_constraints::{
-        Constraint, LocationConstraint, OpenVpnConstraints, RelaySettings as NewRelaySettings,
-        WireguardConstraints,
-    },
-};
-use serde::{Deserialize, Serialize};
+use super::Result;
+use crate::relay_constraints::Constraint;
 use talpid_types::net::TunnelType;
 
 
@@ -20,71 +13,58 @@ impl super::SettingsMigration for Migration {
     fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
         log::info!("Migrating settings format to V2");
 
-        let old_relay_settings: RelaySettings =
-            serde_json::from_value(settings["relay_settings"].clone())
-                .map_err(Error::ParseError)?;
-        let new_relay_settings = migrate_relay_settings(old_relay_settings);
-
-        settings["relay_settings"] = serde_json::json!(new_relay_settings);
-        settings["show_beta_releases"] = serde_json::json!(false);
-        settings["settings_version"] = serde_json::json!(super::SettingsVersion::V2);
-
-        Ok(())
-    }
-}
+        let openvpn_constraints = || -> Option<serde_json::Value> {
+            settings
+                .get("relay_settings")?
+                .get("normal")?
+                .get("tunnel")?
+                .get("only")?
+                .get("openvpn")
+                .cloned()
+        }();
+        let wireguard_constraints = || -> Option<serde_json::Value> {
+            settings
+                .get("relay_settings")?
+                .get("normal")?
+                .get("tunnel")?
+                .get("only")?
+                .get("wireguard")
+                .cloned()
+        }();
 
-fn migrate_relay_settings(relay_settings: RelaySettings) -> NewRelaySettings {
-    match relay_settings {
-        RelaySettings::CustomTunnelEndpoint(endpoint) => {
-            crate::relay_constraints::RelaySettings::CustomTunnelEndpoint(endpoint)
-        }
-        RelaySettings::Normal(old_constraints) => {
-            let mut new_constraints = crate::relay_constraints::RelayConstraints {
-                location: old_constraints.location,
-                ..Default::default()
-            };
-            match old_constraints.tunnel {
-                Constraint::Any => (),
-                Constraint::Only(TunnelConstraints::OpenVpn(constraints)) => {
-                    new_constraints.openvpn_constraints = constraints;
+        if let Some(relay_settings) = settings.get_mut("relay_settings") {
+            if let Some(normal_settings) = relay_settings.get_mut("normal") {
+                if let Some(openvpn_constraints) = openvpn_constraints {
+                    normal_settings["openvpn_constraints"] = openvpn_constraints;
+                    normal_settings["tunnel_protocol"] =
+                        serde_json::json!(Constraint::<TunnelType>::Any);
+                } else if let Some(wireguard_constraints) = wireguard_constraints {
+                    normal_settings["wireguard_constraints"] = wireguard_constraints;
+                    normal_settings["tunnel_protocol"] =
+                        serde_json::json!(Constraint::Only(TunnelType::Wireguard));
+                } else {
+                    normal_settings["tunnel_protocol"] =
+                        serde_json::json!(Constraint::<TunnelType>::Any);
                 }
-                Constraint::Only(TunnelConstraints::Wireguard(constraints)) => {
-                    new_constraints.wireguard_constraints = constraints;
-                    new_constraints.tunnel_protocol = Constraint::Only(TunnelType::Wireguard);
+                if let Some(object) = normal_settings.as_object_mut() {
+                    object.remove("tunnel");
                 }
-            };
-            crate::relay_constraints::RelaySettings::Normal(new_constraints)
+            }
         }
-    }
-}
 
-#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
-#[serde(rename_all = "snake_case")]
-pub enum RelaySettings {
-    CustomTunnelEndpoint(CustomTunnelEndpoint),
-    Normal(RelayConstraints),
-}
-
-#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
-pub struct RelayConstraints {
-    pub location: Constraint<LocationConstraint>,
-    pub tunnel: Constraint<TunnelConstraints>,
-}
+        settings["show_beta_releases"] = serde_json::json!(false);
+        settings["settings_version"] = serde_json::json!(super::SettingsVersion::V2);
 
-#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
-pub enum TunnelConstraints {
-    #[serde(rename = "openvpn")]
-    OpenVpn(OpenVpnConstraints),
-    #[serde(rename = "wireguard")]
-    Wireguard(WireguardConstraints),
+        Ok(())
+    }
 }
 
 #[cfg(test)]
 mod test {
-    use super::super::try_migrate_settings;
+    use super::{super::SettingsMigration, Migration};
     use serde_json;
 
-    pub const NEW_SETTINGS: &str = r#"
+    pub const V2_SETTINGS: &str = r#"
 {
   "account_token": "1234",
   "relay_settings": {
@@ -95,9 +75,6 @@ mod test {
         }
       },
       "tunnel_protocol": "any",
-      "wireguard_constraints": {
-        "port": "any"
-      },
       "openvpn_constraints": {
         "port": {
           "only": 53
@@ -108,12 +85,6 @@ mod test {
       }
     }
   },
-  "bridge_settings": {
-    "normal": {
-      "location": "any"
-    }
-  },
-  "bridge_state": "auto",
   "allow_lan": true,
   "block_when_disconnected": false,
   "auto_connect": false,
@@ -128,7 +99,8 @@ mod test {
       "enable_ipv6": false
     }
   },
-  "settings_version": 5
+  "show_beta_releases": false,
+  "settings_version": 2
 }
 "#;
 
@@ -156,12 +128,6 @@ mod test {
       }
     }
   },
-  "bridge_settings": {
-    "normal": {
-      "location": "any"
-    }
-  },
-  "bridge_state": "auto",
   "allow_lan": true,
   "block_when_disconnected": false,
   "auto_connect": false,
@@ -208,8 +174,7 @@ mod test {
   "auto_connect": false,
   "tunnel_options": {
     "openvpn": {
-      "mssfix": null,
-      "proxy": null
+      "mssfix": null
     },
     "wireguard": {
       "mtu": null
@@ -224,19 +189,27 @@ mod test {
 
     #[test]
     fn test_v1_migration() {
-        let migrated_settings =
-            try_migrate_settings(V1_SETTINGS.as_bytes()).expect("Migration failed");
-        let new_settings = serde_json::from_str(NEW_SETTINGS).unwrap();
+        let mut old_settings = serde_json::from_str(V1_SETTINGS).unwrap();
+
+        let migration = Migration;
+        assert!(migration.version_matches(&mut old_settings));
 
-        assert_eq!(&migrated_settings, &new_settings);
+        migration.migrate(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
+
+        assert_eq!(&old_settings, &new_settings);
     }
 
     #[test]
     fn test_v1_2019v3_migration() {
-        let migrated_settings =
-            try_migrate_settings(V1_SETTINGS_2019V3.as_bytes()).expect("Migration failed");
-        let new_settings = serde_json::from_str(NEW_SETTINGS).unwrap();
+        let mut old_settings = serde_json::from_str(V1_SETTINGS_2019V3).unwrap();
+
+        let migration = Migration;
+        assert!(migration.version_matches(&mut old_settings));
+
+        migration.migrate(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
 
-        assert_eq!(&migrated_settings, &new_settings);
+        assert_eq!(&old_settings, &new_settings);
     }
 }
diff --git a/mullvad-types/src/settings/migrations/v2.rs b/mullvad-types/src/settings/migrations/v2.rs
index 10a255a44c..04d94eacbf 100644
--- a/mullvad-types/src/settings/migrations/v2.rs
+++ b/mullvad-types/src/settings/migrations/v2.rs
@@ -51,6 +51,10 @@ impl super::SettingsMigration for Migration {
 
             settings["tunnel_options"]["wireguard"]["rotation_interval"] =
                 serde_json::json!(new_ivl);
+            settings["tunnel_options"]["wireguard"]
+                .as_object_mut()
+                .ok_or(Error::NoMatchingVersion)?
+                .remove("automatic_rotation");
         }
 
         settings["settings_version"] = serde_json::json!(SettingsVersion::V3);
@@ -61,7 +65,7 @@ impl super::SettingsMigration for Migration {
 
 #[cfg(test)]
 mod test {
-    use super::super::try_migrate_settings;
+    use super::{super::SettingsMigration, Migration};
     use serde_json;
 
     const V2_SETTINGS: &str = r#"
@@ -74,16 +78,16 @@ mod test {
           "country": "se"
         }
       },
-      "tunnel": {
-        "only": {
-          "openvpn": {
-            "port": {
-              "only": 53
-            },
-            "protocol": {
-              "only": "udp"
-            }
-          }
+      "tunnel_protocol": "any",
+      "wireguard_constraints": {
+        "port": "any"
+      },
+      "openvpn_constraints": {
+        "port": {
+          "only": 53
+        },
+        "protocol": {
+          "only": "udp"
         }
       }
     }
@@ -103,16 +107,18 @@ mod test {
     },
     "wireguard": {
       "mtu": null,
-      "automatic_rotation": 24
+      "automatic_rotation": 10
     },
     "generic": {
       "enable_ipv6": false
     }
-  }
+  },
+  "show_beta_releases": null,
+  "settings_version": 2
 }
 "#;
 
-    pub const NEW_SETTINGS: &str = r#"
+    pub const V3_SETTINGS: &str = r#"
 {
   "account_token": "1234",
   "relay_settings": {
@@ -160,17 +166,21 @@ mod test {
       "enable_ipv6": false
     }
   },
-  "settings_version": 5
+  "settings_version": 3
 }
 "#;
 
 
     #[test]
     fn test_v2_migration() {
-        let migrated_settings =
-            try_migrate_settings(V2_SETTINGS.as_bytes()).expect("Migration failed");
-        let new_settings = serde_json::from_str(NEW_SETTINGS).unwrap();
+        let mut old_settings = serde_json::from_str(V2_SETTINGS).unwrap();
+
+        let migration = Migration;
+        assert!(migration.version_matches(&mut old_settings));
+
+        migration.migrate(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(V3_SETTINGS).unwrap();
 
-        assert_eq!(&migrated_settings, &new_settings);
+        assert_eq!(&old_settings, &new_settings);
     }
 }
diff --git a/mullvad-types/src/settings/migrations/v3.rs b/mullvad-types/src/settings/migrations/v3.rs
index 6438b2c981..70b5cef196 100644
--- a/mullvad-types/src/settings/migrations/v3.rs
+++ b/mullvad-types/src/settings/migrations/v3.rs
@@ -50,7 +50,7 @@ impl super::SettingsMigration for Migration {
 
 #[cfg(test)]
 mod test {
-    use super::super::try_migrate_settings;
+    use super::{super::SettingsMigration, Migration};
     use serde_json;
 
     pub const V3_SETTINGS: &str = r#"
@@ -69,7 +69,7 @@ mod test {
       },
       "openvpn_constraints": {
         "port": {
-          "only": 53
+          "only": 1195
         },
         "protocol": {
           "only": "udp"
@@ -112,7 +112,7 @@ mod test {
 }
 "#;
 
-    pub const NEW_SETTINGS: &str = r#"
+    pub const V4_SETTINGS: &str = r#"
 {
   "account_token": "1234",
   "relay_settings": {
@@ -128,7 +128,7 @@ mod test {
       },
       "openvpn_constraints": {
         "port": {
-          "only": 53
+          "only": 1195
         },
         "protocol": {
           "only": "udp"
@@ -173,17 +173,21 @@ mod test {
       }
     }
   },
-  "settings_version": 5
+  "settings_version": 4
 }
 "#;
 
 
     #[test]
     fn test_v3_migration() {
-        let migrated_settings =
-            try_migrate_settings(V3_SETTINGS.as_bytes()).expect("Migration failed");
-        let new_settings = serde_json::from_str(NEW_SETTINGS).unwrap();
+        let mut old_settings = serde_json::from_str(V3_SETTINGS).unwrap();
 
-        assert_eq!(&migrated_settings, &new_settings);
+        let migration = Migration;
+        assert!(migration.version_matches(&mut old_settings));
+
+        migration.migrate(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(V4_SETTINGS).unwrap();
+
+        assert_eq!(&old_settings, &new_settings);
     }
 }
diff --git a/mullvad-types/src/settings/migrations/v4.rs b/mullvad-types/src/settings/migrations/v4.rs
index a479ae847f..f1b3bd43ab 100644
--- a/mullvad-types/src/settings/migrations/v4.rs
+++ b/mullvad-types/src/settings/migrations/v4.rs
@@ -4,6 +4,7 @@ use talpid_types::net::TransportProtocol;
 
 
 const WIREGUARD_TCP_PORTS: [u16; 3] = [80, 443, 5001];
+const OPENVPN_TCP_PORTS: [u16; 2] = [80, 443];
 
 
 pub(super) struct Migration;
@@ -55,13 +56,62 @@ impl super::SettingsMigration for Migration {
                 .remove("protocol");
         }
 
+        let openvpn_constraints = || -> Option<&serde_json::Value> {
+            settings
+                .get("relay_settings")?
+                .get("normal")?
+                .get("openvpn_constraints")
+        }();
+
+        if let Some(constraints) = openvpn_constraints {
+            let port: Constraint<u16> = if let Some(port) = constraints.get("port") {
+                serde_json::from_value(port.clone()).map_err(Error::ParseError)?
+            } else {
+                Constraint::Any
+            };
+            let transport_constraint: Constraint<TransportProtocol> =
+                if let Some(protocol) = constraints.get("protocol") {
+                    serde_json::from_value(protocol.clone()).map_err(Error::ParseError)?
+                } else {
+                    Constraint::Any
+                };
+
+            let port = match (port, transport_constraint) {
+                (Constraint::Only(port), Constraint::Any) => Constraint::Only(TransportPort {
+                    protocol: openvpn_protocol_from_port(port),
+                    port: Constraint::Only(port),
+                }),
+                (port, Constraint::Only(protocol)) => {
+                    Constraint::Only(TransportPort { protocol, port })
+                }
+                (Constraint::Any, Constraint::Any) => Constraint::Any,
+            };
+
+            settings["relay_settings"]["normal"]["openvpn_constraints"]["port"] =
+                serde_json::json!(port);
+            settings["relay_settings"]["normal"]["openvpn_constraints"]
+                .as_object_mut()
+                .ok_or(Error::NoMatchingVersion)?
+                .remove("protocol");
+        }
+
         settings["settings_version"] = serde_json::json!(SettingsVersion::V5);
 
         Ok(())
     }
 }
 
+fn openvpn_protocol_from_port(port: u16) -> TransportProtocol {
+    log::warn!("Inferring transport protocol from port constraint");
+    if OPENVPN_TCP_PORTS.contains(&port) {
+        TransportProtocol::Tcp
+    } else {
+        TransportProtocol::Udp
+    }
+}
+
 fn wg_protocol_from_port(port: u16) -> TransportProtocol {
+    log::warn!("Inferring transport protocol from port constraint");
     if WIREGUARD_TCP_PORTS.contains(&port) {
         TransportProtocol::Tcp
     } else {
@@ -71,7 +121,7 @@ fn wg_protocol_from_port(port: u16) -> TransportProtocol {
 
 #[cfg(test)]
 mod test {
-    use super::super::try_migrate_settings;
+    use super::{super::SettingsMigration, Migration};
     use serde_json;
 
     pub const V4_SETTINGS: &str = r#"
@@ -93,11 +143,9 @@ mod test {
       },
       "openvpn_constraints": {
         "port": {
-          "only": 53
+          "only": 1195
         },
-        "protocol": {
-          "only": "udp"
-        }
+        "protocol": "any"
       }
     }
   },
@@ -142,7 +190,7 @@ mod test {
 }
 "#;
 
-    pub const NEW_SETTINGS: &str = r#"
+    pub const V5_SETTINGS: &str = r#"
 {
   "account_token": "1234",
   "relay_settings": {
@@ -165,10 +213,12 @@ mod test {
       },
       "openvpn_constraints": {
         "port": {
-          "only": 53
-        },
-        "protocol": {
-          "only": "udp"
+          "only": {
+            "protocol": "udp",
+            "port": {
+              "only": 1195
+            }
+          }
         }
       }
     }
@@ -217,10 +267,14 @@ mod test {
 
     #[test]
     fn test_v4_migration() {
-        let migrated_settings =
-            try_migrate_settings(V4_SETTINGS.as_bytes()).expect("Migration failed");
-        let new_settings = serde_json::from_str(NEW_SETTINGS).unwrap();
+        let mut old_settings = serde_json::from_str(V4_SETTINGS).unwrap();
+
+        let migration = Migration;
+        assert!(migration.version_matches(&mut old_settings));
+
+        migration.migrate(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(V5_SETTINGS).unwrap();
 
-        assert_eq!(&migrated_settings, &new_settings);
+        assert_eq!(&old_settings, &new_settings);
     }
 }