Skip to content

Commit b9e00c3

Browse files
AyyanchiraAyyanchira
authored
Merge pull request #899 from Iterable/feature/MOB-11167-fix-anr
[MOB-11167] Add timeout for crypto operations to solve ANRs and config to disable encryption altogether
2 parents 9542c15 + ba7d20c commit b9e00c3

File tree

5 files changed

+152
-37
lines changed

5 files changed

+152
-37
lines changed

‎iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ IterableKeychain getKeychain() {
144144
}
145145
if (keychain == null) {
146146
try {
147-
keychain = new IterableKeychain(getMainActivityContext(), config.decryptionFailureHandler);
147+
keychain = new IterableKeychain(getMainActivityContext(), config.decryptionFailureHandler, null, config.keychainEncryption);
148148
} catch (Exception e) {
149149
IterableLogger.e(TAG, "Failed to create IterableKeychain", e);
150150
}

‎iterableapi/src/main/java/com/iterable/iterableapi/IterableConfig.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ public class IterableConfig {
9393
*/
9494
final boolean enableEmbeddedMessaging;
9595

96+
/**
97+
* When set to true, disables encryption for keychain storage.
98+
* By default, encryption is enabled for storing sensitive user data.
99+
*/
100+
final boolean keychainEncryption;
101+
96102
/**
97103
* Handler for decryption failures of PII information.
98104
* Before calling this handler, the SDK will clear the PII information and create new encryption keys
@@ -121,6 +127,7 @@ private IterableConfig(Builder builder) {
121127
dataRegion = builder.dataRegion;
122128
useInMemoryStorageForInApps = builder.useInMemoryStorageForInApps;
123129
enableEmbeddedMessaging = builder.enableEmbeddedMessaging;
130+
keychainEncryption = builder.keychainEncryption;
124131
decryptionFailureHandler = builder.decryptionFailureHandler;
125132
mobileFrameworkInfo = builder.mobileFrameworkInfo;
126133
}
@@ -141,6 +148,7 @@ public static class Builder {
141148
private IterableDataRegion dataRegion = IterableDataRegion.US;
142149
private boolean useInMemoryStorageForInApps = false;
143150
private boolean enableEmbeddedMessaging = false;
151+
private boolean keychainEncryption = true;
144152
private IterableDecryptionFailureHandler decryptionFailureHandler;
145153
private IterableAPIMobileFrameworkInfo mobileFrameworkInfo;
146154

@@ -288,7 +296,6 @@ public Builder setDataRegion(@NonNull IterableDataRegion dataRegion) {
288296
* Set whether the SDK should store in-apps only in memory, or in file storage
289297
* @param useInMemoryStorageForInApps `true` will have in-apps be only in memory
290298
*/
291-
292299
@NonNull
293300
public Builder setUseInMemoryStorageForInApps(boolean useInMemoryStorageForInApps) {
294301
this.useInMemoryStorageForInApps = useInMemoryStorageForInApps;
@@ -304,6 +311,17 @@ public Builder setEnableEmbeddedMessaging(boolean enableEmbeddedMessaging) {
304311
return this;
305312
}
306313

314+
/**
315+
* When set to true, disables encryption for Iterable's keychain storage.
316+
* By default, encryption is enabled for storing sensitive user data.
317+
* @param keychainEncryption Whether to disable encryption for keychain
318+
*/
319+
@NonNull
320+
public Builder setKeychainEncryption(boolean keychainEncryption) {
321+
this.keychainEncryption = keychainEncryption;
322+
return this;
323+
}
324+
307325
/**
308326
* Set a handler for decryption failures that can be used to handle data recovery
309327
* @param handler Decryption failure handler provided by the app

‎iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package com.iterable.iterableapi
22

33
import android.content.Context
44
import android.content.SharedPreferences
5+
import java.util.concurrent.Callable
6+
import java.util.concurrent.Executors
7+
import java.util.concurrent.TimeUnit
58

69
class IterableKeychain {
710
companion object {
@@ -10,53 +13,73 @@ class IterableKeychain {
1013
const val KEY_USER_ID = "iterable-user-id"
1114
const val KEY_AUTH_TOKEN = "iterable-auth-token"
1215
private const val PLAINTEXT_SUFFIX = "_plaintext"
16+
private const val CRYPTO_OPERATION_TIMEOUT_MS = 500L
17+
private const val KEY_ENCRYPTION_ENABLED = "iterable-encryption-enabled"
18+
19+
private val cryptoExecutor = Executors.newSingleThreadExecutor()
1320
}
1421

1522
private var sharedPrefs: SharedPreferences
16-
internal var encryptor: IterableDataEncryptor
23+
internal var encryptor: IterableDataEncryptor? = null
1724
private val decryptionFailureHandler: IterableDecryptionFailureHandler?
25+
private var encryption: Boolean
1826

1927
@JvmOverloads
2028
constructor(
2129
context: Context,
2230
decryptionFailureHandler: IterableDecryptionFailureHandler? = null,
23-
migrator: IterableKeychainEncryptedDataMigrator? = null
31+
migrator: IterableKeychainEncryptedDataMigrator? = null,
32+
encryption: Boolean = true
2433
) {
25-
this.decryptionFailureHandler = decryptionFailureHandler
2634
sharedPrefs = context.getSharedPreferences(
2735
IterableConstants.SHARED_PREFS_FILE,
2836
Context.MODE_PRIVATE
2937
)
30-
encryptor = IterableDataEncryptor()
31-
IterableLogger.v(TAG, "SharedPreferences being used with encryption")
38+
this.decryptionFailureHandler = decryptionFailureHandler
39+
this.encryption = encryption && sharedPrefs.getBoolean(KEY_ENCRYPTION_ENABLED, true)
3240

33-
try {
34-
val dataMigrator = migrator ?: IterableKeychainEncryptedDataMigrator(context, sharedPrefs, this)
35-
if (!dataMigrator.isMigrationCompleted()) {
36-
dataMigrator.setMigrationCompletionCallback { error ->
37-
error?.let {
38-
IterableLogger.w(TAG, "Migration failed", it)
39-
handleDecryptionError(Exception(it))
41+
if (!encryption) {
42+
IterableLogger.v(TAG, "SharedPreferences being used without encryption")
43+
} else {
44+
encryptor = IterableDataEncryptor()
45+
IterableLogger.v(TAG, "SharedPreferences being used with encryption")
46+
47+
try {
48+
val dataMigrator = migrator ?: IterableKeychainEncryptedDataMigrator(context, sharedPrefs, this)
49+
if (!dataMigrator.isMigrationCompleted()) {
50+
dataMigrator.setMigrationCompletionCallback { error ->
51+
error?.let {
52+
IterableLogger.w(TAG, "Migration failed", it)
53+
handleDecryptionError(Exception(it))
54+
}
4055
}
56+
dataMigrator.attemptMigration()
57+
IterableLogger.v(TAG, "Migration completed")
4158
}
42-
dataMigrator.attemptMigration()
43-
IterableLogger.v(TAG, "Migration completed")
44-
}
45-
} catch (e: Exception) {
46-
IterableLogger.w(TAG, "Migration failed, clearing data", e)
47-
handleDecryptionError(e)
59+
} catch (e: Exception) {
60+
IterableLogger.w(TAG, "Migration failed, clearing data", e)
61+
handleDecryptionError(e)
62+
}
4863
}
4964
}
5065

66+
private fun <T> runWithTimeout(callable: Callable<T>): T {
67+
return cryptoExecutor.submit(callable).get(CRYPTO_OPERATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)
68+
}
69+
5170
private fun handleDecryptionError(e: Exception? = null) {
52-
IterableLogger.w(TAG, "Decryption failed, clearing all data and regenerating key")
71+
IterableLogger.w(TAG, "Decryption failed, permanently disabling encryption for this device. Please login again.")
72+
73+
// Permanently disable encryption for this device
5374
sharedPrefs.edit()
5475
.remove(KEY_EMAIL)
5576
.remove(KEY_USER_ID)
5677
.remove(KEY_AUTH_TOKEN)
78+
.putBoolean(KEY_ENCRYPTION_ENABLED, false)
5779
.apply()
5880

59-
encryptor.resetKeys()
81+
encryption = false
82+
6083
decryptionFailureHandler?.let { handler ->
6184
val exception = e ?: Exception("Unknown decryption error")
6285
try {
@@ -75,13 +98,20 @@ class IterableKeychain {
7598
}
7699

77100
private fun secureGet(key: String): String? {
78-
// First check if it's stored in plaintext
79-
if (sharedPrefs.getBoolean(key + PLAINTEXT_SUFFIX, false)) {
101+
val hasPlainText = sharedPrefs.getBoolean(key + PLAINTEXT_SUFFIX, false)
102+
if (!encryption) {
103+
if (hasPlainText) {
104+
return sharedPrefs.getString(key, null)
105+
} else {
106+
return null
107+
}
108+
} else if (hasPlainText) {
80109
return sharedPrefs.getString(key, null)
81110
}
82111

112+
val encryptedValue = sharedPrefs.getString(key, null) ?: return null
83113
return try {
84-
sharedPrefs.getString(key, null)?.let { encryptor.decrypt(it) }
114+
encryptor?.let { runWithTimeout { it.decrypt(encryptedValue) } }
85115
} catch (e: Exception) {
86116
handleDecryptionError(e)
87117
null
@@ -95,10 +125,18 @@ class IterableKeychain {
95125
return
96126
}
97127

128+
if (!encryption) {
129+
editor.putString(key, value).putBoolean(key + PLAINTEXT_SUFFIX, true).apply()
130+
return
131+
}
132+
98133
try {
99-
editor.putString(key, encryptor.encrypt(value))
100-
.remove(key + PLAINTEXT_SUFFIX)
101-
.apply()
134+
encryptor?.let {
135+
val encrypted = runWithTimeout { it.encrypt(value) }
136+
editor.putString(key, encrypted)
137+
.remove(key + PLAINTEXT_SUFFIX)
138+
.apply()
139+
}
102140
} catch (e: Exception) {
103141
handleDecryptionError(e)
104142
editor.putString(key, value)
@@ -115,4 +153,4 @@ class IterableKeychain {
115153

116154
fun getAuthToken() = secureGet(KEY_AUTH_TOKEN)
117155
fun saveAuthToken(authToken: String?) = secureSave(KEY_AUTH_TOKEN, authToken)
118-
}
156+
}

‎iterableapi/src/test/java/com/iterable/iterableapi/IterableConfigTest.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,19 @@ class IterableConfigTest {
2020
val config: IterableConfig = configBuilder.build()
2121
assertThat(config.dataRegion, `is`(IterableDataRegion.EU))
2222
}
23+
24+
@Test
25+
fun defaultDisableKeychainEncryption() {
26+
val configBuilder: IterableConfig.Builder = IterableConfig.Builder()
27+
val config: IterableConfig = configBuilder.build()
28+
assertTrue(config.keychainEncryption)
29+
}
30+
31+
@Test
32+
fun setDisableKeychainEncryption() {
33+
val configBuilder: IterableConfig.Builder = IterableConfig.Builder()
34+
.setKeychainEncryption(false)
35+
val config: IterableConfig = configBuilder.build()
36+
assertFalse(config.keychainEncryption)
37+
}
2338
}

‎iterableapi/src/test/java/com/iterable/iterableapi/IterableKeychainTest.kt

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,14 @@ class IterableKeychainTest {
6262

6363
// Mock migration-related SharedPreferences calls
6464
`when`(mockSharedPrefs.contains(any<String>())).thenReturn(false)
65-
`when`(mockSharedPrefs.getBoolean(any<String>(), anyBoolean())).thenReturn(false)
65+
// Mock encryption flag to be true by default
66+
`when`(mockSharedPrefs.getBoolean(eq("iterable-encryption-enabled"), anyBoolean())).thenReturn(true)
6667
`when`(mockSharedPrefs.getString(any<String>(), any())).thenReturn(null)
6768

6869
// Mock editor.apply() to do nothing
6970
Mockito.doNothing().`when`(mockEditor).apply()
7071

72+
// Create keychain with encryption enabled (default)
7173
keychain = IterableKeychain(
7274
mockContext,
7375
mockDecryptionFailureHandler
@@ -172,9 +174,6 @@ class IterableKeychainTest {
172174

173175
// Verify failure handler was called with any exception
174176
verify(mockDecryptionFailureHandler).onDecryptionFailed(any())
175-
176-
// Verify encryptor keys were reset
177-
verify(mockEncryptor).resetKeys()
178177

179178
assertNull(result)
180179
}
@@ -193,10 +192,7 @@ class IterableKeychainTest {
193192
assertNull(keychain.getAuthToken())
194193

195194
// Verify failure handler was called exactly once for each operation
196-
verify(mockDecryptionFailureHandler, times(3)).onDecryptionFailed(any())
197-
198-
// Verify keys were reset for each failure
199-
verify(mockEncryptor, times(3)).resetKeys()
195+
verify(mockDecryptionFailureHandler, times(1)).onDecryptionFailed(any())
200196
}
201197

202198
@Test
@@ -326,4 +322,52 @@ class IterableKeychainTest {
326322
verify(mockEncryptor, never()).encrypt(isNull())
327323
verify(mockEncryptor, never()).decrypt(isNull())
328324
}
325+
326+
@Test
327+
fun testEncryptionDisabled() {
328+
// Create a new keychain with encryption disabled
329+
val plaintextKeychain = IterableKeychain(
330+
mockContext,
331+
mockDecryptionFailureHandler,
332+
null,
333+
false // encryption = false means encryption is disabled
334+
)
335+
336+
val testEmail = "[email protected]"
337+
val testUserId = "user123"
338+
val testToken = "auth-token-123"
339+
340+
// Mock the SharedPreferences to return plaintext values
341+
`when`(mockSharedPrefs.getString(eq("iterable-email"), isNull())).thenReturn(testEmail)
342+
`when`(mockSharedPrefs.getString(eq("iterable-user-id"), isNull())).thenReturn(testUserId)
343+
`when`(mockSharedPrefs.getString(eq("iterable-auth-token"), isNull())).thenReturn(testToken)
344+
345+
// Mock plaintext flag checks to return true when encryption is disabled
346+
`when`(mockSharedPrefs.getBoolean(eq("iterable-email_plaintext"), eq(false))).thenReturn(true)
347+
`when`(mockSharedPrefs.getBoolean(eq("iterable-user-id_plaintext"), eq(false))).thenReturn(true)
348+
`when`(mockSharedPrefs.getBoolean(eq("iterable-auth-token_plaintext"), eq(false))).thenReturn(true)
349+
350+
// Test save operations
351+
plaintextKeychain.saveEmail(testEmail)
352+
plaintextKeychain.saveUserId(testUserId)
353+
plaintextKeychain.saveAuthToken(testToken)
354+
355+
// Verify values are stored as plaintext
356+
verify(mockEditor).putString(eq("iterable-email"), eq(testEmail))
357+
verify(mockEditor).putString(eq("iterable-user-id"), eq(testUserId))
358+
verify(mockEditor).putString(eq("iterable-auth-token"), eq(testToken))
359+
360+
// Verify plaintext suffix flags are set for better compatibility
361+
verify(mockEditor).putBoolean(eq("iterable-email_plaintext"), eq(true))
362+
verify(mockEditor).putBoolean(eq("iterable-user-id_plaintext"), eq(true))
363+
verify(mockEditor).putBoolean(eq("iterable-auth-token_plaintext"), eq(true))
364+
365+
// Test get operations
366+
assertEquals(testEmail, plaintextKeychain.getEmail())
367+
assertEquals(testUserId, plaintextKeychain.getUserId())
368+
assertEquals(testToken, plaintextKeychain.getAuthToken())
369+
370+
// Verify IterableDataEncryptor was never created
371+
assertNull(plaintextKeychain.encryptor)
372+
}
329373
}

0 commit comments

Comments
 (0)