Merge #5053

5053: Closes #2229: Encrypted-at-rest FxA state storage support r=csadilek a=grigoryk

This patch adds a version of `AccountStorage` which is backed by an encrypted-at-rest shared-prefs implementation,
`SecureAbove22Preferences`. As the name suggests, encryption at rest is enabled only for Android API levels 23+.
Otherwise, plaintext storage is used. `SecureAbove22Preferences` will handle API level upgrades behind the scenes,
if necessary.

In order to support rolling this out, `SecureAbove22AccountStorage` automatically migrates account state if it was
present in `SharedPrefAccountStorage`. And vice-versa, `SharedPrefAccountStorage` will automatically migrate account
state if it was present in `SecureAbove22AccountStorage`. This allows applications to easily switch between two
implementations, without any ill-effects.

In order to monitor storage implementations for abnormalities (such as disappearing encryption keys), an optional
`CrashReporter` instance may be configured now via FxaAccountManager.

`DeviceConfig` gained a `secureStateAtRest` flag, which allows applications to specify if they'd like to encrypt
account state. This config object isn't a perfect fit for this flag, but it's close enough conceptually.




Co-authored-by: Grisha Kruglov <[email protected]>

components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeatureTest.kt

@@ -30,6 +30,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
+import org.robolectric.annotation.Config
 
 // Same as the actual account manager, except we get to control how FirefoxAccountShaped instances
 // are created. This is necessary because due to some build issues (native dependencies not available
@@ -57,6 +58,10 @@ class FirefoxAccountsAuthFeatureTest {
         }
     }
 
+    // Note that tests that involve secure storage specify API=21, because of issues testing secure storage on
+    // 23+ API levels. See https://github.com/mozilla-mobile/android-components/issues/4956
+
+    @Config(sdk = [22])
     @Test
     fun `begin authentication`() {
         val manager = prepareAccountManagerForSuccessfulAuthentication()
@@ -75,6 +80,7 @@ class FirefoxAccountsAuthFeatureTest {
         assertEquals("auth://url", authLabmda.url)
     }
 
+    @Config(sdk = [22])
     @Test
     fun `begin pairing authentication`() {
         val manager = prepareAccountManagerForSuccessfulAuthentication()
@@ -93,6 +99,7 @@ class FirefoxAccountsAuthFeatureTest {
         assertEquals("auth://url", authLabmda.url)
     }
 
+    @Config(sdk = [22])
     @Test
     fun `begin authentication with errors`() {
         val manager = prepareAccountManagerForFailedAuthentication()
@@ -112,6 +119,7 @@ class FirefoxAccountsAuthFeatureTest {
         assertEquals("https://accounts.firefox.com/signin", authLambda.url)
     }
 
+    @Config(sdk = [22])
     @Test
     fun `begin pairing authentication with errors`() {
         val manager = prepareAccountManagerForFailedAuthentication()
@@ -203,6 +211,7 @@ class FirefoxAccountsAuthFeatureTest {
         )
     }
 
+    @Config(sdk = [22])
     private fun prepareAccountManagerForSuccessfulAuthentication(): TestableFxaAccountManager {
         val mockAccount: OAuthAccount = mock()
         val profile = Profile(uid = "testUID", avatar = null, email = "[email protected]", displayName = "test profile")
@@ -227,6 +236,7 @@ class FirefoxAccountsAuthFeatureTest {
         return manager
     }
 
+    @Config(sdk = [22])
     private fun prepareAccountManagerForFailedAuthentication(): TestableFxaAccountManager {
         val mockAccount: OAuthAccount = mock()
         val profile = Profile(uid = "testUID", avatar = null, email = "[email protected]", displayName = "test profile")

components/service/firefox-accounts/build.gradle

@@ -27,6 +27,23 @@ android {
     }
 }
 
+configurations {
+    // There's an interaction between Gradle's resolution of dependencies with different types
+    // (@jar, @aar) for `implementation` and `testImplementation` and with Android Studio's built-in
+    // JUnit test runner.  The runtime classpath in the built-in JUnit test runner gets the
+    // dependency from the `implementation`, which is type @aar, and therefore the JNA dependency
+    // doesn't provide the JNI dispatch libraries in the correct Java resource directories.  I think
+    // what's happening is that @aar type in `implementation` resolves to the @jar type in
+    // `testImplementation`, and that it wins the dependency resolution battle.
+    //
+    // A workaround is to add a new configuration which depends on the @jar type and to reference
+    // the underlying JAR file directly in `testImplementation`.  This JAR file doesn't resolve to
+    // the @aar type in `implementation`.  This works when invoked via `gradle`, but also sets the
+    // correct runtime classpath when invoked with Android Studio's built-in JUnit test runner.
+    // Success!
+    jnaForTest
+}
+
 dependencies {
     // Types defined in concept-sync are part of the public API of this module.
     api project(':concept-sync')
@@ -40,6 +57,8 @@ dependencies {
     implementation project(':support-sync-telemetry')
     implementation project(':support-ktx')
     implementation project(':lib-dataprotect')
+    // CrashReporter is part of the public API.
+    api project(':lib-crash')
 
     implementation Dependencies.kotlin_stdlib
     implementation Dependencies.kotlin_coroutines
@@ -53,6 +72,11 @@ dependencies {
     testImplementation Dependencies.testing_robolectric
     testImplementation Dependencies.testing_mockito
     testImplementation Dependencies.testing_coroutines
+
+    jnaForTest Dependencies.thirdparty_jna
+    testImplementation files(configurations.jnaForTest.copyRecursive().files)
+
+    testImplementation Dependencies.mozilla_full_megazord_forUnitTests
 }
 
 apply from: '../../../publish.gradle'

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/AccountStorage.kt

@@ -7,21 +7,46 @@ package mozilla.components.service.fxa
 import android.content.Context
 import android.content.SharedPreferences
 import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.lib.crash.CrashReporter
+import mozilla.components.lib.dataprotect.SecureAbove22Preferences
 
 const val FXA_STATE_PREFS_KEY = "fxaAppState"
 const val FXA_STATE_KEY = "fxaState"
 
-interface AccountStorage {
+internal interface AccountStorage {
     @Throws(Exception::class)
     fun read(): OAuthAccount?
     fun write(accountState: String)
     fun clear()
 }
 
-class SharedPrefAccountStorage(val context: Context) : AccountStorage {
+/**
+ * Account storage layer which uses plaintext storage implementation.
+ *
+ * Migration from [SecureAbove22AccountStorage] will happen upon initialization,
+ * unless disabled via [migrateFromSecureStorage].
+ */
+internal class SharedPrefAccountStorage(
+    val context: Context,
+    crashReporter: CrashReporter? = null,
+    migrateFromSecureStorage: Boolean = true
+) : AccountStorage {
+    init {
+        if (migrateFromSecureStorage) {
+            // In case we switched from SecureAbove22AccountStorage to this implementation, migrate persisted account
+            // and clear out the old storage layer.
+            val secureStorage = SecureAbove22AccountStorage(context, crashReporter, migrateFromPlaintextStorage = false)
+            secureStorage.read()?.let { secureAccount ->
+                this.write(secureAccount.toJSONString())
+                secureStorage.clear()
+            }
+        }
+    }
+
     /**
      * @throws FxaException if JSON failed to parse into a [FirefoxAccount].
      */
+    @Throws(FxaException::class)
     override fun read(): OAuthAccount? {
         val savedJSON = accountPreferences().getString(FXA_STATE_KEY, null)
                 ?: return null
@@ -48,3 +73,75 @@ class SharedPrefAccountStorage(val context: Context) : AccountStorage {
         return context.getSharedPreferences(FXA_STATE_PREFS_KEY, Context.MODE_PRIVATE)
     }
 }
+
+/**
+ * A base class for exceptions describing abnormal account storage behaviour.
+ */
+internal abstract class AbnormalAccountStorageEvent : Exception() {
+    /**
+     * Account state was expected to be present, but it wasn't.
+     */
+    internal class UnexpectedlyMissingAccountState : AbnormalAccountStorageEvent()
+}
+
+/**
+ * Account storage layer which uses encrypted-at-rest storage implementation for supported API levels (23+).
+ * On older API versions account state is stored in plaintext.
+ *
+ * Migration from [SharedPrefAccountStorage] will happen upon initialization,
+ * unless disabled via [migrateFromPlaintextStorage].
+ */
+internal class SecureAbove22AccountStorage(
+    context: Context,
+    private val crashReporter: CrashReporter? = null,
+    migrateFromPlaintextStorage: Boolean = true
+) : AccountStorage {
+    companion object {
+        private const val STORAGE_NAME = "fxaStateAC"
+        private const val KEY_ACCOUNT_STATE = "fxaState"
+        private const val PREF_NAME = "fxaStatePrefAC"
+        private const val PREF_KEY_HAS_STATE = "fxaStatePresent"
+    }
+
+    private val store = SecureAbove22Preferences(context, STORAGE_NAME)
+    // Prefs are used here to keep track of abnormal storage behaviour - namely, account state disappearing without
+    // being cleared first through this class. Note that clearing application data will clear both 'store' and 'prefs'.
+    private val prefs = context.getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE)
+
+    init {
+        if (migrateFromPlaintextStorage) {
+            // In case we switched from SharedPrefAccountStorage to this implementation, migrate persisted account
+            // and clear out the old storage layer.
+            val plaintextStorage = SharedPrefAccountStorage(context, migrateFromSecureStorage = false)
+            plaintextStorage.read()?.let { plaintextAccount ->
+                this.write(plaintextAccount.toJSONString())
+                plaintextStorage.clear()
+            }
+        }
+    }
+
+    /**
+     * @throws FxaException if JSON failed to parse into a [FirefoxAccount].
+     */
+    @Throws(FxaException::class)
+    override fun read(): OAuthAccount? {
+        return store.getString(KEY_ACCOUNT_STATE).also {
+            // If account state is missing, but we expected it to be present, report an exception.
+            if (it == null && prefs.getBoolean(PREF_KEY_HAS_STATE, false)) {
+                crashReporter?.submitCaughtException(AbnormalAccountStorageEvent.UnexpectedlyMissingAccountState())
+                // Clear prefs to make sure we only submit this exception once.
+                prefs.edit().clear().apply()
+            }
+        }?.let { FirefoxAccount.fromJSONString(it) }
+    }
+
+    override fun write(accountState: String) {
+        store.putString(KEY_ACCOUNT_STATE, accountState)
+        prefs.edit().putBoolean(PREF_KEY_HAS_STATE, true).apply()
+    }
+
+    override fun clear() {
+        store.clear()
+        prefs.edit().clear().apply()
+    }
+}

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Config.kt

@@ -24,11 +24,21 @@ typealias ServerConfig = mozilla.appservices.fxaclient.Config
  * @property capabilities A set of device capabilities, such as SEND_TAB. This set can be expanded by
  * re-initializing [FxaAccountManager] with a new set (e.g. on app restart).
  * Shrinking a set of capabilities is currently not supported.
+ *
+ * @property secureStateAtRest A flag indicating whether or not to use encrypted storage for the persisted account
+ * state. If set to `true`, [SecureAbove22AccountStorage] will be used as a storage layer. As the name suggests,
+ * account state will only by encrypted on Android API 23+. Otherwise, even if this flag is set to `true`, account state
+ * will be stored in plaintext.
+ *
+ * Default value of `false` configures the plaintext version of account storage to be used, [SharedPrefAccountStorage].
+ *
+ * Switching of this flag's values is supported; account state will be migrated between the underlying storage layers.
  */
 data class DeviceConfig(
     val name: String,
     val type: DeviceType,
-    val capabilities: Set<DeviceCapability>
+    val capabilities: Set<DeviceCapability>,
+    val secureStateAtRest: Boolean = false
 )
 
 /**

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt

@@ -29,13 +29,15 @@ import mozilla.components.concept.sync.DeviceEventsObserver
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.concept.sync.Profile
 import mozilla.components.concept.sync.StatePersistenceCallback
+import mozilla.components.lib.crash.CrashReporter
 import mozilla.components.service.fxa.AccountStorage
 import mozilla.components.service.fxa.DeviceConfig
 import mozilla.components.service.fxa.FxaDeviceSettingsCache
 import mozilla.components.service.fxa.FirefoxAccount
 import mozilla.components.service.fxa.FxaAuthData
 import mozilla.components.service.fxa.FxaException
 import mozilla.components.service.fxa.FxaPanicException
+import mozilla.components.service.fxa.SecureAbove22AccountStorage
 import mozilla.components.service.fxa.ServerConfig
 import mozilla.components.service.fxa.SharedPrefAccountStorage
 import mozilla.components.service.fxa.SyncAuthInfoCache
@@ -125,6 +127,7 @@ open class FxaAccountManager(
     private val deviceConfig: DeviceConfig,
     @Volatile private var syncConfig: SyncConfig?,
     private val applicationScopes: Set<String> = emptySet(),
+    private val crashReporter: CrashReporter? = null,
     // We want a single-threaded execution model for our account-related "actions" (state machine side-effects).
     // That is, we want to ensure a sequential execution flow, but on a background thread.
     private val coroutineContext: CoroutineContext = Executors
@@ -896,13 +899,17 @@ open class FxaAccountManager(
     }
 
     @VisibleForTesting
-    open fun createSyncManager(config: SyncConfig): SyncManager {
+    internal open fun createSyncManager(config: SyncConfig): SyncManager {
         return WorkManagerSyncManager(context, config)
     }
 
     @VisibleForTesting
-    open fun getAccountStorage(): AccountStorage {
-        return SharedPrefAccountStorage(context)
+    internal open fun getAccountStorage(): AccountStorage {
+        return if (deviceConfig.secureStateAtRest) {
+            SecureAbove22AccountStorage(context, crashReporter)
+        } else {
+            SharedPrefAccountStorage(context, crashReporter)
+        }
     }
 
     /**