From db62e7bacc638339d2bdc0a90602421414301e3d Mon Sep 17 00:00:00 2001 From: Gabi Date: Wed, 20 Mar 2024 23:14:01 -0300 Subject: [PATCH] feat(android): detect network and dns changes and send them to connlib (#4163) This completely removes the `get_system_default_resolvers` for android --- .../android/app/src/main/AndroidManifest.xml | 1 + .../firezone/android/tunnel/ConnlibSession.kt | 7 + .../firezone/android/tunnel/NetworkMonitor.kt | 17 ++ .../firezone/android/tunnel/TunnelService.kt | 58 +++-- .../tunnel/callback/ConnlibCallback.kt | 2 - .../android/tunnel/util/DnsServersDetector.kt | 203 ------------------ rust/connlib/clients/android/src/lib.rs | 79 ++++--- 7 files changed, 100 insertions(+), 267 deletions(-) create mode 100644 kotlin/android/app/src/main/java/dev/firezone/android/tunnel/NetworkMonitor.kt delete mode 100644 kotlin/android/app/src/main/java/dev/firezone/android/tunnel/util/DnsServersDetector.kt diff --git a/kotlin/android/app/src/main/AndroidManifest.xml b/kotlin/android/app/src/main/AndroidManifest.xml index 1c3f82cff..48f9b2652 100644 --- a/kotlin/android/app/src/main/AndroidManifest.xml +++ b/kotlin/android/app/src/main/AndroidManifest.xml @@ -3,6 +3,7 @@ + diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/ConnlibSession.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/ConnlibSession.kt index eaac4ac0f..0a63ab1a8 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/ConnlibSession.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/ConnlibSession.kt @@ -14,4 +14,11 @@ object ConnlibSession { ): Long external fun disconnect(connlibSession: Long): Boolean + + external fun setDns( + connlibSession: Long, + dnsList: String, + ): Boolean + + external fun reconnect(connlibSession: Long): Boolean } diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/NetworkMonitor.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/NetworkMonitor.kt new file mode 100644 index 000000000..f6154b608 --- /dev/null +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/NetworkMonitor.kt @@ -0,0 +1,17 @@ +/* Licensed under Apache 2.0 (C) 2024 Firezone, Inc. */ +import android.net.ConnectivityManager +import android.net.LinkProperties +import android.net.Network +import com.google.gson.Gson +import dev.firezone.android.tunnel.ConnlibSession + +class NetworkMonitor(private val connlibSessionPtr: Long) : ConnectivityManager.NetworkCallback() { + override fun onLinkPropertiesChanged( + network: Network, + linkProperties: LinkProperties, + ) { + ConnlibSession.setDns(connlibSessionPtr, Gson().toJson(linkProperties.dnsServers)) + ConnlibSession.reconnect(connlibSessionPtr) + super.onLinkPropertiesChanged(network, linkProperties) + } +} diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt index f4f553ee1..b4bf9662f 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt @@ -1,6 +1,7 @@ /* Licensed under Apache 2.0 (C) 2024 Firezone, Inc. */ package dev.firezone.android.tunnel +import NetworkMonitor import android.app.ActivityManager import android.app.Notification import android.app.NotificationChannel @@ -8,6 +9,9 @@ import android.app.NotificationManager import android.app.PendingIntent import android.content.Context import android.content.Intent +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import android.net.NetworkRequest import android.net.VpnService import android.os.Binder import android.os.Build @@ -27,7 +31,6 @@ import dev.firezone.android.core.presentation.MainActivity import dev.firezone.android.tunnel.callback.ConnlibCallback import dev.firezone.android.tunnel.model.Cidr import dev.firezone.android.tunnel.model.Resource -import dev.firezone.android.tunnel.util.DnsServersDetector import java.nio.file.Files import java.nio.file.Paths import java.util.UUID @@ -52,6 +55,7 @@ class TunnelService : VpnService() { private var connlibSessionPtr: Long? = null private var _tunnelResources: List = emptyList() private var _tunnelState: State = State.DOWN + private var networkCallback: NetworkMonitor? = null var startedByUser: Boolean = false @@ -136,16 +140,6 @@ class TunnelService : VpnService() { return buildVpnService() } - override fun getSystemDefaultResolvers(): Array { - val found = DnsServersDetector(this@TunnelService).servers - Log.d(TAG, "getSystemDefaultResolvers: $found") - Firebase.crashlytics.log("getSystemDefaultResolvers: $found") - - return found.map { - it.address - }.toTypedArray() - } - // Unexpected disconnect, most likely a 401. Clear the token and initiate a stop of the // service. override fun onDisconnect(error: String): Boolean { @@ -188,7 +182,7 @@ class TunnelService : VpnService() { Log.d(TAG, "disconnect") // Connlib should call onDisconnect() when it's done, with no error. - connlibSessionPtr!!.let { + connlibSessionPtr?.let { ConnlibSession.disconnect(it) } @@ -198,6 +192,8 @@ class TunnelService : VpnService() { private fun shutdown() { Log.d(TAG, "shutdown") + stopNetworkMonitoring() + connlibSessionPtr = null stopSelf() tunnelState = State.DOWN @@ -223,6 +219,29 @@ class TunnelService : VpnService() { logFilter = config.logFilter, callback = callback, ) + + startNetworkMonitoring() + } + } + + private fun startNetworkMonitoring() { + networkCallback = NetworkMonitor(connlibSessionPtr!!) + + val networkRequest = + NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) + .build() + val connectivityManager = + getSystemService(ConnectivityManager::class.java) as ConnectivityManager + connectivityManager.requestNetwork(networkRequest, networkCallback!!) + } + + private fun stopNetworkMonitoring() { + networkCallback?.let { + val connectivityManager = + getSystemService(ConnectivityManager::class.java) as ConnectivityManager + connectivityManager.unregisterNetworkCallback(it) + + networkCallback = null } } @@ -312,7 +331,10 @@ class TunnelService : VpnService() { addAddress(tunnelIpv6Address!!, 128) updateAllowedDisallowedApplications("allowedApplications", ::addAllowedApplication) - updateAllowedDisallowedApplications("disallowedApplications", ::addDisallowedApplication) + updateAllowedDisallowedApplications( + "disallowedApplications", + ::addDisallowedApplication, + ) setSession(SESSION_NAME) setMtu(MTU) @@ -354,14 +376,10 @@ class TunnelService : VpnService() { val notificationBuilder = NotificationCompat.Builder(this, NOTIFICATION_CHANNEL_ID) val notification = - notificationBuilder.setOngoing(true) - .setSmallIcon(R.drawable.ic_firezone_logo) - .setContentTitle(NOTIFICATION_TITLE) - .setContentText(message) + notificationBuilder.setOngoing(true).setSmallIcon(R.drawable.ic_firezone_logo) + .setContentTitle(NOTIFICATION_TITLE).setContentText(message) .setPriority(NotificationManager.IMPORTANCE_MIN) - .setCategory(Notification.CATEGORY_SERVICE) - .setContentIntent(configIntent()) - .build() + .setCategory(Notification.CATEGORY_SERVICE).setContentIntent(configIntent()).build() startForeground(STATUS_NOTIFICATION_ID, notification) } diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt index 4398131fb..cbcd37ddf 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt @@ -20,7 +20,5 @@ interface ConnlibCallback { // The JNI doesn't support nullable types, so we need two method signatures fun onDisconnect(error: String): Boolean - fun getSystemDefaultResolvers(): Array - fun protectFileDescriptor(fileDescriptor: Int) } diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/util/DnsServersDetector.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/util/DnsServersDetector.kt deleted file mode 100644 index dfd9c65eb..000000000 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/util/DnsServersDetector.kt +++ /dev/null @@ -1,203 +0,0 @@ -/* Licensed under Apache 2.0 (C) 2024 Firezone, Inc. */ -package dev.firezone.android.tunnel.util - -import android.content.Context -import android.net.ConnectivityManager -import android.net.LinkProperties -import android.os.Build -import android.util.Log -import java.io.BufferedReader -import java.io.InputStreamReader -import java.io.LineNumberReader -import java.net.InetAddress - -/** - * DNS servers detector - * - * IMPORTANT: don't cache the result. - * - * Or if you want to cache the result make sure you invalidate the cache - * on any network change. - * - * It is always better to use a new instance of the detector when you need - * current DNS servers otherwise you may get into trouble because of invalid/changed - * DNS servers. - * - * This class combines various methods and solutions from: - * Dnsjava http://www.xbill.org/dnsjava/ - * Minidns https://github.com/MiniDNS/minidns - * https://stackoverflow.com/a/48973823/1275497 - * - * Unfortunately both libraries are not aware of Oreo changes so new method was added to fix this. - * - * Created by Madalin Grigore-Enescu on 2/24/18. - */ -class DnsServersDetector( - /** - * Holds context this was created under - */ - private val context: Context, -) { - //region - public ////////////////////////////////////////////////////////////////////////////// - // ////////////////////////////////////////////////////////////////////////////////////////////// - val servers: Set - /** - * Returns android DNS servers used for current connected network - * @return Dns servers array - */ - get() { - return serversMethodConnectivityManager - ?.takeIf { it.isNotEmpty() } - ?: serversMethodExec - ?.takeIf { it.isNotEmpty() } - ?: FACTORY_DNS_SERVERS - } - - //endregion - //region - private ///////////////////////////////////////////////////////////////////////////// - // ////////////////////////////////////////////////////////////////////////////////////////////// - private val serversMethodConnectivityManager: Set? - /** - * Detect android DNS servers by using connectivity manager - * - * This method is working in android LOLLIPOP or later - * - * @return Dns servers array - */ - get() { - // This code only works on LOLLIPOP and higher - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { - try { - val priorityServers: MutableSet = HashSet(10) - val servers: MutableSet = HashSet(10) - val connectivityManager = - context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? - if (connectivityManager != null) { - - // Iterate all networks - // Notice that android LOLLIPOP or higher allow iterating multiple connected networks of SAME type - for (network in connectivityManager.allNetworks) { - val networkInfo = connectivityManager.getNetworkInfo(network) - if (networkInfo!!.isConnected) { - val linkProperties = connectivityManager.getLinkProperties(network) - val dnsServersList = linkProperties!!.dnsServers.toSet() - - // Prioritize the DNS servers for link which have a default route - if (linkPropertiesHasDefaultRoute(linkProperties)) { - priorityServers += dnsServersList - } else { - servers += dnsServersList - } - } - } - } - - // Append secondary arrays only if priority is empty - return priorityServers.takeIf { it.isNotEmpty() } ?: servers - } catch (ex: Exception) { - Log.d( - TAG, - "Exception detecting DNS servers using ConnectivityManager method", - ex, - ) - } - } - - return null - } - private val serversMethodExec: Set? - /** - * Detect android DNS servers by executing getprop string command in a separate process - * - * Notice there is an android bug when Runtime.exec() hangs without providing a Process object. - * This problem is fixed in Jelly Bean (Android 4.1) but not in ICS (4.0.4) and probably it will never be fixed in ICS. - * https://stackoverflow.com/questions/8688382/runtime-exec-bug-hangs-without-providing-a-process-object/11362081 - * - * @return Dns servers array - */ - get() { - // We are on the safe side and avoid any bug - try { - val process = Runtime.getRuntime().exec("getprop") - val inputStream = process.inputStream - val lineNumberReader = LineNumberReader(InputStreamReader(inputStream)) - return methodExecParseProps(lineNumberReader) - } catch (ex: Exception) { - Log.d(TAG, "Exception in getServersMethodExec", ex) - } - - return null - } - - /** - * Parse properties produced by executing getprop command - * @param lineNumberReader - * @return Set of parsed properties - * @throws Exception - */ - @Throws(Exception::class) - private fun methodExecParseProps(lineNumberReader: BufferedReader): Set { - var line: String - val serversSet: MutableSet = HashSet(10) - while (lineNumberReader.readLine().also { line = it } != null) { - val split = line.indexOf(METHOD_EXEC_PROP_DELIM) - if (split == -1) { - continue - } - val property = line.substring(1, split) - val valueStart = split + METHOD_EXEC_PROP_DELIM.length - val valueEnd = line.length - 1 - if (valueEnd < valueStart) { - // This can happen if a newline sneaks in as the first character of the property value. For example - // "[propName]: [\n…]". - Log.d(TAG, "Malformed property detected: \"$line\"") - continue - } - val value = line.substring(valueStart, valueEnd) - if (value.isEmpty()) { - continue - } - if (property.endsWith(".dns") || property.endsWith(".dns1") || - property.endsWith(".dns2") || property.endsWith(".dns3") || - property.endsWith(".dns4") - ) { - serversSet.add(InetAddress.getByName(value)) - } - } - - return serversSet - } - - /** - * Returns true if the specified link properties have any default route - * @param linkProperties - * @return true if the specified link properties have default route or false otherwise - */ - private fun linkPropertiesHasDefaultRoute(linkProperties: LinkProperties?): Boolean { - for (route in linkProperties!!.routes) { - if (route.isDefaultRoute) { - return true - } - } - return false - } //endregion - - companion object { - private const val TAG = "DnsServersDetector" - - /** - * Holds some default DNS servers used in case all DNS servers detection methods fail. - * Can be set to null if you want caller to fail in this situation. - */ - private val FACTORY_DNS_SERVERS = - setOf( - InetAddress.getByName("8.8.8.8"), - InetAddress.getByName("8.8.4.4"), - ) - - /** - * Properties delimiter used in exec method of DNS servers detection - */ - private const val METHOD_EXEC_PROP_DELIM = "]: [" - } -} diff --git a/rust/connlib/clients/android/src/lib.rs b/rust/connlib/clients/android/src/lib.rs index 7a5910cac..d0dd3ffb0 100644 --- a/rust/connlib/clients/android/src/lib.rs +++ b/rust/connlib/clients/android/src/lib.rs @@ -8,7 +8,7 @@ use connlib_client_shared::{ ResourceDescription, Session, }; use jni::{ - objects::{GlobalRef, JByteArray, JClass, JObject, JObjectArray, JString, JValue, JValueGen}, + objects::{GlobalRef, JClass, JObject, JString, JValue}, strings::JNIString, JNIEnv, JavaVM, }; @@ -79,21 +79,6 @@ impl CallbackHandler { .map_err(CallbackError::AttachCurrentThreadFailed) .and_then(f) } - - fn get_system_default_resolvers(&self) -> Vec { - self.env(|mut env| { - let name = "getSystemDefaultResolvers"; - let addrs = env - .call_method(&self.callback_handler, name, "()[[B", &[]) - .and_then(JValueGen::l) - .and_then(|arr| convert_byte_array_array(&mut env, arr.into())) - .map_err(|source| CallbackError::CallMethodFailed { name, source })?; - - Ok(Some(addrs.iter().filter_map(|v| to_ip(v)).collect())) - }) - .expect("getSystemDefaultResolvers callback failed") - .unwrap_or_default() - } } fn call_method( @@ -303,29 +288,6 @@ impl Callbacks for CallbackHandler { } } -fn to_ip(val: &[u8]) -> Option { - let addr: Option<[u8; 4]> = val.try_into().ok(); - if let Some(addr) = addr { - return Some(addr.into()); - } - - let addr: [u8; 16] = val.try_into().ok()?; - Some(addr.into()) -} - -fn convert_byte_array_array( - env: &mut JNIEnv, - array: JObjectArray, -) -> jni::errors::Result>> { - let len = env.get_array_length(&array)?; - let mut result = Vec::with_capacity(len as usize); - for i in 0..len { - let arr: JByteArray<'_> = env.get_object_array_element(&array, i)?.into(); - result.push(env.convert_byte_array(arr)?); - } - Ok(result) -} - fn throw(env: &mut JNIEnv, class: &str, msg: impl Into) { if let Err(err) = env.throw_new(class, msg) { // We can't panic, since unwinding across the FFI boundary is UB... @@ -428,13 +390,11 @@ fn connect( login, private_key, Some(os_version), - callback_handler.clone(), + callback_handler, Some(MAX_PARTITION_TIME), runtime.handle().clone(), )?; - session.set_dns(callback_handler.get_system_default_resolvers()); - Ok(SessionWrapper { inner: session, runtime, @@ -508,3 +468,38 @@ pub unsafe extern "system" fn Java_dev_firezone_android_tunnel_ConnlibSession_di Box::from_raw(session).inner.disconnect(); }); } + +/// # Safety +/// Pointers must be valid +#[allow(non_snake_case)] +#[no_mangle] +pub unsafe extern "system" fn Java_dev_firezone_android_tunnel_ConnlibSession_setDns( + mut env: JNIEnv, + _: JClass, + session: *const SessionWrapper, + dns_list: JString, +) { + let dns = String::from( + env.get_string(&dns_list) + .map_err(|source| ConnectError::StringInvalid { + name: "dns_list", + source, + }) + .expect("Invalid string returned from android client"), + ); + let dns: Vec = serde_json::from_str(&dns).unwrap(); + let session = &*session; + session.inner.set_dns(dns); +} + +/// # Safety +/// Pointers must be valid +#[allow(non_snake_case)] +#[no_mangle] +pub unsafe extern "system" fn Java_dev_firezone_android_tunnel_ConnlibSession_reconnect( + _: JNIEnv, + _: JClass, + session: *const SessionWrapper, +) { + (*session).inner.reconnect(); +}