From 61943df5fcf096a1124bb8fb0c4e937076c058eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAkshay?= <“ayyanchira.akshay@gmail.com”> Date: Thu, 21 Sep 2023 15:33:56 -0700 Subject: [PATCH 1/2] [MOB - 6865] - Keychain null crash fix - Handles condition where IterableKeychain creation returns null to Iterable API. SDK will continue to process without crashing. There will be no email fetched from sharedPreferences in this rare case. No email will be persisted and the app will have to setEmail/setUserId on every app session for it to have email in memory than having it persisted for consequent app sessions - Fixed indentation on IterableKeychain file. Using cmd+l on Android Studio - Added method description in IterableConfig class so developers have more context on what `setEncryptionEnforced` means - getKeychain is now annotated @nullable for future kotlin migration purpose --- .../com/iterable/iterableapi/IterableApi.java | 30 +++++++----- .../iterable/iterableapi/IterableConfig.java | 6 +++ .../iterable/iterableapi/IterableKeychain.kt | 48 +++++++------------ 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index 9be104cc2..b23c3d1fa 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -135,7 +135,7 @@ IterableAuthManager getAuthManager() { return authManager; } - @NonNull + @Nullable IterableKeychain getKeychain() { if (keychain == null) { try { @@ -350,15 +350,25 @@ private String getDeviceId() { } private void storeAuthData() { - getKeychain().saveEmail(_email); - getKeychain().saveUserId(_userId); - getKeychain().saveAuthToken(_authToken); + IterableKeychain iterableKeychain = getKeychain(); + if (iterableKeychain != null) { + iterableKeychain.saveEmail(_email); + iterableKeychain.saveUserId(_userId); + iterableKeychain.saveAuthToken(_authToken); + } else { + IterableLogger.e(TAG, "Shared preference creation failed. "); + } } private void retrieveEmailAndUserId() { - _email = getKeychain().getEmail(); - _userId = getKeychain().getUserId(); - _authToken = getKeychain().getAuthToken(); + IterableKeychain iterableKeychain = getKeychain(); + if (iterableKeychain != null) { + _email = iterableKeychain.getEmail(); + _userId = iterableKeychain.getUserId(); + _authToken = iterableKeychain.getAuthToken(); + } else { + IterableLogger.e(TAG, "retrieveEmailAndUserId: Shared preference creation failed. Could not retrieve email/userId"); + } if (config.authHandler != null) { if (_authToken != null) { @@ -370,10 +380,6 @@ private void retrieveEmailAndUserId() { } } - private void updateSDKVersion() { - - } - private class IterableApiAuthProvider implements IterableApiClient.AuthProvider { @Nullable @Override @@ -507,8 +513,6 @@ public static void initialize(@NonNull Context context, @NonNull String apiKey, sharedInstance.config = new IterableConfig.Builder().build(); } - sharedInstance.updateSDKVersion(); - sharedInstance.retrieveEmailAndUserId(); IterableActivityMonitor.getInstance().registerLifecycleCallbacks(context); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableConfig.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableConfig.java index 9c90a2c96..73a8e8397 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableConfig.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableConfig.java @@ -237,6 +237,12 @@ public Builder setAllowedProtocols(@NonNull String[] allowedProtocols) { return this; } + /** + * Set whether the SDK should enforce encryption. If set to `true`, the SDK will not use fallback mechanism + * of storing data in un-encrypted shared preferences if encrypted database is not available. Set this to `true` + * if PII confidentiality is a concern for your app. + * @param encryptionEnforced `true` will have the SDK enforce encryption. + */ public Builder setEncryptionEnforced(boolean encryptionEnforced) { this.encryptionEnforced = encryptionEnforced; return this; diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index 17e0a705d..ac8d59964 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -26,7 +26,8 @@ class IterableKeychain { encryptionEnabled = false sharedPrefs = context.getSharedPreferences( IterableConstants.SHARED_PREFS_FILE, - Context.MODE_PRIVATE) + Context.MODE_PRIVATE + ) IterableLogger.v(TAG, "SharedPreferences being used") } else { // See if EncryptedSharedPreferences can be created successfully @@ -39,7 +40,8 @@ class IterableKeychain { encryptedSharedPrefsFileName, masterKeyAlias, EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM) + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM + ) encryptionEnabled = true } catch (e: Exception) { if (encryptionEnforced) { @@ -49,35 +51,16 @@ class IterableKeychain { } else { sharedPrefs = context.getSharedPreferences( IterableConstants.SHARED_PREFS_FILE, - Context.MODE_PRIVATE) + Context.MODE_PRIVATE + ) encryptionEnabled = false } } //Try to migrate data from SharedPreferences to EncryptedSharedPreferences - if(encryptionEnabled) { + if (encryptionEnabled) { migrateAuthDataFromSharedPrefsToKeychain(context) } - - } - val masterKeyAlias = MasterKey.Builder(context) - .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) - .build() - sharedPrefs = try { - EncryptedSharedPreferences.create( - context, - encryptedSharedPrefsFileName, - masterKeyAlias, - EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM - ) - } catch (e: Exception) { - if (encryptionEnforced) { - IterableLogger.e(TAG, "Error creating EncryptedSharedPreferences", e) - throw e.fillInStackTrace() - } else { - context.getSharedPreferences(encryptedSharedPrefsFileName, Context.MODE_PRIVATE) - } } } @@ -113,17 +96,20 @@ class IterableKeychain { @RequiresApi(api = Build.VERSION_CODES.M) private fun migrateAuthDataFromSharedPrefsToKeychain(context: Context) { - val oldPrefs: SharedPreferences = context.getSharedPreferences( + val oldPrefs: SharedPreferences = context.getSharedPreferences( IterableConstants.SHARED_PREFS_FILE, - Context.MODE_PRIVATE) + Context.MODE_PRIVATE + ) val sharedPrefsEmail = oldPrefs.getString(IterableConstants.SHARED_PREFS_EMAIL_KEY, null) val sharedPrefsUserId = oldPrefs.getString(IterableConstants.SHARED_PREFS_USERID_KEY, null) - val sharedPrefsAuthToken = oldPrefs.getString(IterableConstants.SHARED_PREFS_AUTH_TOKEN_KEY, null) + val sharedPrefsAuthToken = + oldPrefs.getString(IterableConstants.SHARED_PREFS_AUTH_TOKEN_KEY, null) val editor: SharedPreferences.Editor = oldPrefs.edit() if (getEmail() == null && sharedPrefsEmail != null) { saveEmail(sharedPrefsEmail) editor.remove(IterableConstants.SHARED_PREFS_EMAIL_KEY) - IterableLogger.v(TAG, + IterableLogger.v( + TAG, "UPDATED: migrated email from SharedPreferences to IterableKeychain" ) } else if (sharedPrefsEmail != null) { @@ -132,7 +118,8 @@ class IterableKeychain { if (getUserId() == null && sharedPrefsUserId != null) { saveUserId(sharedPrefsUserId) editor.remove(IterableConstants.SHARED_PREFS_USERID_KEY) - IterableLogger.v(TAG, + IterableLogger.v( + TAG, "UPDATED: migrated userId from SharedPreferences to IterableKeychain" ) } else if (sharedPrefsUserId != null) { @@ -141,7 +128,8 @@ class IterableKeychain { if (getAuthToken() == null && sharedPrefsAuthToken != null) { saveAuthToken(sharedPrefsAuthToken) editor.remove(IterableConstants.SHARED_PREFS_AUTH_TOKEN_KEY) - IterableLogger.v(TAG, + IterableLogger.v( + TAG, "UPDATED: migrated authToken from SharedPreferences to IterableKeychain" ) } else if (sharedPrefsAuthToken != null) { From f8a8fa9a0b88e78d1b14a28e469fd8862916cd6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAkshay?= <“ayyanchira.akshay@gmail.com”> Date: Wed, 27 Sep 2023 13:54:36 -0700 Subject: [PATCH 2/2] Catching Error along with Exception 1. Checking for Throwable which will collect both Exception and Error if at all Security library is crashing with a Error and not exception. 2. Updating the dependency to use latest alpha version. Alpha version 5 also seems to fix race condition during creation process. Reference - https://developer.android.com/jetpack/androidx/releases/security#1.1.0-alpha05 3. Added some warning logs if this situation arrives so developers can take some action if it occurs during debugging --- iterableapi/build.gradle | 2 +- .../iterable/iterableapi/IterableKeychain.kt | 21 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/iterableapi/build.gradle b/iterableapi/build.gradle index d7c716e67..f94401e5b 100644 --- a/iterableapi/build.gradle +++ b/iterableapi/build.gradle @@ -42,7 +42,7 @@ dependencies { api 'androidx.appcompat:appcompat:1.0.0' api 'androidx.annotation:annotation:1.0.0' api 'com.google.firebase:firebase-messaging:20.3.0' - implementation "androidx.security:security-crypto:1.1.0-alpha04" + implementation "androidx.security:security-crypto:1.1.0-alpha06" testImplementation 'junit:junit:4.12' testImplementation 'androidx.test:runner:1.3.0' diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt index ac8d59964..71e438fff 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableKeychain.kt @@ -43,16 +43,31 @@ class IterableKeychain { EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM ) encryptionEnabled = true - } catch (e: Exception) { + } catch (e: Throwable) { + if (e is Error) { + IterableLogger.e( + TAG, + "EncryptionSharedPreference creation failed with Error. Attempting to continue" + ) + } + if (encryptionEnforced) { - //TODO: In memory Or similar solution needs to be implemented in future. - IterableLogger.e(TAG, "Error creating EncryptedSharedPreferences", e) + //TODO: In-memory or similar solution needs to be implemented in the future. + IterableLogger.w( + TAG, + "Encryption is enforced. PII will not be persisted due to EncryptionSharedPreference failure. Email/UserId and Auth token will have to be passed for every app session.", + e + ) throw e.fillInStackTrace() } else { sharedPrefs = context.getSharedPreferences( IterableConstants.SHARED_PREFS_FILE, Context.MODE_PRIVATE ) + IterableLogger.w( + TAG, + "Using SharedPreference as EncryptionSharedPreference creation failed." + ) encryptionEnabled = false } }