From e2157dacbf2f52f7dae1d4318eca0107027faf32 Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Thu, 20 Aug 2020 16:55:23 +1000 Subject: [PATCH 1/7] fix typo & ignore error messages before restoration --- .../Loki/Protocol/Meta/SessionMetaProtocol.swift | 6 ++++++ .../src/Messages/OWSMessageDecrypter.m | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift index f545cb2c4..287886a96 100644 --- a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift @@ -94,6 +94,12 @@ public final class SessionMetaProtocol : NSObject { } // MARK: - Receiving + + @objc(isErrorMessageBeforeRestoration:) + public static func isErrorMessageBeforeRestoration(_ errorMessage: TSErrorMessage) -> Bool { + let restorationTimeInMs = UInt64(storage.getRestorationTime() * 1000) + return errorMessage.timestamp < restorationTimeInMs + } @objc(shouldSkipMessageDecryptResult:wrappedIn:) public static func shouldSkipMessageDecryptResult(_ result: OWSMessageDecryptResult, wrappedIn envelope: SSKProtoEnvelope) -> Bool { diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index f346d2d2a..ae7078c40 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -526,7 +526,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes NSError *_Nullable underlyingError; SSKProtoEnvelope *_Nullable identifiedEnvelope; - if (![decryptError.domain isEqualToString:@"SignalMetadataKit.SecretSessionKnownSenderError"]) { + if (![decryptError.domain isEqualToString:@"SessionMetadataKit.SecretSessionKnownSenderError"]) { underlyingError = decryptError; identifiedEnvelope = envelope; } else { @@ -574,7 +574,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes return; } - if ([underlyingError.domain isEqualToString:@"SignalMetadataKit.SMKSecretSessionCipherError"] + if ([underlyingError.domain isEqualToString:@"SessionMetadataKit.SMKSecretSessionCipherError"] && underlyingError.code == SMKSecretSessionCipherErrorSelfSentMessage) { // Self-sent messages can be safely discarded. failureBlock(underlyingError); @@ -672,9 +672,17 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes OWSAssertDebug(errorMessage); if (errorMessage != nil) { - [errorMessage saveWithTransaction:transaction]; [LKSessionManagementProtocol handleDecryptionError:errorMessage.errorType forPublicKey:envelope.source transaction:transaction]; - [self notifyUserForErrorMessage:errorMessage envelope:envelope transaction:transaction]; + if (![LKSessionMetaProtocol isErrorMessageBeforeRestoration:errorMessage]) { + [errorMessage saveWithTransaction:transaction]; + [self notifyUserForErrorMessage:errorMessage envelope:envelope transaction:transaction]; + } else { + // Show the thread if it exists before restoration + NSString *masterPublicKey = [LKDatabaseUtilities getMasterHexEncodedPublicKeyFor:envelope.source in:transaction] ?: envelope.source; + TSThread *contactThread = [TSContactThread getOrCreateThreadWithContactId:masterPublicKey transaction:transaction]; + contactThread.shouldThreadBeVisible = true; + [contactThread saveWithTransaction:transaction]; + } } } error:nil]; } From 5afdd81806599ec1ba54708665bb1d788918182e Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Thu, 20 Aug 2020 16:56:25 +1000 Subject: [PATCH 2/7] fix end session message init function --- .../Interactions/OWSEndSessionMessage.h | 2 ++ .../Interactions/OWSEndSessionMessage.m | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h index cd45fc648..cc7acc768 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h @@ -21,6 +21,8 @@ NS_SWIFT_NAME(EndSessionMessage) contactShare:(nullable OWSContact *)contactShare linkPreview:(nullable OWSLinkPreview *)linkPreview NS_UNAVAILABLE; +- (instancetype)initInThread:(nullable TSThread *)thread messageBody:(nullable NSString *)body attachmentId:(nullable NSString *)attachmentId NS_DESIGNATED_INITIALIZER; + // MJK TODO can we remove the sender timestamp? - (instancetype)initWithTimestamp:(uint64_t)timestamp inThread:(nullable TSThread *)thread NS_DESIGNATED_INITIALIZER; - (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; diff --git a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m index 4138f194b..bfd97e116 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m @@ -5,6 +5,7 @@ #import "OWSEndSessionMessage.h" #import "OWSPrimaryStorage+Loki.h" #import "SignalRecipient.h" +#import #import NS_ASSUME_NONNULL_BEGIN @@ -31,6 +32,26 @@ NS_ASSUME_NONNULL_BEGIN linkPreview:nil]; } +- (instancetype)initInThread:(nullable TSThread *)thread messageBody:(nullable NSString *)body attachmentId:(nullable NSString *)attachmentId +{ + NSMutableArray *attachmentIds = [NSMutableArray new]; + if (attachmentId) { + [attachmentIds addObject:attachmentId]; + } + + return [super initOutgoingMessageWithTimestamp:[NSDate ows_millisecondTimeStamp] + inThread:thread + messageBody:body + attachmentIds:attachmentIds + expiresInSeconds:0 + expireStartedAt:0 + isVoiceMessage:NO + groupMetaMessage:TSGroupMetaMessageUnspecified + quotedMessage:nil + contactShare:nil + linkPreview:nil]; +} + - (BOOL)shouldBeSaved { return NO; From d420d6dd4dc5291d673ad00284355da6a5f06b11 Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Thu, 20 Aug 2020 16:56:52 +1000 Subject: [PATCH 3/7] Automatically rebuild session after restoration --- .../SessionManagementProtocol.swift | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift index 8002e1ecd..bdd55ca56 100644 --- a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift @@ -169,8 +169,13 @@ public final class SessionManagementProtocol : NSObject { // Show the session reset prompt upon certain errors switch type { case .noSession, .invalidMessage, .invalidKeyException: - // Store the source device's public key in case it was a secondary device - thread.addSessionRestoreDevice(publicKey, transaction: transaction) + if storage.getRestorationTime() > 0 { + // Automatically rebuild session after restoration + sendSessionRequestIfNeeded(to: publicKey, using: transaction) + } else { + // Store the source device's public key in case it was a secondary device + thread.addSessionRestoreDevice(publicKey, transaction: transaction) + } default: break } } @@ -178,7 +183,8 @@ public final class SessionManagementProtocol : NSObject { private static func shouldProcessSessionRequest(from publicKey: String, at timestamp: UInt64) -> Bool { let sentTimestamp = Storage.getSessionRequestSentTimestamp(for: publicKey) let processedTimestamp = Storage.getSessionRequestProcessedTimestamp(for: publicKey) - return timestamp > sentTimestamp && timestamp > processedTimestamp + let restorationTimestamp = UInt64(storage.getRestorationTime() * 1000) + return timestamp > sentTimestamp && timestamp > processedTimestamp && timestamp > restorationTimestamp } @objc(handlePreKeyBundleMessageIfNeeded:wrappedIn:transaction:) From 113ad28d5a0d5302b0fa0abea7cf3316f9f27c5e Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Thu, 20 Aug 2020 16:57:18 +1000 Subject: [PATCH 4/7] fix sending logic for prekey bundle message --- SignalServiceKit/src/Messages/OWSMessageSender.m | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 0eec0f8c3..a5e64e1cb 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1450,13 +1450,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; OWSPrimaryStorage *storage = self.primaryStorage; SignalRecipient *recipient = messageSend.recipient; OWSAssertDebug(recipientID.length > 0); - - __block BOOL hasSession; - [LKStorage writeSyncWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - hasSession = [storage containsSession:recipientID deviceId:[deviceId intValue] protocolContext:transaction]; - } error:nil]; - if (hasSession) { return YES; } - + // Discard "typing indicator" messages if there is no existing session with the user. BOOL canSafelyBeDiscarded = messageSend.message.isOnline; if (canSafelyBeDiscarded) { @@ -1467,6 +1461,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; __block NSException *exception; if (!bundle) { + __block BOOL hasSession; + [LKStorage writeSyncWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + hasSession = [storage containsSession:recipientID deviceId:[deviceId intValue] protocolContext:transaction]; + } error:nil]; + if (hasSession) { return YES; } + TSOutgoingMessage *message = messageSend.message; // Loki: Remove this when we have shared sender keys // ======== From 528f930a98156fb338a84ed84e6500e9d467f0aa Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Fri, 21 Aug 2020 09:56:57 +1000 Subject: [PATCH 5/7] clean --- .../Interactions/OWSEndSessionMessage.h | 2 -- .../Interactions/OWSEndSessionMessage.m | 20 ------------------- 2 files changed, 22 deletions(-) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h index cc7acc768..cd45fc648 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.h @@ -21,8 +21,6 @@ NS_SWIFT_NAME(EndSessionMessage) contactShare:(nullable OWSContact *)contactShare linkPreview:(nullable OWSLinkPreview *)linkPreview NS_UNAVAILABLE; -- (instancetype)initInThread:(nullable TSThread *)thread messageBody:(nullable NSString *)body attachmentId:(nullable NSString *)attachmentId NS_DESIGNATED_INITIALIZER; - // MJK TODO can we remove the sender timestamp? - (instancetype)initWithTimestamp:(uint64_t)timestamp inThread:(nullable TSThread *)thread NS_DESIGNATED_INITIALIZER; - (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; diff --git a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m index bfd97e116..01450736f 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSEndSessionMessage.m @@ -32,26 +32,6 @@ NS_ASSUME_NONNULL_BEGIN linkPreview:nil]; } -- (instancetype)initInThread:(nullable TSThread *)thread messageBody:(nullable NSString *)body attachmentId:(nullable NSString *)attachmentId -{ - NSMutableArray *attachmentIds = [NSMutableArray new]; - if (attachmentId) { - [attachmentIds addObject:attachmentId]; - } - - return [super initOutgoingMessageWithTimestamp:[NSDate ows_millisecondTimeStamp] - inThread:thread - messageBody:body - attachmentIds:attachmentIds - expiresInSeconds:0 - expireStartedAt:0 - isVoiceMessage:NO - groupMetaMessage:TSGroupMetaMessageUnspecified - quotedMessage:nil - contactShare:nil - linkPreview:nil]; -} - - (BOOL)shouldBeSaved { return NO; From 8c565c7c5a8e2447bd054d87a7a41147e4b204a6 Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Fri, 21 Aug 2020 12:26:21 +1000 Subject: [PATCH 6/7] prevent infinite loop if errors occur when automatically resetting the session --- .../Session Management/SessionManagementProtocol.swift | 7 ++++--- SignalServiceKit/src/Messages/OWSMessageDecrypter.m | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift index 728680200..b189ab16a 100644 --- a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift @@ -164,14 +164,15 @@ public final class SessionManagementProtocol : NSObject { // MARK: - Receiving @objc(handleDecryptionError:forPublicKey:transaction:) - public static func handleDecryptionError(_ rawValue: Int32, for publicKey: String, using transaction: YapDatabaseReadWriteTransaction) { - let type = TSErrorMessageType(rawValue: rawValue) + public static func handleDecryptionError(_ errorMessage: TSErrorMessage, for publicKey: String, using transaction: YapDatabaseReadWriteTransaction) { + let type = errorMessage.errorType let masterPublicKey = storage.getMasterHexEncodedPublicKey(for: publicKey, in: transaction) ?? publicKey let thread = TSContactThread.getOrCreateThread(withContactId: masterPublicKey, transaction: transaction) + let restorationTimeInMs = UInt64(storage.getRestorationTime() * 1000) // Show the session reset prompt upon certain errors switch type { case .noSession, .invalidMessage, .invalidKeyException: - if storage.getRestorationTime() > 0 { + if restorationTimeInMs > errorMessage.timestamp { // Automatically rebuild session after restoration sendSessionRequestIfNeeded(to: publicKey, using: transaction) } else { diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index 8c0be38f1..e62e7c258 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -690,7 +690,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes OWSAssertDebug(errorMessage); if (errorMessage != nil) { - [LKSessionManagementProtocol handleDecryptionError:errorMessage.errorType forPublicKey:envelope.source transaction:transaction]; + [LKSessionManagementProtocol handleDecryptionError:errorMessage forPublicKey:envelope.source transaction:transaction]; if (![LKSessionMetaProtocol isErrorMessageBeforeRestoration:errorMessage]) { [errorMessage saveWithTransaction:transaction]; [self notifyUserForErrorMessage:errorMessage envelope:envelope transaction:transaction]; From 291216621620239345d3dda608f446fef5e4962a Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Mon, 24 Aug 2020 14:35:33 +1000 Subject: [PATCH 7/7] Rename function --- .../src/Loki/Protocol/Meta/SessionMetaProtocol.swift | 4 ++-- SignalServiceKit/src/Messages/OWSMessageDecrypter.m | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift index 287886a96..f9e031296 100644 --- a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift @@ -95,8 +95,8 @@ public final class SessionMetaProtocol : NSObject { // MARK: - Receiving - @objc(isErrorMessageBeforeRestoration:) - public static func isErrorMessageBeforeRestoration(_ errorMessage: TSErrorMessage) -> Bool { + @objc(isErrorMessageFromBeforeRestoration:) + public static func isErrorMessageFromBeforeRestoration(_ errorMessage: TSErrorMessage) -> Bool { let restorationTimeInMs = UInt64(storage.getRestorationTime() * 1000) return errorMessage.timestamp < restorationTimeInMs } diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index e62e7c258..3a84b0f80 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -691,7 +691,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes OWSAssertDebug(errorMessage); if (errorMessage != nil) { [LKSessionManagementProtocol handleDecryptionError:errorMessage forPublicKey:envelope.source transaction:transaction]; - if (![LKSessionMetaProtocol isErrorMessageBeforeRestoration:errorMessage]) { + if (![LKSessionMetaProtocol isErrorMessageFromBeforeRestoration:errorMessage]) { [errorMessage saveWithTransaction:transaction]; [self notifyUserForErrorMessage:errorMessage envelope:envelope transaction:transaction]; } else {