Skip to content

Commit 24e467c

Browse files
committed
tun: add diags mgr for network stall detection
1 parent cc0d189 commit 24e467c

File tree

3 files changed

+247
-24
lines changed

3 files changed

+247
-24
lines changed

app/src/main/java/com/celzero/bravedns/service/BraveVPNService.kt

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,9 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
194194
@Volatile
195195
private var tunUnderlyingNetworks: String? = null
196196
@Volatile
197-
private var prevDns: MutableSet<InetAddress> = mutableSetOf()
197+
private var prevDns: MutableSet<InetAddress> = mutableSetOf()
198+
@Volatile
199+
private var lastFlowRealtime: Long = elapsedRealtime() // tracks last flow()
198200
private var testFd: AtomicInteger = AtomicInteger(-1)
199201

200202
companion object {
@@ -240,6 +242,8 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
240242

241243
// win last connected threshold in milliseconds
242244
private const val WIN_LAST_CONNECTED_THRESHOLD_MS = 60 * 60 * 1000L // 60 minutes
245+
246+
private const val FLOW_STALL_THRESHOLD_MS = 30 * 1000L // 30 seconds
243247
}
244248

245249
private var lastSubscriptionCheckTime: Long = 0
@@ -1233,6 +1237,9 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
12331237
// family must be either AF_INET (for IPv4) or AF_INET6 (for IPv6)
12341238
}
12351239

1240+
// fixme: remove this when the issue is fixed
1241+
builder.allowBypass()
1242+
12361243
val underlyingNws = getUnderlays()
12371244
builder.setUnderlyingNetworks(underlyingNws)
12381245
tunUnderlyingNetworks = underlyingNws?.joinToString()
@@ -1665,7 +1672,7 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
16651672
vpnScope.launch {
16661673
Logger.i(LOG_TAG_VPN, "start restart manager flow with debounce")
16671674
// create a state flow with debounce to avoid multiple calls in quick succession
1668-
// this should wait for 3 seconds before starting the restart manager flow
1675+
// this should wait for 1.5 seconds before starting the restart manager flow
16691676
vpnRestartTrigger
16701677
.debounce(TimeUnit.MILLISECONDS.toMillis(1500))
16711678
.collect { reason ->
@@ -2690,6 +2697,19 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
26902697
signalStopService("nwRegFail", userInitiated = false)
26912698
}
26922699

2700+
override suspend fun maybeNetworkStall() {
2701+
// these calls are not fool proof, just a mitigation mechanism
2702+
// see if there is no flow call for 30 seconds and this is called, then restart the vpn
2703+
val elapsed = elapsedRealtime()
2704+
if (elapsed >= lastFlowRealtime + FLOW_STALL_THRESHOLD_MS) {
2705+
Logger.w(LOG_TAG_VPN, "nwStall; no flow call for 30 seconds, restarting vpn")
2706+
val reason = "nwStall, time: $elapsed"
2707+
vpnRestartTrigger.value = reason
2708+
} else {
2709+
Logger.d(LOG_TAG_VPN, "nwStall; flow call recd, no restart needed")
2710+
}
2711+
}
2712+
26932713
private fun getUnderlays(): Array<Network>? {
26942714
val networks = underlyingNetworks
26952715
val failOpenOnNoNetwork = persistentState.failOpenOnNoNetwork
@@ -3018,15 +3038,6 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
30183038
}
30193039
}
30203040

3021-
private fun areDnsEqual(prevDns: Set<InetAddress>?, dnsServers: Set<InetAddress>): Boolean {
3022-
if (prevDns == null) {
3023-
return false
3024-
}
3025-
3026-
// ref: kotlinlang.org/docs/equality.html#structural-equality
3027-
return dnsServers == prevDns
3028-
}
3029-
30303041
private fun isDefaultDnsNone(): Boolean {
30313042
// if none is set then the url will either be empty or will not be one of the default dns
30323043
return persistentState.defaultDnsUrl.isEmpty() ||
@@ -3038,7 +3049,7 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
30383049

30393050
Logger.i(LOG_TAG_VPN, "vpn lockdown mode change, restarting")
30403051
io("lockdownSync") {
3041-
val reason = "lockdown: ${isLockdownEnabled}"
3052+
val reason = "lockdown: ${VpnController.isVpnLockdown()}"
30423053
vpnRestartTrigger.value = reason
30433054
vpnAdapter?.notifyLoopback()
30443055
}
@@ -4356,6 +4367,8 @@ class BraveVPNService : VpnService(), ConnectionMonitor.NetworkListener, Bridge,
43564367
logd("flow: $_uid, $src, $dest, $realIps, $d, $blocklists")
43574368
handleVpnLockdownStateAsync()
43584369

4370+
lastFlowRealtime = elapsedRealtime()
4371+
43594372
// in case of double loopback, all traffic will be part of rinr instead of just rethink's
43604373
// own traffic. flip the doubleLoopback flag to true if we need that behavior
43614374
val doubleLoopback = false

app/src/main/java/com/celzero/bravedns/service/ConnectionMonitor.kt

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ import android.net.NetworkRequest
2626
import android.os.Handler
2727
import android.os.Looper
2828
import android.os.SystemClock
29+
import androidx.annotation.RequiresApi
2930
import com.celzero.bravedns.util.ConnectivityCheckHelper
30-
import com.celzero.bravedns.util.Daemons
31-
import com.celzero.bravedns.util.Factory
3231
import com.celzero.bravedns.util.InternetProtocol
3332
import com.celzero.bravedns.util.Utilities.isAtleastQ
33+
import com.celzero.bravedns.util.Utilities.isAtleastR
3434
import com.celzero.bravedns.util.Utilities.isAtleastS
3535
import com.celzero.bravedns.util.Utilities.isNetworkSame
3636
import com.google.common.collect.Sets
@@ -41,7 +41,6 @@ import kotlinx.coroutines.async
4141
import kotlinx.coroutines.channels.Channel
4242
import kotlinx.coroutines.delay
4343
import kotlinx.coroutines.launch
44-
import kotlinx.coroutines.withContext
4544
import org.koin.core.component.KoinComponent
4645
import org.koin.core.component.inject
4746
import java.net.Inet4Address
@@ -54,7 +53,7 @@ import kotlin.math.max
5453
import kotlin.math.min
5554

5655
class ConnectionMonitor(private val networkListener: NetworkListener, private val serializer: CoroutineDispatcher, private val scope: CoroutineScope) :
57-
ConnectivityManager.NetworkCallback(), KoinComponent {
56+
ConnectivityManager.NetworkCallback(), KoinComponent, DiagnosticsManager.DiagnosticsListener {
5857

5958
private val networkSet: MutableSet<Network> = ConcurrentHashMap.newKeySet()
6059

@@ -64,17 +63,22 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
6463
// add cellular, wifi, bluetooth, ethernet, vpn, wifi aware, low pan
6564
private val networkRequest: NetworkRequest =
6665
NetworkRequest.Builder()
66+
// ref: github.com/celzero/rethink-app/issues/347
67+
.apply { if (isAtleastR()) clearCapabilities() else removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) }
6768
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
6869
.apply { if (isAtleastS()) setIncludeOtherUidNetworks(true) }
6970
// api27: .addTransportType(NetworkCapabilities.TRANSPORT_WIFI_AWARE)
7071
// api26: .addTransportType(NetworkCapabilities.TRANSPORT_LOWPAN)
7172
.build()
7273

74+
7375
//private var serviceHandler: NetworkRequestHandler? = null
7476
private val persistentState by inject<PersistentState>()
7577

7678
private lateinit var cm: ConnectivityManager
7779

80+
private var diagsMgr: DiagnosticsManager? = null
81+
7882
companion object {
7983
// add active network as underlying vpn network
8084
const val MSG_ADD_ACTIVE_NETWORK = 1
@@ -145,6 +149,8 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
145149
suspend fun onNetworkConnected(networks: UnderlyingNetworks)
146150

147151
suspend fun onNetworkRegistrationFailed()
152+
153+
suspend fun maybeNetworkStall()
148154
}
149155

150156
override fun onAvailable(network: Network) {
@@ -238,8 +244,7 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
238244
}
239245

240246
Logger.i(LOG_TAG_CONNECTION, "new vpn is created force update the network")
241-
cm =
242-
context.applicationContext.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
247+
cm = context.applicationContext.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
243248

244249
try {
245250
// TODO: use a custom Looper(HandlerThread) to avoid blocking the main thread
@@ -249,6 +254,12 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
249254
networkListener.onNetworkRegistrationFailed()
250255
return@async isNewVpn
251256
}
257+
258+
// register for diagnostics manager if the android version is R or above
259+
if (isAtleastR()) {
260+
registerDiags(context)
261+
}
262+
252263
channel = Channel(Channel.CONFLATED)
253264

254265
scope.launch(CoroutineName("nwHdl") + serializer) {
@@ -282,6 +293,31 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
282293
return deferred.await()
283294
}
284295

296+
@RequiresApi(30)
297+
fun registerDiags(context: Context) {
298+
if (isAtleastR()) {
299+
try {
300+
val diagnosticMgr = DiagnosticsManager(context, scope, this)
301+
diagnosticMgr.register()
302+
} catch (e: Exception) {
303+
Logger.w(LOG_TAG_CONNECTION, "DiagnosticsManager; err while getting connectivity diagnostics manager")
304+
}
305+
}
306+
}
307+
308+
@RequiresApi(30)
309+
fun unregisterDiags() {
310+
if (isAtleastR()) {
311+
// unregister the diag network callback
312+
try {
313+
diagsMgr?.unregister()
314+
diagsMgr = null
315+
} catch (e: Exception) {
316+
Logger.w(LOG_TAG_CONNECTION, "DiagnosticsManager; err while unregistering diag network callback", e)
317+
}
318+
}
319+
}
320+
285321

286322
// Always called from the main thread
287323
suspend fun onVpnStop() {
@@ -292,13 +328,18 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
292328
if (::cm.isInitialized) {
293329
cm.unregisterNetworkCallback(nwCallback)
294330
}
331+
if (isAtleastR()) {
332+
unregisterDiags()
333+
}
295334
networkSet.clear()
296335
if (::channel.isInitialized) {
297336
channel.close()
298337
}
338+
299339
} catch (e: Exception) {
300340
Logger.w(LOG_TAG_CONNECTION, "err while unregistering", e)
301341
}
342+
302343
}
303344
}
304345

@@ -335,6 +376,11 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
335376
return OpPrefs(what, networkSet.toSet(), isForceUpdate, testReachability, failOpenOnNoNetwork, useAutoConnectivityChecks)
336377
}
337378

379+
override suspend fun maybeNetworkStall() {
380+
Logger.i(LOG_TAG_CONNECTION, "onNetworkStallDetected")
381+
networkListener.maybeNetworkStall()
382+
}
383+
338384
data class NetworkProperties(
339385
val network: Network,
340386
val capabilities: NetworkCapabilities,
@@ -448,6 +494,7 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
448494
val isNewNetwork = hasDifference(currentNetworks, newNetworks)
449495
val vpnRoutes = determineVpnProtos(opPrefs.networkSet)
450496
val isDnsChanged = hasNwDnsChanged(currentNetworks, newNetworks)
497+
val isLinkAddressChanged = hasLinkAddrChanged(currentNetworks, newNetworks)
451498

452499
Logger.i(LOG_TAG_CONNECTION, "process message MESSAGE_AVAILABLE_NETWORK, currNws: $currentNetworks ; new? $isNewNetwork, force? ${opPrefs.isForceUpdate}, test? ${opPrefs.testReachability}, cellular? $isActiveNetworkCellular, metered? $isActiveNetworkMetered")
453500
Logger.i(
@@ -458,19 +505,26 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
458505
"cellular? $isActiveNetworkCellular, metered? $isActiveNetworkMetered, dns-changed? $isDnsChanged"
459506
)
460507

461-
if (isNewNetwork || opPrefs.isForceUpdate || isDnsChanged) {
508+
if (isNewNetwork || opPrefs.isForceUpdate || isDnsChanged || isLinkAddressChanged) {
462509
currentNetworks = newNetworks
463510
repopulateTrackedNetworks(opPrefs, currentNetworks)
464511
informListener(true, isActiveNetworkMetered, isActiveNetworkCellular, vpnRoutes)
465512
}
466513
}
467514

468515
private suspend fun hasNwDnsChanged(currNws: Set<NetworkProperties>, newNws: Set<NetworkProperties>): Boolean {
469-
val currDnsServers = currNws.map { it.linkProperties }.mapNotNull { it?.dnsServers }.flatMap { it }.map { it.hostAddress }.toSet()
470-
val newDnsServers = newNws.map { it.linkProperties }.mapNotNull { it?.dnsServers }.flatMap { it }.map { it.hostAddress }.toSet()
516+
// check equality on addr bytes and not on string representation to avoid issues with IPv4-mapped IPv6 addresses
517+
val currDnsServers = currNws.map { it.linkProperties }.mapNotNull { it?.dnsServers }.flatMap { it }.map { it.address }.toSet()
518+
val newDnsServers = newNws.map { it.linkProperties }.mapNotNull { it?.dnsServers }.flatMap { it }.map { it.address }.toSet()
471519
return newDnsServers == currDnsServers
472520
}
473521

522+
private suspend fun hasLinkAddrChanged(currNws: Set<NetworkProperties>, newNws: Set<NetworkProperties>): Boolean {
523+
val currLinkAddresses = currNws.map { it.linkProperties }.mapNotNull { it?.linkAddresses }.flatMap { it }.map { it.address.address }.toSet()
524+
val newLinkAddresses = newNws.map { it.linkProperties }.mapNotNull { it?.linkAddresses }.flatMap { it }.map { it.address.address }.toSet()
525+
return newLinkAddresses == currLinkAddresses
526+
}
527+
474528
/** Adds all the available network to the underlying network. */
475529
private suspend fun processAllNetworks(opPrefs: OpPrefs) {
476530
val newActiveNetwork = cm.activeNetwork
@@ -506,12 +560,12 @@ class ConnectionMonitor(private val networkListener: NetworkListener, private va
506560
* Returns null if no VPN network is found in the provided set.
507561
*/
508562
private suspend fun determineVpnProtos(nws: Set<Network?>): Pair<Boolean, Boolean>? {
509-
var vpnNw = nws.firstOrNull { isVPN(it) == true }
510-
if (vpnNw == null) {
563+
val vpnNw = nws.firstOrNull { isVPN(it) == true }
564+
/*if (vpnNw == null) {
511565
// fallback to the active network if the vpn network is not found
512566
val allNws = cm.allNetworks
513567
vpnNw = allNws.firstOrNull { isVPN(it) == true }
514-
}
568+
}*/
515569
if (vpnNw == null) {
516570
// vpn routes is just the suggestion to mitigate the discrepancy between
517571
// actual vpn routes and the ones handled by BraveVpnService, in that case

0 commit comments

Comments
 (0)