From d598aac81e0a2a181f11878b942464c9b04eecd3 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 31 Mar 2022 13:20:24 -0300 Subject: [PATCH] Clean up system USB storage feature a bit --- .../main/java/com/stevesoltys/seedvault/App.kt | 5 +++-- .../saf/DocumentsProviderStoragePlugin.kt | 10 +++++++--- .../seedvault/plugins/saf/DocumentsStorage.kt | 16 +++++++++------- .../seedvault/settings/SettingsManager.kt | 6 +++--- .../seedvault/storage/SeedvaultStoragePlugin.kt | 3 +++ .../seedvault/ui/storage/StorageRootResolver.kt | 14 +++++++++----- .../seedvault/plugins/saf/StoragePluginTest.kt | 1 + .../plugin/TestSafStoragePlugin.kt | 2 +- .../storage/plugin/saf/SafStoragePlugin.kt | 16 ++++++++++------ 9 files changed, 46 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/App.kt b/app/src/main/java/com/stevesoltys/seedvault/App.kt index bcd3d104..7a053873 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/App.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/App.kt @@ -144,6 +144,7 @@ fun permitDiskReads(func: () -> T): T { } fun Context.getSystemContext(isUsbStorage: () -> Boolean): Context { - return if (checkSelfPermission(INTERACT_ACROSS_USERS_FULL) == PERMISSION_GRANTED - && isUsbStorage()) createContextAsUser(UserHandle.SYSTEM, 0) else this + return if (checkSelfPermission(INTERACT_ACROSS_USERS_FULL) == PERMISSION_GRANTED && + isUsbStorage() + ) createContextAsUser(UserHandle.SYSTEM, 0) else this } diff --git a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt index 55dbd98e..94ff8ad8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt @@ -21,9 +21,13 @@ internal class DocumentsProviderStoragePlugin( private val storage: DocumentsStorage, ) : StoragePlugin { - private val context: Context get() = appContext.getSystemContext { - storage.storage?.isUsb == true - } + /** + * Attention: This context might be from a different user. Use with care. + */ + private val context: Context + get() = appContext.getSystemContext { + storage.storage?.isUsb == true + } private val packageManager: PackageManager = appContext.packageManager diff --git a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsStorage.kt b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsStorage.kt index a174f29d..2ae9699d 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsStorage.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsStorage.kt @@ -45,19 +45,21 @@ internal class DocumentsStorage( private val appContext: Context, private val settingsManager: SettingsManager, ) { - - private val context: Context get() = appContext.getSystemContext { - storage?.isUsb ?: false - } - - private val contentResolver: ContentResolver get() = context.contentResolver - internal var storage: Storage? = null get() { if (field == null) field = settingsManager.getStorage() return field } + /** + * Attention: This context might be from a different user. Use with care. + */ + private val context: Context + get() = appContext.getSystemContext { + storage?.isUsb == true + } + private val contentResolver: ContentResolver get() = context.contentResolver + internal var rootBackupDir: DocumentFile? = null get() = runBlocking { if (field == null) { 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 18179eed..a1fa0ca6 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt @@ -5,11 +5,11 @@ import android.hardware.usb.UsbDevice import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.net.Uri -import android.os.UserHandle import androidx.annotation.UiThread import androidx.annotation.WorkerThread import androidx.documentfile.provider.DocumentFile import androidx.preference.PreferenceManager +import com.stevesoltys.seedvault.getSystemContext import com.stevesoltys.seedvault.permitDiskReads import com.stevesoltys.seedvault.transport.backup.BackupCoordinator import java.util.concurrent.ConcurrentSkipListSet @@ -122,8 +122,8 @@ class SettingsManager(private val context: Context) { @WorkerThread fun canDoBackupNow(): Boolean { val storage = getStorage() ?: return false - return !storage.isUnavailableUsb(context.createContextAsUser(UserHandle.SYSTEM, 0)) - && !storage.isUnavailableNetwork(context) + val systemContext = context.getSystemContext { storage.isUsb } + return !storage.isUnavailableUsb(systemContext) && !storage.isUnavailableNetwork(context) } fun backupApks(): Boolean { diff --git a/app/src/main/java/com/stevesoltys/seedvault/storage/SeedvaultStoragePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/storage/SeedvaultStoragePlugin.kt index 36dd62dc..ca478f14 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/storage/SeedvaultStoragePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/storage/SeedvaultStoragePlugin.kt @@ -13,6 +13,9 @@ internal class SeedvaultStoragePlugin( private val storage: DocumentsStorage, private val keyManager: KeyManager, ) : SafStoragePlugin(appContext) { + /** + * Attention: This context might be from a different user. Use with care. + */ override val context: Context get() = appContext.getSystemContext { storage.storage?.isUsb == true diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageRootResolver.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageRootResolver.kt index fcbeaa0f..4d615e6a 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageRootResolver.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageRootResolver.kt @@ -18,14 +18,13 @@ import android.provider.DocumentsContract.Root.FLAG_SUPPORTS_CREATE import android.provider.DocumentsContract.Root.FLAG_SUPPORTS_IS_CHILD import android.util.Log import com.stevesoltys.seedvault.R +import com.stevesoltys.seedvault.getSystemContext import com.stevesoltys.seedvault.ui.storage.StorageOption.SafOption internal object StorageRootResolver { private val TAG = StorageRootResolver::class.java.simpleName - private const val usbAuthority = "com.android.externalstorage.documents" - fun getStorageRoots(context: Context, authority: String): List { val roots = ArrayList() val rootsUri = DocumentsContract.buildRootsUri(authority) @@ -37,12 +36,17 @@ internal object StorageRootResolver { if (root != null) roots.add(root) } } - if (usbAuthority == authority && UserHandle.myUserId() != UserHandle.USER_SYSTEM) { - val c: Context = context.createContextAsUser(UserHandle.SYSTEM, 0) + // add special system user roots for USB devices + val c = context.getSystemContext { + authority == AUTHORITY_STORAGE && UserHandle.myUserId() != UserHandle.USER_SYSTEM + } + // only proceed if we really got a different [Context], e.g. had permission for it + if (context !== c) { c.contentResolver.query(rootsUri, null, null, null, null)?.use { cursor -> while (cursor.moveToNext()) { - // Pass in context since it is used to query package manager for app icons + // Pass in [context] since it is used to query package manager for app icons val root = getStorageRoot(context, authority, cursor) + // only add USB storage from system user, no others if (root != null && root.isUsb) roots.add(root) } } diff --git a/app/src/test/java/com/stevesoltys/seedvault/plugins/saf/StoragePluginTest.kt b/app/src/test/java/com/stevesoltys/seedvault/plugins/saf/StoragePluginTest.kt index c4a1e074..d06004fd 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/plugins/saf/StoragePluginTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/plugins/saf/StoragePluginTest.kt @@ -39,6 +39,7 @@ internal class StoragePluginTest : BackupTest() { // get current set dir and for that the current token every { storage getProperty "currentToken" } returns token every { settingsManager.getToken() } returns token + every { storage getProperty "storage" } returns null // just to check if isUsb coEvery { storage.getSetDir(token) } returns setDir // delete contents of current set dir coEvery { setDir.listFilesBlocking(context) } returns listOf(backupFile) diff --git a/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafStoragePlugin.kt b/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafStoragePlugin.kt index 9ee75bc7..fef76810 100644 --- a/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafStoragePlugin.kt +++ b/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafStoragePlugin.kt @@ -11,7 +11,7 @@ import javax.crypto.SecretKey @Suppress("BlockingMethodInNonBlockingContext") class TestSafStoragePlugin( - private val appContext: Context, + appContext: Context, private val getLocationUri: () -> Uri?, ) : SafStoragePlugin(appContext) { diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/plugin/saf/SafStoragePlugin.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/plugin/saf/SafStoragePlugin.kt index 9fb988ac..6c143315 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/plugin/saf/SafStoragePlugin.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/plugin/saf/SafStoragePlugin.kt @@ -35,12 +35,15 @@ private const val TAG = "SafStoragePlugin" public abstract class SafStoragePlugin( private val appContext: Context, ) : StoragePlugin { - - private val cache = SafCache() - // In the case of USB storage, if INTERACT_ACROSS_USERS_FULL is granted, this context will match - // the system user's application context. Otherwise, matches appContext. + /** + * Attention: This context could be unexpected. E.g. the system user's application context, + * in the case of USB storage, if INTERACT_ACROSS_USERS_FULL permission is granted. + * Use [appContext], if you need the context of the current app and user + * and [context] for all file access. + */ protected abstract val context: Context protected abstract val root: DocumentFile? + private val cache = SafCache() private val folder: DocumentFile? get() { @@ -48,8 +51,9 @@ public abstract class SafStoragePlugin( if (cache.currentFolder != null) return cache.currentFolder @SuppressLint("HardwareIds") - // this is unique to each combination of app-signing key, user, and device - // so we don't leak anything by not hashing this and can use it as is + // This is unique to each combination of app-signing key, user, and device + // so we don't leak anything by not hashing this and can use it as is. + // Note: Use [appContext] here to not get the wrong ID for a different user. val androidId = Settings.Secure.getString(appContext.contentResolver, ANDROID_ID) // the folder name is our user ID val folderName = "$androidId.sv"