From dfd0f8073d217433a44b522783eee6769ea09ba7 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 7 Jun 2017 23:21:25 -0400 Subject: [PATCH] When failing to send to new identity, save it. Remove creation of "old style" blocking SN alerts // FREEBIE --- .../TSInvalidIdentityKeySendingErrorMessage.h | 5 -- .../TSInvalidIdentityKeySendingErrorMessage.m | 16 ++--- src/Messages/OWSIdentityManager.h | 3 + src/Messages/OWSMessageSender.h | 8 --- src/Messages/OWSMessageSender.m | 69 ++++++++++--------- src/Messages/TSMessagesManager.m | 8 ++- 6 files changed, 48 insertions(+), 61 deletions(-) diff --git a/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.h b/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.h index 37c1fa1ae..08efb2214 100644 --- a/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.h +++ b/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.h @@ -15,11 +15,6 @@ extern NSString *TSInvalidRecipientKey; @interface TSInvalidIdentityKeySendingErrorMessage : TSInvalidIdentityKeyErrorMessage -+ (instancetype)untrustedKeyWithOutgoingMessage:(TSOutgoingMessage *)outgoingMessage - inThread:(TSThread *)thread - forRecipient:(NSString *)recipientId - preKeyBundle:(PreKeyBundle *)preKeyBundle; - @property (nonatomic, readonly) NSString *messageId; @end diff --git a/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m b/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m index 5a04c697b..7502f1385 100644 --- a/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m +++ b/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m @@ -43,20 +43,12 @@ NSString *TSInvalidRecipientKey = @"TSInvalidRecipientKey"; return self; } -+ (instancetype)untrustedKeyWithOutgoingMessage:(TSOutgoingMessage *)outgoingMessage - inThread:(TSThread *)thread - forRecipient:(NSString *)recipientId - preKeyBundle:(PreKeyBundle *)preKeyBundle -{ - TSInvalidIdentityKeySendingErrorMessage *message = [[self alloc] initWithOutgoingMessage:outgoingMessage - inThread:thread - forRecipient:recipientId - preKeyBundle:preKeyBundle]; - return message; -} - - (void)acceptNewIdentityKey { + // Shouldn't really get here, since we're no longer creating blocking SN changes. + // But there may still be some old unaccepted SN errors in the wild that need to be accepted. + OWSFail(@"accepting new identity key is deprecated."); + // Saving a new identity mutates the session store so it must happen on the sessionStoreQueue dispatch_async([OWSDispatch sessionStoreQueue], ^{ [[OWSIdentityManager sharedManager] saveRemoteIdentity:self.newIdentityKey recipientId:self.recipientId]; diff --git a/src/Messages/OWSIdentityManager.h b/src/Messages/OWSIdentityManager.h index 5ea71becb..17f31ddd7 100644 --- a/src/Messages/OWSIdentityManager.h +++ b/src/Messages/OWSIdentityManager.h @@ -13,6 +13,9 @@ extern NSString *const TSStorageManagerTrustedKeysCollection; // or their verification state changes. extern NSString *const kNSNotificationName_IdentityStateDidChange; +// number of bytes in a signal identity key, excluding the key-type byte. +extern const NSUInteger kIdentityKeyLength; + @class OWSRecipientIdentity; @class OWSSignalServiceProtosSyncMessageVerification; diff --git a/src/Messages/OWSMessageSender.h b/src/Messages/OWSMessageSender.h index 588318a59..7c93fa6a3 100644 --- a/src/Messages/OWSMessageSender.h +++ b/src/Messages/OWSMessageSender.h @@ -86,14 +86,6 @@ NS_SWIFT_NAME(MessageSender) success:(void (^)())successHandler failure:(void (^)(NSError *error))failureHandler; -/** - * Resend a message to a select recipient in a thread when previous sending failed due to key error. - * e.g. If a key change prevents one recipient from receiving the message, we don't want to resend to the entire group. - */ -- (void)resendMessageFromKeyError:(TSInvalidIdentityKeySendingErrorMessage *)errorMessage - success:(void (^)())successHandler - failure:(void (^)(NSError *error))failureHandler; - - (void)handleMessageSentRemotely:(TSOutgoingMessage *)message sentAt:(uint64_t)sentAt; /** diff --git a/src/Messages/OWSMessageSender.m b/src/Messages/OWSMessageSender.m index d734000b2..01c3e754e 100644 --- a/src/Messages/OWSMessageSender.m +++ b/src/Messages/OWSMessageSender.m @@ -4,6 +4,7 @@ #import "OWSMessageSender.h" #import "ContactsUpdater.h" +#import "NSData+keyVersionByte.h" #import "NSData+messagePadding.h" #import "OWSBlockingManager.h" #import "OWSDevice.h" @@ -540,34 +541,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; }); } -- (void)resendMessageFromKeyError:(TSInvalidIdentityKeySendingErrorMessage *)errorMessage - success:(void (^)())successHandler - failure:(void (^)(NSError *error))failureHandler -{ - AssertIsOnMainThread(); - OWSAssert(errorMessage); - - NSString *failedMessageId = errorMessage.messageId; - - // Here we remove the existing error message because sending a new message will either - // 1.) succeed and create a new successful message in the thread or... - // 2.) fail and create a new identical error message in the thread. - [errorMessage remove]; - - // The failedMessageId might be nil for transient, unsaved outgoing messages. - // See [TSOutgoingMessage saveWithTransaction:] for details of which messages - // we do not save. - - if (!failedMessageId) { - return; - } - - TSOutgoingMessage *message = [TSOutgoingMessage fetchObjectWithUniqueID:failedMessageId]; - OWSAssert(message); - - return [self sendMessage:message success:successHandler failure:failureHandler]; -} - - (NSArray *)getRecipients:(NSArray *)identifiers error:(NSError **)error { NSMutableArray *recipients = [NSMutableArray new]; @@ -885,20 +858,47 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } @catch (NSException *exception) { deviceMessages = @[]; if ([exception.name isEqualToString:UntrustedIdentityKeyException]) { - [[TSInvalidIdentityKeySendingErrorMessage - untrustedKeyWithOutgoingMessage:message - inThread:thread - forRecipient:exception.userInfo[TSInvalidRecipientKey] - preKeyBundle:exception.userInfo[TSInvalidPreKeyBundleKey]] save]; + // This *can* happen under normal usage, but it should happen relatively rarely. + // We expect it to happen whenever Bob reinstalls, and Alice messages Bob before + // she can pull down his latest identity. + // If it's happening a lot, we should rethink our profile fetching strategy. + OWSAnalyticsInfo(@"Message send failed due to untrusted key."); + NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeUntrustedIdentityKey, NSLocalizedString(@"FAILED_SENDING_BECAUSE_UNTRUSTED_IDENTITY_KEY", @"action sheet header when re-sending message which failed because of untrusted identity keys")); + // Key will continue to be unaccepted, so no need to retry. It'll only cause us to hit the Pre-Key request // rate limit [error setIsRetryable:NO]; // Avoid the "Too many failures with this contact" error rate limiting. [error setIsFatal:YES]; - return failureHandler(error); + + PreKeyBundle *newKeyBundle = exception.userInfo[TSInvalidPreKeyBundleKey]; + if (![newKeyBundle isKindOfClass:[PreKeyBundle class]]) { + OWSFail(@"%@ unexpected TSInvalidPreKeyBundleKey: %@", self.tag, newKeyBundle); + failureHandler(error); + return; + } + + NSData *newIdentityKeyWithVersion = newKeyBundle.identityKey; + if (![newIdentityKeyWithVersion isKindOfClass:[NSData class]]) { + OWSFail(@"%@ unexpected TSInvalidRecipientKey: %@", self.tag, newIdentityKeyWithVersion); + failureHandler(error); + return; + } + + NSData *newIdentityKey = [newIdentityKeyWithVersion removeKeyType]; + if (newIdentityKey.length != kIdentityKeyLength) { + OWSFail(@"%@ unexpected key length: %lu", self.tag, (unsigned long)newIdentityKey.length); + failureHandler(error); + return; + } + + [[OWSIdentityManager sharedManager] saveRemoteIdentity:newIdentityKey recipientId:recipient.recipientId]; + + failureHandler(error); + return; } if ([exception.name isEqualToString:OWSMessageSenderRateLimitedException]) { @@ -1313,6 +1313,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } } +// Called when the server indicates that the devices no longer exist - e.g. when the remote recipient has reinstalled. - (void)handleStaleDevicesWithResponse:(NSData *)responseData recipientId:(NSString *)identifier completion:(void (^)())completionHandler diff --git a/src/Messages/TSMessagesManager.m b/src/Messages/TSMessagesManager.m index 52afb1bda..e3bdce81b 100644 --- a/src/Messages/TSMessagesManager.m +++ b/src/Messages/TSMessagesManager.m @@ -1001,8 +1001,12 @@ NS_ASSUME_NONNULL_BEGIN } else if ([exception.name isEqualToString:InvalidVersionException]) { errorMessage = [TSErrorMessage invalidVersionWithEnvelope:envelope withTransaction:transaction]; } else if ([exception.name isEqualToString:UntrustedIdentityKeyException]) { - errorMessage = - [TSInvalidIdentityKeyReceivingErrorMessage untrustedKeyWithEnvelope:envelope withTransaction:transaction]; + // Should no longer get here, since we now record the new identity for incoming messages. + OWSFail(@"%@ Failed to trust identity on incoming message from: %@.%d", + self.tag, + envelope.source, + envelope.sourceDevice); + return; } else { errorMessage = [TSErrorMessage corruptedMessageWithEnvelope:envelope withTransaction:transaction]; }