Merge branch 'handle-invalid-dns-server-addresses'

5 files changed, 100 insertions, 39 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/notification/TunnelStateNotification.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/notification/TunnelStateNotification.kt
index d352b3fc2a..80ae9da5e4 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/notification/TunnelStateNotification.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/notification/TunnelStateNotification.kt
@@ -8,6 +8,7 @@ import net.mullvad.talpid.tunnel.ActionAfterDisconnect
 import net.mullvad.talpid.tunnel.ErrorState
 import net.mullvad.talpid.tunnel.ErrorStateCause
 import net.mullvad.talpid.tunnel.ParameterGenerationError
+import net.mullvad.talpid.util.addressString
 
 class TunnelStateNotification(
     private val context: Context,
@@ -53,10 +54,27 @@ class TunnelStateNotification(
     }
 
     private fun show(error: ErrorState?) {
-        val cause = error?.cause
+        // if the error state is null, we can assume that we are secure
+        if (error?.isBlocking ?: true) {
+            title = blockingTitle
+            message = error?.cause?.let { cause -> blockingErrorMessage(cause) }
+        } else {
+            title = notBlockingTitle
+            message = notBlockingErrorMessage(error?.cause)
+        }
+
+        shouldShow = true
+    }
+
+    private fun blockingErrorMessage(cause: ErrorStateCause): String {
+        val messageId = when (cause) {
+            is ErrorStateCause.InvalidDnsServers -> {
+                val addresses = cause.addresses
+                    .map { address -> address.addressString() }
+                    .joinToString()
 
-        val messageText = when (cause) {
-            null -> null
+                return context.getString(R.string.invalid_dns_servers, addresses)
+            }
             is ErrorStateCause.AuthFailed -> R.string.auth_failed
             is ErrorStateCause.Ipv6Unavailable -> R.string.ipv6_unavailable
             is ErrorStateCause.SetFirewallPolicyError -> R.string.set_firewall_policy_error
@@ -78,21 +96,16 @@ class TunnelStateNotification(
             is ErrorStateCause.VpnPermissionDenied -> R.string.vpn_permission_denied_error
         }
 
-        // if the error state is null, we can assume that we are secure
-        if (error?.isBlocking ?: true) {
-            title = blockingTitle
-            message = messageText?.let { id -> context.getString(id) }
-        } else {
-            val updatedMessageText = when (cause) {
-                is ErrorStateCause.VpnPermissionDenied -> messageText
-                else -> R.string.failed_to_block_internet
-            }
+        return context.getString(messageId)
+    }
 
-            title = notBlockingTitle
-            message = updatedMessageText?.let { id -> context.getString(id) }
+    private fun notBlockingErrorMessage(cause: ErrorStateCause?): String {
+        val messageId = when (cause) {
+            is ErrorStateCause.VpnPermissionDenied -> R.string.vpn_permission_denied_error
+            else -> R.string.failed_to_block_internet
         }
 
-        shouldShow = true
+        return context.getString(messageId)
     }
 
     private fun hide() {
diff --git a/android/src/main/kotlin/net/mullvad/talpid/CreateTunResult.kt b/android/src/main/kotlin/net/mullvad/talpid/CreateTunResult.kt
new file mode 100644
index 0000000000..dfa82852f1
--- /dev/null
+++ b/android/src/main/kotlin/net/mullvad/talpid/CreateTunResult.kt
@@ -0,0 +1,24 @@
+package net.mullvad.talpid
+
+import java.net.InetAddress
+
+sealed class CreateTunResult {
+    open val isOpen
+        get() = false
+
+    class Success(val tunFd: Int) : CreateTunResult() {
+        override val isOpen
+            get() = true
+    }
+
+    class InvalidDnsServers(
+        val addresses: ArrayList<InetAddress>,
+        val tunFd: Int
+    ) : CreateTunResult() {
+        override val isOpen
+            get() = true
+    }
+
+    class PermissionDenied : CreateTunResult()
+    class TunnelDeviceError : CreateTunResult()
+}
diff --git a/android/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt
index a0dfd029f2..7da32ad71b 100644
--- a/android/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt
+++ b/android/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt
@@ -10,12 +10,21 @@ import kotlin.properties.Delegates.observable
 import net.mullvad.talpid.tun_provider.TunConfig
 
 open class TalpidVpnService : VpnService() {
-    private var activeTunDevice by observable<Int?>(null) { _, oldTunDevice, _ ->
-        oldTunDevice?.let { oldTunFd ->
+    private var activeTunStatus by observable<CreateTunResult?>(null) { _, oldTunStatus, _ ->
+        val oldTunFd = when (oldTunStatus) {
+            is CreateTunResult.Success -> oldTunStatus.tunFd
+            is CreateTunResult.InvalidDnsServers -> oldTunStatus.tunFd
+            else -> null
+        }
+
+        if (oldTunFd != null) {
             ParcelFileDescriptor.adoptFd(oldTunFd).close()
         }
     }
 
+    private val tunIsOpen
+        get() = activeTunStatus?.isOpen ?: false
+
     private var currentTunConfig = defaultTunConfig()
     private var tunIsStale = false
 
@@ -31,52 +40,52 @@ open class TalpidVpnService : VpnService() {
         connectivityListener.unregister()
     }
 
-    fun getTun(config: TunConfig): Int {
+    fun getTun(config: TunConfig): CreateTunResult {
         synchronized(this) {
-            val tunDevice = activeTunDevice
+            val tunStatus = activeTunStatus
 
-            if (config == currentTunConfig && tunDevice != null && !tunIsStale) {
-                return tunDevice
+            if (config == currentTunConfig && tunIsOpen && !tunIsStale) {
+                return tunStatus!!
             } else {
-                val newTunDevice = createTun(config)
+                val newTunStatus = createTun(config)
 
                 currentTunConfig = config
-                activeTunDevice = newTunDevice
+                activeTunStatus = newTunStatus
                 tunIsStale = false
 
-                return newTunDevice
+                return newTunStatus
             }
         }
     }
 
     fun createTun() {
         synchronized(this) {
-            activeTunDevice = createTun(currentTunConfig)
+            activeTunStatus = createTun(currentTunConfig)
         }
     }
 
     fun createTunIfClosed(): Boolean {
         synchronized(this) {
-            if (activeTunDevice == null) {
-                activeTunDevice = createTun(currentTunConfig)
+            if (!tunIsOpen) {
+                activeTunStatus = createTun(currentTunConfig)
             }
 
-            return activeTunDevice?.let { tunFd -> tunFd > 0 } ?: false
+            return tunIsOpen
         }
     }
 
     fun recreateTunIfOpen(config: TunConfig) {
         synchronized(this) {
-            if (activeTunDevice != null) {
+            if (tunIsOpen) {
                 currentTunConfig = config
-                activeTunDevice = createTun(config)
+                activeTunStatus = createTun(config)
             }
         }
     }
 
     fun closeTun() {
         synchronized(this) {
-            activeTunDevice = null
+            activeTunStatus = null
         }
     }
 
@@ -86,19 +95,25 @@ open class TalpidVpnService : VpnService() {
         }
     }
 
-    private fun createTun(config: TunConfig): Int {
+    private fun createTun(config: TunConfig): CreateTunResult {
         if (VpnService.prepare(this) != null) {
             // VPN permission wasn't granted
-            return -1
+            return CreateTunResult.PermissionDenied()
         }
 
+        var invalidDnsServerAddresses = ArrayList<InetAddress>()
+
         val builder = Builder().apply {
             for (address in config.addresses) {
                 addAddress(address, prefixForAddress(address))
             }
 
             for (dnsServer in config.dnsServers) {
-                addDnsServer(dnsServer)
+                try {
+                    addDnsServer(dnsServer)
+                } catch (exception: IllegalArgumentException) {
+                    invalidDnsServerAddresses.add(dnsServer)
+                }
             }
 
             for (route in config.routes) {
@@ -122,12 +137,17 @@ open class TalpidVpnService : VpnService() {
         val vpnInterface = builder.establish()
         val tunFd = vpnInterface?.detachFd()
 
-        if (tunFd != null) {
-            waitForTunnelUp(tunFd, config.routes.any { route -> route.isIpv6 })
-            return tunFd
-        } else {
-            return 0
+        if (tunFd == null) {
+            return CreateTunResult.TunnelDeviceError()
         }
+
+        waitForTunnelUp(tunFd, config.routes.any { route -> route.isIpv6 })
+
+        if (!invalidDnsServerAddresses.isEmpty()) {
+            return CreateTunResult.InvalidDnsServers(invalidDnsServerAddresses, tunFd)
+        }
+
+        return CreateTunResult.Success(tunFd)
     }
 
     fun bypass(socket: Int): Boolean {
diff --git a/android/src/main/kotlin/net/mullvad/talpid/tunnel/ErrorStateCause.kt b/android/src/main/kotlin/net/mullvad/talpid/tunnel/ErrorStateCause.kt
index e65f70be63..22962630b6 100644
--- a/android/src/main/kotlin/net/mullvad/talpid/tunnel/ErrorStateCause.kt
+++ b/android/src/main/kotlin/net/mullvad/talpid/tunnel/ErrorStateCause.kt
@@ -1,10 +1,13 @@
 package net.mullvad.talpid.tunnel
 
+import java.net.InetAddress
+
 sealed class ErrorStateCause {
     class AuthFailed(val reason: String?) : ErrorStateCause()
     class Ipv6Unavailable : ErrorStateCause()
     class SetFirewallPolicyError : ErrorStateCause()
     class SetDnsError : ErrorStateCause()
+    class InvalidDnsServers(val addresses: ArrayList<InetAddress>) : ErrorStateCause()
     class StartTunnelError : ErrorStateCause()
     class TunnelParameterError(val error: ParameterGenerationError) : ErrorStateCause()
     class IsOffline : ErrorStateCause()
diff --git a/android/src/main/res/values/strings.xml b/android/src/main/res/values/strings.xml
index fbb8182bf4..720a406465 100644
--- a/android/src/main/res/values/strings.xml
+++ b/android/src/main/res/values/strings.xml
@@ -118,6 +118,7 @@
     <string name="set_firewall_policy_error">Failed to apply firewall rules. The device might
     currently be unsecured</string>
     <string name="set_dns_error">Failed to set system DNS server</string>
+    <string name="invalid_dns_servers">Custom DNS server addresses %1$s are invalid</string>
     <string name="start_tunnel_error">Failed to start tunnel connection</string>
     <string name="vpn_permission_denied_error">VPN permission was denied when creating the tunnel.
     Please try connecting again.</string>