From 0955ab8662617154edf5afcc68dfa2bbf4ad773b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Sun, 2 Dec 2018 17:30:31 -0500 Subject: [PATCH 1/2] Refine envelope processing. --- .../ViewControllers/DebugUI/DebugUIMessages.m | 1 + .../src/Messages/OWSBatchMessageProcessor.h | 1 + .../src/Messages/OWSBatchMessageProcessor.m | 28 ++++++++--- .../src/Messages/OWSMessageManager.h | 1 + .../src/Messages/OWSMessageManager.m | 46 +++++++++++++------ .../src/Messages/OWSMessageReceiver.m | 11 +++++ 6 files changed, 68 insertions(+), 20 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 9c5f1e91d..51e3edc08 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -3916,6 +3916,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac [OWSPrimaryStorage.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [SSKEnvironment.shared.batchMessageProcessor enqueueEnvelopeData:envelopeData plaintextData:plaintextData + wasReceivedByUD:NO transaction:transaction]; }]; } diff --git a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.h b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.h index 017dc21ab..7dafd743d 100644 --- a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.h +++ b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.h @@ -22,6 +22,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)enqueueEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)handleAnyUnprocessedEnvelopesAsync; diff --git a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m index 49f05fc88..77bdd74fd 100644 --- a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m +++ b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m @@ -33,9 +33,11 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) NSDate *createdAt; @property (nonatomic, readonly) NSData *envelopeData; @property (nonatomic, readonly, nullable) NSData *plaintextData; +@property (nonatomic, readonly) BOOL wasReceivedByUD; - (instancetype)initWithEnvelopeData:(NSData *)envelopeData - plaintextData:(NSData *_Nullable)plaintextData NS_DESIGNATED_INITIALIZER; + plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; - (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; @@ -52,7 +54,9 @@ NS_ASSUME_NONNULL_BEGIN return @"OWSBatchMessageProcessingJob"; } -- (instancetype)initWithEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData +- (instancetype)initWithEnvelopeData:(NSData *)envelopeData + plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD { OWSAssertDebug(envelopeData); @@ -63,6 +67,7 @@ NS_ASSUME_NONNULL_BEGIN _envelopeData = envelopeData; _plaintextData = plaintextData; + _wasReceivedByUD = wasReceivedByUD; _createdAt = [NSDate new]; return self; @@ -148,13 +153,15 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo - (void)addJobWithEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssertDebug(envelopeData); OWSAssertDebug(transaction); - OWSMessageContentJob *job = - [[OWSMessageContentJob alloc] initWithEnvelopeData:envelopeData plaintextData:plaintextData]; + OWSMessageContentJob *job = [[OWSMessageContentJob alloc] initWithEnvelopeData:envelopeData + plaintextData:plaintextData + wasReceivedByUD:wasReceivedByUD]; [job saveWithTransaction:transaction]; } @@ -316,13 +323,17 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo - (void)enqueueEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssertDebug(envelopeData); OWSAssertDebug(transaction); // We need to persist the decrypted envelope data ASAP to prevent data loss. - [self.finder addJobWithEnvelopeData:envelopeData plaintextData:plaintextData transaction:transaction]; + [self.finder addJobWithEnvelopeData:envelopeData + plaintextData:plaintextData + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; } - (void)drainQueue @@ -405,6 +416,7 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo } else { [self.messageManager throws_processEnvelope:envelope plaintextData:job.plaintextData + wasReceivedByUD:job.wasReceivedByUD transaction:transaction]; } } @catch (NSException *exception) { @@ -482,6 +494,7 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo - (void)enqueueEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (envelopeData.length < 1) { @@ -491,7 +504,10 @@ NSString *const OWSMessageContentJobFinderExtensionGroup = @"OWSMessageContentJo OWSAssert(transaction); // We need to persist the decrypted envelope data ASAP to prevent data loss. - [self.processingQueue enqueueEnvelopeData:envelopeData plaintextData:plaintextData transaction:transaction]; + [self.processingQueue enqueueEnvelopeData:envelopeData + plaintextData:plaintextData + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; // The new envelope won't be visible to the finder until this transaction commits, // so drainQueue in the transaction completion. diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.h b/SignalServiceKit/src/Messages/OWSMessageManager.h index fb274d060..b06fb4aa1 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.h +++ b/SignalServiceKit/src/Messages/OWSMessageManager.h @@ -21,6 +21,7 @@ NS_ASSUME_NONNULL_BEGIN // processEnvelope: can be called from any thread. - (void)throws_processEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction; // This should be invoked by the main app when the app is ready. diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 033c1976f..673aa00d9 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -219,6 +219,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)throws_processEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *_Nullable)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -261,7 +262,10 @@ NS_ASSUME_NONNULL_BEGIN OWSFailDebug(@"missing decrypted data for envelope: %@", [self descriptionForEnvelope:envelope]); return; } - [self throws_handleEnvelope:envelope plaintextData:plaintextData transaction:transaction]; + [self throws_handleEnvelope:envelope + plaintextData:plaintextData + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; break; case SSKProtoEnvelopeTypeReceipt: OWSAssertDebug(!plaintextData); @@ -352,6 +356,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)throws_handleEnvelope:(SSKProtoEnvelope *)envelope plaintextData:(NSData *)plaintextData + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -406,7 +411,10 @@ NS_ASSUME_NONNULL_BEGIN [[OWSDeviceManager sharedManager] setHasReceivedSyncMessage]; } else if (contentProto.dataMessage) { - [self handleIncomingEnvelope:envelope withDataMessage:contentProto.dataMessage transaction:transaction]; + [self handleIncomingEnvelope:envelope + withDataMessage:contentProto.dataMessage + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; } else if (contentProto.callMessage) { [self handleIncomingEnvelope:envelope withCallMessage:contentProto.callMessage]; } else if (contentProto.nullMessage) { @@ -427,7 +435,10 @@ NS_ASSUME_NONNULL_BEGIN } OWSLogInfo(@"handling message: ", [self descriptionForDataMessage:dataMessageProto]); - [self handleIncomingEnvelope:envelope withDataMessage:dataMessageProto transaction:transaction]; + [self handleIncomingEnvelope:envelope + withDataMessage:dataMessageProto + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; } else { OWSProdInfoWEnvelope([OWSAnalyticsEvents messageManagerErrorEnvelopeNoActionablePayload], envelope); } @@ -435,6 +446,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope withDataMessage:(SSKProtoDataMessage *)dataMessage + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -517,9 +529,15 @@ NS_ASSUME_NONNULL_BEGIN } else if ((dataMessage.flags & SSKProtoDataMessageFlagsProfileKeyUpdate) != 0) { [self handleProfileKeyMessageWithEnvelope:envelope dataMessage:dataMessage]; } else if (dataMessage.attachments.count > 0) { - [self handleReceivedMediaWithEnvelope:envelope dataMessage:dataMessage transaction:transaction]; + [self handleReceivedMediaWithEnvelope:envelope + dataMessage:dataMessage + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; } else { - [self handleReceivedTextMessageWithEnvelope:envelope dataMessage:dataMessage transaction:transaction]; + [self handleReceivedTextMessageWithEnvelope:envelope + dataMessage:dataMessage + wasReceivedByUD:wasReceivedByUD + transaction:transaction]; if ([self isDataMessageGroupAvatarUpdate:dataMessage]) { OWSLogVerbose(@"Data message had group avatar attachment"); @@ -528,18 +546,11 @@ NS_ASSUME_NONNULL_BEGIN } // Send delivery receipts for "valid data" messages received via UD. - BOOL wasReceivedByUD = [self wasReceivedByUD:envelope]; if (wasReceivedByUD) { [self.outgoingReceiptManager enqueueDeliveryReceiptForEnvelope:envelope]; } } -- (BOOL)wasReceivedByUD:(SSKProtoEnvelope *)envelope -{ - return ( - envelope.type == SSKProtoEnvelopeTypeUnidentifiedSender && (!envelope.hasSource || envelope.source.length < 1)); -} - - (void)sendGroupInfoRequest:(NSData *)groupId envelope:(SSKProtoEnvelope *)envelope transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -702,6 +713,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleReceivedMediaWithEnvelope:(SSKProtoEnvelope *)envelope dataMessage:(SSKProtoDataMessage *)dataMessage + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -734,6 +746,7 @@ NS_ASSUME_NONNULL_BEGIN TSIncomingMessage *_Nullable createdMessage = [self handleReceivedEnvelope:envelope withDataMessage:dataMessage + wasReceivedByUD:wasReceivedByUD attachmentIds:attachmentsProcessor.attachmentIds transaction:transaction]; @@ -983,6 +996,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleReceivedTextMessageWithEnvelope:(SSKProtoEnvelope *)envelope dataMessage:(SSKProtoDataMessage *)dataMessage + wasReceivedByUD:(BOOL)wasReceivedByUD transaction:(YapDatabaseReadWriteTransaction *)transaction { if (!envelope) { @@ -998,7 +1012,11 @@ NS_ASSUME_NONNULL_BEGIN return; } - [self handleReceivedEnvelope:envelope withDataMessage:dataMessage attachmentIds:@[] transaction:transaction]; + [self handleReceivedEnvelope:envelope + withDataMessage:dataMessage + wasReceivedByUD:wasReceivedByUD + attachmentIds:@[] + transaction:transaction]; } - (void)sendGroupUpdateForThread:(TSGroupThread *)gThread message:(TSOutgoingMessage *)message @@ -1106,6 +1124,7 @@ NS_ASSUME_NONNULL_BEGIN - (TSIncomingMessage *_Nullable)handleReceivedEnvelope:(SSKProtoEnvelope *)envelope withDataMessage:(SSKProtoDataMessage *)dataMessage + wasReceivedByUD:(BOOL)wasReceivedByUD attachmentIds:(NSArray *)attachmentIds transaction:(YapDatabaseReadWriteTransaction *)transaction { @@ -1127,7 +1146,6 @@ NS_ASSUME_NONNULL_BEGIN NSData *groupId = dataMessage.group ? dataMessage.group.id : nil; OWSContact *_Nullable contact = [OWSContacts contactForDataMessage:dataMessage transaction:transaction]; NSNumber *_Nullable serverTimestamp = (envelope.hasServerTimestamp ? @(envelope.serverTimestamp) : nil); - BOOL wasReceivedByUD = [self wasReceivedByUD:envelope]; if (dataMessage.group.type == SSKProtoGroupContextTypeRequestInfo) { [self handleGroupInfoRequest:envelope dataMessage:dataMessage transaction:transaction]; diff --git a/SignalServiceKit/src/Messages/OWSMessageReceiver.m b/SignalServiceKit/src/Messages/OWSMessageReceiver.m index 5ad0c8f94..92ffe69cd 100644 --- a/SignalServiceKit/src/Messages/OWSMessageReceiver.m +++ b/SignalServiceKit/src/Messages/OWSMessageReceiver.m @@ -333,6 +333,12 @@ NSString *const OWSMessageDecryptJobFinderExtensionGroup = @"OWSMessageProcessin }]; } +- (BOOL)wasReceivedByUD:(SSKProtoEnvelope *)envelope +{ + return ( + envelope.type == SSKProtoEnvelopeTypeUnidentifiedSender && (!envelope.hasSource || envelope.source.length < 1)); +} + - (void)processJob:(OWSMessageDecryptJob *)job completion:(void (^)(BOOL))completion { AssertOnDispatchQueue(self.serialQueue); @@ -356,6 +362,10 @@ NSString *const OWSMessageDecryptJobFinderExtensionGroup = @"OWSMessageProcessin return; } + // We use the original envelope for this check; + // the decryption process might rewrite the envelope. + BOOL wasReceivedByUD = [self wasReceivedByUD:envelope]; + [self.messageDecrypter decryptEnvelope:envelope envelopeData:job.envelopeData successBlock:^(OWSMessageDecryptResult *result, YapDatabaseReadWriteTransaction *transaction) { @@ -369,6 +379,7 @@ NSString *const OWSMessageDecryptJobFinderExtensionGroup = @"OWSMessageProcessin // since the envelope may be altered by the decryption process in the UD case. [self.batchMessageProcessor enqueueEnvelopeData:result.envelopeData plaintextData:result.plaintextData + wasReceivedByUD:wasReceivedByUD transaction:transaction]; dispatch_async(self.serialQueue, ^{ From 250bb8dc28c268a509c51fd2eb6cbc92c46e8eaa Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Sun, 2 Dec 2018 17:33:14 -0500 Subject: [PATCH 2/2] "Bump build to 2.31.2.0." --- Signal/Signal-Info.plist | 4 ++-- SignalShareExtension/Info.plist | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index 29322c2f6..f6ac95ba3 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -32,7 +32,7 @@ CFBundlePackageType APPL CFBundleShortVersionString - 2.31.1 + 2.31.2 CFBundleSignature ???? CFBundleURLTypes @@ -49,7 +49,7 @@ CFBundleVersion - 2.31.1.3 + 2.31.2.0 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 2882418f6..0d6a76065 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -17,9 +17,9 @@ CFBundlePackageType XPC! CFBundleShortVersionString - 2.31.1 + 2.31.2 CFBundleVersion - 2.31.1.3 + 2.31.2.0 ITSAppUsesNonExemptEncryption NSAppTransportSecurity