From 14775b5c3e052024afcdbca5bc633cc342e72621 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 7 Oct 2024 17:29:59 -0300 Subject: [PATCH] Check that exceptions from file loading get caught a sha256sum mismatch for example can happen, so the exception should get caught e.g. in RestoreCoordinatorState and should not necessarily be fatal --- .../java/com/stevesoltys/seedvault/repo/Loader.kt | 8 ++++++-- .../com/stevesoltys/seedvault/repo/SnapshotManager.kt | 6 ++++-- .../seedvault/restore/install/ApkRestore.kt | 11 +++++++++-- .../seedvault/transport/restore/RestoreCoordinator.kt | 5 +++++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/Loader.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/Loader.kt index 45a6745a..2c527f57 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/repo/Loader.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/repo/Loader.kt @@ -5,7 +5,6 @@ package com.stevesoltys.seedvault.repo -import com.android.internal.R.attr.handle import com.github.luben.zstd.ZstdInputStream import com.stevesoltys.seedvault.backend.BackendManager import com.stevesoltys.seedvault.crypto.Crypto @@ -17,6 +16,7 @@ import org.calyxos.seedvault.core.backends.AppBackupFileType import org.calyxos.seedvault.core.toHexString import java.io.ByteArrayInputStream import java.io.File +import java.io.IOException import java.io.InputStream import java.io.SequenceInputStream import java.security.GeneralSecurityException @@ -38,6 +38,7 @@ internal class Loader( * @param cacheFile if non-null, the ciphertext of the loaded file will be cached there * for later loading with [loadFile]. */ + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) suspend fun loadFile(fileHandle: AppBackupFileType, cacheFile: File? = null): InputStream { val expectedHash = when (fileHandle) { is AppBackupFileType.Snapshot -> fileHandle.hash @@ -49,10 +50,12 @@ internal class Loader( /** * The responsibility with closing the returned stream lies with the caller. */ + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) fun loadFile(file: File, expectedHash: String): InputStream { return loadFromStream(file.inputStream(), expectedHash) } + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) suspend fun loadFiles(handles: List): InputStream { val enumeration: Enumeration = object : Enumeration { val iterator = handles.iterator() @@ -68,6 +71,7 @@ internal class Loader( return SequenceInputStream(enumeration) } + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) private fun loadFromStream( inputStream: InputStream, expectedHash: String, @@ -79,7 +83,7 @@ internal class Loader( // check SHA-256 hash first thing val sha256 = crypto.sha256(cipherText).toHexString() if (sha256 != expectedHash) { - throw GeneralSecurityException("File had wrong SHA-256 hash: $handle") + throw GeneralSecurityException("File had wrong SHA-256 hash: $expectedHash") } // check that we can handle the version of that snapshot val version = cipherText[0] diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt index ca6b8b46..e85f2b05 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt @@ -8,6 +8,7 @@ package com.stevesoltys.seedvault.repo import com.github.luben.zstd.ZstdOutputStream import com.stevesoltys.seedvault.backend.BackendManager import com.stevesoltys.seedvault.crypto.Crypto +import com.stevesoltys.seedvault.header.UnsupportedVersionException import com.stevesoltys.seedvault.header.VERSION import com.stevesoltys.seedvault.proto.Snapshot import io.github.oshai.kotlinlogging.KotlinLogging @@ -18,6 +19,7 @@ import java.io.ByteArrayOutputStream import java.io.File import java.io.IOException import java.nio.ByteBuffer +import java.security.GeneralSecurityException internal const val FOLDER_SNAPSHOTS = "snapshots" @@ -126,7 +128,7 @@ internal class SnapshotManager( * Loads and parses the snapshot referenced by the given [snapshotHandle]. * If a locally cached version exists, the backend will not be hit. */ - @Throws(IOException::class) + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) suspend fun loadSnapshot(snapshotHandle: AppBackupFileType.Snapshot): Snapshot { val file = File(snapshotFolder, snapshotHandle.name) snapshotFolder.mkdirs() @@ -138,7 +140,7 @@ internal class SnapshotManager( return inputStream.use { Snapshot.parseFrom(it) } } - @Throws(IOException::class) + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) fun loadCachedSnapshots(): List { if (!snapshotFolder.isDirectory) return emptyList() return snapshotFolder.listFiles()?.mapNotNull { file -> diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt index 0e82ee67..45e9c859 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt @@ -19,6 +19,7 @@ import com.stevesoltys.seedvault.backend.BackendManager import com.stevesoltys.seedvault.backend.LegacyStoragePlugin import com.stevesoltys.seedvault.crypto.Crypto import com.stevesoltys.seedvault.encodeBase64 +import com.stevesoltys.seedvault.header.UnsupportedVersionException import com.stevesoltys.seedvault.metadata.ApkSplit import com.stevesoltys.seedvault.metadata.PackageMetadata import com.stevesoltys.seedvault.repo.Loader @@ -42,6 +43,7 @@ import java.io.File import java.io.IOException import java.io.InputStream import java.io.OutputStream +import java.security.GeneralSecurityException import java.security.MessageDigest import java.util.Locale @@ -162,7 +164,12 @@ internal class ApkRestore( } @Suppress("ThrowsCount") - @Throws(IOException::class, SecurityException::class) + @Throws( + GeneralSecurityException::class, + UnsupportedVersionException::class, + IOException::class, + SecurityException::class, + ) private suspend fun restore( backup: RestorableBackup, packageName: String, @@ -287,7 +294,7 @@ internal class ApkRestore( * * @return a [Pair] of the cached [File] and SHA-256 hash. */ - @Throws(IOException::class) + @Throws(GeneralSecurityException::class, UnsupportedVersionException::class, IOException::class) private suspend fun cacheApk( backup: RestorableBackup, packageName: String, 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 d6873ac0..99bee8a3 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 @@ -35,6 +35,7 @@ import org.calyxos.seedvault.core.backends.AppBackupFileType import org.calyxos.seedvault.core.backends.Backend import org.calyxos.seedvault.core.backends.LegacyAppBackupFile import java.io.IOException +import java.security.GeneralSecurityException /** * Device name used in AOSP to indicate that a restore set is part of a device-to-device migration. @@ -109,6 +110,10 @@ internal class RestoreCoordinator( } catch (e: SecurityException) { Log.e(TAG, "Error while getting restore set $handle", e) return RestorableBackupResult.ErrorResult(e) + } catch (e: GeneralSecurityException) { + Log.e(TAG, "General security error while decrypting restore set $handle", e) + lastException = e + continue } catch (e: DecryptionFailedException) { Log.e(TAG, "Error while decrypting restore set $handle", e) lastException = e