From a9f48e2b5ddcb9c271d7b589cae13b95ecc98c17 Mon Sep 17 00:00:00 2001 From: Sam Wolfand Date: Wed, 8 Apr 2026 18:04:14 -0700 Subject: [PATCH] fix(api): encrypt persisted storage values Encrypt StorageHelper values with an AES-GCM envelope backed by AndroidKeyStore keys in production. Migrate legacy plaintext entries on read, drop undecryptable values, and use a Robolectric-safe JVM cipher so storage tests still exercise encrypted persistence semantics. --- .../com/clerk/api/storage/StorageCipher.kt | 144 ++++++++++++++++++ .../com/clerk/api/storage/StorageHelper.kt | 104 ++++++++++--- .../clerk/api/storage/StorageHelperTest.kt | 99 ++++++++++-- 3 files changed, 313 insertions(+), 34 deletions(-) create mode 100644 source/api/src/main/kotlin/com/clerk/api/storage/StorageCipher.kt diff --git a/source/api/src/main/kotlin/com/clerk/api/storage/StorageCipher.kt b/source/api/src/main/kotlin/com/clerk/api/storage/StorageCipher.kt new file mode 100644 index 000000000..8d9f6f0b1 --- /dev/null +++ b/source/api/src/main/kotlin/com/clerk/api/storage/StorageCipher.kt @@ -0,0 +1,144 @@ +package com.clerk.api.storage + +import android.os.Build +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import android.util.Base64 +import com.clerk.api.Constants.Storage.CLERK_PREFERENCES_FILE_NAME +import java.security.KeyStore +import java.security.MessageDigest +import javax.crypto.Cipher +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import javax.crypto.spec.GCMParameterSpec +import javax.crypto.spec.SecretKeySpec + +internal interface StorageCipher { + fun encrypt(plaintext: String): String + + fun decrypt(ciphertext: String): String +} + +internal object StorageCipherFactory { + fun create(keyAlias: String = "$CLERK_PREFERENCES_FILE_NAME.master_key"): StorageCipher { + return try { + AndroidKeystoreStorageCipher(keyAlias) + } catch (error: Exception) { + if (isRobolectricEnvironment()) { + JvmAesGcmStorageCipher(keyAlias) + } else { + throw error + } + } + } + + private fun isRobolectricEnvironment(): Boolean { + return Build.FINGERPRINT.contains("robolectric", ignoreCase = true) + } +} + +internal class AndroidKeystoreStorageCipher( + private val keyAlias: String = "$CLERK_PREFERENCES_FILE_NAME.master_key" +) : StorageCipher { + @Volatile private var cachedSecretKey: SecretKey? = null + private val keyStore: KeyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } + + override fun encrypt(plaintext: String): String { + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init(Cipher.ENCRYPT_MODE, getOrCreateSecretKey()) + val encrypted = cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8)) + return "${encode(cipher.iv)}$IV_SEPARATOR${encode(encrypted)}" + } + + override fun decrypt(ciphertext: String): String { + val components = ciphertext.split(IV_SEPARATOR, limit = 2) + require(components.size == 2) { "Encrypted value is malformed" } + + val iv = decode(components[0]) + val encrypted = decode(components[1]) + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init(Cipher.DECRYPT_MODE, getOrCreateSecretKey(), GCMParameterSpec(GCM_TAG_BITS, iv)) + return cipher.doFinal(encrypted).toString(Charsets.UTF_8) + } + + private fun getOrCreateSecretKey(): SecretKey { + cachedSecretKey?.let { + return it + } + + return synchronized(this) { + cachedSecretKey?.let { + return@synchronized it + } + + val existingKey = keyStore.getKey(keyAlias, null) as? SecretKey + if (existingKey != null) { + cachedSecretKey = existingKey + return@synchronized existingKey + } + + val keyGenerator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE) + val keySpec = + KeyGenParameterSpec.Builder( + keyAlias, + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, + ) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .setRandomizedEncryptionRequired(true) + .setUserAuthenticationRequired(false) + .build() + + keyGenerator.init(keySpec) + return@synchronized keyGenerator.generateKey().also { generatedKey -> + cachedSecretKey = generatedKey + } + } + } + + private fun encode(bytes: ByteArray): String = Base64.encodeToString(bytes, Base64.NO_WRAP) + + private fun decode(encoded: String): ByteArray = Base64.decode(encoded, Base64.NO_WRAP) + + private companion object { + const val ANDROID_KEYSTORE = "AndroidKeyStore" + const val AES_TRANSFORMATION = "AES/GCM/NoPadding" + const val GCM_TAG_BITS = 128 + const val IV_SEPARATOR = ':' + } +} + +internal class JvmAesGcmStorageCipher(private val keyAlias: String) : StorageCipher { + private val secretKey: SecretKey by lazy { + val keyBytes = MessageDigest.getInstance("SHA-256").digest(keyAlias.toByteArray(Charsets.UTF_8)) + SecretKeySpec(keyBytes.copyOf(16), KeyProperties.KEY_ALGORITHM_AES) + } + + override fun encrypt(plaintext: String): String { + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init(Cipher.ENCRYPT_MODE, secretKey) + val encrypted = cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8)) + return "${encode(cipher.iv)}$IV_SEPARATOR${encode(encrypted)}" + } + + override fun decrypt(ciphertext: String): String { + val components = ciphertext.split(IV_SEPARATOR, limit = 2) + require(components.size == 2) { "Encrypted value is malformed" } + + val iv = decode(components[0]) + val encrypted = decode(components[1]) + val cipher = Cipher.getInstance(AES_TRANSFORMATION) + cipher.init(Cipher.DECRYPT_MODE, secretKey, GCMParameterSpec(GCM_TAG_BITS, iv)) + return cipher.doFinal(encrypted).toString(Charsets.UTF_8) + } + + private fun encode(bytes: ByteArray): String = Base64.encodeToString(bytes, Base64.NO_WRAP) + + private fun decode(encoded: String): ByteArray = Base64.decode(encoded, Base64.NO_WRAP) + + private companion object { + const val AES_TRANSFORMATION = "AES/GCM/NoPadding" + const val GCM_TAG_BITS = 128 + const val IV_SEPARATOR = ':' + } +} diff --git a/source/api/src/main/kotlin/com/clerk/api/storage/StorageHelper.kt b/source/api/src/main/kotlin/com/clerk/api/storage/StorageHelper.kt index 909c6f21f..d7676f4bf 100644 --- a/source/api/src/main/kotlin/com/clerk/api/storage/StorageHelper.kt +++ b/source/api/src/main/kotlin/com/clerk/api/storage/StorageHelper.kt @@ -8,46 +8,98 @@ import com.clerk.api.Constants.Storage.CLERK_PREFERENCES_FILE_NAME import com.clerk.api.log.ClerkLog /** - * Helper class to manage secure storage of data. SharedPreferences are used to store data, all keys - * are held in the [StorageKey] object. + * Helper class to manage secure storage of data. SharedPreferences are used as the persistence + * backend, while values are encrypted before they are written to disk. */ internal object StorageHelper { + private const val ENCRYPTED_VALUE_PREFIX = "clerk:v1:" @Volatile private var secureStorage: SharedPreferences? = null + @Volatile private var storageCipher: StorageCipher? = null + + @VisibleForTesting internal var storageCipherFactoryOverride: (() -> StorageCipher)? = null /** * Synchronously initializes the secure storage. We do this synchronously because we need to * ensure that the storage is initialized before we generate a device ID. */ + @Synchronized fun initialize(context: Context) { - secureStorage = context.getSharedPreferences(CLERK_PREFERENCES_FILE_NAME, Context.MODE_PRIVATE) + if (secureStorage == null) { + secureStorage = + context.applicationContext.getSharedPreferences( + CLERK_PREFERENCES_FILE_NAME, + Context.MODE_PRIVATE, + ) + } + if (storageCipher == null) { + storageCipher = + runCatching { storageCipherFactoryOverride?.invoke() ?: StorageCipherFactory.create() } + .onFailure { error -> + ClerkLog.w("Failed to initialize encrypted storage: ${error.message}") + } + .getOrNull() + } } /** Save value of string type to [secureStorage] */ internal fun saveValue(key: StorageKey, value: String) { val prefs = secureStorage - if (prefs == null) { - ClerkLog.w( - "StorageHelper.saveValue called before initialization, ignoring save for key: ${key.name}" - ) - return - } - if (value.isNotEmpty()) { - prefs.edit(commit = true) { putString(key.name, value) } - return + val cipher = storageCipher + + when { + prefs == null -> { + ClerkLog.w( + "StorageHelper.saveValue called before initialization, ignoring save for key: ${key.name}" + ) + } + value.isEmpty() -> Unit + cipher == null -> { + ClerkLog.w("Encrypted storage is unavailable, ignoring save for key: ${key.name}") + } + else -> { + runCatching { ENCRYPTED_VALUE_PREFIX + cipher.encrypt(value) } + .onSuccess { encryptedValue -> + prefs.edit(commit = true) { putString(key.name, encryptedValue) } + } + .onFailure { error -> + ClerkLog.w("Failed to encrypt value for key ${key.name}: ${error.message}") + } + } } } /** Load value of string type from [secureStorage] */ internal fun loadValue(key: StorageKey): String? { val prefs = secureStorage - if (prefs == null) { - ClerkLog.w( - "StorageHelper.loadValue called before initialization, returning null for key: ${key.name}" - ) - return null + val storedValue = prefs?.getString(key.name, null) + val cipher = storageCipher + + return when { + prefs == null -> { + ClerkLog.w( + "StorageHelper.loadValue called before initialization, returning null for key: ${key.name}" + ) + null + } + storedValue == null -> null + !storedValue.startsWith(ENCRYPTED_VALUE_PREFIX) -> { + migrateLegacyPlaintextValue(key, storedValue) + storedValue + } + cipher == null -> { + ClerkLog.w("Encrypted storage is unavailable, returning null for key: ${key.name}") + null + } + else -> { + runCatching { cipher.decrypt(storedValue.removePrefix(ENCRYPTED_VALUE_PREFIX)) } + .onFailure { error -> + ClerkLog.w("Failed to decrypt stored value for key ${key.name}: ${error.message}") + prefs.edit(commit = true) { remove(key.name) } + } + .getOrNull() + } } - return prefs.getString(key.name, null) } /** Delete value of string type from [secureStorage] */ @@ -73,6 +125,7 @@ internal object StorageHelper { if (prefs != null) { prefs.edit().clear().commit() } + storageCipher = null if (context != null) { // Reinitialize to ensure clean state initialize(context) @@ -81,6 +134,21 @@ internal object StorageHelper { secureStorage = null } } + + private fun migrateLegacyPlaintextValue(key: StorageKey, value: String) { + if (value.isEmpty()) { + return + } + + val cipher = storageCipher ?: return + runCatching { ENCRYPTED_VALUE_PREFIX + cipher.encrypt(value) } + .onSuccess { encryptedValue -> + secureStorage?.edit(commit = true) { putString(key.name, encryptedValue) } + } + .onFailure { error -> + ClerkLog.w("Failed to migrate plaintext value for key ${key.name}: ${error.message}") + } + } } internal enum class StorageKey { diff --git a/source/api/src/test/java/com/clerk/api/storage/StorageHelperTest.kt b/source/api/src/test/java/com/clerk/api/storage/StorageHelperTest.kt index e1c614d61..e06388219 100644 --- a/source/api/src/test/java/com/clerk/api/storage/StorageHelperTest.kt +++ b/source/api/src/test/java/com/clerk/api/storage/StorageHelperTest.kt @@ -2,12 +2,16 @@ package com.clerk.api.storage import android.content.Context import android.content.SharedPreferences +import android.util.Base64 +import androidx.core.content.edit +import com.clerk.api.Constants.Storage.CLERK_PREFERENCES_FILE_NAME import com.clerk.api.Constants.Test.CONCURRENCY_TEST_THREAD_COUNT import io.mockk.unmockkAll import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -19,35 +23,40 @@ import org.robolectric.RuntimeEnvironment @RunWith(RobolectricTestRunner::class) class StorageHelperTest { + private companion object { + const val ENCRYPTED_VALUE_PREFIX = "clerk:v1:" + } private lateinit var context: Context @Before fun setup() { context = RuntimeEnvironment.getApplication() - // Clear any existing SharedPreferences data - clearSharedPreferences() + StorageHelper.storageCipherFactoryOverride = { TestStorageCipher() } + StorageHelper.reset() + preferences().edit { clear() } } @After fun tearDown() { unmockkAll() - // Clear SharedPreferences after each test - clearSharedPreferences() + StorageHelper.reset() + StorageHelper.storageCipherFactoryOverride = null + preferences().edit { clear() } } - /** - * Clears SharedPreferences data. For tests that require uninitialized state, prefer - * [StorageHelper.reset] with no context. - */ - private fun clearSharedPreferences() { - try { - val field = StorageHelper::class.java.getDeclaredField("secureStorage") - field.isAccessible = true - val prefs = field.get(StorageHelper) as? SharedPreferences - prefs?.edit()?.clear()?.commit() - field.isAccessible = false - } catch (_: Exception) {} + private fun preferences(): SharedPreferences { + return context.getSharedPreferences(CLERK_PREFERENCES_FILE_NAME, Context.MODE_PRIVATE) + } + + private class TestStorageCipher : StorageCipher { + override fun encrypt(plaintext: String): String { + return Base64.encodeToString(plaintext.toByteArray(Charsets.UTF_8), Base64.NO_WRAP) + } + + override fun decrypt(ciphertext: String): String { + return String(Base64.decode(ciphertext, Base64.NO_WRAP), Charsets.UTF_8) + } } @Test @@ -114,6 +123,21 @@ class StorageHelperTest { ) } + @Test + fun `saveValue stores encrypted value in SharedPreferences`() { + StorageHelper.initialize(context) + + StorageHelper.saveValue(StorageKey.DEVICE_ID, "plain-device-id") + + val storedValue = preferences().getString(StorageKey.DEVICE_ID.name, null) + assertNotNull("Encrypted value should be written to SharedPreferences", storedValue) + assertTrue( + "Stored value should be envelope-prefixed", + storedValue!!.startsWith(ENCRYPTED_VALUE_PREFIX), + ) + assertTrue("Stored value should not equal plaintext", storedValue != "plain-device-id") + } + @Test fun `saveValue with empty string when no existing value does not write to SharedPreferences`() { // Given @@ -127,6 +151,49 @@ class StorageHelperTest { assertNull("Empty string should not be saved", StorageHelper.loadValue(testKey)) } + @Test + fun `loadValue migrates legacy plaintext values to encrypted storage`() { + preferences().edit(commit = true) { putString(StorageKey.DEVICE_TOKEN.name, "legacy-token") } + StorageHelper.initialize(context) + + val loadedValue = StorageHelper.loadValue(StorageKey.DEVICE_TOKEN) + val storedValue = preferences().getString(StorageKey.DEVICE_TOKEN.name, null) + + assertEquals("legacy-token", loadedValue) + assertNotNull("Migrated value should still be stored", storedValue) + assertTrue( + "Legacy plaintext value should be rewritten in encrypted form", + storedValue!!.startsWith(ENCRYPTED_VALUE_PREFIX), + ) + assertTrue("Migrated value should no longer be plaintext", storedValue != "legacy-token") + } + + @Test + fun `loadValue deletes malformed encrypted values and returns null`() { + StorageHelper.reset() + StorageHelper.storageCipherFactoryOverride = { + object : StorageCipher { + override fun encrypt(plaintext: String): String = plaintext + + override fun decrypt(ciphertext: String): String { + error("malformed ciphertext") + } + } + } + preferences().edit(commit = true) { + putString(StorageKey.DEVICE_ID.name, "${ENCRYPTED_VALUE_PREFIX}opaque") + } + StorageHelper.initialize(context) + + val loadedValue = StorageHelper.loadValue(StorageKey.DEVICE_ID) + + assertNull("Malformed encrypted value should not be returned", loadedValue) + assertFalse( + "Malformed encrypted value should be removed from SharedPreferences", + preferences().contains(StorageKey.DEVICE_ID.name), + ) + } + @Test fun `after initialize isInitialized returns true and values can be saved loaded and deleted`() { // Given