Skip to content

Commit ccaff30

Browse files
RawaRawa
committed
Merge branch 'remove-wait-for-tunnel-up'
2 parents 612aad8 + c10bffe commit ccaff30

File tree

24 files changed

+828
-413
lines changed

24 files changed

+828
-413
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)