Merge branch 'add-elaborate-tunnel-params-error'

13 files changed, 147 insertions, 54 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d2699c51f9..298cbab8e2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,8 +23,12 @@ Line wrap the file at 100 chars.                                              Th
 
 
 ## [Unreleased]
+### Added
+- Add more details to the block reason shown in GUI when the daemon fails to generate tunnel
+  parameters.
+
 ### Fixed
-- Check and adjust relay and bridge constraints when they are updated, so no incompatbile
+- Check and adjust relay and bridge constraints when they are updated, so no incompatible
   combinations are used.
 
 
diff --git a/Cargo.lock b/Cargo.lock
index a21a47e007..5f8d8b155b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2308,6 +2308,7 @@ name = "talpid-types"
 version = "0.1.0"
 dependencies = [
  "base64 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "err-derive 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipnetwork 0.14.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts
index 24671fac23..4e349aae36 100644
--- a/gui/src/main/daemon-rpc.ts
+++ b/gui/src/main/daemon-rpc.ts
@@ -256,7 +256,6 @@ const tunnelStateSchema = oneOf(
           'set_firewall_policy_error',
           'set_dns_error',
           'start_tunnel_error',
-          'no_matching_relay',
           'is_offline',
           'tap_adapter_problem',
         ),
@@ -265,6 +264,15 @@ const tunnelStateSchema = oneOf(
         reason: enumeration('auth_failed'),
         details: maybe(string),
       }),
+      object({
+        reason: enumeration('tunnel_parameter_error'),
+        details: enumeration(
+          'no_matching_relay',
+          'no_matching_bridge_relay',
+          'no_wireguard_key',
+          'custom_tunnel_host_resultion_error',
+        ),
+      }),
     ),
   }),
   object({
diff --git a/gui/src/renderer/components/NotificationArea.tsx b/gui/src/renderer/components/NotificationArea.tsx
index fcd0872c2e..9e4c971a89 100644
--- a/gui/src/renderer/components/NotificationArea.tsx
+++ b/gui/src/renderer/components/NotificationArea.tsx
@@ -14,7 +14,7 @@ import {
   NotificationTitle,
 } from './NotificationBanner';
 
-import { BlockReason, TunnelState } from '../../shared/daemon-rpc-types';
+import { BlockReason, TunnelParameterError, TunnelState } from '../../shared/daemon-rpc-types';
 import AccountExpiry from '../lib/account-expiry';
 import { parseAuthFailure } from '../lib/auth-failure';
 import { IVersionReduxState } from '../redux/version/reducers';
@@ -40,6 +40,29 @@ type State = NotificationAreaPresentation & {
   visible: boolean;
 };
 
+function getTunnelParameterMessage(err: TunnelParameterError): string {
+  switch (err) {
+    /// TODO: once bridge constraints can be set, add a more descriptive error message
+    case 'no_matching_bridge_relay':
+    case 'no_matching_relay':
+      return messages.pgettext(
+        'in-app-notifications',
+        'No relay server matches the current settings. You can try changing the location or the relay settings.',
+      );
+    case 'no_wireguard_key':
+      return messages.pgettext(
+        'in-app-notifications',
+        'WireGuard key not published to our servers. You can manage your key in Advanced settings.',
+      );
+      break;
+    case 'custom_tunnel_host_resultion_error':
+      return messages.pgettext(
+        'in-app-notifications',
+        'Failed to resolve host of custom tunnel. Consider changing the settings',
+      );
+  }
+}
+
 function getBlockReasonMessage(blockReason: BlockReason): string {
   switch (blockReason.reason) {
     case 'auth_failed':
@@ -70,11 +93,8 @@ function getBlockReasonMessage(blockReason: BlockReason): string {
       return messages.pgettext('in-app-notifications', 'Failed to set system DNS server');
     case 'start_tunnel_error':
       return messages.pgettext('in-app-notifications', 'Failed to start tunnel connection');
-    case 'no_matching_relay':
-      return messages.pgettext(
-        'in-app-notifications',
-        'No relay server matches the current settings. You can try changing the location or the relay settings.',
-      );
+    case 'tunnel_parameter_error':
+      return getTunnelParameterMessage(blockReason.details);
     case 'is_offline':
       return messages.pgettext(
         'in-app-notifications',
diff --git a/gui/src/shared/daemon-rpc-types.ts b/gui/src/shared/daemon-rpc-types.ts
index 8dc87cc2b1..61a0263031 100644
--- a/gui/src/shared/daemon-rpc-types.ts
+++ b/gui/src/shared/daemon-rpc-types.ts
@@ -15,6 +15,12 @@ export interface ILocation {
   bridgeHostname?: string;
 }
 
+export type TunnelParameterError =
+  | 'no_matching_relay'
+  | 'no_matching_bridge_relay'
+  | 'no_wireguard_key'
+  | 'custom_tunnel_host_resultion_error';
+
 export type BlockReason =
   | {
       reason:
@@ -22,10 +28,10 @@ export type BlockReason =
         | 'set_firewall_policy_error'
         | 'set_dns_error'
         | 'start_tunnel_error'
-        | 'no_matching_relay'
         | 'is_offline'
         | 'tap_adapter_problem';
     }
+  | { reason: 'tunnel_parameter_error'; details: TunnelParameterError }
   | { reason: 'auth_failed'; details?: string };
 
 export type AfterDisconnect = 'nothing' | 'block' | 'reconnect';
diff --git a/gui/test/components/NotificationArea.spec.tsx b/gui/test/components/NotificationArea.spec.tsx
index f2674e848c..a92eced532 100644
--- a/gui/test/components/NotificationArea.spec.tsx
+++ b/gui/test/components/NotificationArea.spec.tsx
@@ -141,7 +141,8 @@ describe('components/NotificationArea', () => {
         tunnelState={{
           state: 'blocked',
           details: {
-            reason: 'no_matching_relay',
+            reason: 'tunnel_parameter_error',
+            details: 'no_matching_relay',
           },
         }}
         version={defaultVersion}
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 9fd270c81a..472e62e7d3 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -56,7 +56,7 @@ use talpid_core::{
 };
 use talpid_types::{
     net::{openvpn, TunnelParameters},
-    tunnel::{BlockReason, TunnelStateTransition},
+    tunnel::{BlockReason, ParameterGenerationError, TunnelStateTransition},
     ErrorExt,
 };
 
@@ -133,7 +133,10 @@ pub(crate) enum InternalDaemonEvent {
     /// Tunnel has changed state.
     TunnelStateTransition(TunnelStateTransition),
     /// Request from the `MullvadTunnelParametersGenerator` to obtain a new relay.
-    GenerateTunnelParameters(mpsc::Sender<TunnelParameters>, u32),
+    GenerateTunnelParameters(
+        mpsc::Sender<std::result::Result<TunnelParameters, ParameterGenerationError>>,
+        u32,
+    ),
     /// An event coming from the JSONRPC-2.0 management interface.
     ManagementInterfaceEvent(ManagementCommand),
     /// Triggered if the server hosting the JSONRPC-2.0 management interface dies unexpectedly.
@@ -538,18 +541,21 @@ where
 
     fn handle_generate_tunnel_parameters(
         &mut self,
-        tunnel_parameters_tx: &mpsc::Sender<TunnelParameters>,
+        tunnel_parameters_tx: &mpsc::Sender<
+            std::result::Result<TunnelParameters, ParameterGenerationError>,
+        >,
         retry_attempt: u32,
     ) {
         if let Some(account_token) = self.settings.get_account_token() {
-            if let Err(error_str) = match self.settings.get_relay_settings() {
+            let result = match self.settings.get_relay_settings() {
                 RelaySettings::CustomTunnelEndpoint(custom_relay) => {
                     self.last_generated_relay = None;
                     custom_relay
                         // TODO(emilsp): generate proxy settings for custom tunnels
                         .to_tunnel_parameters(self.settings.get_tunnel_options().clone(), None)
                         .map_err(|e| {
-                            e.display_chain_with_msg("Custom tunnel endpoint could not be resolved")
+                            log::error!("Failed to resolve hostname for custom tunnel config: {}", e);
+                            ParameterGenerationError::CustomTunnelHostResultionError
                         })
                 }
                 RelaySettings::Normal(constraints) => self
@@ -559,30 +565,37 @@ where
                         self.settings.get_bridge_state(),
                         retry_attempt,
                     )
-                    .map_err(|e| {
-                        e.display_chain_with_msg(
-                            "No valid relay servers match the current settings",
-                        )
-                    })
+                    .map_err(|_| ParameterGenerationError::NoMatchingRelay)
                     .and_then(|(relay, endpoint)| {
-                        let result = self
-                            .create_tunnel_parameters(
-                                &relay,
-                                endpoint,
-                                account_token,
-                                retry_attempt,
-                            )
-                            .map_err(|e| e.display_chain());
+                        let result = self.create_tunnel_parameters(
+                            &relay,
+                            endpoint,
+                            account_token,
+                            retry_attempt,
+                        );
                         self.last_generated_relay = Some(relay);
-                        result
+                        match result {
+                            Ok(result) => Ok(result),
+                            Err(Error::NoKeyAvailable) => {
+                                Err(ParameterGenerationError::NoWireguardKey)
+                            }
+                            Err(Error::NoBridgeAvailable) => {
+                                Err(ParameterGenerationError::NoMatchingBridgeRelay)
+                            }
+                            Err(err) => {
+                                log::error!(
+                                    "{}",
+                                    err.display_chain_with_msg(
+                                        "Failed to generate tunnel parameters"
+                                    )
+                                );
+                                Err(ParameterGenerationError::NoMatchingRelay)
+                            }
+                        }
                     }),
-            }
-            .and_then(|tunnel_params| {
-                tunnel_parameters_tx.send(tunnel_params).map_err(|e| {
-                    e.display_chain_with_msg("Tunnel parameters receiver stopped listening")
-                })
-            }) {
-                error!("{}", error_str);
+            };
+            if tunnel_parameters_tx.send(result).is_err() {
+                log::error!("Failed to send tunnel parameters");
             }
         } else {
             error!("No account token configured");
@@ -1439,14 +1452,25 @@ struct MullvadTunnelParametersGenerator {
 }
 
 impl TunnelParametersGenerator for MullvadTunnelParametersGenerator {
-    fn generate(&mut self, retry_attempt: u32) -> Option<TunnelParameters> {
+    fn generate(
+        &mut self,
+        retry_attempt: u32,
+    ) -> std::result::Result<TunnelParameters, ParameterGenerationError> {
         let (response_tx, response_rx) = mpsc::channel();
-        self.tx
-            .send(InternalDaemonEvent::GenerateTunnelParameters(
-                response_tx,
-                retry_attempt,
-            ))
-            .ok()
-            .and_then(|_| response_rx.recv().ok())
+        if let Err(_) = self.tx.send(InternalDaemonEvent::GenerateTunnelParameters(
+            response_tx,
+            retry_attempt,
+        )) {
+            log::error!("Failed to send daemon command to generate tunnel parameters!");
+            return Err(ParameterGenerationError::NoMatchingRelay);
+        }
+
+        match response_rx.recv() {
+            Ok(result) => result,
+            Err(_) => {
+                log::error!("Failed to receive tunnel parameter generation result!");
+                Err(ParameterGenerationError::NoMatchingRelay)
+            }
+        }
     }
 }
diff --git a/mullvad-jni/src/into_java.rs b/mullvad-jni/src/into_java.rs
index 7ee3e6fec4..b726e27bdf 100644
--- a/mullvad-jni/src/into_java.rs
+++ b/mullvad-jni/src/into_java.rs
@@ -648,7 +648,8 @@ impl<'env> IntoJava<'env> for BlockReason {
             BlockReason::SetFirewallPolicyError => "SetFirewallPolicyError",
             BlockReason::SetDnsError => "SetDnsError",
             BlockReason::StartTunnelError => "StartTunnelError",
-            BlockReason::NoMatchingRelay => "NoMatchingRelay",
+            // TODO(emilsp): Fix Android code to handle new TunnelParameterError block reason
+            BlockReason::TunnelParameterError(_) => "NoMatchingRelay",
             BlockReason::IsOffline => "IsOffline",
             BlockReason::TapAdapterProblem => "TapAdapterProblem",
         };
diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml
index ddfe50f5a2..cc40056734 100644
--- a/talpid-core/Cargo.toml
+++ b/talpid-core/Cargo.toml
@@ -50,7 +50,6 @@ netlink-sys = { git = "https://github.com/mullvad/netlink", rev = "b367ac9c012e6
 nftnl = { git = "https://github.com/mullvad/nftnl-rs", rev = "86b30cdc38a6d4b30a900c21f7c644857d6f7401", features = ["nftnl-1-1-0"] }
 mnl = { git = "https://github.com/mullvad/mnl-rs", rev = "f0d19501b9b85be9a1ffaec8317a378bcbdf4fa6", features = ["mnl-1-0-4"] }
 which = "2.0"
-err-derive = "0.1.5"
 tun = "0.4.3"
 
 
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 36424a08be..2c8b112537 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -335,8 +335,8 @@ impl TunnelState for ConnectingState {
             .tunnel_parameters_generator
             .generate(retry_attempt)
         {
-            None => BlockedState::enter(shared_values, BlockReason::NoMatchingRelay),
-            Some(tunnel_parameters) => {
+            Err(err) => BlockedState::enter(shared_values, BlockReason::TunnelParameterError(err)),
+            Ok(tunnel_parameters) => {
                 if let Err(error) = Self::set_firewall_policy(shared_values, &tunnel_parameters) {
                     error!(
                         "{}",
diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs
index d947792da6..e0ea948aa2 100644
--- a/talpid-core/src/tunnel_state_machine/mod.rs
+++ b/talpid-core/src/tunnel_state_machine/mod.rs
@@ -30,7 +30,7 @@ use std::{
 };
 use talpid_types::{
     net::TunnelParameters,
-    tunnel::{BlockReason, TunnelStateTransition},
+    tunnel::{BlockReason, ParameterGenerationError, TunnelStateTransition},
     ErrorExt,
 };
 use tokio_core::reactor::Core;
@@ -280,12 +280,16 @@ impl<T: TunnelState> From<EventConsequence<T>> for TunnelStateMachineAction {
     }
 }
 
+
 /// Trait for any type that can provide a stream of `TunnelParameters` to the `TunnelStateMachine`.
 pub trait TunnelParametersGenerator: Send + 'static {
     /// Given the number of consecutive failed retry attempts, it should yield a `TunnelParameters`
     /// to establish a tunnel with.
     /// If this returns `None` then the state machine goes into the `Blocked` state.
-    fn generate(&mut self, retry_attempt: u32) -> Option<TunnelParameters>;
+    fn generate(
+        &mut self,
+        retry_attempt: u32,
+    ) -> Result<TunnelParameters, ParameterGenerationError>;
 }
 
 /// Values that are common to all tunnel states.
diff --git a/talpid-types/Cargo.toml b/talpid-types/Cargo.toml
index 9fa2bd04a2..bfcb2bed5d 100644
--- a/talpid-types/Cargo.toml
+++ b/talpid-types/Cargo.toml
@@ -13,3 +13,4 @@ base64 = "0.10"
 hex = "0.3"
 x25519-dalek = { version = "0.4.5", features = [ "std", "u64_backend" ], default-features = false }
 rand = "0.7"
+err-derive = "0.1.5"
diff --git a/talpid-types/src/tunnel.rs b/talpid-types/src/tunnel.rs
index 4f2e630c2d..55098ed4af 100644
--- a/talpid-types/src/tunnel.rs
+++ b/talpid-types/src/tunnel.rs
@@ -1,6 +1,6 @@
 use crate::net::TunnelEndpoint;
 use serde::{Deserialize, Serialize};
-use std::fmt;
+use std::{error::Error, fmt};
 
 /// Event resulting from a transition to a new tunnel state.
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -52,14 +52,32 @@ pub enum BlockReason {
     SetDnsError,
     /// Failed to start connection to remote server.
     StartTunnelError,
-    /// No relay server matching the current filter parameters.
-    NoMatchingRelay,
+    /// Tunnel parameter generation failure
+    TunnelParameterError(ParameterGenerationError),
     /// This device is offline, no tunnels can be established.
     IsOffline,
     /// A problem with the TAP adapter has been detected.
     TapAdapterProblem,
 }
 
+/// Errors that can occur when generating tunnel parameters.
+#[derive(err_derive::Error, Debug, Serialize, Clone, PartialEq, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum ParameterGenerationError {
+    /// Failure to select a matching tunnel relay
+    #[error(display = "Failure to select a matching tunnel relay")]
+    NoMatchingRelay,
+    /// Failure to select a matching bridge relay
+    #[error(display = "Failure to select a matching bridge relay")]
+    NoMatchingBridgeRelay,
+    /// Returned when tunnel parameters can't be generated because wireguard key is not available.
+    #[error(display = "No wireguard key available")]
+    NoWireguardKey,
+    /// Failure to resolve the hostname of a custom tunnel configuration
+    #[error(display = "Can't resolve hostname for custom tunnel host")]
+    CustomTunnelHostResultionError,
+}
+
 impl fmt::Display for BlockReason {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::BlockReason::*;
@@ -78,7 +96,13 @@ impl fmt::Display for BlockReason {
             SetFirewallPolicyError => "Failed to set firewall policy",
             SetDnsError => "Failed to set system DNS server",
             StartTunnelError => "Failed to start connection to remote server",
-            NoMatchingRelay => "No relay server matches the current settings",
+            TunnelParameterError(ref err) => {
+                return write!(
+                    f,
+                    "Failure to generate tunnel parameters: {}",
+                    err.description(),
+                );
+            }
             IsOffline => "This device is offline, no tunnels can be established",
             TapAdapterProblem => "A problem with the TAP adapter has been detected",
         };