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..c3d67468064 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 @@ -17,14 +17,21 @@ private const val ENCRYPTED_PREFIX = "bwSecureStorage:$CONFIG_PREFIX" */ class CookieDiskSourceImpl( sharedPreferences: SharedPreferences, + keystoreEncryptedPreferences: SharedPreferences, private val 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) @@ -45,4 +52,8 @@ class CookieDiskSourceImpl( 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/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/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..89c49ba6141 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 @@ -12,18 +12,33 @@ import androidx.security.crypto.EncryptedSharedPreferences 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.withBase(), null) protected fun putEncryptedString( key: String, value: String?, - ): Unit = encryptedSharedPreferences.edit { putString(key.withBase(), value) } + ) { + keystoreEncryptedPreferences.edit { putString(key.withBase(), value) } + } + + protected fun migrateKeyByPrefix(keyPrefix: String) { + encryptedSharedPreferences + .all + .keys + .filter { it.startsWith(prefix = keyPrefix.withBase()) } + .forEach { key -> + // Move the value to the new file. + encryptedSharedPreferences.getString(key, null)?.let { value -> + keystoreEncryptedPreferences.edit(commit = true) { putString(key, value) } + } + // Then ensure the original value is deleted. + encryptedSharedPreferences.edit(commit = true) { remove(key) } + } + } } /** diff --git a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferences.kt b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferences.kt new file mode 100644 index 00000000000..6c97a547113 --- /dev/null +++ b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferences.kt @@ -0,0 +1,184 @@ +package com.bitwarden.data.datasource.disk + +import android.app.Application +import android.content.Context +import android.content.SharedPreferences +import android.security.keystore.KeyProperties +import com.bitwarden.annotation.OmitFromCoverage +import com.bitwarden.data.manager.keystore.KeystoreManager +import java.security.Key +import java.util.Base64 +import javax.crypto.Cipher +import javax.crypto.spec.IvParameterSpec + +private const val SECRET_KEY_ALIAS: String = "KeystoreEncryptedSharedPreferences" +private const val IV_SIZE_BYTES: Int = 16 +private const val CIPHER_TRANSFORMATION: String = KeyProperties.KEY_ALGORITHM_AES + "/" + + KeyProperties.BLOCK_MODE_CBC + "/" + + KeyProperties.ENCRYPTION_PADDING_PKCS7 + +/** + * An implementation of [SharedPreferences] that encrypts the values using the AndroidKeystore. + */ +@OmitFromCoverage +@Suppress("TooManyFunctions") +internal class KeystoreEncryptedSharedPreferences( + app: Application, + private val keystoreManager: KeystoreManager, +) : SharedPreferences { + private val sharedPreferences: SharedPreferences = app.getSharedPreferences( + "${app.packageName}_keystore_encrypted_preferences", + Context.MODE_PRIVATE, + ) + + private val secretKey: Key? get() = keystoreManager.getKeyOrNull(alias = SECRET_KEY_ALIAS) + + override fun contains(key: String?): Boolean = sharedPreferences.contains(key) + + override fun edit(): SharedPreferences.Editor = Editor( + secretKey = secretKey, + editor = sharedPreferences.edit(), + ) + + override fun getAll(): Map = sharedPreferences.all + + override fun getString( + key: String, + defValue: String?, + ): String? = key.decryptedByteArray?.decodeToString() ?: defValue + + override fun getBoolean(key: String, defValue: Boolean): Boolean { + unsupportedFeature("getBoolean") + } + + override fun getFloat(key: String, defValue: Float): Float { + unsupportedFeature("getFloat") + } + + override fun getInt(key: String, defValue: Int): Int { + unsupportedFeature("getInt") + } + + override fun getLong(key: String, defValue: Long): Long { + unsupportedFeature("getLong") + } + + override fun getStringSet(key: String, defValues: Set?): Set? { + unsupportedFeature("getStringSet") + } + + override fun registerOnSharedPreferenceChangeListener( + listener: SharedPreferences.OnSharedPreferenceChangeListener, + ) { + sharedPreferences.registerOnSharedPreferenceChangeListener(listener) + } + + override fun unregisterOnSharedPreferenceChangeListener( + listener: SharedPreferences.OnSharedPreferenceChangeListener, + ) { + sharedPreferences.unregisterOnSharedPreferenceChangeListener(listener) + } + + private val String.decryptedByteArray: ByteArray? + get() = sharedPreferences.getString(this, null)?.decrypt(key = secretKey) +} + +@Suppress("TooManyFunctions") +private class Editor( + private val secretKey: Key?, + private val editor: SharedPreferences.Editor, +) : SharedPreferences.Editor { + override fun apply(): Unit = editor.apply() + + override fun clear(): SharedPreferences.Editor = editor.clear() + + override fun commit(): Boolean = editor.commit() + + override fun putString( + key: String, + value: String?, + ): SharedPreferences.Editor = value + ?.let { encryptAndPutByteArray(key = key, value = it.encodeToByteArray()) } + ?: remove(key = key) + + override fun putBoolean(key: String, value: Boolean): SharedPreferences.Editor { + unsupportedFeature("putBoolean") + } + + override fun putFloat(key: String, value: Float): SharedPreferences.Editor { + unsupportedFeature("putFloat") + } + + override fun putInt(key: String, value: Int): SharedPreferences.Editor { + unsupportedFeature("putInt") + } + + override fun putLong(key: String, value: Long): SharedPreferences.Editor { + unsupportedFeature("putLong") + } + + override fun putStringSet(key: String, values: Set?): SharedPreferences.Editor { + unsupportedFeature("putStringSet") + } + + override fun remove(key: String): SharedPreferences.Editor = editor.remove(key) + + private fun encryptAndPutByteArray( + key: String, + value: ByteArray, + ): SharedPreferences.Editor = editor.putString(key, value.encrypt(key = secretKey)) +} + +private sealed class InitCipher { + data class Decrypt(val iv: ByteArray) : InitCipher() { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + return iv.contentEquals(other = (other as Decrypt).iv) + } + + override fun hashCode(): Int = iv.contentHashCode() + } + + data object Encrypt : InitCipher() +} + +private fun unsupportedFeature(name: String): Nothing { + throw UnsupportedOperationException( + "$name is not supported by KeystoreEncryptedSharedPreferences", + ) +} + +private fun String.decrypt(key: Key?): ByteArray? { + val bytes = Base64.getDecoder().decode(this) + val iv = bytes.copyOfRange(fromIndex = 0, toIndex = IV_SIZE_BYTES) + val cipherText = bytes.copyOfRange(fromIndex = IV_SIZE_BYTES, toIndex = bytes.size) + return key + ?.generateCipher(initCipher = InitCipher.Decrypt(iv = iv)) + ?.doFinal(cipherText) +} + +private fun ByteArray.encrypt(key: Key?): String? = key + ?.generateCipher(initCipher = InitCipher.Encrypt) + ?.let { cipher -> cipher.iv + cipher.doFinal(this) } + ?.let { Base64.getEncoder().encodeToString(it) } + +private fun Key.generateCipher( + initCipher: InitCipher, +): Cipher = Cipher + .getInstance(CIPHER_TRANSFORMATION) + .apply { + when (initCipher) { + is InitCipher.Decrypt -> { + init( + Cipher.DECRYPT_MODE, + this@generateCipher, + IvParameterSpec(initCipher.iv), + ) + } + + is InitCipher.Encrypt -> { + init(Cipher.ENCRYPT_MODE, this@generateCipher) + } + } + } diff --git a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/KeystoreEncryptedPreferences.kt b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/KeystoreEncryptedPreferences.kt new file mode 100644 index 00000000000..c8754f93067 --- /dev/null +++ b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/KeystoreEncryptedPreferences.kt @@ -0,0 +1,11 @@ +package com.bitwarden.data.datasource.disk.di + +import javax.inject.Qualifier + +/** + * Used to denote an instance of [android.content.SharedPreferences] that encrypts its data using + * the Keystore. + */ +@Qualifier +@Retention(AnnotationRetention.RUNTIME) +annotation class KeystoreEncryptedPreferences diff --git a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/PreferenceModule.kt b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/PreferenceModule.kt index cbb559714b6..0beb808c2c6 100644 --- a/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/PreferenceModule.kt +++ b/data/src/main/kotlin/com/bitwarden/data/datasource/disk/di/PreferenceModule.kt @@ -7,6 +7,8 @@ import android.content.Context import android.content.SharedPreferences import androidx.security.crypto.EncryptedSharedPreferences import androidx.security.crypto.MasterKey +import com.bitwarden.data.datasource.disk.KeystoreEncryptedSharedPreferences +import com.bitwarden.data.manager.keystore.KeystoreManager import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -33,6 +35,17 @@ object PreferenceModule { Context.MODE_PRIVATE, ) + @Provides + @Singleton + @KeystoreEncryptedPreferences + fun provideKeystoreEncryptedPreferences( + application: Application, + keystoreManager: KeystoreManager, + ): SharedPreferences = KeystoreEncryptedSharedPreferences( + app = application, + keystoreManager = keystoreManager, + ) + @Provides @Singleton @EncryptedPreferences diff --git a/data/src/main/kotlin/com/bitwarden/data/manager/di/DataManagerModule.kt b/data/src/main/kotlin/com/bitwarden/data/manager/di/DataManagerModule.kt index a5dfca3e178..e07e59f26a5 100644 --- a/data/src/main/kotlin/com/bitwarden/data/manager/di/DataManagerModule.kt +++ b/data/src/main/kotlin/com/bitwarden/data/manager/di/DataManagerModule.kt @@ -17,6 +17,8 @@ import com.bitwarden.data.manager.flightrecorder.FlightRecorderManager import com.bitwarden.data.manager.flightrecorder.FlightRecorderManagerImpl import com.bitwarden.data.manager.flightrecorder.FlightRecorderWriter import com.bitwarden.data.manager.flightrecorder.FlightRecorderWriterImpl +import com.bitwarden.data.manager.keystore.KeystoreManager +import com.bitwarden.data.manager.keystore.KeystoreManagerImpl import com.bitwarden.data.repository.ServerConfigRepository import com.bitwarden.network.service.DownloadService import dagger.Module @@ -90,6 +92,14 @@ object DataManagerModule { serverConfigRepository = serverConfigRepository, ) + @Provides + @Singleton + fun provideKeystoreManager( + buildInfoManager: BuildInfoManager, + ): KeystoreManager = KeystoreManagerImpl( + buildInfoManager = buildInfoManager, + ) + @Provides @Singleton fun provideNativeLibraryManager(): NativeLibraryManager = NativeLibraryManagerImpl() diff --git a/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManager.kt b/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManager.kt new file mode 100644 index 00000000000..083b6be8169 --- /dev/null +++ b/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManager.kt @@ -0,0 +1,34 @@ +package com.bitwarden.data.manager.keystore + +import java.security.Key + +/** + * Responsible for storing, retrieving, and removing symmetric keys in the Android Keystore. + * + * All aliases are automatically namespaced with the application ID before being written to or + * read from the keystore. + */ +interface KeystoreManager { + /** + * Returns the AES [Key] stored under [alias], generating and storing a new key if one + * does not already exist. Returns `null` if the key could not be retrieved or generated. + */ + fun getOrCreateKeyOrNull(alias: String): Key? + + /** + * Returns the AES [Key] stored under [alias], or `null` if no key exists or the key + * could not be recovered. + */ + fun getKeyOrNull(alias: String): Key? + + /** + * Returns `true` if a key exists in the keystore under [alias]. + */ + fun hasKey(alias: String): Boolean + + /** + * Removes the key stored under [alias] from the keystore. Returns `true` if the entry was + * removed successfully or `false` if the operation failed. + */ + fun removeKey(alias: String): Boolean +} diff --git a/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerImpl.kt b/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerImpl.kt new file mode 100644 index 00000000000..2007b99c003 --- /dev/null +++ b/data/src/main/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerImpl.kt @@ -0,0 +1,112 @@ +package com.bitwarden.data.manager.keystore + +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import com.bitwarden.core.data.manager.BuildInfoManager +import timber.log.Timber +import java.security.InvalidAlgorithmParameterException +import java.security.Key +import java.security.KeyStore +import java.security.KeyStoreException +import java.security.NoSuchAlgorithmException +import java.security.NoSuchProviderException +import java.security.ProviderException +import java.security.UnrecoverableKeyException +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey + +private const val KEYSTORE_TYPE_ANDROID: String = "AndroidKeyStore" + +/** + * Default implementation of [KeystoreManager]. + */ +class KeystoreManagerImpl( + private val buildInfoManager: BuildInfoManager, +) : KeystoreManager { + + private val androidKeystore: KeyStore by lazy { + KeyStore + .getInstance(KEYSTORE_TYPE_ANDROID) + .also { it.load(null) } + } + + override fun getOrCreateKeyOrNull( + alias: String, + ): Key? = getKeyOrNull(alias = alias) ?: generateKeyOrNull(alias = alias) + + override fun getKeyOrNull(alias: String): Key? = + try { + androidKeystore.getKey(alias.formatAlias(), null) + } catch (e: KeyStoreException) { + // Keystore was not loaded + Timber.w(e, "getKey failed to retrieve secret key") + null + } catch (e: NoSuchAlgorithmException) { + // Keystore algorithm cannot be found + Timber.w(e, "getKey failed to retrieve secret key") + null + } catch (e: UnrecoverableKeyException) { + // Key could not be recovered + Timber.w(e, "getKey failed to retrieve secret key") + null + } + + override fun hasKey(alias: String): Boolean = + try { + androidKeystore.containsAlias(alias.formatAlias()) + } catch (e: KeyStoreException) { + Timber.w(e, "hasKey failed to query keystore") + false + } + + override fun removeKey(alias: String): Boolean = + try { + androidKeystore.deleteEntry(alias.formatAlias()) + true + } catch (e: KeyStoreException) { + Timber.e(e, "removeKey failed to delete keystore entry") + false + } + + /** + * Generates and stores a new AES [SecretKey] under [alias], or `null` if a key cannot be + * generated. + */ + private fun generateKeyOrNull(alias: String): Key? { + val keyGen = try { + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, KEYSTORE_TYPE_ANDROID) + } catch (e: NoSuchAlgorithmException) { + Timber.w(e, "generateKeyOrNull failed to get key generator instance") + return null + } catch (e: NoSuchProviderException) { + Timber.w(e, "generateKeyOrNull failed to get key generator instance") + return null + } catch (e: IllegalArgumentException) { + Timber.w(e, "generateKeyOrNull failed to get key generator instance") + return null + } + + return try { + keyGen.init(getKeyGenParameterSpec(alias = alias)) + keyGen.generateKey() + } catch (e: InvalidAlgorithmParameterException) { + Timber.w(e, "generateKeyOrNull failed to initialize and generate key") + null + } catch (e: ProviderException) { + Timber.w(e, "generateKeyOrNull failed to initialize and generate key") + null + } + } + + private fun getKeyGenParameterSpec(alias: String): KeyGenParameterSpec = + KeyGenParameterSpec + .Builder( + alias.formatAlias(), + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, + ) + .setBlockModes(KeyProperties.BLOCK_MODE_CBC) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7) + .build() + + private fun String.formatAlias(): String = "${buildInfoManager.applicationId}.$this" +} diff --git a/data/src/test/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferencesTest.kt b/data/src/test/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferencesTest.kt new file mode 100644 index 00000000000..bc59f236879 --- /dev/null +++ b/data/src/test/kotlin/com/bitwarden/data/datasource/disk/KeystoreEncryptedSharedPreferencesTest.kt @@ -0,0 +1,206 @@ +package com.bitwarden.data.datasource.disk + +import android.app.Application +import android.content.Context +import com.bitwarden.data.datasource.disk.base.FakeSharedPreferences +import com.bitwarden.data.manager.keystore.KeystoreManager +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.util.Base64 +import javax.crypto.Cipher +import javax.crypto.SecretKey +import javax.crypto.spec.IvParameterSpec + +class KeystoreEncryptedSharedPreferencesTest { + + private val fakeSharedPreferences = FakeSharedPreferences() + private val mockApplication = mockk { + every { packageName } returns PACKAGE_NAME + every { + getSharedPreferences( + "${PACKAGE_NAME}_keystore_encrypted_preferences", + Context.MODE_PRIVATE, + ) + } returns fakeSharedPreferences + } + private val mockSecretKey = mockk() + private val mockKeystoreManager = mockk { + every { getOrCreateKeyOrNull(alias = SECRET_KEY_ALIAS) } returns mockSecretKey + every { getKeyOrNull(alias = SECRET_KEY_ALIAS) } returns mockSecretKey + } + + // Real Cipher code cannot run in a JVM unit test (the "PKCS7Padding" transformation is only + // registered on Android and MockK cannot call originals on the JPMS-protected + // javax.crypto.Cipher), so the cipher is fully mocked with a symmetric byte transformation + // that produces non-UTF-8 bytes, just like real ciphertext. + private val mockCipher = mockk { + every { init(Cipher.ENCRYPT_MODE, mockSecretKey) } just runs + every { init(Cipher.DECRYPT_MODE, mockSecretKey, any()) } just runs + every { iv } returns IV + every { doFinal(any()) } answers { firstArg().toggleBits() } + } + + private val keystoreEncryptedSharedPreferences = KeystoreEncryptedSharedPreferences( + app = mockApplication, + keystoreManager = mockKeystoreManager, + ) + + @BeforeEach + fun setUp() { + mockkStatic(Cipher::class) + every { Cipher.getInstance("AES/CBC/PKCS7Padding") } returns mockCipher + } + + @AfterEach + fun tearDown() { + unmockkStatic(Cipher::class) + } + + @Test + fun `putString followed by getString should return the original value`() { + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, VALUE) + .apply() + + assertEquals( + VALUE, + keystoreEncryptedSharedPreferences.getString(KEY, null), + ) + verify { + mockCipher.init( + Cipher.DECRYPT_MODE, + mockSecretKey, + match { it.iv.contentEquals(IV) }, + ) + } + } + + @Test + fun `putString should store a base64 encoded IV and ciphertext instead of the plaintext`() { + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, VALUE) + .apply() + + val storedValue = requireNotNull(fakeSharedPreferences.getString(KEY, null)) + assertNotEquals(VALUE, storedValue) + val storedBytes = Base64.getDecoder().decode(storedValue) + assertArrayEquals(IV, storedBytes.copyOfRange(fromIndex = 0, toIndex = IV.size)) + assertArrayEquals( + VALUE.encodeToByteArray().toggleBits(), + storedBytes.copyOfRange(fromIndex = IV.size, toIndex = storedBytes.size), + ) + } + + @Test + fun `putString with a null value should remove the stored value`() { + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, VALUE) + .apply() + + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, null) + .apply() + + assertFalse(fakeSharedPreferences.contains(KEY)) + assertNull(keystoreEncryptedSharedPreferences.getString(KEY, null)) + } + + @Test + fun `getString should return the default value when no value is stored`() { + assertEquals( + "mockDefault", + keystoreEncryptedSharedPreferences.getString(KEY, "mockDefault"), + ) + assertNull(keystoreEncryptedSharedPreferences.getString(KEY, null)) + } + + @Test + fun `getString should return the default value when the secret key is unavailable`() { + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, VALUE) + .apply() + every { mockKeystoreManager.getKeyOrNull(alias = SECRET_KEY_ALIAS) } returns null + + assertEquals( + "mockDefault", + keystoreEncryptedSharedPreferences.getString(KEY, "mockDefault"), + ) + } + + @Test + fun `contains should reflect the underlying preferences`() { + assertFalse(keystoreEncryptedSharedPreferences.contains(KEY)) + + keystoreEncryptedSharedPreferences + .edit() + .putString(KEY, VALUE) + .apply() + + assertTrue(keystoreEncryptedSharedPreferences.contains(KEY)) + } + + @Test + fun `unsupported get operations should throw UnsupportedOperationException`() { + assertThrows { + keystoreEncryptedSharedPreferences.getBoolean(KEY, false) + } + assertThrows { + keystoreEncryptedSharedPreferences.getInt(KEY, 0) + } + assertThrows { + keystoreEncryptedSharedPreferences.getLong(KEY, 0L) + } + assertThrows { + keystoreEncryptedSharedPreferences.getFloat(KEY, 0f) + } + assertThrows { + keystoreEncryptedSharedPreferences.getStringSet(KEY, null) + } + } + + @Test + fun `unsupported put operations should throw UnsupportedOperationException`() { + val editor = keystoreEncryptedSharedPreferences.edit() + assertThrows { editor.putBoolean(KEY, false) } + assertThrows { editor.putInt(KEY, 0) } + assertThrows { editor.putLong(KEY, 0L) } + assertThrows { editor.putFloat(KEY, 0f) } + assertThrows { editor.putStringSet(KEY, null) } + } +} + +/** + * A self-inverting stand-in for encryption that, like real ciphertext, produces bytes that are + * not valid UTF-8. + */ +private fun ByteArray.toggleBits(): ByteArray = + map { (it.toInt() xor 0xAA).toByte() }.toByteArray() + +private const val PACKAGE_NAME: String = "com.mock.app" +private const val SECRET_KEY_ALIAS: String = "KeystoreEncryptedSharedPreferences" +private const val KEY: String = "mockKey" +private const val VALUE: String = "mockValue with unicode 🔐 and quotes “”" + +// 0x80–0x8F are UTF-8 continuation bytes, which are invalid as leading bytes and would be +// destroyed by a UTF-8 decode/encode round trip. +private val IV: ByteArray = ByteArray(16) { (0x80 + it).toByte() } diff --git a/data/src/test/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerTest.kt b/data/src/test/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerTest.kt new file mode 100644 index 00000000000..cc1e0827926 --- /dev/null +++ b/data/src/test/kotlin/com/bitwarden/data/manager/keystore/KeystoreManagerTest.kt @@ -0,0 +1,241 @@ +package com.bitwarden.data.manager.keystore + +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import com.bitwarden.core.data.manager.BuildInfoManager +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.security.InvalidAlgorithmParameterException +import java.security.KeyStore +import java.security.KeyStoreException +import java.security.NoSuchAlgorithmException +import java.security.NoSuchProviderException +import java.security.ProviderException +import java.security.UnrecoverableKeyException +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey + +class KeystoreManagerTest { + + private val mockAndroidKeyStore = mockk(name = "MockAndroidKeyStore") + private val mockKeyGenerator = mockk() + private val mockKeyGenParameterSpec = mockk() + private val mockBuildInfoManager = mockk { + every { applicationId } returns APPLICATION_ID + } + + private val keystoreManager: KeystoreManager = KeystoreManagerImpl( + buildInfoManager = mockBuildInfoManager, + ) + + @BeforeEach + fun setUp() { + mockkStatic(KeyStore::class, KeyGenerator::class) + mockkConstructor(KeyGenParameterSpec.Builder::class) + every { KeyStore.getInstance("AndroidKeyStore") } returns mockAndroidKeyStore + every { mockAndroidKeyStore.load(null) } just runs + } + + @AfterEach + fun tearDown() { + unmockkStatic(KeyStore::class, KeyGenerator::class) + unmockkConstructor(KeyGenParameterSpec.Builder::class) + } + + @Test + fun `getKey should return key stored in keystore`() { + val mockSecretKey = mockk() + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns mockSecretKey + + assertEquals(mockSecretKey, keystoreManager.getKeyOrNull(alias = ALIAS)) + verify { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } + } + + @Test + fun `getKey should return null when no key exists`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + + assertNull(keystoreManager.getKeyOrNull(alias = ALIAS)) + } + + @Test + fun `getKey should return null when keystore throws KeyStoreException`() { + every { + mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) + } throws KeyStoreException() + + assertNull(keystoreManager.getKeyOrNull(alias = ALIAS)) + } + + @Test + fun `getKey should return null when keystore throws NoSuchAlgorithmException`() { + every { + mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) + } throws NoSuchAlgorithmException() + + assertNull(keystoreManager.getKeyOrNull(alias = ALIAS)) + } + + @Test + fun `getKey should return null when keystore throws UnrecoverableKeyException`() { + every { + mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) + } throws UnrecoverableKeyException() + + assertNull(keystoreManager.getKeyOrNull(alias = ALIAS)) + } + + @Test + fun `getOrCreateKey should return existing key without generating a new one`() { + val mockSecretKey = mockk() + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns mockSecretKey + + assertEquals(mockSecretKey, keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + verify(exactly = 0) { KeyGenerator.getInstance(any(), any()) } + } + + @Test + fun `getOrCreateKey should generate and return a new key when none exists`() { + val mockSecretKey = mockk() + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + setupMockKeyGenParameterSpecBuilder() + setupMockKeyGenerator() + every { mockKeyGenerator.generateKey() } returns mockSecretKey + + assertEquals(mockSecretKey, keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + verify { mockKeyGenerator.init(mockKeyGenParameterSpec) } + verify { mockKeyGenerator.generateKey() } + } + + @Suppress("MaxLineLength") + @Test + fun `getOrCreateKey should return null when key generator instance throws NoSuchAlgorithmException`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + every { + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore") + } throws NoSuchAlgorithmException() + + assertNull(keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + } + + @Suppress("MaxLineLength") + @Test + fun `getOrCreateKey should return null when key generator instance throws NoSuchProviderException`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + every { + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore") + } throws NoSuchProviderException() + + assertNull(keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + } + + @Suppress("MaxLineLength") + @Test + fun `getOrCreateKey should return null when key generator instance throws IllegalArgumentException`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + every { + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore") + } throws IllegalArgumentException() + + assertNull(keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + } + + @Suppress("MaxLineLength") + @Test + fun `getOrCreateKey should return null when key generator init throws InvalidAlgorithmParameterException`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + setupMockKeyGenParameterSpecBuilder() + setupMockKeyGenerator() + every { + mockKeyGenerator.init(mockKeyGenParameterSpec) + } throws InvalidAlgorithmParameterException() + + assertNull(keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + } + + @Test + fun `getOrCreateKey should return null when generateKey throws ProviderException`() { + every { mockAndroidKeyStore.getKey(NAMESPACED_ALIAS, null) } returns null + setupMockKeyGenParameterSpecBuilder() + setupMockKeyGenerator() + every { mockKeyGenerator.generateKey() } throws ProviderException() + + assertNull(keystoreManager.getOrCreateKeyOrNull(alias = ALIAS)) + } + + @Test + fun `hasKey should return true when keystore contains alias`() { + every { mockAndroidKeyStore.containsAlias(NAMESPACED_ALIAS) } returns true + + assertTrue(keystoreManager.hasKey(alias = ALIAS)) + } + + @Test + fun `hasKey should return false when keystore does not contain alias`() { + every { mockAndroidKeyStore.containsAlias(NAMESPACED_ALIAS) } returns false + + assertFalse(keystoreManager.hasKey(alias = ALIAS)) + } + + @Test + fun `hasKey should return false when keystore throws KeyStoreException`() { + every { mockAndroidKeyStore.containsAlias(NAMESPACED_ALIAS) } throws KeyStoreException() + + assertFalse(keystoreManager.hasKey(alias = ALIAS)) + } + + @Test + fun `removeKey should delete entry and return true`() { + every { mockAndroidKeyStore.deleteEntry(NAMESPACED_ALIAS) } just runs + + assertTrue(keystoreManager.removeKey(alias = ALIAS)) + verify { mockAndroidKeyStore.deleteEntry(NAMESPACED_ALIAS) } + } + + @Test + fun `removeKey should return false when keystore throws KeyStoreException`() { + every { mockAndroidKeyStore.deleteEntry(NAMESPACED_ALIAS) } throws KeyStoreException() + + assertFalse(keystoreManager.removeKey(alias = ALIAS)) + } + + private fun setupMockKeyGenerator() { + every { + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore") + } returns mockKeyGenerator + every { mockKeyGenerator.init(mockKeyGenParameterSpec) } just runs + } + + private fun setupMockKeyGenParameterSpecBuilder() { + every { + anyConstructed().setBlockModes(any()) + } answers { self as KeyGenParameterSpec.Builder } + every { + anyConstructed().setEncryptionPaddings(any()) + } answers { self as KeyGenParameterSpec.Builder } + every { + anyConstructed().setKeySize(any()) + } answers { self as KeyGenParameterSpec.Builder } + every { + anyConstructed().build() + } returns mockKeyGenParameterSpec + } +} + +private const val APPLICATION_ID: String = "com.mock.app" +private const val ALIAS: String = "mockAlias" +private const val NAMESPACED_ALIAS: String = "$APPLICATION_ID.$ALIAS"