diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt index dbf277b89a..f921387642 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt @@ -18,6 +18,7 @@ import org.signal.core.util.Base64 import org.signal.core.util.SqlUtil import org.signal.core.util.exists import org.signal.core.util.orNull +import org.signal.core.util.readToSingleBoolean import org.signal.core.util.requireLong import org.signal.core.util.requireNonNullString import org.signal.core.util.select @@ -109,6 +110,18 @@ class RecipientTableTest_getAndPossiblyMerge { val record = SignalDatabase.recipients.getRecord(id) assertEquals(RecipientTable.RegisteredState.REGISTERED, record.registered) } + + test("e164+pni+aci insert, pni verified") { + val id = process(E164_A, PNI_A, ACI_A, pniVerified = true) + expect(E164_A, PNI_A, ACI_A) + expectPniVerified() + + val record = SignalDatabase.recipients.getRecord(id) + assertEquals(RecipientTable.RegisteredState.REGISTERED, record.registered) + + process(E164_A, PNI_A, ACI_A, pniVerified = false) + expectPniVerified() + } } @Test @@ -164,6 +177,7 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, ACI_A) expectNoSessionSwitchoverEvent() + expectPniVerified() } test("no match, all fields") { @@ -225,6 +239,8 @@ class RecipientTableTest_getAndPossiblyMerge { given(E164_A, PNI_A, null, pniSession = true) process(E164_A, PNI_A, ACI_A, pniVerified = true) expect(E164_A, PNI_A, ACI_A) + + expectPniVerified() } test("e164 and aci matches, all provided, new pni") { @@ -694,6 +710,8 @@ class RecipientTableTest_getAndPossiblyMerge { expectDeleted() expect(E164_A, PNI_A, ACI_A) + + expectPniVerified() } test("merge, e164+pni & aci, pni session, pni verified") { @@ -706,6 +724,7 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, ACI_A) expectThreadMergeEvent(E164_A) + expectPniVerified() } test("merge, e164+pni & e164+pni+aci, change number") { @@ -1037,6 +1056,10 @@ class RecipientTableTest_getAndPossiblyMerge { if (!test.sessionSwitchoverExpected) { test.expectNoSessionSwitchoverEvent() } + + if (!test.pniVerifiedExpected) { + test.expectPniNotVerified() + } } catch (e: Throwable) { if (e.javaClass != exception) { val error = java.lang.AssertionError("[$name] ${e.message}") @@ -1056,6 +1079,7 @@ class RecipientTableTest_getAndPossiblyMerge { var changeNumberExpected = false var threadMergeExpected = false var sessionSwitchoverExpected = false + var pniVerifiedExpected = false init { // Need to delete these first to prevent foreign key crash @@ -1207,6 +1231,24 @@ class RecipientTableTest_getAndPossiblyMerge { assertNull("Unexpected thread merge event!", getLatestThreadMergeEvent(outputRecipientId)) } + fun expectPniVerified() { + assertTrue("Expected PNI to be verified!", isPniVerified(outputRecipientId)) + pniVerifiedExpected = true + } + + fun expectPniNotVerified() { + assertFalse("Expected PNI to be not be verified!", isPniVerified(outputRecipientId)) + } + + private fun isPniVerified(recipientId: RecipientId): Boolean { + return SignalDatabase.rawDatabase + .select(RecipientTable.PNI_SIGNATURE_VERIFIED) + .from(RecipientTable.TABLE_NAME) + .where("${RecipientTable.ID} = ?", recipientId) + .run() + .readToSingleBoolean(false) + } + private fun insert(e164: String?, pni: PNI?, aci: ACI?): RecipientId { val id: Long = SignalDatabase.rawDatabase.insert( RecipientTable.TABLE_NAME, diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index c15ed20db4..c94342e1b5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -33,6 +33,7 @@ import org.signal.core.util.requireLong import org.signal.core.util.requireNonNullString import org.signal.core.util.requireString import org.signal.core.util.select +import org.signal.core.util.toInt import org.signal.core.util.update import org.signal.core.util.updateAll import org.signal.core.util.withinTransaction @@ -183,6 +184,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da const val REPORTING_TOKEN = "reporting_token" const val PHONE_NUMBER_SHARING = "phone_number_sharing" const val PHONE_NUMBER_DISCOVERABLE = "phone_number_discoverable" + const val PNI_SIGNATURE_VERIFIED = "pni_signature_verified" const val SEARCH_PROFILE_NAME = "search_signal_profile" const val SORT_NAME = "sort_name" @@ -250,7 +252,8 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da $NEEDS_PNI_SIGNATURE INTEGER DEFAULT 0, $REPORTING_TOKEN BLOB DEFAULT NULL, $PHONE_NUMBER_SHARING INTEGER DEFAULT ${PhoneNumberSharingState.UNKNOWN.id}, - $PHONE_NUMBER_DISCOVERABLE INTEGER DEFAULT ${PhoneNumberDiscoverableState.UNKNOWN.id} + $PHONE_NUMBER_DISCOVERABLE INTEGER DEFAULT ${PhoneNumberDiscoverableState.UNKNOWN.id}, + $PNI_SIGNATURE_VERIFIED INTEGER DEFAULT 0 ) """ @@ -829,7 +832,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da val recipientId: RecipientId if (id < 0) { Log.w(TAG, "[applyStorageSyncContactInsert] Failed to insert. Possibly merging.") - recipientId = getAndPossiblyMergePnpVerified(insert.aci.orNull(), insert.pni.orNull(), insert.number.orNull()) + recipientId = getAndPossiblyMerge(aci = insert.aci.orNull(), pni = insert.pni.orNull(), e164 = insert.number.orNull(), pniVerified = insert.isPniSignatureVerified) db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId)) } else { recipientId = RecipientId.from(id) @@ -867,7 +870,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da var recipientId = getByColumn(STORAGE_SERVICE_ID, Base64.encodeWithPadding(update.old.id.raw)).get() Log.w(TAG, "[applyStorageSyncContactUpdate] Found user $recipientId. Possibly merging.") - recipientId = getAndPossiblyMergePnpVerified(update.new.aci.orElse(null), update.new.pni.orElse(null), update.new.number.orElse(null)) + recipientId = getAndPossiblyMerge(aci = update.new.aci.orElse(null), pni = update.new.pni.orElse(null), e164 = update.new.number.orElse(null), pniVerified = update.new.isPniSignatureVerified) Log.w(TAG, "[applyStorageSyncContactUpdate] Merged into $recipientId") db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId)) @@ -1113,6 +1116,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da SYSTEM_NICKNAME, "$TABLE_NAME.$STORAGE_SERVICE_PROTO", "$TABLE_NAME.$UNREGISTERED_TIMESTAMP", + "$TABLE_NAME.$PNI_SIGNATURE_VERIFIED", "${GroupTable.TABLE_NAME}.${GroupTable.V2_MASTER_KEY}", "${ThreadTable.TABLE_NAME}.${ThreadTable.ARCHIVED}", "${ThreadTable.TABLE_NAME}.${ThreadTable.READ}", @@ -2372,7 +2376,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } } - val finalId: RecipientId = writePnpChangeSetToDisk(changeSet, pni) + val finalId: RecipientId = writePnpChangeSetToDisk(changeSet, pni, pniVerified) return ProcessPnpTupleResult( finalId = finalId, @@ -2386,7 +2390,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } @VisibleForTesting - fun writePnpChangeSetToDisk(changeSet: PnpChangeSet, inputPni: PNI?): RecipientId { + fun writePnpChangeSetToDisk(changeSet: PnpChangeSet, inputPni: PNI?, pniVerified: Boolean): RecipientId { var hadThreadMerge = false for (operation in changeSet.operations) { @Exhaustive @@ -2402,7 +2406,10 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da is PnpOperation.RemovePni -> { writableDatabase .update(TABLE_NAME) - .values(PNI_COLUMN to null) + .values( + PNI_COLUMN to null, + PNI_SIGNATURE_VERIFIED to 0 + ) .where("$ID = ?", operation.recipientId) .run() } @@ -2413,7 +2420,8 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da .values( ACI_COLUMN to operation.aci.toString(), REGISTERED to RegisteredState.REGISTERED.id, - UNREGISTERED_TIMESTAMP to 0 + UNREGISTERED_TIMESTAMP to 0, + PNI_SIGNATURE_VERIFIED to pniVerified.toInt() ) .where("$ID = ?", operation.recipientId) .run() @@ -2433,14 +2441,15 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da .values( PNI_COLUMN to operation.pni.toString(), REGISTERED to RegisteredState.REGISTERED.id, - UNREGISTERED_TIMESTAMP to 0 + UNREGISTERED_TIMESTAMP to 0, + PNI_SIGNATURE_VERIFIED to 0 ) .where("$ID = ?", operation.recipientId) .run() } is PnpOperation.Merge -> { - val mergeResult: MergeResult = merge(operation.primaryId, operation.secondaryId, inputPni) + val mergeResult: MergeResult = merge(operation.primaryId, operation.secondaryId, inputPni, pniVerified) hadThreadMerge = hadThreadMerge || mergeResult.neededThreadMerge } @@ -2519,7 +2528,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } is PnpIdResolver.PnpInsert -> { - val id: Long = writableDatabase.insert(TABLE_NAME, null, buildContentValuesForNewUser(changeSet.id.e164, changeSet.id.pni, changeSet.id.aci)) + val id: Long = writableDatabase.insert(TABLE_NAME, null, buildContentValuesForNewUser(changeSet.id.e164, changeSet.id.pni, changeSet.id.aci, pniVerified)) RecipientId.from(id) } } @@ -3843,7 +3852,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da * Merges one ACI recipient with an E164 recipient. It is assumed that the E164 recipient does * *not* have an ACI. */ - private fun merge(primaryId: RecipientId, secondaryId: RecipientId, newPni: PNI? = null): MergeResult { + private fun merge(primaryId: RecipientId, secondaryId: RecipientId, newPni: PNI? = null, pniVerified: Boolean): MergeResult { ensureInTransaction() val db = writableDatabase val primaryRecord = getRecord(primaryId) @@ -3904,7 +3913,8 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da SYSTEM_CONTACT_URI to secondaryRecord.systemContactUri, PROFILE_SHARING to (primaryRecord.profileSharing || secondaryRecord.profileSharing), CAPABILITIES to max(primaryRecord.capabilities.rawBits, secondaryRecord.capabilities.rawBits), - MENTION_SETTING to if (primaryRecord.mentionSetting != MentionSetting.ALWAYS_NOTIFY) primaryRecord.mentionSetting.id else secondaryRecord.mentionSetting.id + MENTION_SETTING to if (primaryRecord.mentionSetting != MentionSetting.ALWAYS_NOTIFY) primaryRecord.mentionSetting.id else secondaryRecord.mentionSetting.id, + PNI_SIGNATURE_VERIFIED to pniVerified.toInt() ) if (primaryRecord.profileSharing || secondaryRecord.profileSharing) { @@ -3929,13 +3939,14 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da check(writableDatabase.inTransaction()) { "Must be in a transaction!" } } - private fun buildContentValuesForNewUser(e164: String?, pni: PNI?, aci: ACI?): ContentValues { + private fun buildContentValuesForNewUser(e164: String?, pni: PNI?, aci: ACI?, pniVerified: Boolean): ContentValues { check(e164 != null || pni != null || aci != null) { "Must provide some sort of identifier!" } val values = contentValuesOf( E164 to e164, ACI_COLUMN to aci?.toString(), PNI_COLUMN to pni?.toString(), + PNI_SIGNATURE_VERIFIED to pniVerified.toInt(), STORAGE_SERVICE_ID to Base64.encodeWithPadding(StorageSyncHelper.generateKey()), AVATAR_COLOR to AvatarColorHash.forAddress((aci ?: pni)?.toString(), e164).serialize() ) @@ -3971,6 +3982,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da put(MUTE_UNTIL, contact.muteUntil) put(STORAGE_SERVICE_ID, Base64.encodeWithPadding(contact.id.raw)) put(HIDDEN, contact.isHidden) + put(PNI_SIGNATURE_VERIFIED, contact.isPniSignatureVerified.toInt()) if (contact.hasUnknownFields()) { put(STORAGE_SERVICE_PROTO, Base64.encodeWithPadding(Objects.requireNonNull(contact.serializeUnknownFields()))) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTableCursorUtil.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTableCursorUtil.kt index 502d8d490b..d3f093447c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTableCursorUtil.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTableCursorUtil.kt @@ -209,25 +209,16 @@ object RecipientTableCursorUtil { } fun getSyncExtras(cursor: Cursor): RecipientRecord.SyncExtras { - val storageProtoRaw = cursor.optionalString(RecipientTable.STORAGE_SERVICE_PROTO).orElse(null) - val storageProto = if (storageProtoRaw != null) Base64.decodeOrThrow(storageProtoRaw) else null - val archived = cursor.optionalBoolean(ThreadTable.ARCHIVED).orElse(false) - val forcedUnread = cursor.optionalInt(ThreadTable.READ).map { status: Int -> status == ThreadTable.ReadStatus.FORCED_UNREAD.serialize() }.orElse(false) - val groupMasterKey = cursor.optionalBlob(GroupTable.V2_MASTER_KEY).map { GroupUtil.requireMasterKey(it) }.orElse(null) - val identityKey = cursor.optionalString(RecipientTable.IDENTITY_KEY).map { Base64.decodeOrThrow(it) }.orElse(null) - val identityStatus = cursor.optionalInt(RecipientTable.IDENTITY_STATUS).map { VerifiedStatus.forState(it) }.orElse(VerifiedStatus.DEFAULT) - val unregisteredTimestamp = cursor.optionalLong(RecipientTable.UNREGISTERED_TIMESTAMP).orElse(0) - val systemNickname = cursor.optionalString(RecipientTable.SYSTEM_NICKNAME).orElse(null) - return RecipientRecord.SyncExtras( - storageProto = storageProto, - groupMasterKey = groupMasterKey, - identityKey = identityKey, - identityStatus = identityStatus, - isArchived = archived, - isForcedUnread = forcedUnread, - unregisteredTimestamp = unregisteredTimestamp, - systemNickname = systemNickname + storageProto = cursor.optionalString(RecipientTable.STORAGE_SERVICE_PROTO).orElse(null)?.let { Base64.decodeOrThrow(it) }, + groupMasterKey = cursor.optionalBlob(GroupTable.V2_MASTER_KEY).map { GroupUtil.requireMasterKey(it) }.orElse(null), + identityKey = cursor.optionalString(RecipientTable.IDENTITY_KEY).map { Base64.decodeOrThrow(it) }.orElse(null), + identityStatus = cursor.optionalInt(RecipientTable.IDENTITY_STATUS).map { VerifiedStatus.forState(it) }.orElse(VerifiedStatus.DEFAULT), + isArchived = cursor.optionalBoolean(ThreadTable.ARCHIVED).orElse(false), + isForcedUnread = cursor.optionalInt(ThreadTable.READ).map { status: Int -> status == ThreadTable.ReadStatus.FORCED_UNREAD.serialize() }.orElse(false), + unregisteredTimestamp = cursor.optionalLong(RecipientTable.UNREGISTERED_TIMESTAMP).orElse(0), + systemNickname = cursor.optionalString(RecipientTable.SYSTEM_NICKNAME).orElse(null), + pniSignatureVerified = cursor.optionalBoolean(RecipientTable.PNI_SIGNATURE_VERIFIED).orElse(false) ) } 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 086c5308ae..7a7c16578c 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 @@ -75,6 +75,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V214_PhoneNumberSha import org.thoughtcrime.securesms.database.helpers.migration.V215_RemoveAttachmentUniqueId import org.thoughtcrime.securesms.database.helpers.migration.V216_PhoneNumberDiscoverable import org.thoughtcrime.securesms.database.helpers.migration.V217_MessageTableExtrasColumn +import org.thoughtcrime.securesms.database.helpers.migration.V218_RecipientPniSignatureVerified /** * Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness. @@ -152,10 +153,11 @@ object SignalDatabaseMigrations { 214 to V214_PhoneNumberSharingColumn, 215 to V215_RemoveAttachmentUniqueId, 216 to V216_PhoneNumberDiscoverable, - 217 to V217_MessageTableExtrasColumn + 217 to V217_MessageTableExtrasColumn, + 218 to V218_RecipientPniSignatureVerified ) - const val DATABASE_VERSION = 217 + const val DATABASE_VERSION = 218 @JvmStatic fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V218_RecipientPniSignatureVerified.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V218_RecipientPniSignatureVerified.kt new file mode 100644 index 0000000000..23b17a92e9 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V218_RecipientPniSignatureVerified.kt @@ -0,0 +1,19 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import net.zetetic.database.sqlcipher.SQLiteDatabase + +/** + * Adds a pni_signature_verified column to the recipient table, letting us track whether the ACI/PNI association is verified and sync that to storage service. + */ +@Suppress("ClassName") +object V218_RecipientPniSignatureVerified : SignalDatabaseMigration { + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + db.execSQL("ALTER TABLE recipient ADD COLUMN pni_signature_verified INTEGER DEFAULT 0") + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt b/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt index 7b4cae3c91..d1e427a4a1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/RecipientRecord.kt @@ -111,7 +111,8 @@ data class RecipientRecord( val isArchived: Boolean, val isForcedUnread: Boolean, val unregisteredTimestamp: Long, - val systemNickname: String? + val systemNickname: String?, + val pniSignatureVerified: Boolean ) data class Capabilities( diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java index aaa67cdc14..aef4c9ba01 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java @@ -216,8 +216,9 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor