Use SensitiveString for passwords (#1692)

* Use SensitiveString for passwords to prevent them from being logged by `toString()` * Add test * Fix other tests * Credentials: equals / hashCode not needed anymore * Add tests for equals
2025-12-23 23:17:50 -05:00 · 2025-09-10 10:31:24 +02:00
parent f4aa55d482
commit fa09a0560f
17 changed files with 158 additions and 50 deletions
--- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt
+++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt
@@ -11,6 +11,7 @@ import at.bitfire.dav4jvm.property.webdav.ResourceType
 import at.bitfire.davdroid.network.HttpClient
 import at.bitfire.davdroid.servicedetection.DavResourceFinder.Configuration.ServiceInfo
 import at.bitfire.davdroid.settings.Credentials
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
 import okhttp3.mockwebserver.Dispatcher
@@ -70,7 +71,7 @@ class DavResourceFinderTest {
            start()
        }

-        val credentials = Credentials(username = "mock", password = "12345".toCharArray())
+        val credentials = Credentials(username = "mock", password = "12345".toSensitiveString())
        client = httpClientBuilder
                .authenticate(host = null, getCredentials = { credentials })
                .build()
--- a/app/src/androidTest/kotlin/at/bitfire/davdroid/ui/setup/LoginActivityTest.kt
+++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/ui/setup/LoginActivityTest.kt
@@ -21,7 +21,7 @@ class LoginActivityTest {
        val loginInfo = LoginActivity.loginInfoFromIntent(intent)
        assertEquals("https://example.com/nextcloud", loginInfo.baseUri.toString())
        assertEquals("user", loginInfo.credentials!!.username)
-        assertEquals("password", loginInfo.credentials.password?.concatToString())
+        assertEquals("password", loginInfo.credentials.password?.asString())
    }

    @Test
@@ -34,7 +34,7 @@ class LoginActivityTest {
        val loginInfo = LoginActivity.loginInfoFromIntent(intent)
        assertEquals("https://example.com:444/nextcloud", loginInfo.baseUri.toString())
        assertEquals("user", loginInfo.credentials!!.username)
-        assertEquals("password", loginInfo.credentials.password?.concatToString())
+        assertEquals("password", loginInfo.credentials.password?.asString())
    }

    @Test
@@ -43,7 +43,7 @@ class LoginActivityTest {
        val loginInfo = LoginActivity.loginInfoFromIntent(intent)
        assertEquals("https://example.com/path", loginInfo.baseUri.toString())
        assertEquals("user", loginInfo.credentials!!.username)
-        assertEquals("password", loginInfo.credentials.password?.concatToString())
+        assertEquals("password", loginInfo.credentials.password?.asString())
    }

    @Test
@@ -52,7 +52,7 @@ class LoginActivityTest {
        val loginInfo = LoginActivity.loginInfoFromIntent(intent)
        assertEquals("https://example.com:0/path", loginInfo.baseUri.toString())
        assertEquals("user", loginInfo.credentials!!.username)
-        assertEquals("password", loginInfo.credentials.password?.concatToString())
+        assertEquals("password", loginInfo.credentials.password?.asString())
    }

    @Test
@@ -61,7 +61,7 @@ class LoginActivityTest {
        val loginInfo = LoginActivity.loginInfoFromIntent(intent)
        assertEquals(null, loginInfo.baseUri)
        assertEquals("user@example.com", loginInfo.credentials!!.username)
-        assertEquals(null, loginInfo.credentials.password?.concatToString())
+        assertEquals(null, loginInfo.credentials.password?.asString())
    }
    
 }
--- a/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/CredentialsStoreTest.kt
+++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/CredentialsStoreTest.kt
@@ -5,6 +5,7 @@
 package at.bitfire.davdroid.webdav

 import at.bitfire.davdroid.settings.Credentials
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
 import org.junit.Assert.assertEquals
@@ -30,8 +31,8 @@ class CredentialsStoreTest {

    @Test
    fun testSetGetDelete() {
-        store.setCredentials(0, Credentials(username = "myname", password = "12345".toCharArray()))
-        assertEquals(Credentials(username = "myname", password = "12345".toCharArray()), store.getCredentials(0))
+        store.setCredentials(0, Credentials(username = "myname", password = "12345".toSensitiveString()))
+        assertEquals(Credentials(username = "myname", password = "12345".toSensitiveString()), store.getCredentials(0))

        store.setCredentials(0, null)
        assertNull(store.getCredentials(0))
--- a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt
@@ -116,7 +116,7 @@ class HttpClient(
                val authHandler = BasicDigestAuthHandler(
                    domain = UrlUtils.hostToDomain(host),
                    username = credentials.username,
-                    password = credentials.password,
+                    password = credentials.password.asCharArray(),
                    insecurePreemptive = true
                )
                authenticationInterceptor = authHandler
--- a/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt
@@ -8,6 +8,7 @@ import at.bitfire.dav4jvm.exception.DavException
 import at.bitfire.dav4jvm.exception.HttpException
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.ui.setup.LoginInfo
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.davdroid.util.withTrailingSlash
 import at.bitfire.vcard4android.GroupMethod
 import kotlinx.coroutines.Dispatchers
@@ -106,7 +107,7 @@ class NextcloudLoginFlow @Inject constructor(
            baseUri = URI(serverUrl).resolve(DAV_PATH),
            credentials = Credentials(
                username = json.getString("loginName"),
-                password = json.getString("appPassword").toCharArray()
+                password = json.getString("appPassword").toSensitiveString()
            ),
            suggestedGroupMethod = GroupMethod.CATEGORIES
        )
--- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
@@ -18,6 +18,7 @@ import at.bitfire.davdroid.sync.AutomaticSyncManager
 import at.bitfire.davdroid.sync.SyncDataType
 import at.bitfire.davdroid.sync.account.InvalidAccountException
 import at.bitfire.davdroid.sync.account.setAndVerifyUserData
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.davdroid.util.trimToNull
 import at.bitfire.vcard4android.GroupMethod
 import dagger.assisted.Assisted
@@ -106,7 +107,7 @@ class AccountSettings @AssistedInject constructor(

    fun credentials() = Credentials(
        accountManager.getUserData(account, KEY_USERNAME),
-        accountManager.getPassword(account)?.toCharArray(),
+        accountManager.getPassword(account)?.toSensitiveString(),

        accountManager.getUserData(account, KEY_CERTIFICATE_ALIAS),

@@ -118,7 +119,7 @@ class AccountSettings @AssistedInject constructor(
    fun credentials(credentials: Credentials) {
        // Basic/Digest auth
        accountManager.setAndVerifyUserData(account, KEY_USERNAME, credentials.username)
-        accountManager.setPassword(account, credentials.password?.concatToString())
+        accountManager.setPassword(account, credentials.password?.asString())

        // client certificate
        accountManager.setAndVerifyUserData(account, KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)
--- a/app/src/main/kotlin/at/bitfire/davdroid/settings/Credentials.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/Credentials.kt
@@ -4,6 +4,7 @@

 package at.bitfire.davdroid.settings

+import at.bitfire.davdroid.util.SensitiveString
 import net.openid.appauth.AuthState

 /**
@@ -16,7 +17,7 @@ data class Credentials(
    /** username for Basic / Digest auth */
    val username: String? = null,
    /** password for Basic / Digest auth */
-    val password: CharArray? = null,
+    val password: SensitiveString? = null,

    /** alias of an client certificate that is present on the system */
    val certificateAlias: String? = null,
@@ -42,27 +43,4 @@ data class Credentials(
        return "Credentials(" + s.joinToString(", ") + ")"
    }

-
-    override fun equals(other: Any?): Boolean {
-        if (this === other) return true
-        if (javaClass != other?.javaClass) return false
-
-        other as Credentials
-
-        if (username != other.username) return false
-        if (!password.contentEquals(other.password)) return false
-        if (certificateAlias != other.certificateAlias) return false
-        if (authState != other.authState) return false
-
-        return true
-    }
-
-    override fun hashCode(): Int {
-        var result = username?.hashCode() ?: 0
-        result = 31 * result + (password?.contentHashCode() ?: 0)
-        result = 31 * result + (certificateAlias?.hashCode() ?: 0)
-        result = 31 * result + (authState?.hashCode() ?: 0)
-        return result
-    }
-
 }
--- a/app/src/main/kotlin/at/bitfire/davdroid/sync/account/SystemAccountUtils.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/account/SystemAccountUtils.kt
@@ -8,6 +8,7 @@ import android.accounts.Account
 import android.accounts.AccountManager
 import android.content.Context
 import android.os.Bundle
+import at.bitfire.davdroid.util.SensitiveString
 import java.util.logging.Logger

 object SystemAccountUtils {
@@ -25,7 +26,7 @@ object SystemAccountUtils {
     * @throws IllegalArgumentException when user data contains non-String values
     * @throws IllegalStateException if user data can't be set
     */
-    fun createAccount(context: Context, account: Account, userData: Bundle, password: CharArray? = null): Boolean {
+    fun createAccount(context: Context, account: Account, userData: Bundle, password: SensitiveString? = null): Boolean {
        // validate user data
        for (key in userData.keySet()) {
            userData.get(key)?.let { entry ->
@@ -36,7 +37,7 @@ object SystemAccountUtils {

        // create account
        val manager = AccountManager.get(context)
-        if (!manager.addAccountExplicitly(account, password?.concatToString(), userData))
+        if (!manager.addAccountExplicitly(account, password?.asString(), userData))
            return false

        // Android seems to lose the initial user data sometimes, so make sure that the values are set
--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/account/AccountSettingsScreen.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/account/AccountSettingsScreen.kt
@@ -67,6 +67,7 @@ import at.bitfire.davdroid.ui.composable.Setting
 import at.bitfire.davdroid.ui.composable.SettingsHeader
 import at.bitfire.davdroid.ui.composable.SwitchSetting
 import at.bitfire.davdroid.util.PermissionUtils
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.vcard4android.GroupMethod
 import kotlinx.coroutines.launch

@@ -578,7 +579,7 @@ fun AuthenticationSettings(
                        initialValue = null, // Do not show the existing password
                        passwordField = true,
                        onValueEntered = { newValue ->
-                            onUpdateCredentials(credentials.copy(password = newValue.toCharArray()))
+                            onUpdateCredentials(credentials.copy(password = newValue.toSensitiveString()))
                        },
                        onDismiss = { showPasswordDialog = false }
                    )
@@ -781,7 +782,7 @@ fun AccountSettingsScreen_Preview() {
            onUpdateIgnoreVpns = {},

            // Authentication Settings
-            credentials = Credentials(username = "test", password = "test".toCharArray()),
+            credentials = Credentials(username = "test", password = "test".toSensitiveString()),
            onUpdateCredentials = {},
            isCredentialsUpdateAllowed = true,

--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AdvancedLoginModel.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AdvancedLoginModel.kt
@@ -10,6 +10,7 @@ import androidx.compose.runtime.setValue
 import androidx.lifecycle.ViewModel
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.util.DavUtils.toURIorNull
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.davdroid.util.trimToNull
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
@@ -46,7 +47,7 @@ class AdvancedLoginModel @AssistedInject constructor(
            baseUri = uri,
            credentials = Credentials(
                username = username.trimToNull(),
-                password = password.trimToNull()?.toCharArray(),
+                password = password.trimToNull()?.toSensitiveString(),
                certificateAlias = certAlias.trimToNull()
            )
        )
@@ -60,7 +61,7 @@ class AdvancedLoginModel @AssistedInject constructor(
        uiState = uiState.copy(
            url = initialLoginInfo.baseUri?.toString()?.removePrefix("https://") ?: "",
            username = initialLoginInfo.credentials?.username ?: "",
-            password = initialLoginInfo.credentials?.password?.concatToString() ?: "",
+            password = initialLoginInfo.credentials?.password?.asString() ?: "",
            certAlias = initialLoginInfo.credentials?.certificateAlias ?: ""
        )
    }
--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EmailLoginModel.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EmailLoginModel.kt
@@ -10,6 +10,7 @@ import androidx.compose.runtime.setValue
 import androidx.lifecycle.ViewModel
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.util.DavUtils.toURIorNull
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
@@ -38,7 +39,7 @@ class EmailLoginModel @AssistedInject constructor(
                baseUri = uri,
                credentials = Credentials(
                    username = email,
-                    password = password.toCharArray()
+                    password = password.toSensitiveString()
                )
            )
        }
@@ -50,7 +51,7 @@ class EmailLoginModel @AssistedInject constructor(
    init {
        uiState = uiState.copy(
            email = initialLoginInfo.credentials?.username ?: "",
-            password = initialLoginInfo.credentials?.password?.concatToString() ?: ""
+            password = initialLoginInfo.credentials?.password?.asString() ?: ""
        )
    }

--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginActivity.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginActivity.kt
@@ -10,6 +10,7 @@ import androidx.activity.compose.setContent
 import androidx.appcompat.app.AppCompatActivity
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.ui.account.AccountActivity
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import dagger.hilt.android.AndroidEntryPoint
 import java.net.URI
 import java.net.URISyntaxException
@@ -139,7 +140,7 @@ class LoginActivity @Inject constructor(): AppCompatActivity() {
                },
                credentials = Credentials(
                    username = givenUsername,
-                    password = givenPassword?.toCharArray()
+                    password = givenPassword?.toSensitiveString()
                )
            )
        }
--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/UrlLoginModel.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/UrlLoginModel.kt
@@ -10,6 +10,7 @@ import androidx.compose.runtime.setValue
 import androidx.lifecycle.ViewModel
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.util.DavUtils.toURIorNull
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.davdroid.util.trimToNull
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
@@ -46,7 +47,7 @@ class UrlLoginModel @AssistedInject constructor(
                baseUri = uri,
                credentials = Credentials(
                    username = username.trimToNull(),
-                    password = password.trimToNull()?.toCharArray()
+                    password = password.trimToNull()?.toSensitiveString()
                )
            )

@@ -59,7 +60,7 @@ class UrlLoginModel @AssistedInject constructor(
        uiState = UiState(
            url = initialLoginInfo.baseUri?.toString()?.removePrefix("https://") ?: "",
            username = initialLoginInfo.credentials?.username ?: "",
-            password = initialLoginInfo.credentials?.password?.concatToString() ?: ""
+            password = initialLoginInfo.credentials?.password?.asString() ?: ""
        )
    }

--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/webdav/AddWebdavMountModel.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/webdav/AddWebdavMountModel.kt
@@ -13,6 +13,7 @@ import androidx.lifecycle.viewModelScope
 import at.bitfire.davdroid.R
 import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.settings.Credentials
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import at.bitfire.davdroid.util.trimToNull
 import at.bitfire.davdroid.webdav.WebDavMountRepository
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -84,7 +85,7 @@ class AddWebdavMountModel @Inject constructor(
        val displayName = uiState.displayName
        val credentials = Credentials(
            username = uiState.username.trimToNull(),
-            password = uiState.password.trimToNull()?.toCharArray(),
+            password = uiState.password.trimToNull()?.toSensitiveString(),
            certificateAlias = uiState.certificateAlias
        )

--- a/app/src/main/kotlin/at/bitfire/davdroid/util/SensitiveString.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/util/SensitiveString.kt
@@ -0,0 +1,69 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.util
+
+/**
+ * Wrapper for passwords and other sensitive strings so that they're not directly [String]s,
+ * so that they're less likely to be used in clear-text unintentionally, like being printed in logs
+ * by [Any.toString].
+ *
+ * This class does not address the issue that clear-text passwords are stored in memory. This problem
+ * could only be reduced if we would consequently store and process only encrypted passwords, with the
+ * exception of some "providePassword" method that provides the clear-text password for a lambda function as
+ * [CharArray] and wipes out the array values after usage.
+ *
+ * See also:
+ *
+ * - https://stackoverflow.com/a/8889285
+ * - https://javaee.github.io/security-api/apidocs/javax/security/enterprise/credential/Password.html and
+ *   https://javaee.github.io/security-api/apidocs/javax/security/enterprise/credential/UsernamePasswordCredential.html
+ */
+class SensitiveString private constructor(
+    private val data: String
+) {
+
+    /**
+     * Returns the sensitive string as a [CharArray].
+     *
+     * _Be careful when using it (for instance, don't print its content unintentionally)._
+     */
+    fun asCharArray() = data.toCharArray()
+
+    /**
+     * Returns the sensitive string as an immutable [String].
+     *
+     * _Be careful when using it (for instance, don't print it unintentionally)._
+     */
+    fun asString() = data
+
+
+    // make comparable by data
+
+    override fun equals(other: Any?) =
+        if (other is SensitiveString)
+            data == other.data
+        else
+            false
+
+    override fun hashCode() = data.hashCode()
+
+
+    /**
+     * Overrides [toString] so that it doesn't expose the clear-text string (password).
+     */
+    override fun toString() = "*****"
+
+
+    companion object {
+
+        fun CharArray.toSensitiveString() =
+            SensitiveString(this.concatToString())
+
+        fun String.toSensitiveString() =
+            SensitiveString(this)
+
+    }
+
+}
--- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/CredentialsStore.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/CredentialsStore.kt
@@ -10,6 +10,7 @@ import androidx.core.content.edit
 import androidx.security.crypto.EncryptedSharedPreferences
 import androidx.security.crypto.MasterKey
 import at.bitfire.davdroid.settings.Credentials
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
 import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject

@@ -47,7 +48,7 @@ class CredentialsStore @Inject constructor(

        return Credentials(
            prefs.getString(keyName(mountId, USER_NAME), null),
-            prefs.getString(keyName(mountId, PASSWORD), null)?.toCharArray(),
+            prefs.getString(keyName(mountId, PASSWORD), null)?.toSensitiveString(),
            prefs.getString(keyName(mountId, CERTIFICATE_ALIAS), null)
        )
    }
@@ -57,7 +58,7 @@ class CredentialsStore @Inject constructor(
            if (credentials != null)
                putBoolean(keyName(mountId, HAS_CREDENTIALS), true)
                    .putString(keyName(mountId, USER_NAME), credentials.username)
-                    .putString(keyName(mountId, PASSWORD), credentials.password?.concatToString())
+                    .putString(keyName(mountId, PASSWORD), credentials.password?.asString())
                    .putString(keyName(mountId, CERTIFICATE_ALIAS), credentials.certificateAlias)
            else
                remove(keyName(mountId, HAS_CREDENTIALS))
--- a/app/src/test/kotlin/at/bitfire/davdroid/util/SensitiveStringTest.kt
+++ b/app/src/test/kotlin/at/bitfire/davdroid/util/SensitiveStringTest.kt
@@ -0,0 +1,49 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.util
+
+import at.bitfire.davdroid.util.SensitiveString.Companion.toSensitiveString
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class SensitiveStringTest {
+
+    private data class UsernameAndPassword(
+        val username: String,
+        val password: SensitiveString
+    )
+
+    @Test
+    fun `equals (other object)`() {
+        val password = "some-password".toSensitiveString()
+        assertFalse(password == Any())
+    }
+
+    @Test
+    fun `equals (other password)`() {
+        val password = "some-password".toSensitiveString()
+        assertFalse(password == "other-password".toSensitiveString())
+    }
+
+    @Test
+    fun `equals (same password)`() {
+        val password = "some-password".toSensitiveString()
+        assertTrue(password == "some-password".toSensitiveString())
+    }
+
+    @Test
+    fun `toString in data class`() {
+        val credentials = UsernameAndPassword(
+            "some-user",
+            "some-password".toSensitiveString()
+        )
+
+        val logMessage = "Credentials: $credentials"
+        assertEquals("Credentials: UsernameAndPassword(username=some-user, password=*****)", logMessage)
+    }
+
+}