Fix interactions between relay and bridge settings.

CHANGELOG.md

@@ -23,6 +23,9 @@ Line wrap the file at 100 chars.                                              Th
 
 
 ## [Unreleased]
+### Fixed
+- Check and adjust relay and bridge constraints when they are updated, so no incompatbile
+  combinations are used.
 
 
 ## [2019.7-beta1] - 2019-08-08

mullvad-daemon/src/lib.rs

@@ -37,8 +37,8 @@ use mullvad_types::{
     endpoint::MullvadEndpoint,
     location::GeoIpLocation,
     relay_constraints::{
-        BridgeSettings, BridgeState, Constraint, InternalBridgeConstraints, OpenVpnConstraints,
-        RelayConstraintsUpdate, RelaySettings, RelaySettingsUpdate, TunnelProtocol,
+        BridgeSettings, BridgeState, Constraint, InternalBridgeConstraints, RelaySettings,
+        RelaySettingsUpdate,
     },
     relay_list::{Relay, RelayList},
     states::{TargetState, TunnelState},
@@ -55,7 +55,7 @@ use talpid_core::{
     tunnel_state_machine::{self, TunnelCommand, TunnelParametersGenerator},
 };
 use talpid_types::{
-    net::{openvpn, TransportProtocol, TunnelParameters},
+    net::{openvpn, TunnelParameters},
     tunnel::{BlockReason, TunnelStateTransition},
     ErrorExt,
 };
@@ -554,7 +554,11 @@ where
                 }
                 RelaySettings::Normal(constraints) => self
                     .relay_selector
-                    .get_tunnel_endpoint(&constraints, retry_attempt)
+                    .get_tunnel_endpoint(
+                        &constraints,
+                        self.settings.get_bridge_state(),
+                        retry_attempt,
+                    )
                     .map_err(|e| {
                         e.display_chain_with_msg(
                             "No valid relay servers match the current settings",
@@ -1136,15 +1140,6 @@ where
         let result = match self.settings.set_bridge_state(bridge_state.clone()) {
             Ok(settings_changed) => {
                 if settings_changed {
-                    if bridge_state == BridgeState::On {
-                        if let Err(e) = self.apply_proxy_constraints() {
-                            log::error!(
-                                "{}",
-                                e.display_chain_with_msg("Failed to apply proxy constraints")
-                            );
-                        }
-                    }
-
                     self.event_listener.notify_settings(self.settings.clone());
                     log::info!("Initiating tunnel restart because bridge state changed");
                     self.reconnect_tunnel();
@@ -1162,21 +1157,6 @@ where
         Self::oneshot_send(tx, result, "on_set_bridge_state response");
     }
 
-    // Set the OpenVPN tunnel to use TCP.
-    fn apply_proxy_constraints(&mut self) -> settings::Result<bool> {
-        let constraints_update = RelayConstraintsUpdate {
-            tunnel_protocol: Some(Constraint::Only(TunnelProtocol::OpenVpn)),
-            openvpn_constraints: Some(OpenVpnConstraints {
-                protocol: Constraint::Only(TransportProtocol::Tcp),
-                port: Constraint::Any,
-            }),
-            ..Default::default()
-        };
-
-        let settings_update = RelaySettingsUpdate::Normal(constraints_update);
-
-        self.settings.update_relay_settings(settings_update)
-    }
 
     fn on_set_enable_ipv6(&mut self, tx: oneshot::Sender<()>, enable_ipv6: bool) {
         let save_result = self.settings.set_enable_ipv6(enable_ipv6);

mullvad-daemon/src/relays.rs

@@ -5,8 +5,8 @@ use mullvad_types::{
     endpoint::MullvadEndpoint,
     location::Location,
     relay_constraints::{
-        Constraint, InternalBridgeConstraints, LocationConstraint, Match, OpenVpnConstraints,
-        RelayConstraints, TunnelProtocol, WireguardConstraints,
+        BridgeState, Constraint, InternalBridgeConstraints, LocationConstraint, Match,
+        OpenVpnConstraints, RelayConstraints, TunnelProtocol, WireguardConstraints,
     },
     relay_list::{OpenVpnEndpointData, Relay, RelayList, RelayTunnels, WireguardEndpointData},
 };
@@ -204,9 +204,11 @@ impl RelaySelector {
     pub fn get_tunnel_endpoint(
         &mut self,
         relay_constraints: &RelayConstraints,
+        bridge_state: &BridgeState,
         retry_attempt: u32,
     ) -> Result<(Relay, MullvadEndpoint), Error> {
-        let preferred_constraints = Self::preferred_constraints(relay_constraints, retry_attempt);
+        let preferred_constraints =
+            Self::preferred_constraints(relay_constraints, bridge_state, retry_attempt);
         if let Some((relay, endpoint)) = self.get_tunnel_endpoint_internal(&preferred_constraints) {
             debug!(
                 "Relay matched on highest preference for retry attempt {}",
@@ -228,16 +230,20 @@ impl RelaySelector {
 
     fn preferred_constraints(
         original_constraints: &RelayConstraints,
+        bridge_state: &BridgeState,
         retry_attempt: u32,
     ) -> RelayConstraints {
         // Prefer UDP by default. But if that has failed a couple of times, then try TCP port 443,
         // which works for many with UDP problems. After that, just alternate between protocols.
-        let (preferred_port, preferred_protocol) = match retry_attempt {
+        let (preferred_port, mut preferred_protocol) = match retry_attempt {
             0 | 1 => (Constraint::Any, TransportProtocol::Udp),
             2 | 3 => (Constraint::Only(443), TransportProtocol::Tcp),
             attempt if attempt % 2 == 0 => (Constraint::Any, TransportProtocol::Udp),
             _ => (Constraint::Any, TransportProtocol::Tcp),
         };
+        if *bridge_state == BridgeState::On {
+            preferred_protocol = TransportProtocol::Tcp;
+        }
 
         let mut relay_constraints = RelayConstraints {
             location: original_constraints.location.clone(),
@@ -251,6 +257,7 @@ impl RelaySelector {
             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,

mullvad-types/src/custom_tunnel.rs

@@ -4,7 +4,7 @@ use std::{
     fmt, io,
     net::{IpAddr, SocketAddr, ToSocketAddrs},
 };
-use talpid_types::net::{openvpn, wireguard, TunnelParameters};
+use talpid_types::net::{openvpn, wireguard, Endpoint, TunnelParameters};
 
 
 #[derive(err_derive::Error, Debug)]
@@ -28,6 +28,13 @@ impl CustomTunnelEndpoint {
         Self { host, config }
     }
 
+    pub fn endpoint(&self) -> Endpoint {
+        match &self.config {
+            ConnectionConfig::OpenVpn(config) => config.endpoint,
+            ConnectionConfig::Wireguard(config) => config.get_endpoint(),
+        }
+    }
+
     pub fn to_tunnel_parameters(
         &self,
         tunnel_options: TunnelOptions,

mullvad-types/src/relay_constraints.rs

@@ -91,6 +91,31 @@ impl RelaySettings {
             }),
         }
     }
+
+    pub(crate) fn ensure_bridge_compatibility(&mut self) {
+        match self {
+            RelaySettings::Normal(ref mut constraints) => {
+                if constraints.tunnel_protocol == Constraint::Only(TunnelProtocol::Wireguard) {
+                    constraints.tunnel_protocol = Constraint::Any;
+                }
+                if constraints.openvpn_constraints.protocol
+                    == Constraint::Only(TransportProtocol::Udp)
+                {
+                    constraints.openvpn_constraints = OpenVpnConstraints {
+                        protocol: Constraint::Any,
+                        port: Constraint::Any,
+                    }
+                }
+            }
+            RelaySettings::CustomTunnelEndpoint(config) => {
+                if config.endpoint().protocol == TransportProtocol::Udp {
+                    log::warn!(
+                        "Using custom tunnel endpoint with UDP, bridges will likely not work"
+                    );
+                }
+            }
+        }
+    }
 }
 
 #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
@@ -353,6 +378,31 @@ pub enum RelaySettingsUpdate {
     Normal(RelayConstraintsUpdate),
 }
 
+impl RelaySettingsUpdate {
+    /// Returns false if the specified relay settings update explicitly do not allow for bridging
+    /// (i.e. use UDP instead of TCP)
+    pub fn supports_bridge(&self) -> bool {
+        match &self {
+            RelaySettingsUpdate::CustomTunnelEndpoint(endpoint) => {
+                endpoint.endpoint().protocol == TransportProtocol::Tcp
+            }
+            RelaySettingsUpdate::Normal(update) => {
+                if let Some(Constraint::Only(TunnelProtocol::Wireguard)) = &update.tunnel_protocol {
+                    false
+                } else if let Some(constraints) = &update.openvpn_constraints {
+                    if let Constraint::Only(TransportProtocol::Udp) = &constraints.protocol {
+                        false
+                    } else {
+                        true
+                    }
+                } else {
+                    true
+                }
+            }
+        }
+    }
+}
+
 #[derive(Debug, Default, Deserialize, Serialize)]
 #[serde(default)]
 pub struct RelayConstraintsUpdate {

mullvad-types/src/settings/mod.rs

@@ -188,8 +188,12 @@ impl Settings {
     }
 
     pub fn update_relay_settings(&mut self, update: RelaySettingsUpdate) -> Result<bool> {
+        let update_supports_bridge = update.supports_bridge();
         let new_settings = self.relay_settings.merge(update);
         if self.relay_settings != new_settings {
+            if !update_supports_bridge && BridgeState::On == self.bridge_state {
+                self.bridge_state = BridgeState::Auto;
+            }
             debug!(
                 "changing relay settings from {} to {}",
                 self.relay_settings, new_settings
@@ -292,6 +296,9 @@ impl Settings {
     pub fn set_bridge_state(&mut self, bridge_state: BridgeState) -> Result<bool> {
         if self.bridge_state != bridge_state {
             self.bridge_state = bridge_state;
+            if self.bridge_state == BridgeState::On {
+                self.relay_settings.ensure_bridge_compatibility();
+            }
             self.save().map(|_| true)
         } else {
             Ok(false)