From 29e3c187c21f899e47b3f3b910fb9711f32e9323 Mon Sep 17 00:00:00 2001 From: kari-ts <135075563+kari-ts@users.noreply.github.com> Date: Wed, 28 Aug 2024 09:45:22 -0700 Subject: [PATCH] android: stop tailscaled when VPN has been revoked (#480) -add new Ipn UI state 'Stopping' to handle the case where the VPN is no longer active and a request to stop Tailscale has been issued (but is not complete yet) and use for optimistic UI -when VPN has been revoked, stop tailscaled and set the state to Stopping -this fixes the race condition where when we tell tailscaled to stop, stopping races against the netmap state updating as a result of the VPN being revoked -add isActive state and use instead of isPrepared for UI showing whether we are connected - we were previously using isPrepared as a proxy for connection, but sometimes the VPN has been prepared but is not active (eg when VPN permissions have been given and VPN has been connected previously, but has been revoked) -refactor network callbacks into its own class for readability Fixes tailscale/tailscale#12850 Signed-off-by: kari-ts --- .../src/main/java/com/tailscale/ipn/App.kt | 60 +++---------------- .../main/java/com/tailscale/ipn/IPNService.kt | 23 +++++-- .../java/com/tailscale/ipn/MainActivity.kt | 4 +- .../tailscale/ipn/NetworkChangeCallback.kt | 58 ++++++++++++++++++ .../java/com/tailscale/ipn/ui/model/Ipn.kt | 6 +- .../com/tailscale/ipn/ui/notifier/Notifier.kt | 39 ++++++------ .../com/tailscale/ipn/ui/view/MainView.kt | 6 +- .../ipn/ui/viewModel/IpnViewModel.kt | 5 -- .../ipn/ui/viewModel/MainViewModel.kt | 32 +++++----- .../ipn/ui/viewModel/VpnViewModel.kt | 13 +++- go.mod | 2 +- libtailscale/backend.go | 21 ++++--- libtailscale/interfaces.go | 6 +- libtailscale/net.go | 35 ++++++++++- 14 files changed, 195 insertions(+), 115 deletions(-) create mode 100644 android/src/main/java/com/tailscale/ipn/NetworkChangeCallback.kt diff --git a/android/src/main/java/com/tailscale/ipn/App.kt b/android/src/main/java/com/tailscale/ipn/App.kt index 3a1e88e..5df2cab 100644 --- a/android/src/main/java/com/tailscale/ipn/App.kt +++ b/android/src/main/java/com/tailscale/ipn/App.kt @@ -12,10 +12,6 @@ import android.content.Intent import android.content.SharedPreferences import android.content.pm.PackageManager import android.net.ConnectivityManager -import android.net.LinkProperties -import android.net.Network -import android.net.NetworkCapabilities -import android.net.NetworkRequest import android.os.Build import android.os.Environment import android.util.Log @@ -45,7 +41,6 @@ import kotlinx.serialization.json.Json import libtailscale.Libtailscale import java.io.File import java.io.IOException -import java.net.InetAddress import java.net.NetworkInterface import java.security.GeneralSecurityException import java.util.Locale @@ -56,11 +51,6 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner { companion object { private const val FILE_CHANNEL_ID = "tailscale-files" private const val TAG = "App" - private val networkConnectivityRequest = - NetworkRequest.Builder() - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) - .build() private lateinit var appInstance: App /** @@ -81,9 +71,6 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner { override val viewModelStore: ViewModelStore get() = appViewModelStore - lateinit var vpnViewModel: VpnViewModel - private set - private val appViewModelStore: ViewModelStore by lazy { ViewModelStore() } var healthNotifier: HealthNotifier? = null @@ -147,7 +134,8 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner { Notifier.start(applicationScope) healthNotifier = HealthNotifier(Notifier.health, applicationScope) connectivityManager = this.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager - setAndRegisterNetworkCallbacks() + NetworkChangeCallback.monitorDnsChanges(connectivityManager, dns) + initViewModels() applicationScope.launch { Notifier.state.collect { state -> val ableToStartVPN = state > Ipn.State.NeedsMachineAuth @@ -161,14 +149,13 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner { QuickToggleService.setVPNRunning(vpnRunning) } } - initViewModels() } private fun initViewModels() { vpnViewModel = ViewModelProvider(this, VpnViewModelFactory(this)).get(VpnViewModel::class.java) } - fun setWantRunning(wantRunning: Boolean) { + fun setWantRunning(wantRunning: Boolean, onSuccess: (() -> Unit)? = null) { val callback: (Result) -> Unit = { result -> result.fold( onSuccess = {}, @@ -180,41 +167,6 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner { .editPrefs(Ipn.MaskedPrefs().apply { WantRunning = wantRunning }, callback) } - // requestNetwork attempts to find the best network that matches the passed NetworkRequest. It is - // possible that this might return an unusuable network, eg a captive portal. - private fun setAndRegisterNetworkCallbacks() { - connectivityManager.requestNetwork( - networkConnectivityRequest, - object : ConnectivityManager.NetworkCallback() { - override fun onAvailable(network: Network) { - super.onAvailable(network) - - val sb = StringBuilder() - val linkProperties: LinkProperties? = connectivityManager.getLinkProperties(network) - val dnsList: MutableList = linkProperties?.dnsServers ?: mutableListOf() - for (ip in dnsList) { - sb.append(ip.hostAddress).append(" ") - } - val searchDomains: String? = linkProperties?.domains - if (searchDomains != null) { - sb.append("\n") - sb.append(searchDomains) - } - - if (dns.updateDNSFromNetwork(sb.toString())) { - Libtailscale.onDNSConfigChanged(linkProperties?.interfaceName) - } - } - - override fun onLost(network: Network) { - super.onLost(network) - if (dns.updateDNSFromNetwork("")) { - Libtailscale.onDNSConfigChanged("") - } - } - }) - } - // encryptToPref a byte array of data using the Jetpack Security // library and writes it to a global encrypted preference store. @Throws(IOException::class, GeneralSecurityException::class) @@ -389,6 +341,8 @@ open class UninitializedApp : Application() { private lateinit var appInstance: UninitializedApp lateinit var notificationManager: NotificationManagerCompat + lateinit var vpnViewModel: VpnViewModel + @JvmStatic fun get(): UninitializedApp { return appInstance @@ -550,6 +504,10 @@ open class UninitializedApp : Application() { return builtInDisallowedPackageNames + userDisallowed } + fun getAppScopedViewModel(): VpnViewModel { + return vpnViewModel + } + val builtInDisallowedPackageNames: List = listOf( // RCS/Jibe https://github.com/tailscale/tailscale/issues/2322 diff --git a/android/src/main/java/com/tailscale/ipn/IPNService.kt b/android/src/main/java/com/tailscale/ipn/IPNService.kt index 193e2ff..53579cc 100644 --- a/android/src/main/java/com/tailscale/ipn/IPNService.kt +++ b/android/src/main/java/com/tailscale/ipn/IPNService.kt @@ -10,33 +10,40 @@ import android.os.Build import android.system.OsConstants import android.util.Log import com.tailscale.ipn.mdm.MDMSettings +import com.tailscale.ipn.ui.model.Ipn +import com.tailscale.ipn.ui.notifier.Notifier import libtailscale.Libtailscale import java.util.UUID open class IPNService : VpnService(), libtailscale.IPNService { private val TAG = "IPNService" private val randomID: String = UUID.randomUUID().toString() + private lateinit var app: App override fun id(): String { return randomID } + override fun updateVpnStatus(status: Boolean) { + app.getAppScopedViewModel().setVpnActive(status) + } + override fun onCreate() { super.onCreate() // grab app to make sure it initializes - App.get() + app = App.get() } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int = when (intent?.action) { ACTION_STOP_VPN -> { - App.get().setWantRunning(false) + app.setWantRunning(false) close() START_NOT_STICKY } ACTION_START_VPN -> { showForegroundNotification() - App.get().setWantRunning(true) + app.setWantRunning(true) Libtailscale.requestVPN(this) START_STICKY } @@ -44,8 +51,8 @@ open class IPNService : VpnService(), libtailscale.IPNService { // This means we were started by Android due to Always On VPN. // We show a non-foreground notification because we weren't // started as a foreground service. - App.get().notifyStatus(true) - App.get().setWantRunning(true) + app.notifyStatus(true) + app.setWantRunning(true) Libtailscale.requestVPN(this) START_STICKY } @@ -64,6 +71,8 @@ open class IPNService : VpnService(), libtailscale.IPNService { } override fun close() { + app.setWantRunning(false) { updateVpnStatus(false) } + Notifier.setState(Ipn.State.Stopping) stopForeground(STOP_FOREGROUND_REMOVE) Libtailscale.serviceDisconnect(this) } @@ -78,6 +87,10 @@ open class IPNService : VpnService(), libtailscale.IPNService { super.onRevoke() } + private fun setVpnPrepared(isPrepared: Boolean) { + app.getAppScopedViewModel().setVpnPrepared(isPrepared) + } + private fun showForegroundNotification() { try { startForeground( diff --git a/android/src/main/java/com/tailscale/ipn/MainActivity.kt b/android/src/main/java/com/tailscale/ipn/MainActivity.kt index 336edd9..bfdfd19 100644 --- a/android/src/main/java/com/tailscale/ipn/MainActivity.kt +++ b/android/src/main/java/com/tailscale/ipn/MainActivity.kt @@ -89,7 +89,7 @@ class MainActivity : ComponentActivity() { private lateinit var vpnPermissionLauncher: ActivityResultLauncher private val viewModel: MainViewModel by lazy { val app = App.get() - vpnViewModel = app.vpnViewModel + vpnViewModel = app.getAppScopedViewModel() ViewModelProvider(this, MainViewModelFactory(vpnViewModel)).get(MainViewModel::class.java) } private lateinit var vpnViewModel: VpnViewModel @@ -137,7 +137,7 @@ class MainActivity : ComponentActivity() { showOtherVPNConflictDialog() } else { Log.d("VpnPermission", "Permission was denied by the user") - viewModel.setVpnPrepared(false) + vpnViewModel.setVpnPrepared(false) } } } diff --git a/android/src/main/java/com/tailscale/ipn/NetworkChangeCallback.kt b/android/src/main/java/com/tailscale/ipn/NetworkChangeCallback.kt new file mode 100644 index 0000000..622df91 --- /dev/null +++ b/android/src/main/java/com/tailscale/ipn/NetworkChangeCallback.kt @@ -0,0 +1,58 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause +package com.tailscale.ipn + +import android.content.Context +import android.net.ConnectivityManager +import android.net.LinkProperties +import android.net.Network +import android.net.NetworkCapabilities +import android.net.NetworkRequest +import android.util.Log +import libtailscale.Libtailscale +import java.net.InetAddress +import java.net.NetworkInterface + +object NetworkChangeCallback { + + // requestNetwork attempts to find the best network that matches the passed NetworkRequest. It is + // possible that this might return an unusuable network, eg a captive portal. + fun monitorDnsChanges(connectivityManager: ConnectivityManager, dns: DnsConfig) { + val networkConnectivityRequest = + NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) + .build() + + connectivityManager.registerNetworkCallback( + networkConnectivityRequest, + object : ConnectivityManager.NetworkCallback() { + override fun onAvailable(network: Network) { + super.onAvailable(network) + + val sb = StringBuilder() + val linkProperties: LinkProperties? = connectivityManager.getLinkProperties(network) + val dnsList: MutableList = linkProperties?.dnsServers ?: mutableListOf() + for (ip in dnsList) { + sb.append(ip.hostAddress).append(" ") + } + val searchDomains: String? = linkProperties?.domains + if (searchDomains != null) { + sb.append("\n") + sb.append(searchDomains) + } + + if (dns.updateDNSFromNetwork(sb.toString())) { + Libtailscale.onDNSConfigChanged(linkProperties?.interfaceName) + } + } + + override fun onLost(network: Network) { + super.onLost(network) + if (dns.updateDNSFromNetwork("")) { + Libtailscale.onDNSConfigChanged("") + } + } + }) + } +} diff --git a/android/src/main/java/com/tailscale/ipn/ui/model/Ipn.kt b/android/src/main/java/com/tailscale/ipn/ui/model/Ipn.kt index 9fceead..b425d14 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/model/Ipn.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/model/Ipn.kt @@ -18,7 +18,11 @@ class Ipn { NeedsMachineAuth(3), Stopped(4), Starting(5), - Running(6); + Running(6), + // Stopping represents a state where a request to stop Tailscale has been issue but has not + // completed. This state allows UI to optimistically reflect a stopped state, and to fallback if + // necessary. + Stopping(7); companion object { fun fromInt(value: Int): State { diff --git a/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt b/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt index c01a142..427397a 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt @@ -33,7 +33,8 @@ object Notifier { private val decoder = Json { ignoreUnknownKeys = true } // General IPN Bus State - val state: StateFlow = MutableStateFlow(Ipn.State.NoState) + private val _state = MutableStateFlow(Ipn.State.NoState) + val state: StateFlow = _state val netmap: StateFlow = MutableStateFlow(null) val prefs: StateFlow = MutableStateFlow(null) val engineStatus: StateFlow = MutableStateFlow(null) @@ -68,22 +69,22 @@ object Notifier { NotifyWatchOpt.Prefs.value or NotifyWatchOpt.InitialState.value or NotifyWatchOpt.InitialHealthState.value - manager = - app.watchNotifications(mask.toLong()) { notification -> - val notify = decoder.decodeFromStream(notification.inputStream()) - notify.State?.let { state.set(Ipn.State.fromInt(it)) } - notify.NetMap?.let(netmap::set) - notify.Prefs?.let(prefs::set) - notify.Engine?.let(engineStatus::set) - notify.TailFSShares?.let(tailFSShares::set) - notify.BrowseToURL?.let(browseToURL::set) - notify.LoginFinished?.let { loginFinished.set(it.property) } - notify.Version?.let(version::set) - notify.OutgoingFiles?.let(outgoingFiles::set) - notify.FilesWaiting?.let(filesWaiting::set) - notify.IncomingFiles?.let(incomingFiles::set) - notify.Health?.let(health::set) - } + manager = + app.watchNotifications(mask.toLong()) { notification -> + val notify = decoder.decodeFromStream(notification.inputStream()) + notify.State?.let { state.set(Ipn.State.fromInt(it)) } + notify.NetMap?.let(netmap::set) + notify.Prefs?.let(prefs::set) + notify.Engine?.let(engineStatus::set) + notify.TailFSShares?.let(tailFSShares::set) + notify.BrowseToURL?.let(browseToURL::set) + notify.LoginFinished?.let { loginFinished.set(it.property) } + notify.Version?.let(version::set) + notify.OutgoingFiles?.let(outgoingFiles::set) + notify.FilesWaiting?.let(filesWaiting::set) + notify.IncomingFiles?.let(incomingFiles::set) + notify.Health?.let(health::set) + } } } @@ -107,4 +108,8 @@ object Notifier { InitialOutgoingFiles(64), InitialHealthState(128), } + + fun setState(newState: Ipn.State) { + _state.value = newState +} } diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt index c12b1b1..8dfb3f4 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt @@ -232,6 +232,9 @@ fun MainView( ConnectView( state, isPrepared, + // If Tailscale is stopping, don't automatically restart; wait for user to take + // action (eg, if the user connected to another VPN). + state != Ipn.State.Stopping, user, { viewModel.toggleVpn() }, { viewModel.login() }, @@ -407,6 +410,7 @@ fun StartingView() { fun ConnectView( state: Ipn.State, isPrepared: Boolean, + shouldStartAutomatically: Boolean, user: IpnLocal.LoginProfile?, connectAction: () -> Unit, loginAction: () -> Unit, @@ -415,7 +419,7 @@ fun ConnectView( showVPNPermissionLauncherIfUnauthorized: () -> Unit ) { LaunchedEffect(isPrepared) { - if (!isPrepared) { + if (!isPrepared && shouldStartAutomatically) { showVPNPermissionLauncherIfUnauthorized() } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt index c38b5c0..3baab13 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt @@ -134,11 +134,6 @@ open class IpnViewModel : ViewModel() { } // VPN Control - - fun setVpnPrepared(prepared: Boolean) { - _vpnPrepared.value = prepared - } - fun startVPN() { UninitializedApp.get().startVPN() } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt index 3ce6a30..cdfaace 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt @@ -72,6 +72,8 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() { val isVpnPrepared: StateFlow = vpnViewModel.vpnPrepared + val isVpnActive: StateFlow = vpnViewModel.vpnActive + // Icon displayed in the button to present the health view val healthIcon: StateFlow = MutableStateFlow(null) @@ -97,30 +99,32 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() { viewModelScope.launch { var previousState: State? = null - combine(Notifier.state, isVpnPrepared) { state, prepared -> state to prepared } - .collect { (currentState, prepared) -> - stateRes.set(userStringRes(currentState, previousState, prepared)) + combine(Notifier.state, isVpnActive) { state, active -> state to active } + .collect { (currentState, active) -> + // Determine the correct state resource string + stateRes.set(userStringRes(currentState, previousState, active)) + // Determine if the VPN toggle should be on val isOn = when { - prepared && currentState == State.Running || currentState == State.Starting -> - true - previousState == State.NoState && currentState == State.Starting -> + active && (currentState == State.Running || currentState == State.Starting) -> true + previousState == State.NoState && currentState == State.Starting -> true else -> false } + // Update the VPN toggle state _vpnToggleState.value = isOn + + // Update the previous state previousState = currentState } } viewModelScope.launch { - searchTerm - .debounce(250L) - .collect { term -> - peers.set(peerCategorizer.groupedAndFilteredPeers(term)) - } + searchTerm.debounce(250L).collect { term -> + peers.set(peerCategorizer.groupedAndFilteredPeers(term)) + } } viewModelScope.launch { @@ -181,17 +185,17 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() { } } -private fun userStringRes(currentState: State?, previousState: State?, vpnPrepared: Boolean): Int { +private fun userStringRes(currentState: State?, previousState: State?, vpnActive: Boolean): Int { return when { previousState == State.NoState && currentState == State.Starting -> R.string.starting currentState == State.NoState -> R.string.placeholder currentState == State.InUseOtherUser -> R.string.placeholder currentState == State.NeedsLogin -> - if (vpnPrepared) R.string.please_login else R.string.connect_to_vpn + if (vpnActive) R.string.please_login else R.string.connect_to_vpn currentState == State.NeedsMachineAuth -> R.string.needs_machine_auth currentState == State.Stopped -> R.string.stopped currentState == State.Starting -> R.string.starting - currentState == State.Running -> if (vpnPrepared) R.string.connected else R.string.placeholder + currentState == State.Running -> if (vpnActive) R.string.connected else R.string.placeholder else -> R.string.placeholder } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/VpnViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/VpnViewModel.kt index f1e33bc..3c4d40e 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/VpnViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/VpnViewModel.kt @@ -26,9 +26,12 @@ class VpnViewModelFactory(private val application: Application) : ViewModelProvi // application scoped because Tailscale might be toggled on and off outside of the activity // lifecycle. class VpnViewModel(application: Application) : AndroidViewModel(application) { - // Whether the VPN is prepared + // Whether the VPN is prepared. This is set to true if the VPN application is already prepared, or if the user has previously consented to the VPN application. This is used to determine whether a VPN permission launcher needs to be shown. val _vpnPrepared = MutableStateFlow(false) val vpnPrepared: StateFlow = _vpnPrepared + // Whether a VPN interface has been established. This is set by net.updateTUN upon VpnServiceBuilder.establish, and consumed by UI to reflect VPN state. + val _vpnActive = MutableStateFlow(false) + val vpnActive: StateFlow = _vpnActive val TAG = "VpnViewModel" init { @@ -49,7 +52,11 @@ class VpnViewModel(application: Application) : AndroidViewModel(application) { } } - fun setVpnPrepared(prepared: Boolean) { - _vpnPrepared.value = prepared + fun setVpnActive(isActive: Boolean) { + _vpnActive.value = isActive + } + + fun setVpnPrepared(isPrepared: Boolean) { + _vpnPrepared.value = isPrepared } } diff --git a/go.mod b/go.mod index 59d0598..9c3291b 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.22.0 require ( github.com/tailscale/wireguard-go v0.0.0-20240731203015-71393c576b98 golang.org/x/mobile v0.0.0-20240319015410-c58ccf4b0c87 - golang.org/x/sys v0.22.0 inet.af/netaddr v0.0.0-20220617031823-097006376321 tailscale.com v1.73.0-pre.0.20240821174438-af3d3c433b67 ) @@ -87,6 +86,7 @@ require ( golang.org/x/mod v0.19.0 // indirect golang.org/x/net v0.27.0 // indirect golang.org/x/sync v0.7.0 // indirect + golang.org/x/sys v0.22.0 // indirect golang.org/x/term v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect golang.org/x/time v0.5.0 // indirect diff --git a/libtailscale/backend.go b/libtailscale/backend.go index f108f10..908b7e2 100644 --- a/libtailscale/backend.go +++ b/libtailscale/backend.go @@ -150,7 +150,6 @@ func (a *App) runBackend(ctx context.Context) error { cfg configPair state ipn.State networkMap *netmap.NetworkMap - service IPNService ) stateCh := make(chan ipn.State) @@ -168,9 +167,9 @@ func (a *App) runBackend(ctx context.Context) error { select { case s := <-stateCh: state = s - if cfg.rcfg != nil && state >= ipn.Starting && service != nil { + if cfg.rcfg != nil && state >= ipn.Starting && vpnService.service != nil { // On state change, check if there are router or config changes requiring an update to VPNBuilder - if err := b.updateTUN(service, cfg.rcfg, cfg.dcfg); err != nil { + if err := b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg); err != nil { if errors.Is(err, errMultipleUsers) { // TODO: surface error to user } @@ -193,13 +192,13 @@ func (a *App) runBackend(ctx context.Context) error { networkMap = n case c := <-configs: cfg = c - if b == nil || service == nil || cfg.rcfg == nil { + if b == nil || vpnService.service == nil || cfg.rcfg == nil { configErrs <- nil break } - configErrs <- b.updateTUN(service, cfg.rcfg, cfg.dcfg) + configErrs <- b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg) case s := <-onVPNRequested: - if service != nil && service.ID() == s.ID() { + if vpnService.service != nil && vpnService.service.ID() == s.ID() { // Still the same VPN instance, do nothing break } @@ -228,24 +227,24 @@ func (a *App) runBackend(ctx context.Context) error { // See https://github.com/tailscale/corp/issues/13814 b.backend.DebugRebind() - service = s + vpnService.service = s if networkMap != nil { // TODO } if cfg.rcfg != nil && state >= ipn.Starting { - if err := b.updateTUN(service, cfg.rcfg, cfg.dcfg); err != nil { + if err := b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg); err != nil { log.Printf("VPN update failed: %v", err) - service.Close() + vpnService.service.Close() b.lastCfg = nil b.CloseTUNs() } } case s := <-onDisconnect: b.CloseTUNs() - if service != nil && service.ID() == s.ID() { + if vpnService.service != nil && vpnService.service.ID() == s.ID() { netns.SetAndroidProtectFunc(nil) - service = nil + vpnService.service = nil } case i := <-onDNSConfigChanged: if b != nil { diff --git a/libtailscale/interfaces.go b/libtailscale/interfaces.go index c6ebf51..f344baa 100644 --- a/libtailscale/interfaces.go +++ b/libtailscale/interfaces.go @@ -3,7 +3,9 @@ package libtailscale -import _ "golang.org/x/mobile/bind" +import ( + _ "golang.org/x/mobile/bind" +) // Start starts the application, storing state in the given dataDir and using // the given appCtx. @@ -73,6 +75,8 @@ type IPNService interface { NewBuilder() VPNServiceBuilder Close() + + UpdateVpnStatus(bool) } // VPNServiceBuilder corresponds to Android's VpnService.Builder. diff --git a/libtailscale/net.go b/libtailscale/net.go index ec3f60f..efa7e8f 100644 --- a/libtailscale/net.go +++ b/libtailscale/net.go @@ -12,9 +12,9 @@ import ( "reflect" "runtime/debug" "strings" + "syscall" "github.com/tailscale/wireguard-go/tun" - "golang.org/x/sys/unix" "inet.af/netaddr" "tailscale.com/net/dns" "tailscale.com/net/netmon" @@ -33,6 +33,15 @@ var errVPNNotPrepared = errors.New("VPN service not prepared or was revoked") // https://github.com/tailscale/tailscale/issues/2180 var errMultipleUsers = errors.New("VPN cannot be created on this device due to an Android bug with multiple users") +// VpnService contains the IPNService class from Android, the file descriptor, and whether the descriptor has been detached. +type VpnService struct { + service IPNService + fd int32 + fdDetached bool +} + +var vpnService = &VpnService{} + // Report interfaces in the device in net.Interface format. func (a *App) getInterfaces() ([]netmon.Interface, error) { var ifaces []netmon.Interface @@ -138,7 +147,8 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O if len(rcfg.LocalAddrs) == 0 { return nil } - builder := service.NewBuilder() + vpnService.service = service + builder := vpnService.service.NewBuilder() b.logger.Logf("updateTUN: got new builder") if err := builder.SetMTU(defaultMTU); err != nil { @@ -193,10 +203,15 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O parcelFD, err := builder.Establish() if err != nil { if strings.Contains(err.Error(), "INTERACT_ACROSS_USERS") { + // Update VPN status if VPN interface cannot be created + b.logger.Logf("updateTUN: could not establish VPN because %v", err) + vpnService.service.UpdateVpnStatus(false) return errMultipleUsers } return fmt.Errorf("VpnService.Builder.establish: %v", err) } + log.Printf("Setting vpn activity status to true") + vpnService.service.UpdateVpnStatus(true) b.logger.Logf("updateTUN: established VPN") if parcelFD == nil { @@ -205,6 +220,9 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O // detachFd. tunFD, err := parcelFD.Detach() + vpnService.fdDetached = true + vpnService.fd = tunFD + if err != nil { return fmt.Errorf("detachFd: %v", err) } @@ -213,7 +231,7 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O // Create TUN device. tunDev, _, err := tun.CreateUnmonitoredTUNFromFD(int(tunFD)) if err != nil { - unix.Close(int(tunFD)) + closeFileDescriptor() return err } b.logger.Logf("updateTUN: created TUN device") @@ -226,10 +244,21 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O return nil } +func closeFileDescriptor() error { + if vpnService.fd != -1 && vpnService.fdDetached { + err := syscall.Close(int(vpnService.fd)) + vpnService.fd = -1 + vpnService.fdDetached = false + return fmt.Errorf("error closing file descriptor: %w", err) + } + return nil +} + // CloseVPN closes any active TUN devices. func (b *backend) CloseTUNs() { b.lastCfg = nil b.devices.Shutdown() + vpnService.service = nil } // ifname is the interface name retrieved from LinkProperties on network change. If a network is lost, an empty string is passed in.