Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Closes #2229: Encrypted-at-rest FxA state storage support
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Grisha Kruglov committed Nov 15, 2019
1 parent 8c74c93 commit 84830fb
Show file tree
Hide file tree
Showing 9 changed files with 329 additions and 9 deletions.
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -75,6 +80,7 @@ class FirefoxAccountsAuthFeatureTest {
assertEquals("auth://url", authLabmda.url)
}

@Config(sdk = [22])
@Test
fun `begin pairing authentication`() {
val manager = prepareAccountManagerForSuccessfulAuthentication()
Expand All @@ -93,6 +99,7 @@ class FirefoxAccountsAuthFeatureTest {
assertEquals("auth://url", authLabmda.url)
}

@Config(sdk = [22])
@Test
fun `begin authentication with errors`() {
val manager = prepareAccountManagerForFailedAuthentication()
Expand All @@ -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()
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down
24 changes: 24 additions & 0 deletions components/service/firefox-accounts/build.gradle
Expand Up @@ -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')
Expand All @@ -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
Expand All @@ -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'
Expand Down
Expand Up @@ -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
Expand All @@ -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()
}
}
Expand Up @@ -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
)

/**
Expand Down
Expand Up @@ -27,13 +27,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
Expand Down Expand Up @@ -116,6 +118,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
Expand Down Expand Up @@ -889,13 +892,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)
}
}

/**
Expand Down

0 comments on commit 84830fb

Please sign in to comment.