refactor(android): Remove excessive debug logging from kotlin codebase (#5748)

- Why: This prevents logging potentially sensitive data to Logcat or
Firebase. For critical codepaths we rely on a non-null `!!` check
anyhow, which will be reported with a crash to Firebase. Now that we
have some confidence the app is reliable, I think we can reflect that
confidence in our code.
- This moves the `loadLibrary` call to the app start, which will surface
issues immediately when launching the app and not when trying to
connect. This also makes connect very slightly faster.
- Finally, `BootShutdownReceiver` is removed since it was essentially a
no-op. There are a few ways we can connect on boot, but this isn't a
good approach since it would ignore Android's Always-on VPN setting.
This commit is contained in:
Jamil
2024-07-04 22:06:02 -07:00
committed by GitHub
parent 3d65be0bb9
commit dd2de0440e
9 changed files with 5 additions and 91 deletions

View File

@@ -79,15 +79,6 @@
</intent-filter>
</service>
<receiver
android:name="dev.firezone.android.features.session.backend.BootShutdownReceiver"
android:exported="true">
<intent-filter>
<action android:name="android.intent.action.ACTION_SHUTDOWN" />
<action android:name="android.intent.action.BOOT_COMPLETED" />
</intent-filter>
</receiver>
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="${applicationId}.provider"

View File

@@ -13,5 +13,9 @@ class FirezoneApp : Application() {
// Disable Crashlytics for debug builds
FirebaseCrashlytics.getInstance().setCrashlyticsCollectionEnabled(!BuildConfig.DEBUG)
// Load the native library immediately after FirebaseCrashlytics
// so we catch any issues with the native library early on.
System.loadLibrary("connlib")
}
}

View File

@@ -7,7 +7,6 @@ import android.app.PendingIntent
import android.content.Context
import android.content.Context.NOTIFICATION_SERVICE
import android.content.Intent
import android.util.Log
import androidx.core.app.NotificationCompat
import dev.firezone.android.R
import dev.firezone.android.core.presentation.MainActivity
@@ -23,7 +22,6 @@ object CustomUriNotification {
context: Context,
status: StatusType,
): NotificationCompat.Builder {
Log.d(TAG, "update")
val manager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager
val chan =
@@ -42,7 +40,6 @@ object CustomUriNotification {
}
private fun configIntent(context: Context): PendingIntent {
Log.d(TAG, "configIntent")
val intent = Intent(context, MainActivity::class.java)
intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK
return PendingIntent.getActivity(

View File

@@ -37,20 +37,15 @@ internal class CustomUriViewModel
when (intent.data?.host) {
PATH_CALLBACK -> {
intent.data?.getQueryParameter(QUERY_ACTOR_NAME)?.let { actorName ->
Log.d(TAG, "Found actor name: $actorName")
repo.saveActorName(actorName).collect()
}
intent.data?.getQueryParameter(QUERY_CLIENT_STATE)?.let { state ->
if (repo.validateState(state).firstOrNull() == true) {
Log.d(TAG, "Valid state parameter. Continuing to save state...")
} else {
if (repo.validateState(state).firstOrNull() != true) {
error("Invalid state parameter $state")
}
}
intent.data?.getQueryParameter(QUERY_CLIENT_AUTH_FRAGMENT)?.let { fragment ->
if (fragment.isNotBlank()) {
Log.d(TAG, "Found valid auth fragment in response")
// Save token, then clear nonce and state since we don't
// need to keep them around anymore
repo.saveToken(fragment).collect()
@@ -66,7 +61,6 @@ internal class CustomUriViewModel
if (accumulatedErrors.isNotEmpty()) {
actionMutableLiveData.postValue(ViewAction.AuthFlowError(accumulatedErrors))
} else {
Log.d(TAG, "Auth flow complete")
actionMutableLiveData.postValue(ViewAction.AuthFlowComplete)
}
}

View File

@@ -1,24 +0,0 @@
/* Licensed under Apache 2.0 (C) 2024 Firezone, Inc. */
package dev.firezone.android.features.session.backend
import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.util.Log
internal class BootShutdownReceiver : BroadcastReceiver() {
override fun onReceive(
context: Context,
intent: Intent,
) {
if (Intent.ACTION_BOOT_COMPLETED == intent.action) {
Log.d("BootShutdownReceiver", "Boot completed. Attempting to connect.")
// TODO: Retrieve the session manager from the application context.
// sessionManager.connect()
} else if (Intent.ACTION_SHUTDOWN == intent.action) {
Log.d("BootShutdownReceiver", "Shutting down. Attempting to disconnect.")
// TODO: Retrieve the session manager from the application context.
// sessionManager.disconnect()
}
}
}

View File

@@ -7,7 +7,6 @@ import android.content.Intent
import android.content.ServiceConnection
import android.os.Bundle
import android.os.IBinder
import android.util.Log
import androidx.activity.viewModels
import androidx.appcompat.app.AppCompatActivity
import androidx.recyclerview.widget.DividerItemDecoration
@@ -30,7 +29,6 @@ internal class SessionActivity : AppCompatActivity() {
name: ComponentName?,
service: IBinder?,
) {
Log.d(TAG, "onServiceConnected")
val binder = service as TunnelService.LocalBinder
tunnelService = binder.getService()
serviceBound = true
@@ -39,7 +37,6 @@ internal class SessionActivity : AppCompatActivity() {
}
override fun onServiceDisconnected(name: ComponentName?) {
Log.d(TAG, "onServiceDisconnected")
serviceBound = false
}
}
@@ -69,14 +66,12 @@ internal class SessionActivity : AppCompatActivity() {
private fun setupViews() {
binding.btSignOut.setOnClickListener {
Log.d(TAG, "Sign out button clicked")
viewModel.clearToken()
viewModel.clearActorName()
tunnelService?.disconnect()
}
binding.btSettings.setOnClickListener {
Log.d(TAG, "Settings button clicked")
val intent = Intent(this, SettingsActivity::class.java)
intent.putExtra("isUserSignedIn", true)
startActivity(intent)

View File

@@ -2,7 +2,6 @@
import android.net.ConnectivityManager
import android.net.LinkProperties
import android.net.Network
import android.util.Log
import com.google.gson.Gson
import dev.firezone.android.tunnel.ConnlibSession
import dev.firezone.android.tunnel.TunnelService
@@ -17,8 +16,6 @@ class NetworkMonitor(private val tunnelService: TunnelService) : ConnectivityMan
network: Network,
linkProperties: LinkProperties,
) {
Log.d("NetworkMonitor", "OnLinkPropertiesChanged: $network: $linkProperties")
// Acquire mutex lock
if (tunnelService.lock.tryLock()) {
if (tunnelService.tunnelState != TunnelService.Companion.State.UP) {

View File

@@ -15,7 +15,6 @@ import android.os.Binder
import android.os.Build
import android.os.Bundle
import android.os.IBinder
import android.util.Log
import androidx.lifecycle.MutableLiveData
import com.google.firebase.crashlytics.ktx.crashlytics
import com.google.firebase.ktx.Firebase
@@ -90,8 +89,6 @@ class TunnelService : VpnService() {
private val callback: ConnlibCallback =
object : ConnlibCallback {
override fun onUpdateResources(resourceListJSON: String) {
Log.d(TAG, "onUpdateResources: $resourceListJSON")
Firebase.crashlytics.log("onUpdateResources: $resourceListJSON")
moshi.adapter<List<Resource>>().fromJson(resourceListJSON)?.let {
tunnelResources = it
}
@@ -102,9 +99,6 @@ class TunnelService : VpnService() {
addressIPv6: String,
dnsAddresses: String,
): Int {
Log.d(TAG, "onSetInterfaceConfig: $addressIPv4, $addressIPv6, $dnsAddresses")
Firebase.crashlytics.log("onSetInterfaceConfig: $addressIPv4, $addressIPv6, $dnsAddresses")
// init tunnel config
tunnelDnsAddresses = moshi.adapter<MutableList<String>>().fromJson(dnsAddresses)!!
tunnelIpv4Address = addressIPv4
@@ -118,8 +112,6 @@ class TunnelService : VpnService() {
routes4JSON: String,
routes6JSON: String,
): Int {
Log.d(TAG, "onUpdateRoutes: $routes4JSON, $routes6JSON")
Firebase.crashlytics.log("onUpdateRoutes: $routes4JSON, $routes6JSON")
val routes4 = moshi.adapter<MutableList<Cidr>>().fromJson(routes4JSON)!!
val routes6 = moshi.adapter<MutableList<Cidr>>().fromJson(routes6JSON)!!
@@ -133,9 +125,6 @@ class TunnelService : VpnService() {
// Unexpected disconnect, most likely a 401. Clear the token and initiate a stop of the
// service.
override fun onDisconnect(error: String): Boolean {
Log.d(TAG, "onDisconnect: $error")
Firebase.crashlytics.log("onDisconnect: $error")
stopNetworkMonitoring()
// Clear any user tokens and actorNames
@@ -162,7 +151,6 @@ class TunnelService : VpnService() {
context: Context,
intent: Intent,
) {
Log.d(TAG, "onReceive")
val restrictionsManager = context.getSystemService(Context.RESTRICTIONS_SERVICE) as android.content.RestrictionsManager
val newAppRestrictions = restrictionsManager.applicationRestrictions
val changed = MANAGED_CONFIGURATIONS.any { newAppRestrictions.getString(it) != appRestrictions.getString(it) }
@@ -186,7 +174,6 @@ class TunnelService : VpnService() {
flags: Int,
startId: Int,
): Int {
Log.d(TAG, "onStartCommand")
if (intent?.getBooleanExtra("startedByUser", false) == true) {
startedByUser = true
}
@@ -196,26 +183,21 @@ class TunnelService : VpnService() {
override fun onCreate() {
super.onCreate()
Log.d(TAG, "onCreate")
registerReceiver(restrictionsReceiver, restrictionsFilter)
}
override fun onDestroy() {
Log.d(TAG, "onDestroy")
unregisterReceiver(restrictionsReceiver)
super.onDestroy()
}
override fun onRevoke() {
Log.d(TAG, "onRevoke")
disconnect()
super.onRevoke()
}
// Call this to stop the tunnel and shutdown the service, leaving the token intact.
fun disconnect() {
Log.d(TAG, "disconnect")
// Acquire mutex lock
lock.lock()
@@ -232,8 +214,6 @@ class TunnelService : VpnService() {
}
private fun shutdown() {
Log.d(TAG, "shutdown")
connlibSessionPtr = null
stopSelf()
tunnelState = State.DOWN
@@ -246,7 +226,6 @@ class TunnelService : VpnService() {
if (!token.isNullOrBlank()) {
tunnelState = State.CONNECTING
updateStatusNotification(TunnelStatusNotification.Connecting)
System.loadLibrary("connlib")
connlibSessionPtr =
ConnlibSession.connect(
@@ -315,8 +294,6 @@ class TunnelService : VpnService() {
repo.saveDeviceIdSync(newDeviceId)
newDeviceId
}
Log.d(TAG, "Device ID: $deviceId")
return deviceId
}
@@ -329,36 +306,24 @@ class TunnelService : VpnService() {
private fun buildVpnService(): Int {
Builder().apply {
Firebase.crashlytics.log("Building VPN service")
// Allow traffic to bypass the VPN interface when Always-on VPN is enabled.
allowBypass()
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Firebase.crashlytics.log("Setting transport info")
setMetered(false) // Inherit the metered status from the underlying networks.
}
Firebase.crashlytics.log("Setting underlying networks")
setUnderlyingNetworks(null) // Use all available networks.
Log.d(TAG, "Routes: $tunnelRoutes")
Firebase.crashlytics.log("Routes: $tunnelRoutes")
tunnelRoutes.forEach {
addRoute(it.address, it.prefix)
}
Log.d(TAG, "DNS Servers: $tunnelDnsAddresses")
Firebase.crashlytics.log("DNS Servers: $tunnelDnsAddresses")
tunnelDnsAddresses.forEach { dns ->
addDnsServer(dns)
}
Log.d(TAG, "IPv4 Address: $tunnelIpv4Address")
Firebase.crashlytics.log("IPv4 Address: $tunnelIpv4Address")
addAddress(tunnelIpv4Address!!, 32)
Log.d(TAG, "IPv6 Address: $tunnelIpv6Address")
Firebase.crashlytics.log("IPv6 Address: $tunnelIpv6Address")
addAddress(tunnelIpv6Address!!, 128)
updateAllowedDisallowedApplications("allowedApplications", ::addAllowedApplication)
@@ -379,7 +344,6 @@ class TunnelService : VpnService() {
allowOrDisallow: (String) -> Unit,
) {
val applications = appRestrictions.getString(key)
Log.d(TAG, "$key: $applications")
Firebase.crashlytics.log("$key: $applications")
applications?.let {
if (it.isNotBlank()) {
@@ -433,7 +397,6 @@ class TunnelService : VpnService() {
}
fun start(context: Context) {
Log.d(TAG, "Starting TunnelService")
val intent = Intent(context, TunnelService::class.java)
intent.putExtra("startedByUser", true)
context.startService(intent)

View File

@@ -8,7 +8,6 @@ import android.app.PendingIntent
import android.content.Context
import android.content.Context.NOTIFICATION_SERVICE
import android.content.Intent
import android.util.Log
import androidx.core.app.NotificationCompat
import dev.firezone.android.R
import dev.firezone.android.core.presentation.MainActivity
@@ -25,7 +24,6 @@ object TunnelStatusNotification {
context: Context,
status: StatusType,
): NotificationCompat.Builder {
Log.d(TAG, "update")
val manager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager
val chan =
@@ -44,7 +42,6 @@ object TunnelStatusNotification {
}
private fun configIntent(context: Context): PendingIntent {
Log.d(TAG, "configIntent")
val intent = Intent(context, MainActivity::class.java)
intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK
return PendingIntent.getActivity(