From 3f4dcecf158b33ba25d1a87ef3153cf3ef59a2bd Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 26 Jul 2017 15:59:31 -0400 Subject: [PATCH] ensure blocking keychange message has identityKey before proceeding This is increasingly irrelevant due to recent safety number changes, but legacy clients with old messages can still run into this. FIXES: https://github.com/WhisperSystems/Signal-iOS/issues/2346 // FREEBIE --- Signal/src/util/ThreadUtil.m | 9 ++++++++- .../TSInvalidIdentityKeyErrorMessage.h | 8 ++------ .../TSInvalidIdentityKeyErrorMessage.m | 2 +- .../TSInvalidIdentityKeyReceivingErrorMessage.m | 6 +++--- .../TSInvalidIdentityKeySendingErrorMessage.m | 10 ++++++++-- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index d5460e387..d3205b0fb 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -287,7 +287,14 @@ NS_ASSUME_NONNULL_BEGIN if (!isMissing) { continue; } - [missingUnseenSafetyNumberChanges addObject:safetyNumberChange.newIdentityKey]; + + NSData *_Nullable newIdentityKey = safetyNumberChange.newIdentityKey; + if (newIdentityKey == nil) { + OWSFail(@"Safety number change was missing it's new identity key."); + continue; + } + + [missingUnseenSafetyNumberChanges addObject:newIdentityKey]; } // Count the de-duplicated "blocking" safety number changes and all diff --git a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.h b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.h index 84588736c..2ef76e8ad 100644 --- a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.h +++ b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.h @@ -1,9 +1,5 @@ // -// TSInvalidIdentityKeyErrorMessage.h -// Signal -// -// Created by Frederic Jacobs on 15/02/15. -// Copyright (c) 2015 Open Whisper Systems. All rights reserved. +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. // #import "TSErrorMessage.h" @@ -15,7 +11,7 @@ NS_ASSUME_NONNULL_BEGIN @interface TSInvalidIdentityKeyErrorMessage : TSErrorMessage - (void)acceptNewIdentityKey; -- (NSData *)newIdentityKey; +- (nullable NSData *)newIdentityKey; - (NSString *)theirSignalId; @end diff --git a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.m b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.m index 0274fb662..05606cfc2 100644 --- a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.m +++ b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyErrorMessage.m @@ -13,7 +13,7 @@ NS_ASSUME_NONNULL_BEGIN NSAssert(NO, @"Method needs to be implemented in subclasses of TSInvalidIdentityKeyErrorMessage."); } -- (NSString *)newIdentityKey +- (nullable NSData *)newIdentityKey { NSAssert(NO, @"Method needs to be implemented in subclasses of TSInvalidIdentityKeyErrorMessage."); return nil; diff --git a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyReceivingErrorMessage.m b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyReceivingErrorMessage.m index 85eefd652..67bcb20c4 100644 --- a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyReceivingErrorMessage.m +++ b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeyReceivingErrorMessage.m @@ -74,9 +74,9 @@ NS_ASSUME_NONNULL_BEGIN return; } - NSData *newKey = [self newIdentityKey]; + NSData *_Nullable newKey = [self newIdentityKey]; if (!newKey) { - DDLogError(@"Couldn't extract identity key to accept"); + OWSFail(@"Couldn't extract identity key to accept"); return; } @@ -101,7 +101,7 @@ NS_ASSUME_NONNULL_BEGIN }); } -- (NSData *)newIdentityKey +- (nullable NSData *)newIdentityKey { if (!self.envelope) { DDLogError(@"Error message had no envelope data to extract key from"); diff --git a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m index 7502f1385..072705a11 100644 --- a/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m +++ b/SignalServiceKit/src/Messages/InvalidKeyMessages/TSInvalidIdentityKeySendingErrorMessage.m @@ -50,12 +50,18 @@ NSString *TSInvalidRecipientKey = @"TSInvalidRecipientKey"; OWSFail(@"accepting new identity key is deprecated."); // Saving a new identity mutates the session store so it must happen on the sessionStoreQueue + NSData *_Nullable newIdentityKey = self.newIdentityKey; + if (!newIdentityKey) { + OWSFail(@"newIdentityKey is unexpectedly nil. Bad Prekey bundle?: %@", self.preKeyBundle); + return; + } + dispatch_async([OWSDispatch sessionStoreQueue], ^{ - [[OWSIdentityManager sharedManager] saveRemoteIdentity:self.newIdentityKey recipientId:self.recipientId]; + [[OWSIdentityManager sharedManager] saveRemoteIdentity:newIdentityKey recipientId:self.recipientId]; }); } -- (NSData *)newIdentityKey +- (nullable NSData *)newIdentityKey { return [self.preKeyBundle.identityKey removeKeyType]; }