From 01039fbc5c2016a5889fa7cf4f707a3f98e954ac Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 21 Jan 2021 13:50:39 -0300 Subject: [PATCH] Refine handling of unverified safety number changes. --- .../CVComponentSystemMessage.swift | 30 ++++++++----------- .../Messages/Interactions/TSErrorMessage.h | 14 ++++----- .../Messages/Interactions/TSErrorMessage.m | 20 +++++-------- .../Interactions/TSErrorMessage.swift | 6 ++-- .../src/Messages/OWSIdentityManager.m | 24 +++++++++++---- .../src/Messages/OWSMessageDecrypter.m | 7 ----- 6 files changed, 48 insertions(+), 53 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSystemMessage.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSystemMessage.swift index c30df51583..559a22d177 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSystemMessage.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSystemMessage.swift @@ -670,10 +670,16 @@ extension CVComponentSystemMessage { return nil } - return CVMessageAction(title: NSLocalizedString("SYSTEM_MESSAGE_ACTION_VERIFY_SAFETY_NUMBER", - comment: "Label for button to verify a user's safety number."), - accessibilityIdentifier: "verify_safety_number", - action: .cvc_didTapNonBlockingIdentityChange(address: address)) + if message.wasIdentityVerified { + return CVMessageAction(title: NSLocalizedString("SYSTEM_MESSAGE_ACTION_VERIFY_SAFETY_NUMBER", + comment: "Label for button to verify a user's safety number."), + accessibilityIdentifier: "verify_safety_number", + action: .cvc_didTapNonBlockingIdentityChange(address: address)) + } else { + return CVMessageAction(title: CommonStrings.learnMore, + accessibilityIdentifier: "learn_more", + action: .cvc_didTapNonBlockingIdentityChange(address: address)) + } case .wrongTrustedIdentityKey: guard let message = message as? TSInvalidIdentityKeyErrorMessage else { owsFailDebug("Invalid interaction.") @@ -692,19 +698,9 @@ extension CVComponentSystemMessage { accessibilityIdentifier: "reset_session", action: .cvc_didTapCorruptedMessage(errorMessage: message)) case .sessionRefresh: - switch message.sessionRefreshType { - case .wasVerified: - return Action(title: CommonStrings.learnMore, - accessibilityIdentifier: "learn_more", - action: .cvc_didTapSessionRefreshMessage(errorMessage: message)) - case .wasNotVerified: - return Action(title: CommonStrings.learnMore, - accessibilityIdentifier: "learn_more", - action: .cvc_didTapSessionRefreshMessage(errorMessage: message)) - @unknown default: - owsFailDebug("Unknown sessionRefreshType.") - return nil - } + return Action(title: CommonStrings.learnMore, + accessibilityIdentifier: "learn_more", + action: .cvc_didTapSessionRefreshMessage(errorMessage: message)) case .duplicateMessage, .invalidVersion: return nil diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.h b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.h index b4625777df..959ad51945 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.h @@ -29,11 +29,6 @@ typedef NS_ENUM(int32_t, TSErrorMessageType) { TSErrorMessageSessionRefresh }; -typedef NS_ENUM(int32_t, SessionRefreshType) { - SessionRefreshTypeWasNotVerified = 0, - SessionRefreshTypeWasVerified, -}; - extern NSUInteger TSErrorMessageSchemaVersion; @interface ThreadlessErrorMessage : NSObject @@ -121,16 +116,17 @@ NS_DESIGNATED_INITIALIZER NS_SWIFT_NAME(init(grdbId:uniqueId:receivedAtTimestamp withTransaction:(SDSAnyWriteTransaction *)transaction; + (instancetype)sessionRefreshWithEnvelope:(SSKProtoEnvelope *)envelope - sessionRefreshType:(SessionRefreshType)sessionRefreshType withTransaction:(SDSAnyWriteTransaction *)transaction; -+ (instancetype)nonblockingIdentityChangeInThread:(TSThread *)thread address:(SignalServiceAddress *)address; ++ (instancetype)nonblockingIdentityChangeInThread:(TSThread *)thread + address:(SignalServiceAddress *)address + wasIdentityVerified:(BOOL)wasIdentityVerified; @property (nonatomic, readonly) TSErrorMessageType errorType; @property (nullable, nonatomic, readonly) SignalServiceAddress *recipientAddress; -// This property only applies if errorType == TSErrorMessageSessionRefresh. -@property (nonatomic, readonly) SessionRefreshType sessionRefreshType; +// This property only applies if errorType == .nonBlockingIdentityChange. +@property (nonatomic, readonly) BOOL wasIdentityVerified; @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m index 023336bede..a35bcdc35e 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m @@ -18,9 +18,6 @@ NSUInteger TSErrorMessageSchemaVersion = 2; @property (nonatomic, readonly) TSErrorMessageType errorType; -// This property only applies if errorType == TSErrorMessageSessionRefresh. -@property (nonatomic) SessionRefreshType sessionRefreshType; - @end #pragma mark - @@ -110,7 +107,7 @@ NSUInteger TSErrorMessageSchemaVersion = 2; _errorType = errorMessageBuilder.errorType; _recipientAddress = errorMessageBuilder.recipientAddress; _errorMessageSchemaVersion = TSErrorMessageSchemaVersion; - _sessionRefreshType = errorMessageBuilder.sessionRefreshType; + _wasIdentityVerified = errorMessageBuilder.wasIdentityVerified; if (self.isDynamicInteraction) { self.read = YES; @@ -268,22 +265,21 @@ NSUInteger TSErrorMessageSchemaVersion = 2; } + (instancetype)sessionRefreshWithEnvelope:(SSKProtoEnvelope *)envelope - sessionRefreshType:(SessionRefreshType)sessionRefreshType withTransaction:(SDSAnyWriteTransaction *)transaction { - TSErrorMessageBuilder *builder = - [TSErrorMessageBuilder errorMessageBuilderWithErrorType:TSErrorMessageSessionRefresh - envelope:envelope - transaction:transaction]; - builder.sessionRefreshType = sessionRefreshType; - return [builder build]; + return [[TSErrorMessageBuilder errorMessageBuilderWithErrorType:TSErrorMessageSessionRefresh + envelope:envelope + transaction:transaction] build]; } -+ (instancetype)nonblockingIdentityChangeInThread:(TSThread *)thread address:(SignalServiceAddress *)address ++ (instancetype)nonblockingIdentityChangeInThread:(TSThread *)thread + address:(SignalServiceAddress *)address + wasIdentityVerified:(BOOL)wasIdentityVerified { TSErrorMessageBuilder *builder = [TSErrorMessageBuilder errorMessageBuilderWithThread:thread errorType:TSErrorMessageNonBlockingIdentityChange]; builder.recipientAddress = address; + builder.wasIdentityVerified = wasIdentityVerified; return [builder build]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.swift b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.swift index 84b3e638e4..9e56754015 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.swift +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.swift @@ -15,7 +15,7 @@ public class TSErrorMessageBuilder: TSMessageBuilder { @objc public var recipientAddress: SignalServiceAddress? @objc - public var sessionRefreshType: SessionRefreshType + public var wasIdentityVerified: Bool public required init(thread: TSThread, timestamp: UInt64? = nil, @@ -30,11 +30,11 @@ public class TSErrorMessageBuilder: TSMessageBuilder { isViewOnceMessage: Bool = false, errorType: TSErrorMessageType, recipientAddress: SignalServiceAddress? = nil, - sessionRefreshType: SessionRefreshType = .wasNotVerified) { + wasIdentityVerified: Bool = false) { self.errorType = errorType self.recipientAddress = recipientAddress - self.sessionRefreshType = sessionRefreshType + self.wasIdentityVerified = wasIdentityVerified super.init(thread: thread, timestamp: timestamp, diff --git a/SignalServiceKit/src/Messages/OWSIdentityManager.m b/SignalServiceKit/src/Messages/OWSIdentityManager.m index 60158b4956..e5807d86e2 100644 --- a/SignalServiceKit/src/Messages/OWSIdentityManager.m +++ b/SignalServiceKit/src/Messages/OWSIdentityManager.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// Copyright (c) 2021 Open Whisper Systems. All rights reserved. // #import "OWSIdentityManager.h" @@ -261,13 +261,16 @@ NSNotificationName const kNSNotificationNameIdentityStateDidChange = @"kNSNotifi if (![existingIdentity.identityKey isEqual:identityKey]) { OWSVerificationState verificationState; + BOOL wasIdentityVerified; switch (existingIdentity.verificationState) { case OWSVerificationStateDefault: verificationState = OWSVerificationStateDefault; + wasIdentityVerified = NO; break; case OWSVerificationStateVerified: case OWSVerificationStateNoLongerVerified: verificationState = OWSVerificationStateNoLongerVerified; + wasIdentityVerified = YES; break; } @@ -275,7 +278,10 @@ NSNotificationName const kNSNotificationNameIdentityStateDidChange = @"kNSNotifi accountId, OWSVerificationStateToString(existingIdentity.verificationState), OWSVerificationStateToString(verificationState)); - [self createIdentityChangeInfoMessageForAccountId:accountId transaction:transaction]; + + [self createIdentityChangeInfoMessageForAccountId:accountId + wasIdentityVerified:wasIdentityVerified + transaction:transaction]; [[[OWSRecipientIdentity alloc] initWithAccountId:accountId identityKey:identityKey @@ -554,6 +560,7 @@ NSNotificationName const kNSNotificationNameIdentityStateDidChange = @"kNSNotifi } - (void)createIdentityChangeInfoMessageForAccountId:(NSString *)accountId + wasIdentityVerified:(BOOL)wasIdentityVerified transaction:(SDSAnyWriteTransaction *)transaction { SignalServiceAddress *_Nullable address = [[OWSAccountIdFinder new] addressForAccountId:accountId @@ -564,10 +571,13 @@ NSNotificationName const kNSNotificationNameIdentityStateDidChange = @"kNSNotifi return; } - [self createIdentityChangeInfoMessageForAddress:address transaction:transaction]; + [self createIdentityChangeInfoMessageForAddress:address + wasIdentityVerified:wasIdentityVerified + transaction:transaction]; } - (void)createIdentityChangeInfoMessageForAddress:(SignalServiceAddress *)address + wasIdentityVerified:(BOOL)wasIdentityVerified transaction:(SDSAnyWriteTransaction *)transaction { OWSAssertDebug(address.isValid); @@ -579,11 +589,15 @@ NSNotificationName const kNSNotificationNameIdentityStateDidChange = @"kNSNotifi transaction:transaction]; OWSAssertDebug(contactThread != nil); - TSErrorMessage *errorMessage = [TSErrorMessage nonblockingIdentityChangeInThread:contactThread address:address]; + TSErrorMessage *errorMessage = [TSErrorMessage nonblockingIdentityChangeInThread:contactThread + address:address + wasIdentityVerified:wasIdentityVerified]; [messages addObject:errorMessage]; for (TSGroupThread *groupThread in [TSGroupThread groupThreadsWithAddress:address transaction:transaction]) { - [messages addObject:[TSErrorMessage nonblockingIdentityChangeInThread:groupThread address:address]]; + [messages addObject:[TSErrorMessage nonblockingIdentityChangeInThread:groupThread + address:address + wasIdentityVerified:wasIdentityVerified]]; } // MJK TODO - why not save immediately, why build up this array? diff --git a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m index 389783b247..5957eca39a 100644 --- a/SignalServiceKit/src/Messages/OWSMessageDecrypter.m +++ b/SignalServiceKit/src/Messages/OWSMessageDecrypter.m @@ -751,14 +751,7 @@ NSError *EnsureDecryptError(NSError *_Nullable error, NSString *fallbackErrorDes deviceId:envelope.sourceDevice transaction:transaction]; - // Always notify the user that we have performed an automatic archive. - BOOL isVerified = ([self.identityManager verificationStateForAddress:envelope.sourceAddress - transaction:transaction] - == OWSVerificationStateVerified); - SessionRefreshType sessionRefreshType - = (isVerified ? SessionRefreshTypeWasVerified : SessionRefreshTypeWasNotVerified); errorMessage = [TSErrorMessage sessionRefreshWithEnvelope:envelope - sessionRefreshType:sessionRefreshType withTransaction:transaction]; NSDate *_Nullable lastNullMessageDate = [self.keyValueStore getDate:senderId transaction:transaction];