Merge branch 'update-wg-windows-routing-errors'

9 files changed, 150 insertions, 25 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 469bbe90a7..7de0c43522 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,6 +53,7 @@ Line wrap the file at 100 chars.                                              Th
 
 #### Windows
 - Fix "cannot find the file" error while creating a Wintun adapter by upgrading Wintun.
+- Retry when creating a WireGuard tunnel fails due to no default routes being found.
 
 
 ## [2021.2] - 2021-02-18
diff --git a/talpid-core/src/routing/windows.rs b/talpid-core/src/routing/windows.rs
index 47a45672dc..25c80899e5 100644
--- a/talpid-core/src/routing/windows.rs
+++ b/talpid-core/src/routing/windows.rs
@@ -20,7 +20,7 @@ pub enum Error {
     FailedToStartManager,
     /// Failure to add routes
     #[error(display = "Failed to add routes")]
-    AddRoutesFailed,
+    AddRoutesFailed(#[error(source)] winnet::Error),
     /// Failure to clear routes
     #[error(display = "Failed to clear applied routes")]
     ClearRoutesFailed,
@@ -118,11 +118,9 @@ impl RouteManager {
                         })
                         .collect();
 
-                    if winnet::routing_manager_add_routes(&routes) {
-                        let _ = tx.send(Ok(()));
-                    } else {
-                        let _ = tx.send(Err(Error::AddRoutesFailed));
-                    }
+                    let _ = tx.send(
+                        winnet::routing_manager_add_routes(&routes).map_err(Error::AddRoutesFailed),
+                    );
                 }
                 RouteManagerCommand::Shutdown => {
                     break;
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 7ec0aaf55a..897f4435fc 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -29,6 +29,9 @@ use talpid_types::{
     ErrorExt,
 };
 
+#[cfg(windows)]
+use crate::{routing, winnet};
+
 #[cfg(target_os = "android")]
 use crate::tunnel::tun_provider;
 
@@ -343,8 +346,10 @@ impl ConnectingState {
 }
 
 fn should_retry(error: &tunnel::Error) -> bool {
+    use tunnel::wireguard::Error;
     #[cfg(not(windows))]
-    use tunnel::wireguard::{Error, TunnelError};
+    use tunnel::wireguard::TunnelError;
+
     match error {
         #[cfg(not(windows))]
         tunnel::Error::WireguardTunnelMonitoringError(Error::TunnelError(
@@ -356,6 +361,24 @@ fn should_retry(error: &tunnel::Error) -> bool {
             TunnelError::BypassError(_),
         )) => true,
 
+        #[cfg(windows)]
+        tunnel::Error::WireguardTunnelMonitoringError(Error::SetupRoutingError(error)) => {
+            is_recoverable_routing_error(error)
+        }
+
+        _ => false,
+    }
+}
+
+#[cfg(windows)]
+fn is_recoverable_routing_error(error: &crate::routing::Error) -> bool {
+    match error {
+        routing::Error::AddRoutesFailed(route_error) => match route_error {
+            winnet::Error::GetDefaultRoute
+            | winnet::Error::GetDeviceByName
+            | winnet::Error::GetDeviceByGateway => true,
+            _ => false,
+        },
         _ => false,
     }
 }
diff --git a/talpid-core/src/winnet.rs b/talpid-core/src/winnet.rs
index c00a22d73f..79008f8cbc 100644
--- a/talpid-core/src/winnet.rs
+++ b/talpid-core/src/winnet.rs
@@ -28,6 +28,18 @@ pub enum Error {
     #[error(display = "Failed to obtain default route")]
     GetDefaultRoute,
 
+    /// Failed to get a network device.
+    #[error(display = "Failed to obtain network interface by name")]
+    GetDeviceByName,
+
+    /// Failed to get a network device.
+    #[error(display = "Failed to obtain network interface by gateway")]
+    GetDeviceByGateway,
+
+    /// Unexpected error while adding routes
+    #[error(display = "Winnet returned an error while adding routes")]
+    GeneralAddRoutesError,
+
     /// Failed to obtain an IP address given a LUID.
     #[error(display = "Failed to obtain IP address for the given interface")]
     GetIpAddressFromLuid,
@@ -310,10 +322,16 @@ pub fn add_default_route_change_callback<T: 'static>(
     }
 }
 
-pub fn routing_manager_add_routes(routes: &[WinNetRoute]) -> bool {
+pub fn routing_manager_add_routes(routes: &[WinNetRoute]) -> Result<(), Error> {
     let ptr = routes.as_ptr();
     let length: u32 = routes.len() as u32;
-    unsafe { WinNet_AddRoutes(ptr, length) }
+    match unsafe { WinNet_AddRoutes(ptr, length) } {
+        WinNetAddRouteStatus::Success => Ok(()),
+        WinNetAddRouteStatus::GeneralError => Err(Error::GeneralAddRoutesError),
+        WinNetAddRouteStatus::NoDefaultRoute => Err(Error::GetDefaultRoute),
+        WinNetAddRouteStatus::NameNotFound => Err(Error::GetDeviceByName),
+        WinNetAddRouteStatus::GatewayNotFound => Err(Error::GetDeviceByGateway),
+    }
 }
 
 pub fn routing_manager_delete_applied_routes() -> bool {
@@ -394,15 +412,28 @@ mod api {
         Failure = 2,
     }
 
+    #[allow(dead_code)]
+    #[repr(u32)]
+    pub enum WinNetAddRouteStatus {
+        Success = 0,
+        GeneralError = 1,
+        NoDefaultRoute = 2,
+        NameNotFound = 3,
+        GatewayNotFound = 4,
+    }
+
     extern "system" {
         #[link_name = "WinNet_ActivateRouteManager"]
         pub fn WinNet_ActivateRouteManager(sink: Option<LogSink>, sink_context: *const u8) -> bool;
 
         #[link_name = "WinNet_AddRoutes"]
-        pub fn WinNet_AddRoutes(routes: *const super::WinNetRoute, num_routes: u32) -> bool;
+        pub fn WinNet_AddRoutes(
+            routes: *const super::WinNetRoute,
+            num_routes: u32,
+        ) -> WinNetAddRouteStatus;
 
         // #[link_name = "WinNet_AddRoute"]
-        // pub fn WinNet_AddRoute(route: *const super::WinNetRoute) -> bool;
+        // pub fn WinNet_AddRoute(route: *const super::WinNetRoute) -> WinNetAddRouteStatus;
 
         // #[link_name = "WinNet_DeleteRoutes"]
         // pub fn WinNet_DeleteRoutes(routes: *const super::WinNetRoute, num_routes: u32) -> bool;
diff --git a/windows/windows-libraries b/windows/windows-libraries
-Subproject add0a436ab87b55b39cb2b46165971e8beadd1e
+Subproject 391d06a9f01d144e4efd47185adf2d31c08b6cc
diff --git a/windows/winnet/src/winnet/routing/routemanager.cpp b/windows/winnet/src/winnet/routing/routemanager.cpp
index dc38466440..2c2257e916 100644
--- a/windows/winnet/src/winnet/routing/routemanager.cpp
+++ b/windows/winnet/src/winnet/routing/routemanager.cpp
@@ -52,7 +52,7 @@ NET_LUID InterfaceLuidFromGateway(const NodeAddress &gateway)
 
 	if (matches.empty())
 	{
-		THROW_ERROR("Unable to find network adapter with specified gateway");
+		THROW_ERROR_TYPE(error::DeviceGatewayNotFound, "Unable to find network adapter with specified gateway");
 	}
 
 	//
@@ -128,7 +128,7 @@ InterfaceAndGateway ResolveNode(ADDRESS_FAMILY family, const std::optional<Node>
 		const auto default_route = GetBestDefaultRoute(family);
 		if (!default_route.has_value())
 		{
-			THROW_ERROR("Unable to determine details of default route");
+			THROW_ERROR_TYPE(error::NoDefaultRoute, "Unable to determine details of default route");
 		}
 		return default_route.value();
 	}
@@ -145,8 +145,7 @@ InterfaceAndGateway ResolveNode(ADDRESS_FAMILY family, const std::optional<Node>
 		{
 			const auto msg = std::string("Unable to derive interface LUID from interface alias: ")
 				.append(common::string::ToAnsi(deviceName));
-
-			THROW_ERROR(msg.c_str());
+			THROW_ERROR_TYPE(error::DeviceNameNotFound, msg.c_str());
 		}
 
 		auto onLinkProvider = [&family]()
@@ -242,11 +241,15 @@ void RouteManager::addRoutes(const std::vector<Route> &routes)
 				*existingRecord = std::move(newRecord);
 			}
 		}
+		catch (const error::RouteManagerError&)
+		{
+			undoEvents(eventLog);
+			throw;
+		}
 		catch (...)
 		{
 			undoEvents(eventLog);
-
-			THROW_ERROR("Failed during batch insertion of routes");
+			THROW_ERROR("Unexpected error during batch insertion of routes");
 		}
 	}
 }
diff --git a/windows/winnet/src/winnet/routing/routemanager.h b/windows/winnet/src/winnet/routing/routemanager.h
index 07cc7dbf40..42949a657b 100644
--- a/windows/winnet/src/winnet/routing/routemanager.h
+++ b/windows/winnet/src/winnet/routing/routemanager.h
@@ -18,6 +18,51 @@
 namespace winnet::routing
 {
 
+namespace error
+{
+
+class RouteManagerError : public std::runtime_error
+{
+public:
+
+	RouteManagerError(const char* message)
+		: std::runtime_error(message)
+	{
+	}
+};
+
+class NoDefaultRoute : public RouteManagerError
+{
+public:
+
+	NoDefaultRoute(const char* message)
+		: RouteManagerError(message)
+	{
+	}
+};
+
+class DeviceNameNotFound : public RouteManagerError
+{
+public:
+
+	DeviceNameNotFound(const char* message)
+		: RouteManagerError(message)
+	{
+	}
+};
+
+class DeviceGatewayNotFound : public RouteManagerError
+{
+public:
+
+	DeviceGatewayNotFound(const char* message)
+		: RouteManagerError(message)
+	{
+	}
+};
+
+}
+
 class RouteManager
 {
 public:
diff --git a/windows/winnet/src/winnet/winnet.cpp b/windows/winnet/src/winnet/winnet.cpp
index 9ed2ec103e..e82345ebee 100644
--- a/windows/winnet/src/winnet/winnet.cpp
+++ b/windows/winnet/src/winnet/winnet.cpp
@@ -280,7 +280,7 @@ WinNet_ActivateRouteManager(
 

 extern "C"

 WINNET_LINKAGE

-bool

+WINNET_AR_STATUS

 WINNET_API

 WinNet_AddRoutes(

 	const WINNET_ROUTE *routes,

@@ -291,7 +291,7 @@ WinNet_AddRoutes(
 

 	if (nullptr == g_RouteManager)

 	{

-		return false;

+		return WINNET_AR_STATUS_GENERAL_ERROR;

 	}

 

 	try

@@ -302,22 +302,37 @@ WinNet_AddRoutes(
 		}

 

 		g_RouteManager->addRoutes(winnet::ConvertRoutes(routes, numRoutes));

-		return true;

+		return WINNET_AR_STATUS_SUCCESS;

+	}

+	catch (const winnet::routing::error::NoDefaultRoute &err)

+	{

+		common::error::UnwindException(err, g_RouteManagerLogSink);

+		return WINNET_AR_STATUS_NO_DEFAULT_ROUTE;

+	}

+	catch (const winnet::routing::error::DeviceNameNotFound &err)

+	{

+		common::error::UnwindException(err, g_RouteManagerLogSink);

+		return WINNET_AR_STATUS_NAME_NOT_FOUND;

+	}

+	catch (const winnet::routing::error::DeviceGatewayNotFound &err)

+	{

+		common::error::UnwindException(err, g_RouteManagerLogSink);

+		return WINNET_AR_STATUS_GATEWAY_NOT_FOUND;

 	}

 	catch (const std::exception &err)

 	{

 		common::error::UnwindException(err, g_RouteManagerLogSink);

-		return false;

+		return WINNET_AR_STATUS_GENERAL_ERROR;

 	}

 	catch (...)

 	{

-		return false;

+		return WINNET_AR_STATUS_GENERAL_ERROR;

 	}

 }

 

 extern "C"

 WINNET_LINKAGE

-bool

+WINNET_AR_STATUS

 WINNET_API

 WinNet_AddRoute(

 	const WINNET_ROUTE *route

diff --git a/windows/winnet/src/winnet/winnet.h b/windows/winnet/src/winnet/winnet.h
index 32d377e73f..8cd1703f79 100644
--- a/windows/winnet/src/winnet/winnet.h
+++ b/windows/winnet/src/winnet/winnet.h
@@ -96,9 +96,18 @@ WinNet_ActivateRouteManager(
 	void *logSinkContext
 );
 
+enum WINNET_AR_STATUS
+{
+	WINNET_AR_STATUS_SUCCESS = 0,
+	WINNET_AR_STATUS_GENERAL_ERROR = 1,
+	WINNET_AR_STATUS_NO_DEFAULT_ROUTE = 2,
+	WINNET_AR_STATUS_NAME_NOT_FOUND = 3,
+	WINNET_AR_STATUS_GATEWAY_NOT_FOUND = 4,
+};
+
 extern "C"
 WINNET_LINKAGE
-bool
+WINNET_AR_STATUS
 WINNET_API
 WinNet_AddRoutes(
 	const WINNET_ROUTE *routes,
@@ -107,7 +116,7 @@ WinNet_AddRoutes(
 
 extern "C"
 WINNET_LINKAGE
-bool
+WINNET_AR_STATUS
 WINNET_API
 WinNet_AddRoute(
 	const WINNET_ROUTE *route