From c43f96096eeceb52dc09e8618caee5cdb3c65ef0 Mon Sep 17 00:00:00 2001 From: Emily Kager Date: Mon, 4 Nov 2019 14:02:16 -0800 Subject: [PATCH] For #5074 - Sync Logins, Uses KeySharedPreferences for Passwords Encryption Key --- app/build.gradle | 1 + .../java/org/mozilla/fenix/FeatureFlags.kt | 2 +- .../fenix/components/BackgroundServices.kt | 23 +++++++++---- .../mozilla/fenix/components/Components.kt | 9 +++++- .../java/org/mozilla/fenix/components/Core.kt | 32 +++++++++++++++++-- .../fenix/logins/SavedLoginsFragment.kt | 2 +- .../java/org/mozilla/fenix/utils/Settings.kt | 10 ++++++ app/src/main/res/values/preference_keys.xml | 2 ++ .../components/BackgroundServicesTest.kt | 15 +++++++-- buildSrc/src/main/java/Dependencies.kt | 1 + 10 files changed, 82 insertions(+), 15 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 4e676fdd3..5aed36c49 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -402,6 +402,7 @@ dependencies { implementation Deps.mozilla_lib_crash implementation Deps.mozilla_lib_push_firebase + implementation Deps.mozilla_lib_dataprotect debugImplementation Deps.leakcanary implementation Deps.androidx_legacy diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index c61ec6436..81de7e6ef 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -39,5 +39,5 @@ object FeatureFlags { /** * Gives option in Settings to see logins and sync logins */ - const val logins = false + val logins = Config.channel.isNightlyOrDebug } diff --git a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt index 042c6081b..ce8e86820 100644 --- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt +++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt @@ -26,6 +26,7 @@ import mozilla.components.feature.push.PushConfig import mozilla.components.feature.push.PushSubscriptionObserver import mozilla.components.feature.push.PushType import mozilla.components.lib.crash.CrashReporter +import mozilla.components.lib.dataprotect.SecureAbove22Preferences import mozilla.components.service.fxa.DeviceConfig import mozilla.components.service.fxa.ServerConfig import mozilla.components.service.fxa.SyncConfig @@ -33,6 +34,7 @@ import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.fxa.manager.SCOPE_SYNC import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider +import mozilla.components.service.sync.logins.SyncableLoginsStore import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.Experiments import org.mozilla.fenix.R @@ -53,7 +55,9 @@ class BackgroundServices( private val context: Context, crashReporter: CrashReporter, historyStorage: PlacesHistoryStorage, - bookmarkStorage: PlacesBookmarksStorage + bookmarkStorage: PlacesBookmarksStorage, + passwordsStorage: SyncableLoginsStore, + secureAbove22Preferences: SecureAbove22Preferences ) { // // A malformed string is causing crashes. // This will be removed when the string is fixed. See #5552 @@ -87,8 +91,9 @@ class BackgroundServices( val syncConfig = if (context.isInExperiment(Experiments.asFeatureSyncDisabled)) { null } else { - // TODO Add Passwords Here Waiting On https://github.com/mozilla-mobile/android-components/issues/4741 - SyncConfig(setOf(SyncEngine.History, SyncEngine.Bookmarks), syncPeriodInMinutes = 240L) // four hours + SyncConfig( + setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords), + syncPeriodInMinutes = 240L) // four hours } private val pushService by lazy { FirebasePush() } @@ -96,11 +101,11 @@ class BackgroundServices( val push by lazy { makePushConfig()?.let { makePush(it) } } init { - // Make the "history", "bookmark", and "logins" stores accessible to workers spawned by the sync manager. + // Make the "history", "bookmark", and "passwords" stores accessible to workers spawned by the sync manager. GlobalSyncableStoreProvider.configureStore(SyncEngine.History to historyStorage) GlobalSyncableStoreProvider.configureStore(SyncEngine.Bookmarks to bookmarkStorage) - // TODO Add Passwords Here Waiting On https://github.com/mozilla-mobile/android-components/issues/4741 - // GlobalSyncableStoreProvider.configureStore(SyncEngine.Passwords to loginsStorage) + GlobalSyncableStoreProvider.configureStore(SyncEngine.Passwords to passwordsStorage) + GlobalSyncableStoreProvider.configureKeyStorage(secureAbove22Preferences) } private val deviceEventObserver = object : DeviceEventsObserver { @@ -170,7 +175,11 @@ class BackgroundServices( ).also { accountManager -> // TODO this needs to change once we have a SyncManager context.settings().fxaHasSyncedItems = syncConfig?.supportedEngines?.isNotEmpty() ?: false - accountManager.registerForDeviceEvents(deviceEventObserver, ProcessLifecycleOwner.get(), false) + accountManager.registerForDeviceEvents( + deviceEventObserver, + ProcessLifecycleOwner.get(), + false + ) // Register a telemetry account observer to keep track of FxA auth metrics. accountManager.register(telemetryAccountObserver) diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index e222f736e..db217ec40 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -15,7 +15,14 @@ import org.mozilla.fenix.utils.ClipboardHandler @Mockable class Components(private val context: Context) { val backgroundServices by lazy { - BackgroundServices(context, analytics.crashReporter, core.historyStorage, core.bookmarksStorage) + BackgroundServices( + context, + analytics.crashReporter, + core.historyStorage, + core.bookmarksStorage, + core.passwordsStorage, + core.secureAbove22Preferences + ) } val services by lazy { Services(context, backgroundServices.accountManager) } val core by lazy { Core(context) } diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 6f27a9c04..871f5f806 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.components import GeckoProvider import android.content.Context import android.content.res.Configuration +import io.sentry.Sentry import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope @@ -34,6 +35,8 @@ import mozilla.components.feature.pwa.ManifestStorage import mozilla.components.feature.pwa.WebAppShortcutManager import mozilla.components.feature.session.HistoryDelegate import mozilla.components.feature.webcompat.WebCompatFeature +import mozilla.components.lib.dataprotect.SecureAbove22Preferences +import mozilla.components.lib.dataprotect.generateEncryptionKey import mozilla.components.service.sync.logins.AsyncLoginsStorageAdapter import mozilla.components.service.sync.logins.SyncableLoginsStore import org.mozilla.fenix.AppRequestInterceptor @@ -171,7 +174,7 @@ class Core(private val context: Context) { val webAppManifestStorage by lazy { ManifestStorage(context) } - val loginsStorage by lazy { + val passwordsStorage by lazy { SyncableLoginsStore( AsyncLoginsStorageAdapter.forDatabase( File( @@ -180,10 +183,29 @@ class Core(private val context: Context) { ).canonicalPath ) ) { - CompletableDeferred("very-insecure-key") + CompletableDeferred(passwordsEncryptionKey) } } + /** + * Shared Preferences that encrypt/decrypt using Android KeyStore and lib-dataprotect for 23+ + * otherwise simply stored + */ + val secureAbove22Preferences by lazy { + SecureAbove22Preferences(context, KEY_STORAGE_NAME) + } + + private val passwordsEncryptionKey = + secureAbove22Preferences.getString(PASSWORDS_KEY) + ?: generateEncryptionKey(KEY_STRENGTH).also { + if (context.settings().passwordsEncryptionKeyGenerated) { + // We already had previously generated an encryption key, but we have lost it + Sentry.capture("Passwords encryption key for passwords storage was lost and we generated a new one") + } + context.settings().recordPasswordsEncryptionKeyGenerated() + secureAbove22Preferences.putString(PASSWORDS_KEY, it) + } + /** * Constructs a [TrackingProtectionPolicy] based on current preferences. * @@ -223,4 +245,10 @@ class Core(private val context: Context) { else -> PreferredColorScheme.Light } } + + companion object { + private const val KEY_STRENGTH = 256 + private const val KEY_STORAGE_NAME = "core_prefs" + private const val PASSWORDS_KEY = "passwords" + } } diff --git a/app/src/main/java/org/mozilla/fenix/logins/SavedLoginsFragment.kt b/app/src/main/java/org/mozilla/fenix/logins/SavedLoginsFragment.kt index 32dbdf6d6..737df5753 100644 --- a/app/src/main/java/org/mozilla/fenix/logins/SavedLoginsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/logins/SavedLoginsFragment.kt @@ -80,7 +80,7 @@ class SavedLoginsFragment : Fragment() { private fun loadAndMapLogins() { lifecycleScope.launch(IO) { val syncedLogins = async { - context!!.components.core.loginsStorage.withUnlocked { + context!!.components.core.passwordsStorage.withUnlocked { it.list().await().map { item -> SavedLoginsItem( item.hostname, diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 5789ac3eb..269a16555 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -213,6 +213,16 @@ class Settings private constructor( fun shouldDeleteAnyDataOnQuit() = DeleteBrowsingDataOnQuitType.values().any { getDeleteDataOnQuit(it) } + val passwordsEncryptionKeyGenerated by booleanPreference( + appContext.getPreferenceKey(R.string.pref_key_encryption_key_generated), + false + ) + + fun recordPasswordsEncryptionKeyGenerated() = preferences.edit().putBoolean( + appContext.getPreferenceKey(R.string.pref_key_encryption_key_generated), + true + ).apply() + val themeSettingString: String get() = when { shouldFollowDeviceTheme -> appContext.getString(R.string.preference_follow_device_theme) diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 9c216399d..a4a8a7176 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -117,4 +117,6 @@ pref_key_testing_stage pref_key_total_uri + + pref_key_encryption_key_generated diff --git a/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt b/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt index 31b277430..a25eb84ca 100644 --- a/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt @@ -50,15 +50,24 @@ class BackgroundServicesTest { val context = mockk(relaxed = true) every { context.isInExperiment(eq(Experiments.asFeatureWebChannelsDisabled)) } returns false - assertEquals("urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel", FxaServer.redirectUrl(context)) + assertEquals( + "urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel", + FxaServer.redirectUrl(context) + ) every { context.isInExperiment(eq(Experiments.asFeatureWebChannelsDisabled)) } returns true - assertEquals("https://accounts.firefox.com/oauth/success/a2270f727f45f648", FxaServer.redirectUrl(context)) + assertEquals( + "https://accounts.firefox.com/oauth/success/a2270f727f45f648", + FxaServer.redirectUrl(context) + ) every { context.isInExperiment(eq(Experiments.asFeatureSyncDisabled)) } returns false var backgroundServices = TestableBackgroundServices(context) assertEquals( - SyncConfig(setOf(SyncEngine.History, SyncEngine.Bookmarks), syncPeriodInMinutes = 240L), + SyncConfig( + setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords), + syncPeriodInMinutes = 240L + ), backgroundServices.syncConfig ) diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 8693ae33f..ce55403f8 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -150,6 +150,7 @@ object Deps { const val mozilla_lib_crash = "org.mozilla.components:lib-crash:${Versions.mozilla_android_components}" const val mozilla_lib_push_firebase = "org.mozilla.components:lib-push-firebase:${Versions.mozilla_android_components}" + const val mozilla_lib_dataprotect = "org.mozilla.components:lib-dataprotect:${Versions.mozilla_android_components}" const val mozilla_ui_publicsuffixlist = "org.mozilla.components:lib-publicsuffixlist:${Versions.mozilla_android_components}"