From ff16d4c7f3629365e37e5f91953c5156bae89aab Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 21 Jul 2026 14:34:15 -0500 Subject: [PATCH] PM-40154: feat: Migrate to new keystore encrypted shared preferences --- .../datasource/disk/AuthDiskSourceImpl.kt | 24 +++- .../auth/datasource/disk/di/AuthDiskModule.kt | 3 + .../datasource/disk/CookieDiskSourceImpl.kt | 20 ++- .../datasource/disk/di/PlatformDiskModule.kt | 3 + .../datasource/disk/AuthDiskSourceTest.kt | 122 ++++++++++++------ .../datasource/disk/CookieDiskSourceTest.kt | 44 +++++++ .../datasource/disk/AuthDiskSourceImpl.kt | 13 ++ .../auth/datasource/disk/di/AuthDiskModule.kt | 3 + .../datasource/disk/AuthDiskSourceTest.kt | 50 +++++-- .../disk/BaseEncryptedDiskSource.kt | 41 ++++-- 10 files changed, 249 insertions(+), 74 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt index dbaa71ab248..c75fe8d2124 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt @@ -32,6 +32,7 @@ private const val BIOMETRICS_UNLOCK_KEY = "userKeyBiometricUnlock" private const val USER_AUTO_UNLOCK_KEY_KEY = "userKeyAutoUnlock" private const val DEVICE_KEY_KEY = "deviceKey" private const val PENDING_ADMIN_AUTH_REQUEST_KEY = "pendingAdminAuthRequest" +private const val ACCOUNT_CRYPTOGRAPHIC_STATE_KEY = "accountCryptographicState" // These keys should not be encrypted private const val UNIQUE_APP_ID_KEY = "appId" @@ -57,7 +58,6 @@ private const val ONBOARDING_STATUS_KEY = "onboardingStatus" private const val SHOW_IMPORT_LOGINS_KEY = "showImportLogins" private const val LAST_LOCK_TIMESTAMP = "lastLockTimestamp" private const val PROFILE_ACCOUNT_KEYS_KEY = "profileAccountKeys" -private const val ACCOUNT_CRYPTOGRAPHIC_STATE_KEY = "accountCryptographicState" /** * Primary implementation of [AuthDiskSource]. @@ -65,11 +65,13 @@ private const val ACCOUNT_CRYPTOGRAPHIC_STATE_KEY = "accountCryptographicState" @Suppress("TooManyFunctions") class AuthDiskSourceImpl( encryptedSharedPreferences: SharedPreferences, + keystoreEncryptedPreferences: SharedPreferences, sharedPreferences: SharedPreferences, legacySecureStorageMigrator: LegacySecureStorageMigrator, private val json: Json, ) : BaseEncryptedDiskSource( encryptedSharedPreferences = encryptedSharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, sharedPreferences = sharedPreferences, ), AuthDiskSource { @@ -125,6 +127,9 @@ class AuthDiskSourceImpl( // We must migrate the Private Key and Account Keys to use the Account Cryptographic state // from now on. migrateAccountKeys() + + // Migrate to the Keystore Encrypted SharedPreferences. + migrateToKeystoreEncryption() } override var authenticatorSyncSymmetricKey: ByteArray? @@ -270,10 +275,7 @@ class AuthDiskSourceImpl( } override fun getUserAutoUnlockKey(userId: String): String? = - getEncryptedString( - key = USER_AUTO_UNLOCK_KEY_KEY.appendIdentifier(userId), - default = null, - ) + getEncryptedString(key = USER_AUTO_UNLOCK_KEY_KEY.appendIdentifier(userId)) override fun storeUserAutoUnlockKey( userId: String, @@ -688,4 +690,16 @@ class AuthDiskSourceImpl( } } } + + private fun migrateToKeystoreEncryption() { + migrateKeyByPrefix(keyPrefix = AUTHENTICATOR_SYNC_SYMMETRIC_KEY) + migrateKeyByPrefix(keyPrefix = AUTHENTICATOR_SYNC_UNLOCK_KEY) + migrateKeyByPrefix(keyPrefix = ACCOUNT_CRYPTOGRAPHIC_STATE_KEY) + migrateKeyByPrefix(keyPrefix = USER_AUTO_UNLOCK_KEY_KEY) + migrateKeyByPrefix(keyPrefix = DEVICE_KEY_KEY) + migrateKeyByPrefix(keyPrefix = PENDING_ADMIN_AUTH_REQUEST_KEY) + migrateKeyByPrefix(keyPrefix = BIOMETRICS_INIT_VECTOR_KEY) + migrateKeyByPrefix(keyPrefix = BIOMETRICS_UNLOCK_KEY) + migrateKeyByPrefix(keyPrefix = ACCOUNT_TOKENS_KEY) + } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/di/AuthDiskModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/di/AuthDiskModule.kt index 330b3c87ed3..9b0b5e8a25f 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/di/AuthDiskModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/di/AuthDiskModule.kt @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.auth.datasource.disk.di import android.content.SharedPreferences import com.bitwarden.data.datasource.disk.di.EncryptedPreferences +import com.bitwarden.data.datasource.disk.di.KeystoreEncryptedPreferences import com.bitwarden.data.datasource.disk.di.UnencryptedPreferences import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSourceImpl @@ -24,12 +25,14 @@ object AuthDiskModule { @Singleton fun provideAuthDiskSource( @EncryptedPreferences encryptedSharedPreferences: SharedPreferences, + @KeystoreEncryptedPreferences keystoreEncryptedPreferences: SharedPreferences, @UnencryptedPreferences sharedPreferences: SharedPreferences, legacySecureStorageMigrator: LegacySecureStorageMigrator, json: Json, ): AuthDiskSource = AuthDiskSourceImpl( encryptedSharedPreferences = encryptedSharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, sharedPreferences = sharedPreferences, legacySecureStorageMigrator = legacySecureStorageMigrator, json = json, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceImpl.kt index f4ec6bffee9..4641a3568dc 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceImpl.kt @@ -8,7 +8,6 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.model.CookieConfigurati import kotlinx.serialization.json.Json private const val CONFIG_PREFIX = "elb_cookie_config" -private const val ENCRYPTED_PREFIX = "bwSecureStorage:$CONFIG_PREFIX" /** * Implementation of [CookieDiskSource] using encrypted SharedPreferences. @@ -17,14 +16,21 @@ private const val ENCRYPTED_PREFIX = "bwSecureStorage:$CONFIG_PREFIX" */ class CookieDiskSourceImpl( sharedPreferences: SharedPreferences, - private val encryptedSharedPreferences: SharedPreferences, + private val keystoreEncryptedPreferences: SharedPreferences, + encryptedSharedPreferences: SharedPreferences, private val json: Json, ) : CookieDiskSource, BaseEncryptedDiskSource( sharedPreferences = sharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, encryptedSharedPreferences = encryptedSharedPreferences, ) { + init { + // Migrate to the Keystore Encrypted SharedPreferences. + migrateToKeystoreEncryption() + } + override fun getCookieConfig(hostname: String): CookieConfigurationData? { val key = CONFIG_PREFIX.appendIdentifier(hostname) return getEncryptedString(key) @@ -37,12 +43,16 @@ class CookieDiskSourceImpl( } override fun clearCookies() { - val keysToRemove = encryptedSharedPreferences + val keysToRemove = keystoreEncryptedPreferences .all .keys - .filter { it.startsWith(ENCRYPTED_PREFIX) } - encryptedSharedPreferences.edit { + .filter { it.startsWith(CONFIG_PREFIX) } + keystoreEncryptedPreferences.edit { keysToRemove.forEach { key -> remove(key) } } } + + private fun migrateToKeystoreEncryption() { + migrateKeyByPrefix(keyPrefix = CONFIG_PREFIX) + } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/di/PlatformDiskModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/di/PlatformDiskModule.kt index 1fbabc3a6b7..2e6e3d96650 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/di/PlatformDiskModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/di/PlatformDiskModule.kt @@ -7,6 +7,7 @@ import androidx.room.Room import com.bitwarden.core.data.manager.dispatcher.DispatcherManager import com.bitwarden.data.datasource.disk.FlightRecorderDiskSource import com.bitwarden.data.datasource.disk.di.EncryptedPreferences +import com.bitwarden.data.datasource.disk.di.KeystoreEncryptedPreferences import com.bitwarden.data.datasource.disk.di.UnencryptedPreferences import com.x8bit.bitwarden.data.platform.datasource.disk.CookieDiskSource import com.x8bit.bitwarden.data.platform.datasource.disk.CookieDiskSourceImpl @@ -162,10 +163,12 @@ object PlatformDiskModule { @Singleton fun provideCookieDiskSource( @UnencryptedPreferences sharedPreferences: SharedPreferences, + @KeystoreEncryptedPreferences keystoreEncryptedPreferences: SharedPreferences, @EncryptedPreferences encryptedSharedPreferences: SharedPreferences, json: Json, ): CookieDiskSource = CookieDiskSourceImpl( sharedPreferences = sharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, encryptedSharedPreferences = encryptedSharedPreferences, json = json, ) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index 6133de6d19c..b31c1e7d551 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -38,6 +38,7 @@ import java.time.Instant @Suppress("LargeClass") class AuthDiskSourceTest { private val fakeEncryptedSharedPreferences = FakeSharedPreferences() + private val fakeKeystoreEncryptedPreferences = FakeSharedPreferences() private val fakeSharedPreferences = FakeSharedPreferences().apply { edit(commit = true) { putString("bwPreferencesStorage:masterKeyEncryptedUserKey", "testUserKey") @@ -51,6 +52,7 @@ class AuthDiskSourceTest { private val authDiskSource = AuthDiskSourceImpl( encryptedSharedPreferences = fakeEncryptedSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, sharedPreferences = fakeSharedPreferences, legacySecureStorageMigrator = legacySecureStorageMigrator, json = json, @@ -61,6 +63,42 @@ class AuthDiskSourceTest { verify(exactly = 1) { legacySecureStorageMigrator.migrateIfNecessary() } } + @Test + @Suppress("MaxLineLength") + fun `initialization should migrate legacy encrypted values to the keystore encrypted preferences`() { + val mockUserId = "mockUserId" + val legacyValues = mapOf( + "authenticatorSyncSymmetric" to "mockSyncSymmetricKey", + "authenticatorSyncUnlock_$mockUserId" to "mockSyncUnlockKey", + "accountCryptographicState_$mockUserId" to "mockCryptographicState", + "userKeyAutoUnlock_$mockUserId" to "mockUserAutoUnlockKey", + "deviceKey_$mockUserId" to "mockDeviceKey", + "pendingAdminAuthRequest_$mockUserId" to "mockPendingAuthRequest", + "biometricInitializationVector_$mockUserId" to "mockInitVector", + "userKeyBiometricUnlock_$mockUserId" to "mockBiometricsKey", + "accountTokens_$mockUserId" to "mockAccountTokens", + ) + fakeEncryptedSharedPreferences.edit { + legacyValues.forEach { (key, value) -> + putString("bwSecureStorage:$key", value) + } + } + + AuthDiskSourceImpl( + encryptedSharedPreferences = fakeEncryptedSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, + sharedPreferences = fakeSharedPreferences, + legacySecureStorageMigrator = legacySecureStorageMigrator, + json = json, + ) + + // The values are moved to the keystore encrypted preferences without the legacy prefix. + assertTrue(fakeEncryptedSharedPreferences.all.isEmpty()) + legacyValues.forEach { (key, value) -> + assertEquals(value, fakeKeystoreEncryptedPreferences.getString(key, null)) + } + } + @Test fun `initialization should trigger a legacy user key removal`() { assertNull( @@ -441,12 +479,12 @@ class AuthDiskSourceTest { @Test fun `getAccountCryptographicState should pull from SharedPreferences`() { - val accountKeysBaseKey = "bwSecureStorage:accountCryptographicState" + val accountKeysBaseKey = "accountCryptographicState" val mockUserId = "mockUserId" val mockAccountCryptographicState = WrappedAccountCryptographicState.V1( privateKey = "privateKey", ) - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString( "${accountKeysBaseKey}_$mockUserId", json.encodeToString( @@ -464,7 +502,7 @@ class AuthDiskSourceTest { @Test fun `storeAccountCryptographicState should update sharedPreferences`() { - val accountKeysBaseKey = "bwSecureStorage:accountCryptographicState" + val accountKeysBaseKey = "accountCryptographicState" val mockUserId = "mockUserId" val mockAccountCryptographicState = WrappedAccountCryptographicState.V1( privateKey = "privateKey", @@ -473,7 +511,7 @@ class AuthDiskSourceTest { userId = mockUserId, accountCryptographicState = mockAccountCryptographicState, ) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( "${accountKeysBaseKey}_$mockUserId", null, ) @@ -526,10 +564,10 @@ class AuthDiskSourceTest { @Test fun `getUserAutoUnlockKey should pull from SharedPreferences`() { - val userAutoUnlockKeyBaseKey = "bwSecureStorage:userKeyAutoUnlock" + val userAutoUnlockKeyBaseKey = "userKeyAutoUnlock" val mockUserId = "mockUserId" val mockUserAutoUnlockKey = "mockUserAutoUnlockKey" - fakeEncryptedSharedPreferences + fakeKeystoreEncryptedPreferences .edit { putString( "${userAutoUnlockKeyBaseKey}_$mockUserId", @@ -545,14 +583,14 @@ class AuthDiskSourceTest { @Test fun `storeUserAutoUnlockKey should update SharedPreferences`() { - val userAutoUnlockKeyBaseKey = "bwSecureStorage:userKeyAutoUnlock" + val userAutoUnlockKeyBaseKey = "userKeyAutoUnlock" val mockUserId = "mockUserId" val mockUserAutoUnlockKey = "mockUserAutoUnlockKey" authDiskSource.storeUserAutoUnlockKey( userId = mockUserId, userAutoUnlockKey = mockUserAutoUnlockKey, ) - val actual = fakeEncryptedSharedPreferences + val actual = fakeKeystoreEncryptedPreferences .getString( "${userAutoUnlockKeyBaseKey}_$mockUserId", null, @@ -565,23 +603,23 @@ class AuthDiskSourceTest { @Test fun `getDeviceKey should pull from SharedPreferences`() { - val deviceKeyBaseKey = "bwSecureStorage:deviceKey" + val deviceKeyBaseKey = "deviceKey" val mockUserId = "mockUserId" val deviceKeyKey = "${deviceKeyBaseKey}_$mockUserId" val devicesKey = "1234" - fakeEncryptedSharedPreferences.edit { putString(deviceKeyKey, devicesKey) } + fakeKeystoreEncryptedPreferences.edit { putString(deviceKeyKey, devicesKey) } val actual = authDiskSource.getDeviceKey(userId = mockUserId) assertEquals(devicesKey, actual) } @Test fun `storeDeviceKey for non-null values should update SharedPreferences`() { - val deviceKeyBaseKey = "bwSecureStorage:deviceKey" + val deviceKeyBaseKey = "deviceKey" val mockUserId = "mockUserId" val deviceKeyKey = "${deviceKeyBaseKey}_$mockUserId" val devicesKey = "1234" authDiskSource.storeDeviceKey(userId = mockUserId, deviceKey = devicesKey) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = deviceKeyKey, defaultValue = null, ) @@ -590,21 +628,21 @@ class AuthDiskSourceTest { @Test fun `storeDeviceKey for null values should clear SharedPreferences`() { - val deviceKeyBaseKey = "bwSecureStorage:deviceKey" + val deviceKeyBaseKey = "deviceKey" val mockUserId = "mockUserId" val deviceKeyKey = "${deviceKeyBaseKey}_$mockUserId" val deviceKey = "1234" - fakeEncryptedSharedPreferences.edit { putString(deviceKeyKey, deviceKey) } + fakeKeystoreEncryptedPreferences.edit { putString(deviceKeyKey, deviceKey) } authDiskSource.storeDeviceKey(userId = mockUserId, deviceKey = null) - assertFalse(fakeEncryptedSharedPreferences.contains(deviceKeyKey)) + assertFalse(fakeKeystoreEncryptedPreferences.contains(deviceKeyKey)) } @Test fun `getPendingAuthRequest should pull from SharedPreferences`() { - val pendingAdminAuthRequestBaseKey = "bwSecureStorage:pendingAdminAuthRequest" + val pendingAdminAuthRequestBaseKey = "pendingAdminAuthRequest" val mockUserId = "mockUserId" val pendingAdminAuthRequestKey = "${pendingAdminAuthRequestBaseKey}_$mockUserId" - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString( pendingAdminAuthRequestKey, """ @@ -631,7 +669,7 @@ class AuthDiskSourceTest { @Test fun `storePendingAuthRequest for non-null values should update SharedPreferences`() { - val pendingAdminAuthRequestKeyBaseKey = "bwSecureStorage:pendingAdminAuthRequest" + val pendingAdminAuthRequestKeyBaseKey = "pendingAdminAuthRequest" val mockUserId = "mockUserId" val pendingAuthRequestKey = "${pendingAdminAuthRequestKeyBaseKey}_$mockUserId" val pendingAdminAuthRequest = PendingAuthRequestJson( @@ -644,7 +682,7 @@ class AuthDiskSourceTest { userId = mockUserId, pendingAuthRequest = pendingAdminAuthRequest, ) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = pendingAuthRequestKey, defaultValue = null, ) @@ -666,11 +704,11 @@ class AuthDiskSourceTest { @Test fun `getUserBiometricUnlockKey should pull from SharedPreferences`() { - val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val biometricsKeyBaseKey = "userKeyBiometricUnlock" val mockUserId = "mockUserId" val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" val biometricsKey = "1234" - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString(biometricsKeyKey, biometricsKey) } val actual = authDiskSource.getUserBiometricUnlockKey(userId = mockUserId) @@ -698,12 +736,12 @@ class AuthDiskSourceTest { @Test fun `storeUserBiometricInitVector for non-null values should update SharedPreferences`() { - val biometricsInitVectorBaseKey = "bwSecureStorage:biometricInitializationVector" + val biometricsInitVectorBaseKey = "biometricInitializationVector" val mockUserId = "mockUserId" val biometricsInitVectorKey = "${biometricsInitVectorBaseKey}_$mockUserId" val initVector = byteArrayOf(1, 2) authDiskSource.storeUserBiometricInitVector(userId = mockUserId, iv = initVector) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = biometricsInitVectorKey, defaultValue = null, ) @@ -712,20 +750,20 @@ class AuthDiskSourceTest { @Test fun `storeUserBiometricInitVector for null values should clear SharedPreferences`() { - val biometricsInitVectorBaseKey = "bwSecureStorage:biometricInitializationVector" + val biometricsInitVectorBaseKey = "biometricInitializationVector" val mockUserId = "mockUserId" val biometricsInitVectorKey = "${biometricsInitVectorBaseKey}_$mockUserId" val initVector = "1234" - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString(biometricsInitVectorKey, initVector) } authDiskSource.storeUserBiometricInitVector(userId = mockUserId, iv = null) - assertFalse(fakeEncryptedSharedPreferences.contains(biometricsInitVectorKey)) + assertFalse(fakeKeystoreEncryptedPreferences.contains(biometricsInitVectorKey)) } @Test fun `storeUserBiometricUnlockKey for non-null values should update SharedPreferences`() { - val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val biometricsKeyBaseKey = "userKeyBiometricUnlock" val mockUserId = "mockUserId" val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" val biometricsKey = "1234" @@ -733,7 +771,7 @@ class AuthDiskSourceTest { userId = mockUserId, biometricsKey = biometricsKey, ) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = biometricsKeyKey, defaultValue = null, ) @@ -742,18 +780,18 @@ class AuthDiskSourceTest { @Test fun `storeUserBiometricUnlockKey for null values should clear SharedPreferences`() { - val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val biometricsKeyBaseKey = "userKeyBiometricUnlock" val mockUserId = "mockUserId" val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" val biometricsKey = "1234" - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString(biometricsKeyKey, biometricsKey) } authDiskSource.storeUserBiometricUnlockKey( userId = mockUserId, biometricsKey = null, ) - assertFalse(fakeEncryptedSharedPreferences.contains(biometricsKeyKey)) + assertFalse(fakeKeystoreEncryptedPreferences.contains(biometricsKeyKey)) } @Suppress("MaxLineLength") @@ -1180,13 +1218,13 @@ class AuthDiskSourceTest { @Test fun `getAccountTokens should pull from SharedPreferences`() { - val baseKey = "bwSecureStorage:accountTokens" + val baseKey = "accountTokens" val mockUserId = "mockUserId" val accountTokens = AccountTokensJson( accessToken = "accessToken", refreshToken = "refreshToken", ) - fakeEncryptedSharedPreferences.edit { + fakeKeystoreEncryptedPreferences.edit { putString("${baseKey}_$mockUserId", json.encodeToString(accountTokens)) } val actual = authDiskSource.getAccountTokens(userId = mockUserId) @@ -1223,7 +1261,7 @@ class AuthDiskSourceTest { @Test fun `storeAccountTokens should update SharedPreferences`() { - val baseKey = "bwSecureStorage:accountTokens" + val baseKey = "accountTokens" val mockUserId = "mockUserId" val accountTokens = AccountTokensJson( accessToken = "accessToken", @@ -1233,7 +1271,7 @@ class AuthDiskSourceTest { userId = mockUserId, accountTokens = accountTokens, ) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = "${baseKey}_$mockUserId", defaultValue = null, ) @@ -1245,10 +1283,10 @@ class AuthDiskSourceTest { @Test fun `getAuthenticatorSyncUnlockKey should pull from SharedPreferences`() { - val authenticatorSyncUnlockKey = "bwSecureStorage:authenticatorSyncUnlock" + val authenticatorSyncUnlockKey = "authenticatorSyncUnlock" val mockUserId = "mockUserId" val mockAuthenticatorSyncUnlockKey = "mockAuthSyncUnlockKey" - fakeEncryptedSharedPreferences + fakeKeystoreEncryptedPreferences .edit { putString( "${authenticatorSyncUnlockKey}_$mockUserId", @@ -1264,7 +1302,7 @@ class AuthDiskSourceTest { @Test fun `storeAuthenticatorSyncUnlockKey should update SharedPreferences`() { - val authenticatorSyncUnlockKey = "bwSecureStorage:authenticatorSyncUnlock" + val authenticatorSyncUnlockKey = "authenticatorSyncUnlock" val mockUserId = "mockUserId" val mockAuthenticatorSyncUnlockKey = "mockAuthSyncUnlockKey" authDiskSource.storeAuthenticatorSyncUnlockKey( @@ -1272,7 +1310,7 @@ class AuthDiskSourceTest { authenticatorSyncUnlockKey = mockAuthenticatorSyncUnlockKey, ) - val actual = fakeEncryptedSharedPreferences.getString( + val actual = fakeKeystoreEncryptedPreferences.getString( key = "${authenticatorSyncUnlockKey}_$mockUserId", defaultValue = null, ) @@ -1332,18 +1370,18 @@ class AuthDiskSourceTest { @Test fun `authenticatorSyncSymmetricKey should store and update from EncryptedSharedPreferences`() { - val sharedPrefsKey = "bwSecureStorage:authenticatorSyncSymmetric" + val sharedPrefsKey = "authenticatorSyncSymmetric" // Shared preferences and the repository start with the same value: assertNull(authDiskSource.authenticatorSyncSymmetricKey) - assertNull(fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null)) + assertNull(fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null)) // Updating the repository updates shared preferences: val symmetricKey = generateSecretKey().getOrThrow().encoded authDiskSource.authenticatorSyncSymmetricKey = symmetricKey assertEquals( symmetricKey.toString(Charsets.ISO_8859_1), - fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null), + fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null), ) // Retrieving the key from repository should give same byte array despite String conversion: diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceTest.kt index 9f35cb3b629..90b6bb6bc3b 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/CookieDiskSourceTest.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.platform.datasource.disk +import androidx.core.content.edit import com.bitwarden.core.di.CoreModule import com.bitwarden.data.datasource.disk.base.FakeSharedPreferences import com.x8bit.bitwarden.data.platform.datasource.disk.model.CookieConfigurationData @@ -10,15 +11,58 @@ import org.junit.jupiter.api.Test class CookieDiskSourceTest { private val fakeEncryptedSharedPreferences = FakeSharedPreferences() + private val fakeKeystoreEncryptedPreferences = FakeSharedPreferences() private val fakeSharedPreferences = FakeSharedPreferences() private val json = CoreModule.providesJson(buildInfoManager = mockk(relaxed = true)) private val cookieDiskSource: CookieDiskSource = CookieDiskSourceImpl( sharedPreferences = fakeSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, encryptedSharedPreferences = fakeEncryptedSharedPreferences, json = json, ) + @Test + @Suppress("MaxLineLength") + fun `initialization should migrate legacy encrypted values to the keystore encrypted preferences`() { + val hostname = "vault.bitwarden.com" + val config = CookieConfigurationData( + hostname = hostname, + cookies = listOf( + CookieConfigurationData.Cookie( + name = "BW_SESSION", + value = "encrypted_cookie_value", + ), + ), + ) + val legacyKey = "bwSecureStorage:elb_cookie_config_$hostname" + fakeEncryptedSharedPreferences.edit { + putString(legacyKey, json.encodeToString(config)) + } + + val diskSource = CookieDiskSourceImpl( + sharedPreferences = fakeSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, + encryptedSharedPreferences = fakeEncryptedSharedPreferences, + json = json, + ) + + // The value is moved to the keystore encrypted preferences without the legacy prefix. + assertNull(fakeEncryptedSharedPreferences.getString(legacyKey, null)) + assertEquals( + json.parseToJsonElement(json.encodeToString(config)), + json.parseToJsonElement( + requireNotNull( + fakeKeystoreEncryptedPreferences.getString( + "elb_cookie_config_$hostname", + null, + ), + ), + ), + ) + assertEquals(config, diskSource.getCookieConfig(hostname)) + } + @Test fun `getCookieConfig should return null when no config exists`() { assertNull(cookieDiskSource.getCookieConfig("example.com")) diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceImpl.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceImpl.kt index c590cf4f679..a8c40e6ec0b 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceImpl.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceImpl.kt @@ -18,14 +18,21 @@ private const val UNIQUE_APP_ID_KEY = "appId" */ class AuthDiskSourceImpl( encryptedSharedPreferences: SharedPreferences, + keystoreEncryptedPreferences: SharedPreferences, sharedPreferences: SharedPreferences, ) : BaseEncryptedDiskSource( encryptedSharedPreferences = encryptedSharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, sharedPreferences = sharedPreferences, ), AuthDiskSource { private val mutableUserBiometricUnlockKeyFlow = bufferedMutableSharedFlow(replay = 1) + init { + // Migrate to the Keystore Encrypted SharedPreferences. + migrateToKeystoreEncryption() + } + override val uniqueAppId: String get() = getString(key = UNIQUE_APP_ID_KEY) ?: generateAndStoreUniqueAppId() @@ -86,4 +93,10 @@ class AuthDiskSourceImpl( .also { putString(key = UNIQUE_APP_ID_KEY, value = it) } + + private fun migrateToKeystoreEncryption() { + migrateKeyByPrefix(keyPrefix = BIOMETRICS_UNLOCK_KEY) + migrateKeyByPrefix(keyPrefix = BIOMETRICS_INIT_VECTOR_KEY) + migrateKeyByPrefix(keyPrefix = AUTHENTICATOR_SYNC_SYMMETRIC_KEY) + } } diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/di/AuthDiskModule.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/di/AuthDiskModule.kt index 7f7e2af7eed..57237306010 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/di/AuthDiskModule.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/di/AuthDiskModule.kt @@ -4,6 +4,7 @@ import android.content.SharedPreferences import com.bitwarden.authenticator.data.auth.datasource.disk.AuthDiskSource import com.bitwarden.authenticator.data.auth.datasource.disk.AuthDiskSourceImpl import com.bitwarden.data.datasource.disk.di.EncryptedPreferences +import com.bitwarden.data.datasource.disk.di.KeystoreEncryptedPreferences import com.bitwarden.data.datasource.disk.di.UnencryptedPreferences import dagger.Module import dagger.Provides @@ -22,10 +23,12 @@ object AuthDiskModule { @Singleton fun provideAuthDiskSource( @EncryptedPreferences encryptedSharedPreferences: SharedPreferences, + @KeystoreEncryptedPreferences keystoreEncryptedPreferences: SharedPreferences, @UnencryptedPreferences sharedPreferences: SharedPreferences, ): AuthDiskSource = AuthDiskSourceImpl( encryptedSharedPreferences = encryptedSharedPreferences, + keystoreEncryptedPreferences = keystoreEncryptedPreferences, sharedPreferences = sharedPreferences, ) } diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceTest.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceTest.kt index 4b97102b52e..83a9092efe2 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -11,28 +11,61 @@ import org.junit.jupiter.api.Test class AuthDiskSourceTest { private val fakeEncryptedSharedPreferences = FakeSharedPreferences() + private val fakeKeystoreEncryptedPreferences = FakeSharedPreferences() private val fakeSharedPreferences = FakeSharedPreferences() private val authDiskSource = AuthDiskSourceImpl( encryptedSharedPreferences = fakeEncryptedSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, sharedPreferences = fakeSharedPreferences, ) @Test @Suppress("MaxLineLength") - fun `authenticatorBridgeSymmetricSyncKey should store and update from EncryptedSharedPreferences`() { - val sharedPrefsKey = "bwSecureStorage:authenticatorSyncSymmetricKey" + fun `initialization should migrate legacy encrypted values to the keystore encrypted preferences`() { + fakeEncryptedSharedPreferences.edit { + putString("bwSecureStorage:userKeyBiometricUnlock", "biometricsKey") + putString("bwSecureStorage:biometricInitializationVector", "initVector") + putString("bwSecureStorage:authenticatorSyncSymmetricKey", "symmetricKey") + } + + AuthDiskSourceImpl( + encryptedSharedPreferences = fakeEncryptedSharedPreferences, + keystoreEncryptedPreferences = fakeKeystoreEncryptedPreferences, + sharedPreferences = fakeSharedPreferences, + ) + + // The values are moved to the keystore encrypted preferences without the legacy prefix. + assertTrue(fakeEncryptedSharedPreferences.all.isEmpty()) + assertEquals( + "biometricsKey", + fakeKeystoreEncryptedPreferences.getString("userKeyBiometricUnlock", null), + ) + assertEquals( + "initVector", + fakeKeystoreEncryptedPreferences.getString("biometricInitializationVector", null), + ) + assertEquals( + "symmetricKey", + fakeKeystoreEncryptedPreferences.getString("authenticatorSyncSymmetricKey", null), + ) + } + + @Test + @Suppress("MaxLineLength") + fun `authenticatorBridgeSymmetricSyncKey should store and update from keystore encrypted preferences`() { + val sharedPrefsKey = "authenticatorSyncSymmetricKey" // Shared preferences and the repository start with the same value: assertNull(authDiskSource.authenticatorBridgeSymmetricSyncKey) - assertNull(fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null)) + assertNull(fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null)) // Updating the repository updates shared preferences: val symmetricKey = generateSecretKey().getOrThrow().encoded authDiskSource.authenticatorBridgeSymmetricSyncKey = symmetricKey assertEquals( symmetricKey.toString(Charsets.ISO_8859_1), - fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null), + fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null), ) // Retrieving the key from repository should give same byte array despite String conversion: @@ -40,19 +73,20 @@ class AuthDiskSourceTest { } @Test - fun `userBiometricKeyInitVector should store and update from EncryptedSharedPreferences`() { - val sharedPrefsKey = "bwSecureStorage:biometricInitializationVector" + @Suppress("MaxLineLength") + fun `userBiometricKeyInitVector should store and update from keystore encrypted preferences`() { + val sharedPrefsKey = "biometricInitializationVector" // Shared preferences and the repository start with the same value: assertNull(authDiskSource.userBiometricKeyInitVector) - assertNull(fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null)) + assertNull(fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null)) // Updating the repository updates shared preferences: val userBiometricKeyInitVector = byteArrayOf(3, 4) authDiskSource.userBiometricKeyInitVector = userBiometricKeyInitVector assertEquals( userBiometricKeyInitVector.toString(Charsets.ISO_8859_1), - fakeEncryptedSharedPreferences.getString(sharedPrefsKey, null), + fakeKeystoreEncryptedPreferences.getString(sharedPrefsKey, null), ) // Retrieving the key from repository should give same byte array despite String conversion: diff --git a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/BaseEncryptedDiskSource.kt b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/BaseEncryptedDiskSource.kt index 7a47b3b44d6..abe37e4fffd 100644 --- a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/BaseEncryptedDiskSource.kt +++ b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/BaseEncryptedDiskSource.kt @@ -2,31 +2,44 @@ package com.bitwarden.data.datasource.disk import android.content.SharedPreferences import androidx.core.content.edit -import androidx.security.crypto.EncryptedSharedPreferences + +private const val LEGACY_PREFIX: String = "bwSecureStorage:" /** - * Base class for simplifying interactions with [SharedPreferences] and - * [EncryptedSharedPreferences]. + * Base class for simplifying interactions with [SharedPreferences], this includes both regular + * and encrypted storage. */ @Suppress("UnnecessaryAbstractClass") abstract class BaseEncryptedDiskSource( sharedPreferences: SharedPreferences, private val encryptedSharedPreferences: SharedPreferences, -) : BaseDiskSource( - sharedPreferences = sharedPreferences, -) { + private val keystoreEncryptedPreferences: SharedPreferences, +) : BaseDiskSource(sharedPreferences = sharedPreferences) { protected fun getEncryptedString( key: String, - default: String? = null, - ): String? = encryptedSharedPreferences.getString(key.withBase(), default) + ): String? = keystoreEncryptedPreferences.getString(key, null) protected fun putEncryptedString( key: String, value: String?, - ): Unit = encryptedSharedPreferences.edit { putString(key.withBase(), value) } -} + ) { + keystoreEncryptedPreferences.edit { putString(key, value) } + } -/** - * Helper method for prepending the key with the appropriate base storage key. - */ -private fun String.withBase(): String = "bwSecureStorage:$this" + protected fun migrateKeyByPrefix(keyPrefix: String) { + encryptedSharedPreferences + .all + .keys + .filter { it.startsWith(prefix = "$LEGACY_PREFIX$keyPrefix") } + .forEach { key -> + // Move the value to the new file without the base prefix. + encryptedSharedPreferences.getString(key, null)?.let { value -> + keystoreEncryptedPreferences.edit(commit = true) { + putString(key.removePrefix(prefix = LEGACY_PREFIX), value) + } + } + // Then ensure the original value is deleted. + encryptedSharedPreferences.edit(commit = true) { remove(key) } + } + } +}