From ad6f593591e171a0f23a3adeb359b165ee0b01f2 Mon Sep 17 00:00:00 2001 From: mariiapanasetskaia Date: Wed, 19 Aug 2026 23:26:00 +0200 Subject: [PATCH 1/3] credential leaks + logout fix + diagnostics --- .../app/navigation/SessionReconciler.kt | 25 ++-- .../android/auth/AuthTokenServiceImpl.kt | 17 ++- .../hedvig/android/auth/MemberIdService.kt | 18 ++- .../android/auth/token/LocalAccessToken.kt | 8 +- .../android/auth/token/LocalRefreshToken.kt | 8 +- .../android/auth/AuthTokenServiceImplTest.kt | 55 +++++++- .../com/hedvig/authlib/AuthRepository.kt | 2 +- .../hedvig/authlib/NetworkAuthRepository.kt | 2 +- app/feature/feature-login/build.gradle.kts | 1 + .../feature/login/swedishlogin/BankIdState.kt | 3 +- .../swedishlogin/SwedishLoginPresenter.kt | 13 +- .../swedishlogin/SwedishLoginViewModel.kt | 4 +- .../swedishlogin/SwedishLoginPresenterTest.kt | 4 + .../android/network/clients/TlsDiagnostics.kt | 123 ++++++++++++++++++ 14 files changed, 250 insertions(+), 33 deletions(-) create mode 100644 app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/SessionReconciler.kt b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/SessionReconciler.kt index 74dc2d0107..6b4207fe97 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/SessionReconciler.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/SessionReconciler.kt @@ -120,18 +120,7 @@ internal class SessionReconciler( */ private suspend fun logoutOnInvalidCredentials() { val authStatusLog: (AuthStatus?) -> Unit = { authStatus -> - logcat { - buildString { - append("Owner: MainActivity | Received authStatus: ") - append( - when (authStatus) { - is AuthStatus.LoggedIn -> "LoggedIn" - AuthStatus.LoggedOut -> "LoggedOut" - null -> "null" - }, - ) - } - } + logcat { "Owner: MainActivity | Received authStatus: ${authStatus.logName()}" } } combine( authTokenService.authStatus.onEach(authStatusLog).filterNotNull().distinctUntilChanged(), @@ -140,7 +129,7 @@ internal class SessionReconciler( ) { authStatus: AuthStatus, isDemoMode: Boolean, isLoggedIn: Boolean -> logcat { "SessionReconciler.logoutOnInvalidCredentials: " + - "authStatus:$authStatus | " + + "authStatus:${authStatus.logName()} | " + "isDemoMode:$isDemoMode | " + "isLoggedIn:$isLoggedIn" } @@ -153,3 +142,13 @@ internal class SessionReconciler( }.collect() } } + +/** + * Renders only the variant name. [AuthStatus.LoggedIn] holds the access and refresh tokens, and this + * value is logged at INFO, which reaches Datadog and Crashlytics breadcrumbs. + */ +private fun AuthStatus?.logName(): String = when (this) { + is AuthStatus.LoggedIn -> "LoggedIn" + AuthStatus.LoggedOut -> "LoggedOut" + null -> "null" +} diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt index fbc29e9995..e211fbad88 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/AuthTokenServiceImpl.kt @@ -60,12 +60,25 @@ internal class AuthTokenServiceImpl( override suspend fun refreshAndGetAccessToken(): AccessToken? { val refreshToken = getRefreshToken() ?: return null return when (val result = authRepository.exchange(RefreshTokenGrant(refreshToken.token))) { - is AuthTokenResult.Error -> { - logcat { "Refreshing token failed. Invalidating present tokens" } + is AuthTokenResult.Error.BackendErrorResponse -> { + logcat { "Refreshing token was rejected by the backend. Invalidating present tokens" } logoutAndInvalidateTokens() null } + is AuthTokenResult.Error.IOError -> { + // The backend was never reached, so this says nothing about whether the tokens are still + // valid. Keeping them lets a later request retry the refresh; discarding them would force a + // full BankID re-login over the same network that just failed one small request. + logcat(LogPriority.WARN) { "Refreshing token could not reach the backend. Keeping present tokens" } + null + } + + is AuthTokenResult.Error.UnknownError -> { + logcat(LogPriority.WARN) { "Refreshing token failed with an unknown error. Keeping present tokens" } + null + } + is AuthTokenResult.Success -> { logcat(LogPriority.VERBOSE) { "Refreshing token success. Updating tokens" } authTokenStorage.updateTokens(result.accessToken, result.refreshToken) diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/MemberIdService.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/MemberIdService.kt index 3bd581e8e9..59df8c4f52 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/MemberIdService.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/MemberIdService.kt @@ -46,21 +46,31 @@ class MemberIdService( .decodeToString() val payloadJsonObject: JsonObject = Json.parseToJsonElement(decodedPayload).jsonObject val subContent: JsonElement = payloadJsonObject.getOrElse("sub") { - logcat(LogPriority.ERROR) { "Failed to find `sub` in jsonElement for accessToken: $accessToken" } + logcat(LogPriority.ERROR) { "Failed to find `sub` in access token payload. ${accessToken.tokenShape()}" } return null } val subText = subContent.jsonPrimitive.content if (!subText.startsWith("mem_")) { - logcat(LogPriority.ERROR) { "Failed to find the `mem_` prefix for accessToken: $accessToken" } + logcat(LogPriority.ERROR) { "Access token `sub` lacks the `mem_` prefix. ${accessToken.tokenShape()}" } return null } subText.removePrefix("mem_") } catch (exception: SerializationException) { - logcat(LogPriority.ERROR, exception) { "Got serializationException for accessToken: $accessToken" } + logcat(LogPriority.ERROR, exception) { + "Got serializationException parsing access token. ${accessToken.tokenShape()}" + } null } catch (exception: IllegalArgumentException) { - logcat(LogPriority.ERROR, exception) { "Got illegalArgumentException for accessToken: $accessToken" } + logcat(LogPriority.ERROR, exception) { + "Got illegalArgumentException parsing access token. ${accessToken.tokenShape()}" + } null } } } + +/** + * Enough to tell a malformed token from a well-formed one, without putting a live bearer credential + * into logs that ship to Datadog and Crashlytics. + */ +private fun String.tokenShape(): String = "segments=${split(".").size}, length=$length" diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalAccessToken.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalAccessToken.kt index 563f3b0201..ebf5c84dbd 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalAccessToken.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalAccessToken.kt @@ -5,4 +5,10 @@ import kotlin.time.Instant data class LocalAccessToken( val token: String, val expiryDate: Instant, -) +) { + /** + * Redacted, so that interpolating this token, or anything holding it, can never publish a live + * bearer credential to a log sink. + */ + override fun toString(): String = "LocalAccessToken(token=REDACTED, expiryDate=$expiryDate)" +} diff --git a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalRefreshToken.kt b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalRefreshToken.kt index a2cb51ae81..10ea56e54c 100644 --- a/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalRefreshToken.kt +++ b/app/auth/auth-core-public/src/main/kotlin/com/hedvig/android/auth/token/LocalRefreshToken.kt @@ -5,4 +5,10 @@ import kotlin.time.Instant data class LocalRefreshToken( val token: String, val expiryDate: Instant, -) +) { + /** + * Redacted, so that interpolating this token, or anything holding it, can never publish a live + * bearer credential to a log sink. + */ + override fun toString(): String = "LocalRefreshToken(token=REDACTED, expiryDate=$expiryDate)" +} diff --git a/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt index bcc5f7d4a3..1ec65b2f1b 100644 --- a/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt +++ b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/AuthTokenServiceImplTest.kt @@ -3,6 +3,8 @@ package com.hedvig.android.auth import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf +import assertk.assertions.isNotNull +import assertk.assertions.isNull import com.hedvig.android.auth.event.AuthEventStorage import com.hedvig.android.auth.storage.AuthTokenStorage import com.hedvig.android.auth.test.FakeAuthRepository @@ -11,6 +13,8 @@ import com.hedvig.android.core.datastore.TestPreferencesDataStore import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.test.clock.TestClock import com.hedvig.authlib.AccessToken +import com.hedvig.authlib.AuthRepository +import com.hedvig.authlib.AuthTokenResult import com.hedvig.authlib.RefreshToken import kotlin.time.Clock import kotlin.time.Duration.Companion.seconds @@ -78,14 +82,51 @@ internal class AuthTokenServiceImplTest { assertThat(status).isEqualTo(AuthStatus.LoggedOut) } - private fun TestScope.authTokenService(storage: AuthTokenStorage, clock: Clock): AuthTokenService = - AuthTokenServiceImpl( - storage, - FakeAuthRepository(), - AuthEventStorage(), - ApplicationScope(backgroundScope), - clock, + @Test + fun `a network error while refreshing keeps the stored tokens`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 6000), ) + val authRepository = FakeAuthRepository() + val service = authTokenService(storage, clock, authRepository) + authRepository.exchangeResponse.add(AuthTokenResult.Error.IOError("no network")) + + service.refreshAndGetAccessToken() + + assertThat(storage.getTokens().first()).isNotNull() + } + + @Test + fun `the backend rejecting the refresh token clears the stored tokens`() = runTest { + val clock = TestClock() + val storage = authTokenStorage(clock) + storage.updateTokens( + accessToken = AccessToken("access", expiryInSeconds = 60), + refreshToken = RefreshToken("refresh", expiryInSeconds = 6000), + ) + val authRepository = FakeAuthRepository() + val service = authTokenService(storage, clock, authRepository) + authRepository.exchangeResponse.add(AuthTokenResult.Error.BackendErrorResponse("invalid_grant")) + + service.refreshAndGetAccessToken() + + assertThat(storage.getTokens().first()).isNull() + } + + private fun TestScope.authTokenService( + storage: AuthTokenStorage, + clock: Clock, + authRepository: AuthRepository = FakeAuthRepository(), + ): AuthTokenService = AuthTokenServiceImpl( + storage, + authRepository, + AuthEventStorage(), + ApplicationScope(backgroundScope), + clock, + ) private fun TestScope.authTokenStorage(clock: Clock) = AuthTokenStorage( TestPreferencesDataStore( diff --git a/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/AuthRepository.kt b/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/AuthRepository.kt index f15e7fb2e0..ff5dd82e72 100644 --- a/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/AuthRepository.kt +++ b/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/AuthRepository.kt @@ -51,7 +51,7 @@ public sealed interface AuthAttemptResult { public data class BackendErrorResponse(val message: String) : Error - public data class IOError(val message: String) : Error + public data class IOError(val message: String, val throwable: Throwable? = null) : Error public data class UnknownError(val message: String) : Error } diff --git a/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/NetworkAuthRepository.kt b/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/NetworkAuthRepository.kt index 28c7931f66..5165a3824f 100644 --- a/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/NetworkAuthRepository.kt +++ b/app/authlib/src/commonMain/kotlin/com/hedvig/authlib/NetworkAuthRepository.kt @@ -93,7 +93,7 @@ public class NetworkAuthRepository( when (e) { is CancellationException -> throw e - is IOException -> AuthAttemptResult.Error.IOError("IO Error with message: ${e.message ?: "unknown message"}") + is IOException -> AuthAttemptResult.Error.IOError("IO Error with message: ${e.message ?: "unknown message"}", e) is NoTransformationFoundException -> AuthAttemptResult.Error.BackendErrorResponse( e.message ?: "unknown error", diff --git a/app/feature/feature-login/build.gradle.kts b/app/feature/feature-login/build.gradle.kts index b781ac3f5a..0ab9ea9bfc 100644 --- a/app/feature/feature-login/build.gradle.kts +++ b/app/feature/feature-login/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { implementation(projects.navigationCommon) implementation(projects.navigationCompose) implementation(projects.navigationCore) + implementation(projects.networkClients) testImplementation(libs.androidx.datastore.core) testImplementation(libs.androidx.junit) diff --git a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt index 4bacc9f4c6..dd19633954 100644 --- a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt +++ b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt @@ -51,7 +51,8 @@ private class BankIdStateImpl( fun initialize() { canOpenBankId = context.canBankIdAppHandleUri(bankIdUri).also { - logcat { "Trying to resolve BankID app with bankIdUri:$bankIdUri | result: canOpenBankId=$it" } + // The URI carries a single-use autostart token, so log only what identifies the target. + logcat { "Trying to resolve BankID app for ${bankIdUri.host} | result: canOpenBankId=$it" } } } diff --git a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenter.kt b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenter.kt index feec4cef1a..64f1f5f61f 100644 --- a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenter.kt +++ b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenter.kt @@ -26,6 +26,8 @@ import com.hedvig.android.logger.LogPriority import com.hedvig.android.logger.logcat import com.hedvig.android.molecule.public.MoleculePresenter import com.hedvig.android.molecule.public.MoleculePresenterScope +import com.hedvig.android.network.clients.TLS_DIAG_TAG +import com.hedvig.android.network.clients.TlsDiagnostics import com.hedvig.authlib.AuthAttemptResult import com.hedvig.authlib.AuthRepository import com.hedvig.authlib.AuthTokenResult @@ -40,6 +42,7 @@ internal class SwedishLoginPresenter( private val authTokenService: AuthTokenService, private val authRepository: AuthRepository, private val demoManager: DemoManager, + private val tlsDiagnostics: TlsDiagnostics, private val savedStateHandle: SavedStateHandle, ) : MoleculePresenter { @Composable @@ -123,7 +126,15 @@ internal class SwedishLoginPresenter( } is AuthAttemptResult.Error -> { - logcat(LogPriority.ERROR) { "Got Error when signing in with BankId: $result" } + val tlsFailure = (result as? AuthAttemptResult.Error.IOError) + ?.let { tlsDiagnostics.describe(it.throwable) } + if (tlsFailure != null) { + // Nothing reached us, so this is the member's network refusing the handshake rather than a + // fault of ours. WARN keeps it out of the app's error rate while the evidence stays greppable. + logcat(LogPriority.WARN, tag = TLS_DIAG_TAG) { "BankId login blocked by certificate trust. $tlsFailure" } + } else { + logcat(LogPriority.ERROR) { "Got Error when signing in with BankId: $result" } + } startLoginAttemptFailed = true } diff --git a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginViewModel.kt b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginViewModel.kt index a4caee1cd6..283456e273 100644 --- a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginViewModel.kt +++ b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginViewModel.kt @@ -6,6 +6,7 @@ import com.hedvig.android.core.common.di.ActivityRetainedScope import com.hedvig.android.core.common.di.HedvigViewModel import com.hedvig.android.core.demomode.DemoManager import com.hedvig.android.molecule.public.MoleculeViewModel +import com.hedvig.android.network.clients.TlsDiagnostics import com.hedvig.authlib.AuthRepository import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedInject @@ -17,9 +18,10 @@ internal class SwedishLoginViewModel( authTokenService: AuthTokenService, authRepository: AuthRepository, demoManager: DemoManager, + tlsDiagnostics: TlsDiagnostics, @Assisted savedStateHandle: SavedStateHandle, ) : MoleculeViewModel( SwedishLoginUiState(BankIdUiState.Loading, false), - SwedishLoginPresenter(authTokenService, authRepository, demoManager, savedStateHandle), + SwedishLoginPresenter(authTokenService, authRepository, demoManager, tlsDiagnostics, savedStateHandle), SharingStarted.WhileSubscribed(), ) diff --git a/app/feature/feature-login/src/test/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenterTest.kt b/app/feature/feature-login/src/test/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenterTest.kt index a830682c97..23df20fe80 100644 --- a/app/feature/feature-login/src/test/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenterTest.kt +++ b/app/feature/feature-login/src/test/kotlin/com/hedvig/android/feature/login/swedishlogin/SwedishLoginPresenterTest.kt @@ -17,6 +17,7 @@ import com.hedvig.android.auth.test.TestAuthTokenService import com.hedvig.android.core.demomode.DemoManager import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.molecule.test.test +import com.hedvig.android.network.clients.TlsDiagnostics import com.hedvig.authlib.AccessToken import com.hedvig.authlib.AuthAttemptResult import com.hedvig.authlib.AuthRepository @@ -303,6 +304,9 @@ class SwedishLoginPresenterTest { override suspend fun setDemoMode(demoMode: Boolean) {} }, + object : TlsDiagnostics { + override suspend fun describe(throwable: Throwable?): String? = null + }, savedStateHandle, ) } diff --git a/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt new file mode 100644 index 0000000000..fb969567a8 --- /dev/null +++ b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt @@ -0,0 +1,123 @@ +package com.hedvig.android.network.clients + +import android.content.Context +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import android.os.Build +import com.hedvig.android.core.buildconstants.HedvigBuildConstants +import com.hedvig.android.core.common.di.AppScope +import dev.zacsweers.metro.ContributesBinding +import dev.zacsweers.metro.Inject +import java.net.InetAddress +import java.net.URI +import java.security.KeyStore +import java.security.cert.CertPathBuilderException +import java.security.cert.CertPathValidatorException +import java.security.cert.CertificateException +import java.security.cert.X509Certificate +import javax.net.ssl.SSLHandshakeException +import javax.security.auth.x500.X500Principal +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext + +const val TLS_DIAG_TAG = "TlsDiag" + +/** + * Evidence for why a TLS handshake was refused, gathered at the point of failure. + * + * A trust failure means a certificate did arrive and we declined it, so the questions worth answering + * are where the hostname pointed and whether this device trusts anything unusual. An address outside + * our hosting means something other than us answered the name; a non-zero count of added CAs means + * traffic is being intercepted on the device itself. + */ +interface TlsDiagnostics { + /** Null when [throwable] is not a certificate-trust failure, so callers can fall through. */ + suspend fun describe(throwable: Throwable?): String? +} + +@Inject +@ContributesBinding(AppScope::class) +internal class AndroidTlsDiagnostics( + private val context: Context, + private val buildConstants: HedvigBuildConstants, +) : TlsDiagnostics { + override suspend fun describe(throwable: Throwable?): String? { + val causes = generateSequence(throwable) { it.cause }.take(MAX_CAUSE_DEPTH).toList() + val trustFailure = causes.firstOrNull { it.isTrustFailure() } ?: return null + return withContext(Dispatchers.IO) { + buildString { + append("failure=").append(trustFailure::class.java.name) + append(" chain=").append(trustFailure.servedChain()) + append(" resolvedIps=").append(resolvedAuthHostIps()) + append(" transport=").append(activeTransports()) + append(" userCaCount=").append(userInstalledCaCount()) + append(" api=").append(Build.VERSION.SDK_INT) + } + } + } + + /** + * Which exception carries a trust failure varies by platform and provider — Android reports + * [CertPathValidatorException], the desktop JVM a [CertPathBuilderException] — so match on the + * message too rather than on one type. + */ + private fun Throwable.isTrustFailure(): Boolean { + if (this is CertPathValidatorException || this is CertPathBuilderException) return true + if (this is SSLHandshakeException || this is CertificateException) { + val message = message ?: return false + return message.contains("certification path", ignoreCase = true) || + message.contains("trust anchor", ignoreCase = true) + } + return false + } + + /** + * Only [CertPathValidatorException] can carry the chain, and even then it may be absent, so the rest + * of the report has to stand on its own. + */ + private fun Throwable.servedChain(): String { + val certificates = (this as? CertPathValidatorException)?.certPath?.certificates.orEmpty() + .filterIsInstance() + if (certificates.isEmpty()) return "unavailable" + return certificates.joinToString( + separator = " | ", + prefix = "[", + postfix = "]", + ) { "${it.subjectX500Principal.commonName()} issuedBy ${it.issuerX500Principal.commonName()}" } + } + + /** An address outside our hosting means the name was answered by something that isn't us. */ + private fun resolvedAuthHostIps(): String = runCatching { + val host = URI(buildConstants.urlAuthService).host ?: return@runCatching "unknown-host" + InetAddress.getAllByName(host).joinToString(",") { it.hostAddress ?: "?" } + }.getOrElse { "unresolved(${it::class.simpleName})" } + + private fun activeTransports(): String = runCatching { + val connectivityManager = context.getSystemService(ConnectivityManager::class.java) + val capabilities = connectivityManager.getNetworkCapabilities(connectivityManager.activeNetwork) + ?: return@runCatching "none" + listOfNotNull( + "vpn".takeIf { capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN) }, + "wifi".takeIf { capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) }, + "cellular".takeIf { capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) }, + ).joinToString("+").ifEmpty { "other" } + }.getOrElse { "unknown" } + + /** + * How many CAs the member or an MDM profile added. Count only — an alias can name an employer. + * A non-zero count is the strongest single signal that traffic is intercepted on the device. + */ + private fun userInstalledCaCount(): Int = runCatching { + KeyStore.getInstance("AndroidCAStore").apply { load(null) } + .aliases() + .asSequence() + .count { it.startsWith("user:") } + }.getOrElse { -1 } + + private fun X500Principal.commonName(): String = COMMON_NAME.find(name)?.groupValues?.get(1) ?: "?" + + private companion object { + const val MAX_CAUSE_DEPTH = 10 + val COMMON_NAME = Regex("CN=([^,]+)") + } +} From 34415d35289e7a66e623853c0334b1596e5a84fb Mon Sep 17 00:00:00 2001 From: mariiapanasetskaia Date: Thu, 20 Aug 2026 10:22:21 +0200 Subject: [PATCH 2/3] second leak fix + ipKind change --- .../android/auth/token/TokenRedactionTest.kt | 40 +++++++++++++++++++ .../feature/login/swedishlogin/BankIdState.kt | 2 +- .../android/network/clients/TlsDiagnostics.kt | 25 ++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/token/TokenRedactionTest.kt diff --git a/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/token/TokenRedactionTest.kt b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/token/TokenRedactionTest.kt new file mode 100644 index 0000000000..12d5356157 --- /dev/null +++ b/app/auth/auth-core-public/src/test/kotlin/com/hedvig/android/auth/token/TokenRedactionTest.kt @@ -0,0 +1,40 @@ +package com.hedvig.android.auth.token + +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.doesNotContain +import com.hedvig.android.auth.AuthStatus +import kotlin.time.Instant +import org.junit.Test + +/** + * These values get interpolated into logs that ship to Datadog and Crashlytics, so a token surviving + * [toString] is a live credential leaving the device. + */ +internal class TokenRedactionTest { + private val accessToken = LocalAccessToken("access-token-secret", Instant.fromEpochSeconds(1)) + private val refreshToken = LocalRefreshToken("refresh-token-secret", Instant.fromEpochSeconds(2)) + + @Test + fun `access token is not rendered by toString`() { + assertThat(accessToken.toString()).doesNotContain("access-token-secret") + } + + @Test + fun `refresh token is not rendered by toString`() { + assertThat(refreshToken.toString()).doesNotContain("refresh-token-secret") + } + + @Test + fun `expiry stays visible so the log remains useful`() { + assertThat(accessToken.toString()).contains("expiryDate") + } + + @Test + fun `rendering the whole auth status leaks neither token`() { + val rendered = AuthStatus.LoggedIn(accessToken, refreshToken).toString() + + assertThat(rendered).doesNotContain("access-token-secret") + assertThat(rendered).doesNotContain("refresh-token-secret") + } +} diff --git a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt index dd19633954..4095ecda99 100644 --- a/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt +++ b/app/feature/feature-login/src/main/kotlin/com/hedvig/android/feature/login/swedishlogin/BankIdState.kt @@ -71,7 +71,7 @@ private class BankIdStateImpl( true } catch (e: PackageManager.NameNotFoundException) { logcat(LogPriority.WARN) { - "Could not resolve BankID app with bankIdUri:$uri + exception: $e" + "Could not resolve BankID app for ${uri.host} + exception: $e" } false } diff --git a/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt index fb969567a8..447407d1f9 100644 --- a/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt +++ b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt @@ -48,7 +48,7 @@ internal class AndroidTlsDiagnostics( buildString { append("failure=").append(trustFailure::class.java.name) append(" chain=").append(trustFailure.servedChain()) - append(" resolvedIps=").append(resolvedAuthHostIps()) + append(" authHost=").append(resolvedAuthHost()) append(" transport=").append(activeTransports()) append(" userCaCount=").append(userInstalledCaCount()) append(" api=").append(Build.VERSION.SDK_INT) @@ -86,10 +86,27 @@ internal class AndroidTlsDiagnostics( ) { "${it.subjectX500Principal.commonName()} issuedBy ${it.issuerX500Principal.commonName()}" } } - /** An address outside our hosting means the name was answered by something that isn't us. */ - private fun resolvedAuthHostIps(): String = runCatching { + /** + * Reported as a category rather than raw addresses: our hosting answers from a rotating public pool, + * so a specific address proves nothing, whereas a loopback, unspecified or private answer means a + * filter on the device or the local network answered the name instead of us. + */ + private fun resolvedAuthHost(): String = runCatching { val host = URI(buildConstants.urlAuthService).host ?: return@runCatching "unknown-host" - InetAddress.getAllByName(host).joinToString(",") { it.hostAddress ?: "?" } + val addresses = InetAddress.getAllByName(host) + if (addresses.isEmpty()) return@runCatching "no-addresses" + addresses + .map { address -> + when { + address.isAnyLocalAddress -> "unspecified" + address.isLoopbackAddress -> "loopback" + address.isSiteLocalAddress || address.isLinkLocalAddress -> "private" + else -> "public" + } + } + .distinct() + .sorted() + .joinToString("+") }.getOrElse { "unresolved(${it::class.simpleName})" } private fun activeTransports(): String = runCatching { From 31b10bfbdcbd5d435ae6a193a313066c4dfa74eb Mon Sep 17 00:00:00 2001 From: mariiapanasetskaia Date: Thu, 20 Aug 2026 10:42:11 +0200 Subject: [PATCH 3/3] log change --- .../android/network/clients/TlsDiagnostics.kt | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt index 447407d1f9..e10f866367 100644 --- a/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt +++ b/app/network/network-clients/src/androidMain/kotlin/com/hedvig/android/network/clients/TlsDiagnostics.kt @@ -14,9 +14,7 @@ import java.security.KeyStore import java.security.cert.CertPathBuilderException import java.security.cert.CertPathValidatorException import java.security.cert.CertificateException -import java.security.cert.X509Certificate import javax.net.ssl.SSLHandshakeException -import javax.security.auth.x500.X500Principal import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -47,19 +45,24 @@ internal class AndroidTlsDiagnostics( return withContext(Dispatchers.IO) { buildString { append("failure=").append(trustFailure::class.java.name) - append(" chain=").append(trustFailure.servedChain()) append(" authHost=").append(resolvedAuthHost()) append(" transport=").append(activeTransports()) append(" userCaCount=").append(userInstalledCaCount()) append(" api=").append(Build.VERSION.SDK_INT) + // Last, and quoted: it is the only field containing spaces, so the rest stay parseable. + append(" msg=\"").append(trustFailure.messageForLog()).append('"') } } } /** - * Which exception carries a trust failure varies by platform and provider — Android reports - * [CertPathValidatorException], the desktop JVM a [CertPathBuilderException] — so match on the - * message too rather than on one type. + * On Android the trust failure arrives as an [SSLHandshakeException] whose message merely names + * `CertPathValidatorException`, so the type checks below never match there and the message check is + * what identifies it. The type checks still hold on other providers, which report a + * [CertPathValidatorException] or [CertPathBuilderException] directly. + * + * Should the wording ever change, these stop being recognised and fall back to the pre-existing + * error log rather than disappearing, so the regression is visible in Datadog. */ private fun Throwable.isTrustFailure(): Boolean { if (this is CertPathValidatorException || this is CertPathBuilderException) return true @@ -72,18 +75,12 @@ internal class AndroidTlsDiagnostics( } /** - * Only [CertPathValidatorException] can carry the chain, and even then it may be absent, so the rest - * of the report has to stand on its own. + * The wording is what identifies a trust failure on Android, so record it verbatim: it is the only + * way to notice the provider changing it, or differing between OEMs and API levels. */ - private fun Throwable.servedChain(): String { - val certificates = (this as? CertPathValidatorException)?.certPath?.certificates.orEmpty() - .filterIsInstance() - if (certificates.isEmpty()) return "unavailable" - return certificates.joinToString( - separator = " | ", - prefix = "[", - postfix = "]", - ) { "${it.subjectX500Principal.commonName()} issuedBy ${it.issuerX500Principal.commonName()}" } + private fun Throwable.messageForLog(): String { + val message = message ?: return "none" + return message.replace('\n', ' ').take(MAX_MESSAGE_CHARS) } /** @@ -131,10 +128,8 @@ internal class AndroidTlsDiagnostics( .count { it.startsWith("user:") } }.getOrElse { -1 } - private fun X500Principal.commonName(): String = COMMON_NAME.find(name)?.groupValues?.get(1) ?: "?" - private companion object { const val MAX_CAUSE_DEPTH = 10 - val COMMON_NAME = Regex("CN=([^,]+)") + const val MAX_MESSAGE_CHARS = 200 } }