From 4d1c38cc457694d73098501bb310593b8513f2b9 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 13:10:40 -0500 Subject: [PATCH 01/11] Never failover message sends. --- SignalServiceKit/src/Messages/OWSMessageSender.m | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 6c7205249..7c5b6655b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1070,15 +1070,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; OWSLogWarn(@"Sending a message with no device messages."); } - // NOTE: canFailoverUDAuth depends on whether or not we're sending a - // sync message because sync messages use different device lists - // for UD-auth and Non-UD-auth requests. - // - // Therefore, for sync messages, we can't use OWSRequestMaker's - // retry/failover logic; we need to use the message sender's retry - // logic that will build a new set of device messages. - BOOL isSyncMessageSend = messageSend.isLocalNumber; - BOOL canFailoverUDAuth = !isSyncMessageSend; OWSRequestMaker *requestMaker = [[OWSRequestMaker alloc] initWithLabel:@"Message Send" requestFactoryBlock:^(SMKUDAccessKey *_Nullable udAccessKey) { return [OWSRequestFactory submitMessageRequestWithRecipient:recipient.recipientId @@ -1098,7 +1089,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } recipientId:recipient.recipientId udAccess:messageSend.udAccess - canFailoverUDAuth:canFailoverUDAuth]; + canFailoverUDAuth:NO]; [[requestMaker makeRequestObjc] .then(^(OWSRequestMakerResult *result) { dispatch_async([OWSDispatch sendingQueue], ^{ From b290c9a89f597fd665a80139ade4f7f579fbd251 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 13:01:43 -0500 Subject: [PATCH 02/11] Fix headers for censorship circumvention. --- .../src/Network/API/TSNetworkManager.m | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 94e7448f9..9af915388 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -27,6 +27,7 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) // This property should only be accessed on udSerialQueue. @property (atomic, readonly) AFHTTPSessionManager *udSessionManager; +@property (atomic, readonly) NSDictionary *udSessionManagerDefaultHeaders; @property (atomic, readonly) dispatch_queue_t udSerialQueue; @@ -139,9 +140,12 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); - (AFHTTPSessionManager *)udSessionManager { if (!_udSessionManager) { - _udSessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager; - _udSessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + AFHTTPSessionManager *udSessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager; + udSessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); // NOTE: We could enable HTTPShouldUsePipelining here. + _udSessionManager = udSessionManager; + // Make a copy of the default headers for this session manager. + _udSessionManagerDefaultHeaders = [udSessionManager.requestSerializer.HTTPRequestHeaders copy]; } return _udSessionManager; @@ -169,10 +173,16 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); AFHTTPSessionManager *sessionManager = self.udSessionManager; - // Honor the request's headers. + // Clear all headers so that we don't retain headers from previous requests. for (NSString *headerField in sessionManager.requestSerializer.HTTPRequestHeaders.allKeys.copy) { [sessionManager.requestSerializer setValue:nil forHTTPHeaderField:headerField]; } + // Apply the default headers for this session manager. + for (NSString *headerField in self.udSessionManagerDefaultHeaders) { + NSString *headerValue = self.udSessionManagerDefaultHeaders[headerField]; + [sessionManager.requestSerializer setValue:headerValue forHTTPHeaderField:headerField]; + } + // Honor the request's headers. for (NSString *headerField in request.allHTTPHeaderFields) { NSString *headerValue = request.allHTTPHeaderFields[headerField]; [sessionManager.requestSerializer setValue:headerValue forHTTPHeaderField:headerField]; From d78371be74c944463038e7b400b4ae9eae961d8a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 19 Nov 2018 15:57:25 -0500 Subject: [PATCH 03/11] Don't use UD for "self" profile fetches. --- SignalMessaging/profiles/ProfileFetcherJob.swift | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index 1261b3b0e..fae0485e5 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -62,6 +62,10 @@ public class ProfileFetcherJob: NSObject { return SignalServiceRestClient() } + private var tsAccountManager: TSAccountManager { + return SSKEnvironment.shared.tsAccountManager + } + // MARK: - public func run(recipientIds: [String]) { @@ -135,8 +139,13 @@ public class ProfileFetcherJob: NSObject { Logger.error("getProfile: \(recipientId)") - let udAccess = udManager.udAccess(forRecipientId: recipientId, - requireSyncAccess: false) + // Don't use UD for "self" profile fetches. + var udAccess: OWSUDAccess? + if recipientId != tsAccountManager.localNumber() { + udAccess = udManager.udAccess(forRecipientId: recipientId, + requireSyncAccess: false) + } + return requestProfile(recipientId: recipientId, udAccess: udAccess, canFailoverUDAuth: true) From d6b107ec18fe223633325ca32b0c151f86c01bfb Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 13:40:31 -0500 Subject: [PATCH 04/11] "Bump build to 2.31.1.1." --- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index d6be6f842..748f59de7 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -49,7 +49,7 @@ CFBundleVersion - 2.31.1.0 + 2.31.1.1 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 3bd19ef94..359e83def 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 2.31.1 CFBundleVersion - 2.31.1.0 + 2.31.1.1 ITSAppUsesNonExemptEncryption NSAppTransportSecurity From a6cef1c4cc32023c6d2d203cccf9c9f2d4994654 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 14:30:03 -0500 Subject: [PATCH 05/11] Update UD indicators. --- SignalServiceKit/src/Messages/OWSMessageManager.m | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 68a20d28c..033c1976f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -528,12 +528,18 @@ NS_ASSUME_NONNULL_BEGIN } // Send delivery receipts for "valid data" messages received via UD. - BOOL wasReceivedByUD = envelope.type == SSKProtoEnvelopeTypeUnidentifiedSender; + 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 @@ -1121,7 +1127,7 @@ 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 = envelope.type == SSKProtoEnvelopeTypeUnidentifiedSender; + BOOL wasReceivedByUD = [self wasReceivedByUD:envelope]; if (dataMessage.group.type == SSKProtoGroupContextTypeRequestInfo) { [self handleGroupInfoRequest:envelope dataMessage:dataMessage transaction:transaction]; From c183aeca8e524a10aa4f0c53d1c19eb0340c4092 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 14:30:16 -0500 Subject: [PATCH 06/11] Refine asserts around message sending. --- SignalServiceKit/src/Messages/OWSMessageSender.m | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 7c5b6655b..d0c0e37e3 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1053,6 +1053,17 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } } + for (NSDictionary *deviceMessage in deviceMessages) { + NSNumber *_Nullable messageType = deviceMessage[@"type"]; + OWSAssertDebug(messageType); + if (messageSend.isUDSend) { + OWSAssertDebug([messageType isEqualToNumber:@(TSUnidentifiedSenderMessageType)]); + } else { + OWSAssertDebug([messageType isEqualToNumber:@(TSEncryptedWhisperMessageType)] || + [messageType isEqualToNumber:@(TSPreKeyWhisperMessageType)]); + } + } + if (deviceMessages.count == 0) { // This might happen: // From a2dfcd0288962fec3ea8ba2504d368961118e523 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 15:00:37 -0500 Subject: [PATCH 07/11] Respond to CR. --- SignalServiceKit/src/Messages/OWSMessageSender.m | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index d0c0e37e3..53c359a67 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1056,12 +1056,20 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; for (NSDictionary *deviceMessage in deviceMessages) { NSNumber *_Nullable messageType = deviceMessage[@"type"]; OWSAssertDebug(messageType); + BOOL hasValidMessageType; if (messageSend.isUDSend) { - OWSAssertDebug([messageType isEqualToNumber:@(TSUnidentifiedSenderMessageType)]); + hasValidMessageType = [messageType isEqualToNumber:@(TSUnidentifiedSenderMessageType)]; } else { - OWSAssertDebug([messageType isEqualToNumber:@(TSEncryptedWhisperMessageType)] || + hasValidMessageType = ([messageType isEqualToNumber:@(TSEncryptedWhisperMessageType)] || [messageType isEqualToNumber:@(TSPreKeyWhisperMessageType)]); } + + if (!hasValidMessageType) { + OWSFailDebug(@"Invalid message type: %@", messageType); + NSError *error = OWSErrorMakeFailedToSendOutgoingMessageError(); + [error setIsRetryable:NO]; + return messageSend.failure(error); + } } if (deviceMessages.count == 0) { From 5ce4f4a18c2c145028f425baa0d1a9177844fc9f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 15:27:23 -0500 Subject: [PATCH 08/11] "Bump build to 2.31.1.2." --- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index 748f59de7..cb125cd51 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -49,7 +49,7 @@ CFBundleVersion - 2.31.1.1 + 2.31.1.2 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 359e83def..6bac35ef8 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 2.31.1 CFBundleVersion - 2.31.1.1 + 2.31.1.2 ITSAppUsesNonExemptEncryption NSAppTransportSecurity From 4ab37a0f380a720ca5552907f0fe1f4a4714014a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 18:04:59 -0500 Subject: [PATCH 09/11] Update Cocoapods. --- Podfile.lock | 2 +- Pods | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Podfile.lock b/Podfile.lock index 0b7c433b0..736e2e0a9 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -261,7 +261,7 @@ CHECKOUT OPTIONS: :commit: b60dc7d58dfc93ca6eafbb3ea5300c6d67ebc69a :git: https://github.com/signalapp/SignalCoreKit.git SignalMetadataKit: - :commit: a5473c8d33602775e00253afce78eef01a69260e + :commit: 56f28fc3a6e35d548d034ef7d0009f233ca0aa62 :git: https://github.com/signalapp/SignalMetadataKit SocketRocket: :commit: 9f9563a83cd8960503074aa8de72206f83fb7a69 diff --git a/Pods b/Pods index 37cee04ee..f701ff448 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 37cee04eebbe0ecbf61bd415383be9b1773dccef +Subproject commit f701ff448b3426345539a3c2e72b73d8ca663994 From 2b43fe31ed5f9b0b1d081db26c1670e842867429 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Thu, 29 Nov 2018 15:30:07 -0700 Subject: [PATCH 10/11] verify serialzed message exists --- SignalServiceKit/src/Messages/OWSMessageSender.m | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 53c359a67..d0dfba4d6 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1458,7 +1458,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; // we open a transaction. [self throws_ensureRecipientHasSessionForMessageSend:messageSend deviceId:deviceId]; - __block NSDictionary *messageDict; + __block NSDictionary *_Nullable messageDict; __block NSException *encryptionException; [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { @@ -1626,10 +1626,10 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; }) retainUntilComplete]; } -- (NSDictionary *)throws_encryptedMessageForMessageSend:(OWSMessageSend *)messageSend - deviceId:(NSNumber *)deviceId - plainText:(NSData *)plainText - transaction:(YapDatabaseReadWriteTransaction *)transaction +- (nullable NSDictionary *)throws_encryptedMessageForMessageSend:(OWSMessageSend *)messageSend + deviceId:(NSNumber *)deviceId + plainText:(NSData *)plainText + transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssertDebug(messageSend); OWSAssertDebug(deviceId); @@ -1679,6 +1679,10 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; protocolContext:transaction error:&error]; SCKRaiseIfExceptionWrapperError(error); + if (!serializedMessage || error) { + OWSFailDebug(@"error while UD encrypting message: %@", error); + return nil; + } messageType = TSUnidentifiedSenderMessageType; } else { // This may throw an exception. From f9bdf574fb2d8f6295420714ecacc5d56b06001e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 29 Nov 2018 18:07:29 -0500 Subject: [PATCH 11/11] "Bump build to 2.31.1.3." --- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index cb125cd51..29322c2f6 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -49,7 +49,7 @@ CFBundleVersion - 2.31.1.2 + 2.31.1.3 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 6bac35ef8..2882418f6 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 2.31.1 CFBundleVersion - 2.31.1.2 + 2.31.1.3 ITSAppUsesNonExemptEncryption NSAppTransportSecurity