Commit 4b4ade17 authored by MozLando's avatar MozLando
Browse files

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: default avatarGrisha Kruglov <gkruglov@mozilla.com>
parents 1dcfb602 3061e01c

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

+10 −0
Original line number Diff line number Diff line
@@ -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 = "test@example.com", 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 = "test@example.com", displayName = "test profile")

components/service/firefox-accounts/build.gradle

+24 −0
Original line number Diff line number Diff line
@@ -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

+99 −2
Original line number Diff line number Diff line
@@ -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

+11 −1
Original line number Diff line number Diff line
@@ -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

+10 −3
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ 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
@@ -36,6 +37,7 @@ 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)

        }

    }



    /**