From ca3187d0b8605846103d25b6ee1d8d017f52c6ff Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 10 Aug 2023 11:08:28 -0400 Subject: [PATCH] Ungate some PNP receive-side behavior. --- .../PendingPniSignatureMessageTable.kt | 6 ---- .../securesms/database/RecipientTable.kt | 4 --- .../securesms/groups/GroupManagerV2.java | 2 +- .../securesms/jobs/RetrieveProfileJob.java | 4 +-- .../securesms/messages/MessageDecryptor.kt | 31 +++++++------------ .../securesms/recipients/Recipient.java | 13 ++------ 6 files changed, 18 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt index 7011a22467..06877b573f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/PendingPniSignatureMessageTable.kt @@ -8,7 +8,6 @@ import org.signal.core.util.logging.Log import org.signal.core.util.update import org.signal.core.util.withinTransaction import org.thoughtcrime.securesms.recipients.RecipientId -import org.thoughtcrime.securesms.util.FeatureFlags import org.whispersystems.signalservice.api.messages.SendMessageResult /** @@ -43,8 +42,6 @@ class PendingPniSignatureMessageTable(context: Context, databaseHelper: SignalDa } fun insertIfNecessary(recipientId: RecipientId, sentTimestamp: Long, result: SendMessageResult) { - if (!FeatureFlags.phoneNumberPrivacy()) return - if (!result.isSuccess) { return } @@ -63,8 +60,6 @@ class PendingPniSignatureMessageTable(context: Context, databaseHelper: SignalDa } fun acknowledgeReceipts(recipientId: RecipientId, sentTimestamps: Collection, deviceId: Int) { - if (!FeatureFlags.phoneNumberPrivacy()) return - writableDatabase.withinTransaction { db -> val count = db .delete(TABLE_NAME) @@ -93,7 +88,6 @@ class PendingPniSignatureMessageTable(context: Context, databaseHelper: SignalDa * Deletes all record of pending PNI verification messages. Should only be called after the user changes their number. */ fun deleteAll() { - if (!FeatureFlags.phoneNumberPrivacy()) return writableDatabase.delete(TABLE_NAME).run() } 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 88ba2551ff..3288f6d776 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -440,10 +440,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da * It is assumed that the tuple is verified. Do not give this method an untrusted association. */ fun getAndPossiblyMergePnpVerified(aci: ACI?, pni: PNI?, e164: String?): RecipientId { - if (!FeatureFlags.phoneNumberPrivacy()) { - throw AssertionError() - } - return getAndPossiblyMerge(aci = aci, pni = pni, e164 = e164, pniVerified = true, changeSelf = false) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java index f1785f9ed8..3c49450bbc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -568,7 +568,7 @@ final class GroupManagerV2 { if (aciInPending.isPresent()) { return commitChangeWithConflictResolution(selfAci, groupOperations.createAcceptInviteChange(groupCandidate.requireExpiringProfileKeyCredential())); - } else if (pniInPending.isPresent() && FeatureFlags.phoneNumberPrivacy()) { + } else if (pniInPending.isPresent()) { return commitChangeWithConflictResolution(selfPni, groupOperations.createAcceptPniInviteChange(groupCandidate.requireExpiringProfileKeyCredential())); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.java index 94e3c4678b..2e55e3c54e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.java @@ -301,8 +301,8 @@ public class RetrieveProfileJob extends BaseJob { }); recipientTable.markProfilesFetched(success, System.currentTimeMillis()); - // XXX The service hasn't implemented profiles for PNIs yet, so if using PNP CDS we don't want to mark users without profiles as unregistered. - if ((operationState.unregistered.size() > 0 || newlyRegistered.size() > 0) && !FeatureFlags.phoneNumberPrivacy()) { + + if (operationState.unregistered.size() > 0 || newlyRegistered.size() > 0) { Log.i(TAG, "Marking " + newlyRegistered.size() + " users as registered and " + operationState.unregistered.size() + " users as unregistered."); recipientTable.bulkUpdatedRegisteredStatus(newlyRegistered, operationState.unregistered); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptor.kt index f3fbd67b7b..c8d4cb45dd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptor.kt @@ -88,7 +88,17 @@ object MessageDecryptor { val selfAci: ACI = SignalStore.account().requireAci() val selfPni: PNI = SignalStore.account().requirePni() - val destination: ServiceId = envelope.getDestination(selfAci, selfPni) + val destination: ServiceId? = ServiceId.parseOrNull(envelope.destinationServiceId) + + if (destination == null) { + Log.w(TAG, "${logPrefix(envelope)} Missing destination address! Invalid message, ignoring.") + return Result.Ignore(envelope, serverDeliveredTimestamp, emptyList()) + } + + if (destination != selfAci && destination != selfPni) { + Log.w(TAG, "${logPrefix(envelope)} Destination address does not match our ACI or PNI! Invalid message, ignoring.") + return Result.Ignore(envelope, serverDeliveredTimestamp, emptyList()) + } if (destination == selfPni && envelope.hasSourceServiceId()) { Log.i(TAG, "${logPrefix(envelope)} Received a message at our PNI. Marking as needing a PNI signature.") @@ -158,7 +168,7 @@ object MessageDecryptor { ) } - if (FeatureFlags.phoneNumberPrivacy() && cipherResult.content.hasPniSignatureMessage()) { + if (cipherResult.content.hasPniSignatureMessage()) { if (cipherResult.metadata.sourceServiceId is ACI) { handlePniSignatureMessage( envelope, @@ -411,23 +421,6 @@ object MessageDecryptor { } } - private fun Envelope.getDestination(selfAci: ServiceId, selfPni: ServiceId): ServiceId { - return if (!FeatureFlags.phoneNumberPrivacy()) { - selfAci - } else if (this.hasDestinationServiceId()) { - val serviceId = ServiceId.parseOrThrow(this.destinationServiceId) - if (serviceId == selfAci || serviceId == selfPni) { - serviceId - } else { - Log.w(TAG, "Destination of $serviceId does not match our ACI ($selfAci) or PNI ($selfPni)! Defaulting to ACI.") - selfAci - } - } else { - Log.w(TAG, "No destinationUuid set! Defaulting to ACI.") - selfAci - } - } - private fun Int.toCiphertextMessageType(): Int { return when (this) { Envelope.Type.CIPHERTEXT_VALUE -> CiphertextMessage.WHISPER_TYPE diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java index a8a19f0b18..87cb912602 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java @@ -227,15 +227,8 @@ public class Recipient { RecipientTable db = SignalDatabase.recipients(); - RecipientId recipientId; - - if (FeatureFlags.phoneNumberPrivacy()) { - recipientId = db.getAndPossiblyMergePnpVerified(aci, pni, e164); - } else { - recipientId = db.getAndPossiblyMerge(aci, e164); - } - - Recipient resolved = resolved(recipientId); + RecipientId recipientId = db.getAndPossiblyMergePnpVerified(aci, pni, e164); + Recipient resolved = resolved(recipientId); if (!resolved.getId().equals(recipientId)) { Log.w(TAG, "Resolved " + recipientId + ", but got back a recipient with " + resolved.getId()); @@ -1219,7 +1212,7 @@ public class Recipient { } public boolean needsPniSignature() { - return FeatureFlags.phoneNumberPrivacy() && needsPniSignature; + return needsPniSignature; } public boolean isCallLink() {