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
seedvault-app · Oct 8, 2024 · 8728025 · 8728025
1 parent 715fa6c
commit 8728025
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 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
@@ -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<AppBackupFileType>): InputStream {
         val enumeration: Enumeration<InputStream> = object : Enumeration<InputStream> {
             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
@@ -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<Snapshot> {
         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
@@ -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
@@ -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