Skip to content

Commit 341c10b

Browse files
RawaRawa
committed
Replace old waitForTunnelUp function
After invoking VpnService.establish() we will get a tunnel file descriptor that corresponds to the interface that was created. However, this has no guarantee of the routing table beeing up to date, and we might thus send traffic outside the tunnel. Previously this was done through looking at the tunFd to see that traffic is sent to verify that the routing table has changed. If no traffic is seen some traffic is induced to a random IP address to ensure traffic can be seen. This new implementation is slower but won't risk sending UDP traffic to a random public address at the internet.
1 parent 612aad8 commit 341c10b

File tree

24 files changed

+748
-412
lines changed

24 files changed

+748
-412
lines changed

‎Cargo.lock

+13-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎android/app/src/test/kotlin/net/mullvad/talpid/TalpidVpnServiceFallbackDnsTest.kt

+146
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package net.mullvad.talpid
2+
3+
import android.net.VpnService
4+
import android.os.ParcelFileDescriptor
5+
import arrow.core.right
6+
import io.mockk.MockKAnnotations
7+
import io.mockk.coVerify
8+
import io.mockk.every
9+
import io.mockk.mockk
10+
import io.mockk.mockkConstructor
11+
import io.mockk.mockkStatic
12+
import io.mockk.spyk
13+
import java.net.InetAddress
14+
import net.mullvad.mullvadvpn.lib.common.test.assertLists
15+
import net.mullvad.mullvadvpn.lib.common.util.prepareVpnSafe
16+
import net.mullvad.mullvadvpn.lib.model.Prepared
17+
import net.mullvad.talpid.model.CreateTunResult
18+
import net.mullvad.talpid.model.InetNetwork
19+
import net.mullvad.talpid.model.TunConfig
20+
import org.junit.jupiter.api.BeforeEach
21+
import org.junit.jupiter.api.Test
22+
import org.junit.jupiter.api.assertInstanceOf
23+
24+
class TalpidVpnServiceFallbackDnsTest {
25+
lateinit var talpidVpnService: TalpidVpnService
26+
var builderMockk = mockk<VpnService.Builder>()
27+
28+
@BeforeEach
29+
fun setup() {
30+
MockKAnnotations.init(this)
31+
mockkStatic(VPN_SERVICE_EXTENSION)
32+
33+
talpidVpnService = spyk<TalpidVpnService>(recordPrivateCalls = true)
34+
every { talpidVpnService.prepareVpnSafe() } returns Prepared.right()
35+
builderMockk = mockk<VpnService.Builder>()
36+
37+
mockkConstructor(VpnService.Builder::class)
38+
every { anyConstructed<VpnService.Builder>().setMtu(any()) } returns builderMockk
39+
every { anyConstructed<VpnService.Builder>().setBlocking(any()) } returns builderMockk
40+
every { anyConstructed<VpnService.Builder>().addAddress(any<InetAddress>(), any()) } returns
41+
builderMockk
42+
every { anyConstructed<VpnService.Builder>().addRoute(any<InetAddress>(), any()) } returns
43+
builderMockk
44+
every {
45+
anyConstructed<VpnService.Builder>()
46+
.addDnsServer(TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER)
47+
} returns builderMockk
48+
val parcelFileDescriptor: ParcelFileDescriptor = mockk()
49+
every { anyConstructed<VpnService.Builder>().establish() } returns parcelFileDescriptor
50+
every { parcelFileDescriptor.detachFd() } returns 1
51+
}
52+
53+
@Test
54+
fun `opening tun with no DnsServers should add fallback DNS server`() {
55+
val tunConfig = baseTunConfig.copy(dnsServers = arrayListOf())
56+
57+
val result = talpidVpnService.openTun(tunConfig)
58+
59+
assertInstanceOf<CreateTunResult.Success>(result)
60+
61+
// Fallback DNS server should be added if no DNS servers are provided
62+
coVerify(exactly = 1) {
63+
anyConstructed<VpnService.Builder>()
64+
.addDnsServer(TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER)
65+
}
66+
}
67+
68+
@Test
69+
fun `opening tun with all bad DnsServers should return InvalidDnsServers and add fallback`() {
70+
val badDns1 = InetAddress.getByName("0.0.0.0")
71+
val badDns2 = InetAddress.getByName("255.255.255.255")
72+
every { anyConstructed<VpnService.Builder>().addDnsServer(badDns1) } throws
73+
IllegalArgumentException()
74+
every { anyConstructed<VpnService.Builder>().addDnsServer(badDns2) } throws
75+
IllegalArgumentException()
76+
77+
val tunConfig = baseTunConfig.copy(dnsServers = arrayListOf(badDns1, badDns2))
78+
val result = talpidVpnService.openTun(tunConfig)
79+
80+
assertInstanceOf<CreateTunResult.InvalidDnsServers>(result)
81+
assertLists(tunConfig.dnsServers, result.addresses)
82+
// Fallback DNS server should be added if no valid DNS servers are provided
83+
coVerify(exactly = 1) {
84+
anyConstructed<VpnService.Builder>()
85+
.addDnsServer(TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER)
86+
}
87+
}
88+
89+
@Test
90+
fun `opening tun with 1 good and 1 bad DnsServers should return InvalidDnsServers`() {
91+
val goodDnsServer = InetAddress.getByName("1.1.1.1")
92+
val badDns = InetAddress.getByName("255.255.255.255")
93+
every { anyConstructed<VpnService.Builder>().addDnsServer(goodDnsServer) } returns
94+
builderMockk
95+
every { anyConstructed<VpnService.Builder>().addDnsServer(badDns) } throws
96+
IllegalArgumentException()
97+
98+
val tunConfig = baseTunConfig.copy(dnsServers = arrayListOf(goodDnsServer, badDns))
99+
val result = talpidVpnService.openTun(tunConfig)
100+
101+
assertInstanceOf<CreateTunResult.InvalidDnsServers>(result)
102+
assertLists(arrayListOf(badDns), result.addresses)
103+
104+
// Fallback DNS server should not be added since we have 1 good DNS server
105+
coVerify(exactly = 0) {
106+
anyConstructed<VpnService.Builder>()
107+
.addDnsServer(TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER)
108+
}
109+
}
110+
111+
@Test
112+
fun `providing good dns servers should not add the fallback dns and return success`() {
113+
val goodDnsServer = InetAddress.getByName("1.1.1.1")
114+
every { anyConstructed<VpnService.Builder>().addDnsServer(goodDnsServer) } returns
115+
builderMockk
116+
117+
val tunConfig = baseTunConfig.copy(dnsServers = arrayListOf(goodDnsServer))
118+
val result = talpidVpnService.openTun(tunConfig)
119+
120+
assertInstanceOf<CreateTunResult.Success>(result)
121+
122+
// Fallback DNS server should not be added since we have good DNS servers.
123+
coVerify(exactly = 0) {
124+
anyConstructed<VpnService.Builder>()
125+
.addDnsServer(TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER)
126+
}
127+
}
128+
129+
companion object {
130+
private const val VPN_SERVICE_EXTENSION =
131+
"net.mullvad.mullvadvpn.lib.common.util.VpnServiceUtilsKt"
132+
133+
val baseTunConfig =
134+
TunConfig(
135+
addresses = arrayListOf(InetAddress.getByName("45.83.223.209")),
136+
dnsServers = arrayListOf(),
137+
routes =
138+
arrayListOf(
139+
InetNetwork(InetAddress.getByName("0.0.0.0"), 0),
140+
InetNetwork(InetAddress.getByName("::"), 0),
141+
),
142+
mtu = 1280,
143+
excludedPackages = arrayListOf(),
144+
)
145+
}
146+
}

‎android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/VpnServiceUtils.kt

+43-3
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,23 @@ package net.mullvad.mullvadvpn.lib.common.util
22

33
import android.content.Context
44
import android.content.Intent
5+
import android.net.VpnService
56
import android.net.VpnService.prepare
7+
import android.os.ParcelFileDescriptor
68
import arrow.core.Either
7-
import arrow.core.flatten
9+
import arrow.core.flatMap
810
import arrow.core.left
11+
import arrow.core.raise.either
12+
import arrow.core.raise.ensureNotNull
913
import arrow.core.right
1014
import co.touchlab.kermit.Logger
1115
import net.mullvad.mullvadvpn.lib.common.util.SdkUtils.getInstalledPackagesList
1216
import net.mullvad.mullvadvpn.lib.model.PrepareError
1317
import net.mullvad.mullvadvpn.lib.model.Prepared
1418

1519
/**
20+
* Prepare to establish a VPN connection safely.
21+
*
1622
* Invoking VpnService.prepare() can result in 3 out comes:
1723
* 1. IllegalStateException - There is a legacy VPN profile marked as always on
1824
* 2. Intent
@@ -34,7 +40,7 @@ fun Context.prepareVpnSafe(): Either<PrepareError, Prepared> =
3440
else -> throw it
3541
}
3642
}
37-
.map { intent ->
43+
.flatMap { intent ->
3844
if (intent == null) {
3945
Prepared.right()
4046
} else {
@@ -46,7 +52,6 @@ fun Context.prepareVpnSafe(): Either<PrepareError, Prepared> =
4652
}
4753
}
4854
}
49-
.flatten()
5055

5156
fun Context.getAlwaysOnVpnAppName(): String? {
5257
return resolveAlwaysOnVpnPackageName()
@@ -59,3 +64,38 @@ fun Context.getAlwaysOnVpnAppName(): String? {
5964
?.loadLabel(packageManager)
6065
?.toString()
6166
}
67+
68+
/**
69+
* Establish a VPN connection safely.
70+
*
71+
* This function wraps the [VpnService.Builder.establish] function and catches any exceptions that
72+
* may be thrown and type them to a more specific error.
73+
*
74+
* @return [ParcelFileDescriptor] if successful, [EstablishError] otherwise
75+
*/
76+
fun VpnService.Builder.establishSafe(): Either<EstablishError, ParcelFileDescriptor> = either {
77+
val vpnInterfaceFd =
78+
Either.catch { establish() }
79+
.mapLeft {
80+
when (it) {
81+
is IllegalStateException -> EstablishError.ParameterNotApplied(it)
82+
is IllegalArgumentException -> EstablishError.ParameterNotAccepted(it)
83+
else -> EstablishError.UnknownError(it)
84+
}
85+
}
86+
.bind()
87+
88+
ensureNotNull(vpnInterfaceFd) { EstablishError.NullVpnInterface }
89+
90+
vpnInterfaceFd
91+
}
92+
93+
sealed interface EstablishError {
94+
data class ParameterNotApplied(val exception: IllegalStateException) : EstablishError
95+
96+
data class ParameterNotAccepted(val exception: IllegalArgumentException) : EstablishError
97+
98+
data object NullVpnInterface : EstablishError
99+
100+
data class UnknownError(val error: Throwable) : EstablishError
101+
}

‎android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt

+3-7
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ import net.mullvad.mullvadvpn.lib.model.DnsState
3636
import net.mullvad.mullvadvpn.lib.model.Endpoint
3737
import net.mullvad.mullvadvpn.lib.model.ErrorState
3838
import net.mullvad.mullvadvpn.lib.model.ErrorStateCause
39-
import net.mullvad.mullvadvpn.lib.model.ErrorStateCause.AuthFailed
40-
import net.mullvad.mullvadvpn.lib.model.ErrorStateCause.OtherAlwaysOnApp
41-
import net.mullvad.mullvadvpn.lib.model.ErrorStateCause.TunnelParameterError
4239
import net.mullvad.mullvadvpn.lib.model.FeatureIndicator
4340
import net.mullvad.mullvadvpn.lib.model.GeoIpLocation
4441
import net.mullvad.mullvadvpn.lib.model.GeoLocationId
@@ -125,7 +122,7 @@ private fun ManagementInterface.TunnelState.Error.toDomain(): TunnelState.Error
125122
val otherAlwaysOnAppError =
126123
errorState.let {
127124
if (it.hasOtherAlwaysOnAppError()) {
128-
OtherAlwaysOnApp(it.otherAlwaysOnAppError.appName)
125+
ErrorStateCause.OtherAlwaysOnApp(it.otherAlwaysOnAppError.appName)
129126
} else {
130127
null
131128
}
@@ -238,7 +235,7 @@ internal fun ManagementInterface.ErrorState.toDomain(
238235
cause =
239236
when (cause!!) {
240237
ManagementInterface.ErrorState.Cause.AUTH_FAILED ->
241-
AuthFailed(authFailedError.toDomain())
238+
ErrorStateCause.AuthFailed(authFailedError.toDomain())
242239
ManagementInterface.ErrorState.Cause.IPV6_UNAVAILABLE ->
243240
ErrorStateCause.Ipv6Unavailable
244241
ManagementInterface.ErrorState.Cause.SET_FIREWALL_POLICY_ERROR ->
@@ -247,15 +244,14 @@ internal fun ManagementInterface.ErrorState.toDomain(
247244
ManagementInterface.ErrorState.Cause.START_TUNNEL_ERROR ->
248245
ErrorStateCause.StartTunnelError
249246
ManagementInterface.ErrorState.Cause.TUNNEL_PARAMETER_ERROR ->
250-
TunnelParameterError(parameterError.toDomain())
247+
ErrorStateCause.TunnelParameterError(parameterError.toDomain())
251248
ManagementInterface.ErrorState.Cause.IS_OFFLINE -> ErrorStateCause.IsOffline
252249
ManagementInterface.ErrorState.Cause.SPLIT_TUNNEL_ERROR ->
253250
ErrorStateCause.StartTunnelError
254251
ManagementInterface.ErrorState.Cause.UNRECOGNIZED,
255252
ManagementInterface.ErrorState.Cause.NEED_FULL_DISK_PERMISSIONS,
256253
ManagementInterface.ErrorState.Cause.CREATE_TUNNEL_DEVICE ->
257254
throw IllegalArgumentException("Unrecognized error state cause")
258-
259255
ManagementInterface.ErrorState.Cause.NOT_PREPARED -> ErrorStateCause.NotPrepared
260256
ManagementInterface.ErrorState.Cause.OTHER_ALWAYS_ON_APP -> otherAlwaysOnApp!!
261257
ManagementInterface.ErrorState.Cause.OTHER_LEGACY_ALWAYS_ON_VPN ->

0 commit comments

Comments
 (0)