From 150e98bbc128e565834a05df75a3a7f0a760df35 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Tue, 12 Aug 2025 09:57:29 -0400 Subject: [PATCH] Fix attachment reconciliation bugs stemming from incorrect base64 encoded remote_key. --- .../attachments/ArchivedAttachment.kt | 2 +- .../securesms/database/AttachmentTable.kt | 18 ++++---- .../helpers/SignalDatabaseMigrations.kt | 6 ++- .../migration/V286_FixRemoteKeyEncoding.kt | 41 +++++++++++++++++++ .../ArchiveAttachmentReconciliationJob.kt | 5 +++ 5 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V286_FixRemoteKeyEncoding.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/attachments/ArchivedAttachment.kt b/app/src/main/java/org/thoughtcrime/securesms/attachments/ArchivedAttachment.kt index 0dd4b92971..8f725ad361 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/attachments/ArchivedAttachment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/attachments/ArchivedAttachment.kt @@ -55,7 +55,7 @@ class ArchivedAttachment : Attachment { fileName = fileName, cdn = Cdn.fromCdnNumber(cdn), remoteLocation = cdnKey, - remoteKey = Base64.encodeWithoutPadding(key), + remoteKey = Base64.encodeWithPadding(key), remoteDigest = null, incrementalDigest = incrementalMac, fastPreflightId = null, diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt index 994cf43229..dcd78c3c15 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt @@ -521,7 +521,7 @@ class AttachmentTable( file = File(it.requireNonNullString(DATA_FILE)), random = it.requireNonNullBlob(DATA_RANDOM), size = it.requireLong(DATA_SIZE), - remoteKey = it.requireBlob(REMOTE_KEY)!!, + remoteKey = Base64.decode(it.requireNonNullString(REMOTE_KEY)), plaintextHash = Base64.decode(it.requireNonNullString(DATA_HASH_END)) ) } @@ -545,8 +545,8 @@ class AttachmentTable( attachmentId = AttachmentId(it.requireLong(ID)), mmsId = it.requireLong(MESSAGE_ID), size = it.requireLong(DATA_SIZE), - plaintextHash = it.requireBlob(DATA_HASH_END), - remoteKey = it.requireBlob(REMOTE_KEY) + plaintextHash = it.requireString(DATA_HASH_END)?.let { hash -> Base64.decode(hash) }, + remoteKey = it.requireString(REMOTE_KEY)?.let { key -> Base64.decode(key) } ) } } @@ -569,8 +569,8 @@ class AttachmentTable( attachmentId = AttachmentId(it.requireLong(ID)), mmsId = it.requireLong(MESSAGE_ID), size = it.requireLong(DATA_SIZE), - plaintextHash = it.requireBlob(DATA_HASH_END), - remoteKey = it.requireBlob(REMOTE_KEY) + plaintextHash = it.requireString(DATA_HASH_END)?.let { hash -> Base64.decode(hash) }, + remoteKey = it.requireString(REMOTE_KEY)?.let { key -> Base64.decode(key) } ) } } @@ -588,8 +588,8 @@ class AttachmentTable( attachmentId = AttachmentId(it.requireLong(ID)), mmsId = it.requireLong(MESSAGE_ID), size = it.requireLong(DATA_SIZE), - plaintextHash = it.requireBlob(DATA_HASH_END), - remoteKey = it.requireBlob(REMOTE_KEY) + plaintextHash = it.requireString(DATA_HASH_END)?.let { hash -> Base64.decode(hash) }, + remoteKey = it.requireString(REMOTE_KEY)?.let { key -> Base64.decode(key) } ) } } @@ -606,8 +606,8 @@ class AttachmentTable( attachmentId = AttachmentId(it.requireLong(ID)), mmsId = it.requireLong(MESSAGE_ID), size = it.requireLong(DATA_SIZE), - plaintextHash = it.requireNonNullBlob(DATA_HASH_END), - remoteKey = it.requireNonNullBlob(REMOTE_KEY) + plaintextHash = it.requireString(DATA_HASH_END)?.let { hash -> Base64.decode(hash) }, + remoteKey = it.requireString(REMOTE_KEY)?.let { key -> Base64.decode(key) } ) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index 1bf07d2a4a..6622127b68 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -140,6 +140,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V282_AddSnippetMess import org.thoughtcrime.securesms.database.helpers.migration.V283_ViewOnceRemoteDataCleanup import org.thoughtcrime.securesms.database.helpers.migration.V284_SetPlaceholderGroupFlag import org.thoughtcrime.securesms.database.helpers.migration.V285_AddEpochToCallLinksTable +import org.thoughtcrime.securesms.database.helpers.migration.V286_FixRemoteKeyEncoding import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** @@ -285,10 +286,11 @@ object SignalDatabaseMigrations { 282 to V282_AddSnippetMessageIdColumnToThreadTable, 283 to V283_ViewOnceRemoteDataCleanup, 284 to V284_SetPlaceholderGroupFlag, - 285 to V285_AddEpochToCallLinksTable + 285 to V285_AddEpochToCallLinksTable, + 286 to V286_FixRemoteKeyEncoding ) - const val DATABASE_VERSION = 285 + const val DATABASE_VERSION = 286 @JvmStatic fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V286_FixRemoteKeyEncoding.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V286_FixRemoteKeyEncoding.kt new file mode 100644 index 0000000000..2cb908797a --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V286_FixRemoteKeyEncoding.kt @@ -0,0 +1,41 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import androidx.core.content.contentValuesOf +import org.signal.core.util.Base64 +import org.signal.core.util.forEach +import org.signal.core.util.logging.Log +import org.signal.core.util.requireLong +import org.signal.core.util.requireNonNullString +import org.thoughtcrime.securesms.database.SQLiteDatabase + +/** + * Ensure remote_key is encoded with padding, fixing archive restore attachments not using padding. + */ +@Suppress("ClassName") +object V286_FixRemoteKeyEncoding : SignalDatabaseMigration { + + private val TAG = Log.tag(V286_FixRemoteKeyEncoding::class) + + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + var updated = 0 + db.query("SELECT _id, remote_key FROM attachment WHERE remote_key is not null AND LENGTH(remote_key) = 86").forEach { + val id = it.requireLong("_id") + val remoteKey = Base64.encodeWithPadding(Base64.decode(it.requireNonNullString("remote_key"))) + + updated += db.update( + "attachment", + contentValuesOf("remote_key" to remoteKey), + "_id = ? AND remote_key != ?", + arrayOf(id.toString(), remoteKey) + ) + } + + Log.i(TAG, "Updated $updated attachment remote_keys") + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt index 84d9f9fe0a..81edc17a4a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt @@ -73,6 +73,11 @@ class ArchiveAttachmentReconciliationJob private constructor( override fun getFactoryKey(): String = KEY override fun run(): Result { + if (!SignalStore.backup.hasBackupBeenUploaded) { + Log.w(TAG, "No backup has been uploaded yet! Skipping.") + return Result.success() + } + if (!SignalStore.backup.backsUpMedia) { Log.w(TAG, "This user doesn't back up media! Skipping.") return Result.success()