Make not finding a suitable relay block the network

4 files changed, 81 insertions, 65 deletions

diff --git a/gui/packages/desktop/src/renderer/errors.js b/gui/packages/desktop/src/renderer/errors.js
index ddf0c239bb..b333f66221 100644
--- a/gui/packages/desktop/src/renderer/errors.js
+++ b/gui/packages/desktop/src/renderer/errors.js
@@ -11,6 +11,12 @@ export class BlockedError extends Error {
       case 'start_tunnel_error':
         super('Failed to start tunnel connection');
         break;
+      case 'no_matching_relay':
+        super('No relay server matches the current settings');
+        break;
+      case 'no_account_token':
+        super('No account token configured');
+        break;
       default:
         super(`Unknown error: ${(reason: empty)}`);
     }
diff --git a/gui/packages/desktop/src/renderer/lib/daemon-rpc.js b/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
index ae2e501268..0408017d5a 100644
--- a/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
+++ b/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
@@ -41,7 +41,11 @@ const LocationSchema = object({
   mullvad_exit_ip: boolean,
 });
 
-export type BlockReason = 'set_security_policy_error' | 'start_tunnel_error';
+export type BlockReason =
+  | 'set_security_policy_error'
+  | 'start_tunnel_error'
+  | 'no_matching_relay'
+  | 'no_account_token';
 export type DisconnectedState = {
   state: 'disconnected',
 };
@@ -218,7 +222,12 @@ const AccountDataSchema = object({
   expiry: string,
 });
 
-const allBlockReasons: Array<BlockReason> = ['set_security_policy_error', 'start_tunnel_error'];
+const allBlockReasons: Array<BlockReason> = [
+  'set_security_policy_error',
+  'start_tunnel_error',
+  'no_matching_relay',
+  'no_account_token',
+];
 const BlockedStateSchema = object({
   state: enumeration('blocked'),
   details: enumeration(...allBlockReasons),
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index 4385b3a291..b37de6c93d 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -80,7 +80,7 @@ use std::{mem, thread};
 use talpid_core::mpsc::IntoSender;
 use talpid_core::tunnel_state_machine::{self, TunnelCommand, TunnelParameters};
 use talpid_types::net::TunnelOptions;
-use talpid_types::tunnel::TunnelStateTransition;
+use talpid_types::tunnel::{BlockReason, TunnelStateTransition};
 
 
 error_chain!{
@@ -99,15 +99,13 @@ error_chain!{
             description("Error in the management interface")
             display("Management interface error: {}", msg)
         }
-        InvalidSettings(msg: &'static str) {
-            description("Invalid settings")
-            display("Invalid Settings: {}", msg)
+        NoAccountToken {
+            description("No account token configured")
         }
-        NoRelay {
-            description("Found no valid relays to connect to")
+        NoMatchingRelay {
+            description("No valid relay servers match the current settings")
         }
     }
-
     links {
         TunnelError(tunnel_state_machine::Error, tunnel_state_machine::ErrorKind);
     }
@@ -306,7 +304,7 @@ impl Daemon {
     pub fn run(mut self) -> Result<()> {
         if self.settings.get_auto_connect() {
             info!("Automatically connecting since auto-connect is turned on");
-            self.set_target_state(TargetState::Secured)?;
+            self.set_target_state(TargetState::Secured);
         }
         while let Ok(event) = self.rx.recv() {
             self.handle_event(event)?;
@@ -355,7 +353,7 @@ impl Daemon {
     fn handle_management_interface_event(&mut self, event: ManagementCommand) -> Result<()> {
         use ManagementCommand::*;
         match event {
-            SetTargetState(state) => self.on_set_target_state(state),
+            SetTargetState(state) => Ok(self.on_set_target_state(state)),
             GetState(tx) => Ok(self.on_get_state(tx)),
             GetCurrentLocation(tx) => Ok(self.on_get_current_location(tx)),
             GetAccountData(tx, account_token) => Ok(self.on_get_account_data(tx, account_token)),
@@ -379,12 +377,11 @@ impl Daemon {
         }
     }
 
-    fn on_set_target_state(&mut self, new_target_state: TargetState) -> Result<()> {
+    fn on_set_target_state(&mut self, new_target_state: TargetState) {
         if self.state.is_running() {
-            self.set_target_state(new_target_state)
+            self.set_target_state(new_target_state);
         } else {
             warn!("Ignoring target state change request due to shutdown");
-            Ok(())
         }
     }
 
@@ -447,10 +444,10 @@ impl Daemon {
                 if account_changed {
                     if account_token_cleared {
                         info!("Disconnecting because account token was cleared");
-                        self.set_target_state(TargetState::Unsecured)?;
+                        self.set_target_state(TargetState::Unsecured);
                     } else {
                         info!("Initiating tunnel restart because the account token changed");
-                        self.reconnect_tunnel()?;
+                        self.reconnect_tunnel();
                     }
                 }
             }
@@ -499,7 +496,7 @@ impl Daemon {
 
                 if changed {
                     info!("Initiating tunnel restart because the relay settings changed");
-                    self.reconnect_tunnel()?;
+                    self.reconnect_tunnel();
                 }
             }
             Err(e) => error!("{}", e.display_chain()),
@@ -517,9 +514,7 @@ impl Daemon {
         match save_result.chain_err(|| "Unable to save settings") {
             Ok(settings_changed) => {
                 if settings_changed {
-                    self.tunnel_command_tx
-                        .send(TunnelCommand::AllowLan(allow_lan))
-                        .expect("Tunnel state machine has stopped");
+                    self.send_tunnel_command(TunnelCommand::AllowLan(allow_lan));
                 }
                 Self::oneshot_send(tx, (), "set_allow_lan response");
             }
@@ -575,7 +570,7 @@ impl Daemon {
 
                 if settings_changed {
                     info!("Initiating tunnel restart because the enable IPv6 setting changed");
-                    self.reconnect_tunnel()?;
+                    self.reconnect_tunnel();
                 }
             }
             Err(e) => error!("{}", e.display_chain()),
@@ -609,76 +604,70 @@ impl Daemon {
 
     /// Set the target state of the client. If it changed trigger the operations needed to
     /// progress towards that state.
-    fn set_target_state(&mut self, new_state: TargetState) -> Result<()> {
+    fn set_target_state(&mut self, new_state: TargetState) {
         if new_state != self.target_state {
             debug!("Target state {:?} => {:?}", self.target_state, new_state);
             self.target_state = new_state;
-            self.apply_target_state()
-        } else {
-            Ok(())
-        }
-    }
-
-    fn apply_target_state(&mut self) -> Result<()> {
-        match self.target_state {
-            TargetState::Secured => {
-                debug!("Triggering tunnel start");
-                if let Err(e) = self.connect_tunnel().chain_err(|| "Failed to start tunnel") {
-                    error!("{}", e.display_chain());
-                    self.current_relay = None;
-                    self.management_interface_broadcaster.notify_error(&e);
-                    self.set_target_state(TargetState::Unsecured)?;
-                }
+            match self.target_state {
+                TargetState::Secured => self.connect_tunnel(),
+                TargetState::Unsecured => self.disconnect_tunnel(),
             }
-            TargetState::Unsecured => self.disconnect_tunnel(),
         }
-
-        Ok(())
     }
 
-    fn connect_tunnel(&mut self) -> Result<()> {
-        let parameters = self.build_tunnel_parameters()?;
-
-        self.tunnel_command_tx
-            .send(TunnelCommand::Connect(parameters))
-            .expect("Tunnel state machine has stopped");
-
-        Ok(())
+    fn connect_tunnel(&mut self) {
+        debug!("Triggering tunnel start");
+        let command = match self.build_tunnel_parameters() {
+            Ok(parameters) => TunnelCommand::Connect(parameters),
+            Err(error) => {
+                error!("{}", error.display_chain());
+                let reason = match error.kind() {
+                    ErrorKind::NoMatchingRelay => BlockReason::NoMatchingRelay,
+                    ErrorKind::NoAccountToken => BlockReason::NoAccountToken,
+                    _ => {
+                        error!(
+                            "Invalid error from build_tunnel_parameters: {}",
+                            error.display_chain()
+                        );
+                        BlockReason::NoMatchingRelay
+                    }
+                };
+                TunnelCommand::Block(reason)
+            }
+        };
+        self.send_tunnel_command(command);
     }
 
     fn disconnect_tunnel(&mut self) {
-        self.tunnel_command_tx
-            .send(TunnelCommand::Disconnect)
-            .expect("Tunnel state machine has stopped");
+        self.send_tunnel_command(TunnelCommand::Disconnect);
     }
 
-    fn reconnect_tunnel(&mut self) -> Result<()> {
-        match self.target_state {
-            TargetState::Secured => self.connect_tunnel(),
-            TargetState::Unsecured => Ok(()),
+    fn reconnect_tunnel(&mut self) {
+        if self.target_state == TargetState::Secured {
+            self.connect_tunnel()
         }
     }
 
     fn build_tunnel_parameters(&mut self) -> Result<TunnelParameters> {
+        let account_token = self
+            .settings
+            .get_account_token()
+            .ok_or(ErrorKind::NoAccountToken)?;
+
         let endpoint = match self.settings.get_relay_settings() {
             RelaySettings::CustomTunnelEndpoint(custom_relay) => custom_relay
                 .to_tunnel_endpoint()
-                .chain_err(|| ErrorKind::NoRelay)?,
+                .chain_err(|| ErrorKind::NoMatchingRelay)?,
             RelaySettings::Normal(constraints) => {
                 let (relay, tunnel_endpoint) = self
                     .relay_selector
                     .get_tunnel_endpoint(&constraints)
-                    .chain_err(|| ErrorKind::NoRelay)?;
+                    .chain_err(|| ErrorKind::NoMatchingRelay)?;
                 self.current_relay = Some(relay);
                 tunnel_endpoint
             }
         };
 
-        let account_token = self
-            .settings
-            .get_account_token()
-            .ok_or(ErrorKind::InvalidSettings("No account token"))?;
-
         Ok(TunnelParameters {
             endpoint,
             options: self.settings.get_tunnel_options().clone(),
@@ -689,6 +678,12 @@ impl Daemon {
         })
     }
 
+    fn send_tunnel_command(&mut self, command: TunnelCommand) {
+        self.tunnel_command_tx
+            .send(command)
+            .expect("Tunnel state machine has stopped");
+    }
+
     pub fn shutdown_handle(&self) -> DaemonShutdownHandle {
         DaemonShutdownHandle {
             tx: self.tx.clone(),
diff --git a/talpid-types/src/tunnel.rs b/talpid-types/src/tunnel.rs
index e5454988c5..77a1ffacb5 100644
--- a/talpid-types/src/tunnel.rs
+++ b/talpid-types/src/tunnel.rs
@@ -21,10 +21,14 @@ pub enum TunnelStateTransition {
 #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum BlockReason {
-    /// Failed to set security policy
+    /// Failed to set security policy.
     SetSecurityPolicyError,
-    /// Failed to start connection to remote server
+    /// Failed to start connection to remote server.
     StartTunnelError,
+    /// No relay server matching the current filter parameters.
+    NoMatchingRelay,
+    /// No account token configured.
+    NoAccountToken,
 }
 
 impl fmt::Display for BlockReason {
@@ -32,6 +36,8 @@ impl fmt::Display for BlockReason {
         let description = match *self {
             BlockReason::SetSecurityPolicyError => "Failed to set security policy",
             BlockReason::StartTunnelError => "Failed to start connection to remote server",
+            BlockReason::NoMatchingRelay => "No relay server matches the current settings",
+            BlockReason::NoAccountToken => "No account token configured",
         };
 
         write!(formatter, "{}", description)