From 1e8d589aed8c8967d97f78ac26f1fbcc7375afbc Mon Sep 17 00:00:00 2001 From: Andras Sarro Date: Fri, 7 Feb 2025 16:11:01 +0100 Subject: [PATCH] feat(shared-pref-crypto): improve error handling and logging SDK-217 Co-authored-by: davidSchuppa <32750715+davidSchuppa@users.noreply.github.com> Co-authored-by: LasOri <24588073+LasOri@users.noreply.github.com> Co-authored-by: megamegax Co-authored-by: matusekma <36794575+matusekma@users.noreply.github.com> --- .../core/crypto/SharedPreferenceCryptoTest.kt | 204 +++++++----------- ...EmarsysEncryptedSharedPreferencesV3Test.kt | 22 +- .../SharedPreferencesV3ProviderTest.kt | 6 +- .../core/crypto/SharedPreferenceCrypto.kt | 112 ++++++---- .../EmarsysEncryptedSharedPreferencesV3.kt | 12 +- 5 files changed, 165 insertions(+), 191 deletions(-) diff --git a/core/src/androidTest/java/com/emarsys/core/crypto/SharedPreferenceCryptoTest.kt b/core/src/androidTest/java/com/emarsys/core/crypto/SharedPreferenceCryptoTest.kt index 1937dce1..dfee9b71 100644 --- a/core/src/androidTest/java/com/emarsys/core/crypto/SharedPreferenceCryptoTest.kt +++ b/core/src/androidTest/java/com/emarsys/core/crypto/SharedPreferenceCryptoTest.kt @@ -1,12 +1,9 @@ -import android.security.keystore.KeyGenParameterSpec -import android.util.Base64 -import com.emarsys.core.crypto.SharedPreferenceCrypto +package com.emarsys.core.crypto + +import android.security.keystore.KeyProperties import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe -import io.mockk.Runs import io.mockk.every -import io.mockk.just -import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkAll import io.mockk.verify @@ -17,168 +14,119 @@ import java.security.GeneralSecurityException import java.security.KeyStore import javax.crypto.Cipher import javax.crypto.KeyGenerator -import javax.crypto.SecretKey -import javax.crypto.spec.GCMParameterSpec -class SharedPreferenceCryptoTest { - private companion object { - const val encryptedBase64 = "Base64EncryptedBase64IV123123" +class SharedPreferenceCryptoTest { + private lateinit var keyStore: KeyStore + @Before + fun setup() { + keyStore = KeyStore.getInstance("AndroidKeyStore") + keyStore.load(null) + keyStore.deleteEntry("emarsys_sdk_key_shared_pref_key_v3") } - private lateinit var sharedPreferenceCrypto: SharedPreferenceCrypto - private lateinit var mockKeyStore: KeyStore - private lateinit var mockKeyGenerator: KeyGenerator - private lateinit var mockSecretKey: SecretKey - private lateinit var mockCipher: Cipher + @After + fun tearDown() { + unmockkAll() + } - @Before - fun setup() { - mockkStatic(KeyStore::class) + @Test + fun init_shouldGenerateKey_ifNotPresent_inKeyStore() { mockkStatic(KeyGenerator::class) - mockkStatic(Cipher::class) - mockkStatic(Base64::class) - - mockKeyStore = mockk() - mockKeyGenerator = mockk() - mockSecretKey = mockk() - mockCipher = mockk() - every { KeyStore.getInstance(any()) } returns mockKeyStore - every { KeyGenerator.getInstance(any(), any()) } returns mockKeyGenerator - every { Cipher.getInstance(any()) } returns mockCipher + SharedPreferenceCrypto() - sharedPreferenceCrypto = SharedPreferenceCrypto() + verify { KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES) } } - @After - fun tearDown() { - unmockkAll() + @Test + fun init_shouldNotGenerateKey_ifPresent_inKeyStore() { + mockkStatic(KeyGenerator::class) + + SharedPreferenceCrypto() + SharedPreferenceCrypto() + + verify(exactly = 1) { KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES) } } @Test - fun testGetOrCreateSecretKey_KeyExists() { - every { mockKeyStore.load(null) } just Runs - every { mockKeyStore.containsAlias(any()) } returns true - every { mockKeyStore.getKey(any(), null) } returns mockSecretKey + fun encrypt_decrypt_shouldWork() { + val testValue = "testValue" - val result = sharedPreferenceCrypto.getOrCreateSecretKey() + val testCrypto = SharedPreferenceCrypto() + val encrypted = testCrypto.encrypt(testValue) - result shouldBe mockSecretKey - verify { mockKeyStore.getKey(any(), null) } + testCrypto.decrypt(encrypted) shouldBe testValue } @Test - fun testGetOrCreateSecretKey_KeyDoesNotExist() { - every { mockKeyStore.load(null) } just Runs - every { mockKeyStore.containsAlias(any()) } returns false - every { mockKeyGenerator.init(any()) } just Runs - every { mockKeyGenerator.generateKey() } returns mockSecretKey + fun encrypt_shouldGenerateNewSecretKey_andRetryEncrypting_once_andReturnEncryptedValue_ifSucceeds() { + val testValue = "testValue" + mockkStatic(KeyGenerator::class) + mockkStatic(Cipher::class) + every { Cipher.getInstance("AES/GCM/NoPadding") } throws GeneralSecurityException("Test exception") andThenAnswer { callOriginal() } + + val testCrypto = SharedPreferenceCrypto() - val result = sharedPreferenceCrypto.getOrCreateSecretKey() + val result = testCrypto.encrypt(testValue) - result shouldBe mockSecretKey - verify { mockKeyGenerator.generateKey() } + verify(exactly = 2) { KeyGenerator.getInstance(any()) } + + result shouldNotBe testValue } @Test - fun testEncrypt_Success() { - val value = "test_value" - val encryptedBytes = byteArrayOf(1, 2, 3, 4) - val iv = byteArrayOf(5, 6, 7, 8) + fun encrypt_shouldGenerateNewSecretKey_andRetryEncrypting_once_andReturnInitialValue_ifFails() { + val testValue = "testValue" + mockkStatic(KeyGenerator::class) + mockkStatic(Cipher::class) + every { Cipher.getInstance("AES/GCM/NoPadding") } throws GeneralSecurityException("Test exception") - every { mockCipher.init(Cipher.ENCRYPT_MODE, mockSecretKey) } just Runs - every { mockCipher.doFinal(any()) } returns encryptedBytes - every { mockCipher.iv } returns iv - every { Base64.encodeToString(any(), Base64.DEFAULT) } returns "encodedString" + val testCrypto = SharedPreferenceCrypto() - val result = sharedPreferenceCrypto.encrypt(value, mockSecretKey) + val result = testCrypto.encrypt(testValue) - result shouldNotBe value - result shouldBe "encodedStringencodedString" + verify(exactly = 2) { KeyGenerator.getInstance(any()) } + result shouldBe testValue } @Test - fun testEncrypt_Exception() { - val value = "test_value" + fun decrypt_shouldReturn_null_andGenerateNewSecretKey_ifGeneralSecurityExceptionHappens() { + val testValue = "dGVzdFZhbHVlU2hvdWxkQmVTaXh0ZWVuQ2hhcnNMb25n" + mockkStatic(KeyGenerator::class) + mockkStatic(Cipher::class) + every { Cipher.getInstance("AES/GCM/NoPadding") } throws GeneralSecurityException("Test exception") - every { - mockCipher.init( - Cipher.ENCRYPT_MODE, - mockSecretKey - ) - } throws GeneralSecurityException("Encryption failed") + val testCrypto = SharedPreferenceCrypto() - val result = sharedPreferenceCrypto.encrypt(value, mockSecretKey) + testCrypto.decrypt(testValue) shouldBe null - result shouldBe value + verify { KeyGenerator.getInstance(any()) } } @Test - fun testDecrypt_Success() { - val ivBytes = byteArrayOf(1, 2, 3, 4) - val encryptedBytes = byteArrayOf(5, 6, 7, 8) - val decryptedBytes = "decrypted".toByteArray() - - every { Base64.decode(any(), Base64.DEFAULT) } returnsMany listOf( - ivBytes, - encryptedBytes - ) - every { - mockCipher.init( - Cipher.DECRYPT_MODE, - mockSecretKey, - any() - ) - } just Runs - every { mockCipher.doFinal(encryptedBytes) } returns decryptedBytes - - val result = sharedPreferenceCrypto.decrypt(encryptedBase64, mockSecretKey) - - result shouldBe "decrypted" + fun decrypt_shouldReturn_encryptedValue_ifIllegalArgumentException_withBase64ErrorHappens() { + val testValue = "testValueShouldBeSixteenCharsLong" + + val testCrypto = SharedPreferenceCrypto() + testCrypto.decrypt(testValue) shouldBe testValue } @Test - fun testDecrypt_Exception() { - val IVValue = "Base64EncryptedBase64IV123" - val decryptedBytes = encryptedBase64.toByteArray() - every { - mockCipher.init(any(), mockSecretKey, any()) - } just Runs - every { - mockCipher.doFinal(any()) - } returns decryptedBytes - every { - Base64.decode( - IVValue, - Base64.DEFAULT - ) - } throws GeneralSecurityException("Decryption failed") - - val result = sharedPreferenceCrypto.decrypt(encryptedBase64, mockSecretKey) - - result shouldBe null + fun decrypt_shouldReturn_null_ifIllegalArgumentExceptionHappens() { + val testValue = "dGVzdFZhbHVlU2hvdWxkQmVTaXh0ZWVuQ2hhcnNMb25n" + mockkStatic(Cipher::class) + every { Cipher.getInstance("AES/GCM/NoPadding") } throws IllegalArgumentException("Test exception") + + val testCrypto = SharedPreferenceCrypto() + testCrypto.decrypt(testValue) shouldBe null } @Test - fun testDecrypt_IllegalArgumentException() { - val IVValue = "Base64EncryptedBase64IV123" - val decryptedBytes = encryptedBase64.toByteArray() - every { - mockCipher.init(any(), mockSecretKey, any()) - } just Runs - every { - mockCipher.doFinal(any()) - } returns decryptedBytes - every { - Base64.decode( - IVValue, - Base64.DEFAULT - ) - } throws IllegalArgumentException("bad base-64") - - val result = sharedPreferenceCrypto.decrypt(encryptedBase64, mockSecretKey) - - result shouldBe null + fun decrypt_shouldReturn_null_ifExceptionHappens() { + val testValue = "testValue" + + val testCrypto = SharedPreferenceCrypto() + testCrypto.decrypt(testValue) shouldBe null } } \ No newline at end of file diff --git a/core/src/androidTest/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3Test.kt b/core/src/androidTest/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3Test.kt index 090c5555..79a9de2f 100644 --- a/core/src/androidTest/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3Test.kt +++ b/core/src/androidTest/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3Test.kt @@ -28,11 +28,10 @@ class EmarsysEncryptedSharedPreferencesV3Test { mockSecretKey = mockk() every { mockContext.getSharedPreferences(any(), any()) } returns mockRealPreferences - every { mockSharedPreferenceCrypto.getOrCreateSecretKey() } returns mockSecretKey mockInternalEditor = mockk(relaxed = true) every { mockRealPreferences.edit() } returns mockInternalEditor - every { mockSharedPreferenceCrypto.encrypt(any(), any()) } returns "encryptedValue" + every { mockSharedPreferenceCrypto.encrypt(any()) } returns "encryptedValue" emarsysEncryptedSharedPreferencesV3 = EmarsysEncryptedSharedPreferencesV3( mockContext, @@ -54,7 +53,6 @@ class EmarsysEncryptedSharedPreferencesV3Test { every { mockRealPreferences.all } returns encryptedMap every { mockSharedPreferenceCrypto.decrypt( - any(), any() ) } returnsMany listOf("decryptedValue1", "decryptedValue2", "decryptedValue3") @@ -76,8 +74,7 @@ class EmarsysEncryptedSharedPreferencesV3Test { every { mockRealPreferences.getString("testKey", null) } returns "encryptedValue" every { mockSharedPreferenceCrypto.decrypt( - "encryptedValue", - mockSecretKey + "encryptedValue" ) } returns "decryptedValue" @@ -91,8 +88,7 @@ class EmarsysEncryptedSharedPreferencesV3Test { every { mockRealPreferences.getString("testKey", null) } returns "encryptedValue" every { mockSharedPreferenceCrypto.decrypt( - "encryptedValue", - mockSecretKey + "encryptedValue" ) } returns null @@ -107,14 +103,12 @@ class EmarsysEncryptedSharedPreferencesV3Test { every { mockRealPreferences.getStringSet("testKey", null) } returns encryptedSet every { mockSharedPreferenceCrypto.decrypt( - "encryptedValue1", - mockSecretKey + "encryptedValue1" ) } returns "decryptedValue1" every { mockSharedPreferenceCrypto.decrypt( - "encryptedValue2", - mockSecretKey + "encryptedValue2" ) } returns "decryptedValue2" @@ -196,9 +190,9 @@ class EmarsysEncryptedSharedPreferencesV3Test { editor.commit() - verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("testValue", mockSecretKey) } - verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("value1", mockSecretKey) } - verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("value2", mockSecretKey) } + verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("testValue") } + verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("value1") } + verify(exactly = 1) { mockSharedPreferenceCrypto.encrypt("value2") } verify(exactly = 1) { mockInternalEditor.putString("testKey", "encryptedValue") } verify(exactly = 1) { mockInternalEditor.putInt("testIntKey", 42) } diff --git a/core/src/androidTest/java/com/emarsys/core/storage/SharedPreferencesV3ProviderTest.kt b/core/src/androidTest/java/com/emarsys/core/storage/SharedPreferencesV3ProviderTest.kt index 88780a08..398c8a5b 100644 --- a/core/src/androidTest/java/com/emarsys/core/storage/SharedPreferencesV3ProviderTest.kt +++ b/core/src/androidTest/java/com/emarsys/core/storage/SharedPreferencesV3ProviderTest.kt @@ -1,9 +1,8 @@ +package com.emarsys.core.storage + import android.content.Context import android.content.SharedPreferences import com.emarsys.core.crypto.SharedPreferenceCrypto -import com.emarsys.core.storage.EmarsysEncryptedSharedPreferencesV3 -import com.emarsys.core.storage.EncryptedSharedPreferencesToSharedPreferencesMigration -import com.emarsys.core.storage.SharedPreferencesV3Provider import com.emarsys.testUtil.ReflectionTestUtils import io.kotest.matchers.shouldBe import io.mockk.Runs @@ -36,7 +35,6 @@ class SharedPreferencesV3ProviderTest { any() ) } returns mockRealSharedPrefs - every { mockCrypto.getOrCreateSecretKey() } returns mockk() every { mockMigration.migrate(any(), any()) } just Runs } diff --git a/core/src/main/java/com/emarsys/core/crypto/SharedPreferenceCrypto.kt b/core/src/main/java/com/emarsys/core/crypto/SharedPreferenceCrypto.kt index 1ddb1e3e..34f45d2b 100644 --- a/core/src/main/java/com/emarsys/core/crypto/SharedPreferenceCrypto.kt +++ b/core/src/main/java/com/emarsys/core/crypto/SharedPreferenceCrypto.kt @@ -3,6 +3,8 @@ package com.emarsys.core.crypto import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyProperties import android.util.Base64 +import com.emarsys.core.util.log.Logger +import com.emarsys.core.util.log.entry.StatusLog import java.security.GeneralSecurityException import java.security.KeyStore import javax.crypto.Cipher @@ -15,44 +17,24 @@ class SharedPreferenceCrypto { private const val KEYSTORE_ALIAS = "emarsys_sdk_key_shared_pref_key_v3" } - fun getOrCreateSecretKey(): SecretKey { - val keyStore = KeyStore.getInstance("AndroidKeyStore") - keyStore.load(null) + private var secretKey: SecretKey = getOrCreateSecretKey() - if (!keyStore.containsAlias(KEYSTORE_ALIAS)) { - val keyGenerator = - KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore") - val keyGenParameterSpec = KeyGenParameterSpec.Builder( - KEYSTORE_ALIAS, - KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT - ) - .setBlockModes(KeyProperties.BLOCK_MODE_GCM) - .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) - .setKeySize(256) - .build() - keyGenerator.init(keyGenParameterSpec) - return keyGenerator.generateKey() - } - - return keyStore.getKey(KEYSTORE_ALIAS, null) as SecretKey - } - - fun encrypt(value: String, secretKey: SecretKey): String { + fun encrypt(value: String): String { return try { - val cipher = Cipher.getInstance("AES/GCM/NoPadding") - cipher.init(Cipher.ENCRYPT_MODE, secretKey) - val encrypted = cipher.doFinal(value.toByteArray()) - val iv = cipher.iv - val ivBase64 = Base64.encodeToString(iv, Base64.DEFAULT) - val encryptedBase64 = Base64.encodeToString(encrypted, Base64.DEFAULT) - "$ivBase64$encryptedBase64" - } catch (e: GeneralSecurityException) { - e.printStackTrace() - value + tryEncrypt(value) + } catch (exception: GeneralSecurityException) { + logCryptoError(value, "encrypt", exception) + secretKey = createSecretKey() + try { + tryEncrypt(value) + } catch (exception: Exception) { + logCryptoError(value, "encrypt", exception) + value + } } } - fun decrypt(value: String, secretKey: SecretKey): String? { + fun decrypt(value: String): String? { return try { val ivBytes = Base64.decode(value.substring(0, 16), Base64.DEFAULT) val encryptedBytes = Base64.decode(value.substring(16), Base64.DEFAULT) @@ -61,12 +43,68 @@ class SharedPreferenceCrypto { cipher.init(Cipher.DECRYPT_MODE, secretKey, GCMParameterSpec(128, ivBytes)) val decrypted = cipher.doFinal(encryptedBytes) String(decrypted) - } catch (e: GeneralSecurityException) { - e.printStackTrace() + } catch (exception: GeneralSecurityException) { + logCryptoError(value, "decrypt", exception) + secretKey = createSecretKey() null - } catch (e: IllegalArgumentException) { - e.printStackTrace() + } catch (exception: IllegalArgumentException) { + logCryptoError(value, "decrypt", exception) + if (exception.message?.contains("bad base-64") == true) { + value + } else { + null + } + } catch (exception: Exception) { + logCryptoError(value, "decrypt", exception) null } } + + private fun getOrCreateSecretKey(): SecretKey { + val keyStore = KeyStore.getInstance("AndroidKeyStore") + keyStore.load(null) + + if (!keyStore.containsAlias(KEYSTORE_ALIAS)) { + return createSecretKey() + } + + return keyStore.getKey(KEYSTORE_ALIAS, null) as SecretKey + } + + private fun createSecretKey(): SecretKey { + val keyGenerator = + KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES) + val keyGenParameterSpec = KeyGenParameterSpec.Builder( + KEYSTORE_ALIAS, + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT + ) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .setKeySize(256) + .build() + keyGenerator.init(keyGenParameterSpec) + return keyGenerator.generateKey() + } + + private fun tryEncrypt(value: String): String { + val cipher = Cipher.getInstance("AES/GCM/NoPadding") + cipher.init(Cipher.ENCRYPT_MODE, secretKey) + val encrypted = cipher.doFinal(value.toByteArray()) + val iv = cipher.iv + val ivBase64 = Base64.encodeToString(iv, Base64.DEFAULT) + val encryptedBase64 = Base64.encodeToString(encrypted, Base64.DEFAULT) + return "$ivBase64$encryptedBase64" + } + + private fun logCryptoError(value: String, methodName: String, exception: Exception) { + val logEntry = StatusLog( + SharedPreferenceCrypto::class.java, + methodName, + mapOf( + "reason" to "Failed to decrypt value: $value", + "exception" to exception.message + ) + ) + Logger.debug(logEntry) + } } \ No newline at end of file diff --git a/core/src/main/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3.kt b/core/src/main/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3.kt index 27b41f27..80b18309 100644 --- a/core/src/main/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3.kt +++ b/core/src/main/java/com/emarsys/core/storage/EmarsysEncryptedSharedPreferencesV3.kt @@ -3,7 +3,6 @@ package com.emarsys.core.storage import android.content.Context import android.content.SharedPreferences import com.emarsys.core.crypto.SharedPreferenceCrypto -import javax.crypto.SecretKey class EmarsysEncryptedSharedPreferencesV3( context: Context, @@ -11,13 +10,11 @@ class EmarsysEncryptedSharedPreferencesV3( private val sharedPreferenceCrypto: SharedPreferenceCrypto ) : SharedPreferences { - private val secretKey: SecretKey = sharedPreferenceCrypto.getOrCreateSecretKey() private val realPreferences: SharedPreferences = context.getSharedPreferences(fileName, Context.MODE_PRIVATE) private val editor: SharedPreferences.Editor = Editor( realPreferences, - sharedPreferenceCrypto, - secretKey + sharedPreferenceCrypto ) override fun getAll(): Map { @@ -80,13 +77,12 @@ class EmarsysEncryptedSharedPreferencesV3( } private fun decryptString(value: String): String? { - return sharedPreferenceCrypto.decrypt(value, secretKey) + return sharedPreferenceCrypto.decrypt(value) } private class Editor( realPreferences: SharedPreferences, - private val sharedPreferenceCrypto: SharedPreferenceCrypto, - private val secretKey: SecretKey + private val sharedPreferenceCrypto: SharedPreferenceCrypto ) : SharedPreferences.Editor { private val editor: SharedPreferences.Editor = realPreferences.edit() @@ -145,7 +141,7 @@ class EmarsysEncryptedSharedPreferencesV3( private fun encryptString(value: String?): String? { value ?: return null - return sharedPreferenceCrypto.encrypt(value, secretKey) + return sharedPreferenceCrypto.encrypt(value) } } }