From f0575ddd6ad28068e0542f6ee968f615a52864e4 Mon Sep 17 00:00:00 2001 From: Oliver Scott Date: Wed, 16 Nov 2022 13:08:51 -0500 Subject: [PATCH] Add support for d2d Co-authored-by: Tommy Webb Change-Id: I61d88a511a0f81e6d384e3650f6797725da79814 --- .../seedvault/settings/SettingsManager.kt | 23 ++++++++- .../transport/ConfigurableBackupTransport.kt | 8 +-- .../ConfigurableBackupTransportService.kt | 10 +++- .../transport/backup/BackupCoordinator.kt | 13 ----- .../transport/backup/BackupModule.kt | 2 +- .../transport/backup/PackageService.kt | 37 +++++++++----- .../transport/restore/RestoreCoordinator.kt | 11 +++- .../seedvault/settings/SettingsManagerTest.kt | 51 +++++++++++++++++++ .../transport/backup/BackupCoordinatorTest.kt | 16 ------ .../restore/RestoreCoordinatorTest.kt | 2 +- 10 files changed, 123 insertions(+), 50 deletions(-) create mode 100644 app/src/test/java/com/stevesoltys/seedvault/settings/SettingsManagerTest.kt diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt index a1fa0ca6..58119f67 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt @@ -5,7 +5,10 @@ import android.hardware.usb.UsbDevice import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.net.Uri +import android.util.Log import androidx.annotation.UiThread +import androidx.annotation.VisibleForTesting +import androidx.annotation.VisibleForTesting.PRIVATE import androidx.annotation.WorkerThread import androidx.documentfile.provider.DocumentFile import androidx.preference.PreferenceManager @@ -34,6 +37,8 @@ private const val PREF_KEY_UNLIMITED_QUOTA = "unlimited_quota" class SettingsManager(private val context: Context) { + private val TAG = SettingsManager::class.java.simpleName + private val prefs = permitDiskReads { PreferenceManager.getDefaultSharedPreferences(context) } @@ -41,12 +46,19 @@ class SettingsManager(private val context: Context) { @Volatile private var token: Long? = null + /** + * The StoragePlugin provider package name is assigned by ConfigurableBackupTransportService + * to ensure it is excluded; if the provider is killed, the backup or restore will fail. + */ + var pluginProviderPackageName: String? = null + /** * This gets accessed by non-UI threads when saving with [PreferenceManager] * and when [isBackupEnabled] is called during a backup run. * Therefore, it is implemented with a thread-safe [ConcurrentSkipListSet]. */ - private val blacklistedApps: MutableSet by lazy { + @VisibleForTesting(otherwise = PRIVATE) + val blacklistedApps: MutableSet by lazy { ConcurrentSkipListSet(prefs.getStringSet(PREF_KEY_BACKUP_APP_BLACKLIST, emptySet())) } @@ -132,6 +144,15 @@ class SettingsManager(private val context: Context) { fun isBackupEnabled(packageName: String) = !blacklistedApps.contains(packageName) + fun isAppAllowedForBackup(packageName: String): Boolean { + // Check that the app is not blacklisted by the user + val enabled = isBackupEnabled(packageName) + if (!enabled) Log.w(TAG, "Backup of $packageName disabled by user.") + // We need to exclude the DocumentsProvider used to store backup data. + // Otherwise, it gets killed when we back it up, terminating our backup. + return enabled && packageName != pluginProviderPackageName + } + fun isStorageBackupEnabled() = prefs.getBoolean(PREF_KEY_BACKUP_STORAGE, false) @UiThread diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransport.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransport.kt index 0d293f15..feb5daff 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransport.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransport.kt @@ -12,6 +12,7 @@ import android.os.ParcelFileDescriptor import android.util.Log import com.stevesoltys.seedvault.R import com.stevesoltys.seedvault.settings.SettingsActivity +import com.stevesoltys.seedvault.settings.SettingsManager import com.stevesoltys.seedvault.transport.backup.BackupCoordinator import com.stevesoltys.seedvault.transport.restore.RestoreCoordinator import kotlinx.coroutines.runBlocking @@ -22,7 +23,7 @@ import org.koin.core.component.inject val TRANSPORT_ID: String = ConfigurableBackupTransport::class.java.name // Since there seems to be consensus in the community to pose as device to device transport, -// we are pretending to be one here. This will backup opt-out apps that target API 30. +// we are pretending to be one here. This will backup opt-out apps that target at least API 30. const val TRANSPORT_FLAGS = FLAG_CLIENT_SIDE_ENCRYPTION_ENABLED or FLAG_DEVICE_TO_DEVICE_TRANSFER private const val TRANSPORT_DIRECTORY_NAME = @@ -38,6 +39,7 @@ class ConfigurableBackupTransport internal constructor(private val context: Cont private val backupCoordinator by inject() private val restoreCoordinator by inject() + private val settingsManager by inject() override fun transportDirName(): String { return TRANSPORT_DIRECTORY_NAME @@ -120,9 +122,9 @@ class ConfigurableBackupTransport internal constructor(private val context: Cont override fun isAppEligibleForBackup( targetPackage: PackageInfo, - isFullBackup: Boolean, + @Suppress("UNUSED_PARAMETER") isFullBackup: Boolean, ): Boolean { - return backupCoordinator.isAppEligibleForBackup(targetPackage, isFullBackup) + return settingsManager.isAppAllowedForBackup(targetPackage.packageName) } override fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long = runBlocking { diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt index 1d67988d..bccaee98 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt @@ -11,6 +11,8 @@ import android.util.Log import androidx.annotation.WorkerThread import com.stevesoltys.seedvault.BackupMonitor import com.stevesoltys.seedvault.crypto.KeyManager +import com.stevesoltys.seedvault.plugins.StoragePlugin +import com.stevesoltys.seedvault.settings.SettingsManager import com.stevesoltys.seedvault.transport.backup.PackageService import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import com.stevesoltys.seedvault.ui.notification.NotificationBackupObserver @@ -65,7 +67,13 @@ fun requestBackup(context: Context) { val backupManager: IBackupManager = get().get() if (backupManager.isBackupEnabled) { val packageService: PackageService = get().get() - val packages = packageService.eligiblePackages + val settingsManager: SettingsManager = get().get() + val plugin: StoragePlugin = get().get() + + // SettingsManager must know the StoragePlugin provider package name to exclude it when + // isAppAllowedForBackup is called. + settingsManager.pluginProviderPackageName = plugin.providerPackageName + val packages = packageService.requestedPackages val appTotals = packageService.expectedAppTotals val result = try { diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index 5d791fc4..7e043b7a 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -138,19 +138,6 @@ internal class BackupCoordinator( TRANSPORT_ERROR } - fun isAppEligibleForBackup( - targetPackage: PackageInfo, - @Suppress("UNUSED_PARAMETER") isFullBackup: Boolean, - ): Boolean { - val packageName = targetPackage.packageName - // Check that the app is not blacklisted by the user - val enabled = settingsManager.isBackupEnabled(packageName) - if (!enabled) Log.w(TAG, "Backup of $packageName disabled by user.") - // We need to exclude the DocumentsProvider used to store backup data. - // Otherwise, it gets killed when we back it up, terminating our backup. - return enabled && targetPackage.packageName != plugin.providerPackageName - } - /** * Ask the transport about current quota for backup size of the package. * diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt index 8be00ac6..b5afe05c 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt @@ -8,7 +8,7 @@ val backupModule = module { single { PackageService( context = androidContext(), - backupManager = get() + settingsManager = get(), ) } single { diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt index fbadc1a4..c86378fd 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt @@ -1,6 +1,5 @@ package com.stevesoltys.seedvault.transport.backup -import android.app.backup.IBackupManager import android.content.Context import android.content.pm.ApplicationInfo.FLAG_ALLOW_BACKUP import android.content.pm.ApplicationInfo.FLAG_STOPPED @@ -17,6 +16,7 @@ import android.util.Log import android.util.Log.INFO import androidx.annotation.WorkerThread import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER +import com.stevesoltys.seedvault.settings.SettingsManager private val TAG = PackageService::class.java.simpleName @@ -28,13 +28,13 @@ private const val LOG_MAX_PACKAGES = 100 */ internal class PackageService( private val context: Context, - private val backupManager: IBackupManager, + private val settingsManager: SettingsManager, ) { private val packageManager: PackageManager = context.packageManager private val myUserId = UserHandle.myUserId() - val eligiblePackages: Array + val requestedPackages: Array @WorkerThread @Throws(RemoteException::class) get() { @@ -45,22 +45,27 @@ internal class PackageService( // log packages if (Log.isLoggable(TAG, INFO)) { Log.i(TAG, "Got ${packages.size} packages:") - packages.chunked(LOG_MAX_PACKAGES).forEach { - Log.i(TAG, it.toString()) - } + logPackages(packages) } - val eligibleApps = - backupManager.filterAppsEligibleForBackupForUser(myUserId, packages.toTypedArray()) + // As a result of switching to D2D, we can no longer use BackupManager's method + // filterAppsEligibleForBackupForUser, because it will not return all of the expected + // apps; it is not designed to determine MIGRATION eligibility, only eligibility for + // inclusion in *scheduled* BACKUPs, which are implicitly *not* D2D migrations. + // None of the other eligibility methods are exposed by AOSP APIs. On the other hand, + // the actual backup process properly utilizes OperationType.MIGRATION and performs its + // own checks as to whether apps are allowed to be backed up. All we need to do now is + // filter out apps that *we* want to be excluded. The system will do the rest later. + val requestedApps = packages.filter { settingsManager.isAppAllowedForBackup(it) } - // log eligible packages + // log requested packages if (Log.isLoggable(TAG, INFO)) { - Log.i(TAG, "Filtering left ${eligibleApps.size} eligible packages:") - logPackages(eligibleApps.toList()) + Log.i(TAG, "Filtering left ${requestedApps.size} requested packages:") + logPackages(requestedApps.toList()) } // add magic @pm@ package (PACKAGE_MANAGER_SENTINEL) which holds package manager data - val packageArray = eligibleApps.toMutableList() + val packageArray = requestedApps.toMutableList() packageArray.add(MAGIC_PACKAGE_MANAGER) return packageArray.toTypedArray() @@ -159,7 +164,13 @@ internal fun PackageInfo.isSystemApp(): Boolean { internal fun PackageInfo.allowsBackup(): Boolean { if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false - return applicationInfo.flags and FLAG_ALLOW_BACKUP != 0 + + // At backup time, the system will filter out any apps that *it* does not want to be backed up. + // Now that we have switched to D2D, *we* generally want to back up as much as possible; + // part of the point of D2D is to ignore FLAG_ALLOW_BACKUP (allowsBackup). So, we return true. + // See frameworks/base/services/backup/java/com/android/server/backup/utils/ + // BackupEligibilityRules.java lines 74-81 and 163-167 (tag: android-13.0.0_r8). + return true } /** diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt index 23d8b6a6..f6a00ecc 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt @@ -26,6 +26,14 @@ import com.stevesoltys.seedvault.transport.TRANSPORT_FLAGS import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import java.io.IOException +/** + * Device name used in AOSP to indicate that a restore set is part of a device-to-device migration. + * See getBackupEligibilityRules in frameworks/base/services/backup/java/com/android/server/ + * backup/restore/ActiveRestoreSession.java. AOSP currently relies on this constant, and it is not + * publicly exposed. Framework code indicates they intend to use a flag, instead, in the future. + */ +internal const val DEVICE_NAME_FOR_D2D_SET = "D2D" + private data class RestoreCoordinatorState( val token: Long, val packages: Iterator, @@ -92,7 +100,8 @@ internal class RestoreCoordinator( **/ suspend fun getAvailableRestoreSets(): Array? { return getAvailableMetadata()?.map { (_, metadata) -> - RestoreSet(metadata.deviceName, metadata.deviceName, metadata.token, TRANSPORT_FLAGS) + RestoreSet(metadata.deviceName /* name */, DEVICE_NAME_FOR_D2D_SET /* device */, + metadata.token, TRANSPORT_FLAGS) }?.toTypedArray() } diff --git a/app/src/test/java/com/stevesoltys/seedvault/settings/SettingsManagerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/settings/SettingsManagerTest.kt new file mode 100644 index 00000000..b19af0d3 --- /dev/null +++ b/app/src/test/java/com/stevesoltys/seedvault/settings/SettingsManagerTest.kt @@ -0,0 +1,51 @@ +package com.stevesoltys.seedvault.settings + +import com.stevesoltys.seedvault.transport.TransportTest +import io.mockk.every +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +internal class SettingsManagerTest : TransportTest() { + + private val userExcludedApps = mutableSetOf() + + @Test + fun `isAppAllowedForBackup() exempts plugin provider and blacklisted apps`() { + val pkgName = packageInfo.packageName + + preparePartiallyMockedSettingsManager() + + settingsManager.pluginProviderPackageName = pkgName + assertFalse(settingsManager.isAppAllowedForBackup(pkgName), + "Plugin provider must not be allowed for backup") + + settingsManager.pluginProviderPackageName = "new.package" + userExcludedApps.add(pkgName) + assertFalse(settingsManager.isAppAllowedForBackup(pkgName), + "Excluded package must not be allowed for backup") + + userExcludedApps.remove(pkgName) + assertTrue(settingsManager.isAppAllowedForBackup(pkgName), + "Non-excluded package should be allowed if it is not the plugin provider") + } + + private fun preparePartiallyMockedSettingsManager() { + // Use our own app exclusion set to avoid the uninitialized set in mocked SettingsManager. + every { settingsManager.blacklistedApps } answers { userExcludedApps } + + // Always call isAppAllowedForBackup unmocked rather than an unimplemented mock. + every { settingsManager.isAppAllowedForBackup(any()) } answers { callOriginal() } + + // Always call isBackupEnabled unmocked. + every { settingsManager.isBackupEnabled(any()) } answers { callOriginal() } + + // Always set the underlying provider name field for use by unmocked isAppAllowedForBackup. + every { + settingsManager.pluginProviderPackageName = any() + } propertyType String::class answers { + fieldValue = value + } + } + +} diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index d5facae7..e9148947 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt @@ -32,8 +32,6 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.IOException import java.io.OutputStream @@ -183,20 +181,6 @@ internal class BackupCoordinatorTest : BackupTest() { verify { metadataOutputStream.close() } } - @Test - fun `isAppEligibleForBackup() exempts plugin provider and blacklisted apps`() { - every { - settingsManager.isBackupEnabled(packageInfo.packageName) - } returns true andThen false andThen true - every { - plugin.providerPackageName - } returns packageInfo.packageName andThen "new.package" andThen "new.package" - - assertFalse(backup.isAppEligibleForBackup(packageInfo, true)) - assertFalse(backup.isAppEligibleForBackup(packageInfo, true)) - assertTrue(backup.isAppEligibleForBackup(packageInfo, true)) - } - @Test fun `clearing KV backup data throws`() = runBlocking { every { settingsManager.getToken() } returns token diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt index c7c11dd8..4f704dee 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt @@ -87,7 +87,7 @@ internal class RestoreCoordinatorTest : TransportTest() { val sets = restore.getAvailableRestoreSets() ?: fail() assertEquals(2, sets.size) - assertEquals(metadata.deviceName, sets[0].device) + assertEquals(DEVICE_NAME_FOR_D2D_SET, sets[0].device) assertEquals(metadata.deviceName, sets[0].name) assertEquals(metadata.token, sets[0].token) }