fix(android): prevent onLost from triggering a session disconnect when the VPN is re-established just after (#6605)

On Android when when we receive the `onLost` callback for the VPN we
disconnect the tunnel, since that's the only way to find when the user
has disconnected the VPN from outside Firezone, otherwise this would
kill network connection when it's this happens.

But `onLost` was also called when we call `establish` on the tunnel to
recreate it and there's no obvious way to distinguish between both
cases.

So we previously just stopped monitoring `onLost` while we were
executing `establish`. The problem with this approach was that sometimes
`onLost` might have a delayed since `establish` is called.

Therefore, to fix this, we changed the `onLost` call to delay the
execution of `session.disconnect` for 2 second, if within this grace
period we get a `linkPropertiesChanged` for our interface, which is
always called after the network is re-created, we cancel the execution,
otherwise we go ahead an disconnect the session.
This commit is contained in:
Gabi
2024-09-05 14:35:43 -07:00
committed by GitHub
parent da81fb7f41
commit a97f0156aa
2 changed files with 25 additions and 5 deletions

View File

@@ -2,6 +2,9 @@
import android.net.ConnectivityManager
import android.net.LinkProperties
import android.net.Network
import android.os.Handler
import android.os.Looper
import android.util.Log
import dev.firezone.android.tunnel.TunnelService
// None of the TunnelService lifecycle callbacks are called when a user disconnects the VPN
@@ -10,12 +13,17 @@ import dev.firezone.android.tunnel.TunnelService
class DisconnectMonitor(private val tunnelService: TunnelService) : ConnectivityManager.NetworkCallback() {
private var vpnNetwork: Network? = null
// This handler is used to stop connlib when the VPN fd is lost
private var sessionStopperHandle: Handler = Handler(Looper.getMainLooper())
// Android doesn't provide a good way to associate a network with a VPN service, so we
// have to use the IP addresses of the tunnel to determine if the network is our VPN.
override fun onLinkPropertiesChanged(
network: Network,
linkProperties: LinkProperties,
) {
Log.d("DisconnectMonitor", "properties: $linkProperties")
super.onLinkPropertiesChanged(network, linkProperties)
if (tunnelService.tunnelIpv4Address.isNullOrBlank() || tunnelService.tunnelIpv6Address.isNullOrBlank()) {
@@ -26,6 +34,9 @@ class DisconnectMonitor(private val tunnelService: TunnelService) : Connectivity
val ipv6Found = linkProperties.linkAddresses.find { it.address.hostAddress == tunnelService.tunnelIpv6Address }
if (ipv4Found != null && ipv6Found != null) {
// When we get onLinkPropertiesChanged it means the interface is available again and we
// should abort stopping the session
sessionStopperHandle.removeCallbacksAndMessages(null)
// Matched both IPv4 and IPv6 addresses, this is our VPN network
vpnNetwork = network
}
@@ -33,7 +44,18 @@ class DisconnectMonitor(private val tunnelService: TunnelService) : Connectivity
override fun onLost(network: Network) {
if (network == vpnNetwork) {
tunnelService.disconnect()
Log.d("DisconnectMonitor", "Scheduling a session disconnect for $network")
// We delay the execution of disconnecting the tunnel when the network is lost since
// when the tunnel is rebuild we get an onLost just like with disabling the VPN and we
// can't distinguish between those save for getting an onLinkProperties changed later
sessionStopperHandle.postDelayed(
{
Log.d("DisconnectMonitor", "Disconnect tunnel service due to network lost $network")
tunnelService.disconnect()
},
2000,
)
}
super.onLost(network)

View File

@@ -136,6 +136,7 @@ class TunnelService : VpnService() {
// service.
override fun onDisconnect(error: String): Boolean {
stopNetworkMonitoring()
stopDisconnectMonitoring()
// Clear any user tokens and actorNames
repo.clearToken()
@@ -164,8 +165,6 @@ class TunnelService : VpnService() {
}
}
stopDisconnectMonitoring()
Builder().apply {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
setMetered(false) // Inherit the metered status from the underlying networks.
@@ -203,8 +202,6 @@ class TunnelService : VpnService() {
ConnlibSession.setTun(it, fd)
}
}
startDisconnectMonitoring()
}
private val restrictionsFilter = IntentFilter(Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED)
@@ -333,6 +330,7 @@ class TunnelService : VpnService() {
)
startNetworkMonitoring()
startDisconnectMonitoring()
}
}