From 351a010fe0b644d96639e339f2f741242bf05272 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 10 Feb 2017 13:20:11 -0500 Subject: [PATCH] Clean up prekey usage. // FREEBIE --- src/Account/TSPreKeyManager.h | 2 + src/Account/TSPreKeyManager.m | 73 +++++++++++++++++-- src/Contacts/PhoneNumberUtil.m | 5 +- src/Messages/OWSMessageSender.m | 21 ++++++ .../TSStorageManager+SignedPreKeyStore.h | 19 +++-- .../TSStorageManager+SignedPreKeyStore.m | 46 ++++++++++++ src/Storage/TSStorageManager.h | 5 +- src/Storage/TSStorageManager.m | 28 ++++++- src/TSPrefix.h | 10 ++- src/Util/OWSError.h | 2 + src/Util/OWSError.m | 7 ++ src/Util/constraints/BadState.m | 6 +- src/Util/constraints/OperationFailed.m | 6 +- 13 files changed, 206 insertions(+), 24 deletions(-) diff --git a/src/Account/TSPreKeyManager.h b/src/Account/TSPreKeyManager.h index f981eff1d..e6ce9a716 100644 --- a/src/Account/TSPreKeyManager.h +++ b/src/Account/TSPreKeyManager.h @@ -20,6 +20,8 @@ typedef NS_ENUM(NSInteger, RefreshPreKeysMode) { @interface TSPreKeyManager : NSObject ++ (BOOL)isAppLockedDueToPreKeyUpdateFailures; + + (void)registerPreKeysWithMode:(RefreshPreKeysMode)mode success:(void (^)())successHandler failure:(void (^)(NSError *error))failureHandler; diff --git a/src/Account/TSPreKeyManager.m b/src/Account/TSPreKeyManager.m index 9e05eacb4..0844c7f29 100644 --- a/src/Account/TSPreKeyManager.m +++ b/src/Account/TSPreKeyManager.m @@ -7,6 +7,7 @@ #import "TSNetworkManager.h" #import "TSRegisterSignedPrekeyRequest.h" #import "TSStorageHeaders.h" +#import "TSStorageManager+SignedPreKeyStore.h" // Time before deletion of signed prekeys (measured in seconds) // @@ -16,7 +17,7 @@ static const CGFloat kSignedPreKeysDeletionTime = 14 * 24 * 60 * 60; // Time before rotation of signed prekeys (measured in seconds) // // Currently we rotate signed prekeys every 2 days (48 hours). -static const CGFloat kSignedPreKeysRotationTime = 2 * 24 * 60 * 60; +static const CGFloat kSignedPreKeyRotationTime = 2 * 24 * 60 * 60; // How often we check prekey state on app activation. // @@ -30,8 +31,50 @@ static const NSUInteger kEphemeralPreKeysMinimumCount = 35; // This global should only be accessed on prekeyQueue. static NSDate *lastPreKeyCheckTimestamp = nil; +// Maximum number of failures while updating signed prekeys +// before the message sending is disabled. +static const NSUInteger kMaxPrekeyUpdateFailureCount = 5; + +// Maximum amount of time that can elapse without updating signed prekeys +// before the message sending is disabled. +// +// Current value is 10 days (240 hours). +static const CGFloat kSignedPreKeyUpdateFailureMaxFailureDuration = 10 * 24 * 60 * 60; + +#pragma mark - + @implementation TSPreKeyManager ++ (BOOL)isAppLockedDueToPreKeyUpdateFailures +{ + // Only disable message sending if we have failed more than N times + // over a period of at least M days. + TSStorageManager *storageManager = [TSStorageManager sharedManager]; + return ([storageManager prekeyUpdateFailureCount] >= kMaxPrekeyUpdateFailureCount && + [storageManager firstPrekeyUpdateFailureDate] != nil + && fabs([[storageManager firstPrekeyUpdateFailureDate] timeIntervalSinceNow]) + >= kSignedPreKeyUpdateFailureMaxFailureDuration); +} + ++ (void)incrementPreKeyUpdateFailureCount +{ + // Record a prekey update failure. + TSStorageManager *storageManager = [TSStorageManager sharedManager]; + int failureCount = [storageManager incrementPrekeyUpdateFailureCount]; + if (failureCount == 1 || ![storageManager firstPrekeyUpdateFailureDate]) { + // If this is the "first" failure, record the timestamp of that + // failure. + [storageManager setFirstPrekeyUpdateFailureDate:[NSDate new]]; + } +} + ++ (void)clearPreKeyUpdateFailureCount +{ + TSStorageManager *storageManager = [TSStorageManager sharedManager]; + [storageManager clearFirstPrekeyUpdateFailureDate]; + [storageManager clearPrekeyUpdateFailureCount]; +} + // We should never dispatch sync to this queue. + (dispatch_queue_t)prekeyQueue { @@ -99,17 +142,32 @@ static NSDate *lastPreKeyCheckTimestamp = nil; [[TSNetworkManager sharedManager] makeRequest:request success:^(NSURLSessionDataTask *task, id responseObject) { - DDLogInfo(@"%@ Successfully registered %@.", self.tag, description); + OWSAnalyticsInfo(@"Prekey update success: %@", description); + if (modeCopy == RefreshPreKeysMode_SignedAndOneTime) { [storageManager storePreKeyRecords:preKeys]; } [storageManager storeSignedPreKey:signedPreKey.Id signedPreKeyRecord:signedPreKey]; successHandler(); + + [TSPreKeyManager clearPreKeyUpdateFailureCount]; } failure:^(NSURLSessionDataTask *task, NSError *error) { - DDLogError(@"%@ Failed to register %@.", self.tag, description); + OWSAnalyticsError(@"Prekey update failed (%@): %@", description, error); + failureHandler(error); + + NSInteger statusCode = 0; + if ([task.response isKindOfClass:[NSHTTPURLResponse class]]) { + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; + statusCode = httpResponse.statusCode; + } + if (statusCode >= 400 && statusCode <= 599) { + // Only treat 4xx and 5xx errors from the service as failures. + // Ignore network failures, for example. + [TSPreKeyManager incrementPreKeyUpdateFailureCount]; + } }]; }); } @@ -137,7 +195,7 @@ static NSDate *lastPreKeyCheckTimestamp = nil; [self clearSignedPreKeyRecords]; } failure:^(NSError *error) { - DDLogWarn(@"%@ Failed to update prekeys with the server", self.tag); + DDLogWarn(@"%@ Failed to update prekeys with the server: %@", self.tag, error); }]; }; @@ -158,7 +216,7 @@ static NSDate *lastPreKeyCheckTimestamp = nil; OWSAssert(currentRecord); BOOL shouldUpdateSignedPrekey - = fabs([currentRecord.generatedAt timeIntervalSinceNow]) >= kSignedPreKeysRotationTime; + = fabs([currentRecord.generatedAt timeIntervalSinceNow]) >= kSignedPreKeyRotationTime; if (shouldUpdateSignedPrekey) { DDLogInfo(@"%@ Updating signed prekey due to rotation period.", self.tag); updatePreKeys(RefreshPreKeysMode_SignedOnly); @@ -236,9 +294,8 @@ static NSDate *lastPreKeyCheckTimestamp = nil; break; } - DDLogInfo(@"%@ Deleting old signed prekey: %@", - self.tag, - [dateFormatter stringFromDate:deletionCandidate.generatedAt]); + OWSAnalyticsInfo( + @"Deleting old signed prekey: %@", [dateFormatter stringFromDate:deletionCandidate.generatedAt]); [storageManager removeSignedPreKey:deletionCandidate.Id]; deletedCount++; } diff --git a/src/Contacts/PhoneNumberUtil.m b/src/Contacts/PhoneNumberUtil.m index b9749cf99..af5fcc9a4 100644 --- a/src/Contacts/PhoneNumberUtil.m +++ b/src/Contacts/PhoneNumberUtil.m @@ -1,5 +1,8 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + #import "PhoneNumberUtil.h" -#import "Constraints.h" #import "ContactsManagerProtocol.h" #import "FunctionalUtil.h" #import "TextSecureKitEnv.h" diff --git a/src/Messages/OWSMessageSender.m b/src/Messages/OWSMessageSender.m index 70ac96b9c..4705c0960 100644 --- a/src/Messages/OWSMessageSender.m +++ b/src/Messages/OWSMessageSender.m @@ -23,6 +23,7 @@ #import "TSInvalidIdentityKeySendingErrorMessage.h" #import "TSNetworkManager.h" #import "TSOutgoingMessage.h" +#import "TSPreKeyManager.h" #import "TSStorageManager+IdentityKeyStore.h" #import "TSStorageManager+PreKeyStore.h" #import "TSStorageManager+SignedPreKeyStore.h" @@ -411,6 +412,26 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; { DDLogDebug(@"%@ sending message to service: %@", self.tag, message.debugDescription); + if ([TSPreKeyManager isAppLockedDueToPreKeyUpdateFailures]) { + OWSAnalyticsError(@"Message send failed due to prekey update failures"); + + // Retry prekey update every time user tries to send a message while app + // is disabled due to prekey update failures. + // + // Only try to update the signed prekey; updating it is sufficient to + // re-enable message sending. + [TSPreKeyManager registerPreKeysWithMode:RefreshPreKeysMode_SignedOnly + success:^{ + DDLogInfo(@"%@ New prekeys registered with server.", self.tag); + } + failure:^(NSError *error) { + DDLogWarn(@"%@ Failed to update prekeys with the server: %@", self.tag, error); + }]; + + DDLogError(@"%@ Message send failed due to repeated inability to update prekeys.", self.tag); + return failureHandler(OWSErrorMakeMessageSendDisabledDueToPreKeyUpdateFailuresError()); + } + if (remainingAttempts <= 0) { // We should always fail with a specific error. DDLogError(@"%@ Unexpected generic failure.", self.tag); diff --git a/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.h b/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.h index 5f03e6d2c..40dc72916 100644 --- a/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.h +++ b/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.h @@ -1,19 +1,22 @@ // -// TSStorageManager+SignedPreKeyStore.h -// TextSecureKit -// -// Created by Frederic Jacobs on 06/11/14. -// Copyright (c) 2014 Open Whisper Systems. All rights reserved. +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. // #import #import "TSStorageManager.h" - -#define TSStorageManagerSignedPreKeyStoreCollection @"TSStorageManagerSignedPreKeyStoreCollection" - @interface TSStorageManager (SignedPreKeyStore) - (SignedPreKeyRecord *)generateRandomSignedRecord; +#pragma mark - Prekey update failures + +- (int)prekeyUpdateFailureCount; +- (void)clearPrekeyUpdateFailureCount; +- (int)incrementPrekeyUpdateFailureCount; + +- (nullable NSDate *)firstPrekeyUpdateFailureDate; +- (void)setFirstPrekeyUpdateFailureDate:(nonnull NSDate *)value; +- (void)clearFirstPrekeyUpdateFailureDate; + @end diff --git a/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.m b/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.m index c2c156ea3..193aaf216 100644 --- a/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.m +++ b/src/Storage/AxolotlStore/TSStorageManager+SignedPreKeyStore.m @@ -11,6 +11,11 @@ #import #import +NSString *const TSStorageManagerSignedPreKeyStoreCollection = @"TSStorageManagerSignedPreKeyStoreCollection"; +NSString *const TSStorageManagerSignedPreKeyMetadataCollection = @"TSStorageManagerSignedPreKeyMetadataCollection"; +NSString *const TSStorageManagerKeyPrekeyUpdateFailureCount = @"prekeyUpdateFailureCount"; +NSString *const TSStorageManagerKeyFirstPrekeyUpdateFailureDate = @"firstPrekeyUpdateFailureDate"; + @implementation TSStorageManager (SignedPreKeyStore) - (SignedPreKeyRecord *)generateRandomSignedRecord { @@ -68,4 +73,45 @@ [self removeObjectForKey:[self keyFromInt:signedPrekeyId] inCollection:TSStorageManagerSignedPreKeyStoreCollection]; } +#pragma mark - Prekey update failures + +- (int)prekeyUpdateFailureCount; +{ + NSNumber *value = [TSStorageManager.sharedManager objectForKey:TSStorageManagerKeyPrekeyUpdateFailureCount + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; + // Will default to zero. + return [value intValue]; +} + +- (void)clearPrekeyUpdateFailureCount +{ + [TSStorageManager.sharedManager removeObjectForKey:TSStorageManagerKeyPrekeyUpdateFailureCount + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; +} + +- (int)incrementPrekeyUpdateFailureCount +{ + return [TSStorageManager.sharedManager incrementIntForKey:TSStorageManagerKeyPrekeyUpdateFailureCount + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; +} + +- (nullable NSDate *)firstPrekeyUpdateFailureDate +{ + return [TSStorageManager.sharedManager dateForKey:TSStorageManagerKeyFirstPrekeyUpdateFailureDate + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; +} + +- (void)setFirstPrekeyUpdateFailureDate:(nonnull NSDate *)value +{ + [TSStorageManager.sharedManager setDate:value + forKey:TSStorageManagerKeyFirstPrekeyUpdateFailureDate + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; +} + +- (void)clearFirstPrekeyUpdateFailureDate +{ + [TSStorageManager.sharedManager removeObjectForKey:TSStorageManagerKeyFirstPrekeyUpdateFailureDate + inCollection:TSStorageManagerSignedPreKeyMetadataCollection]; +} + @end diff --git a/src/Storage/TSStorageManager.h b/src/Storage/TSStorageManager.h index 46d0727fe..08f41e367 100644 --- a/src/Storage/TSStorageManager.h +++ b/src/Storage/TSStorageManager.h @@ -33,15 +33,16 @@ extern NSString *const TSUIDatabaseConnectionDidUpdateNotification; - (YapDatabase *)database; - (YapDatabaseConnection *)newDatabaseConnection; - - (void)setObject:(id)object forKey:(NSString *)key inCollection:(NSString *)collection; - (void)removeObjectForKey:(NSString *)string inCollection:(NSString *)collection; - - (BOOL)boolForKey:(NSString *)key inCollection:(NSString *)collection; - (int)intForKey:(NSString *)key inCollection:(NSString *)collection; - (void)setInt:(int)integer forKey:(NSString *)key inCollection:(NSString *)collection; - (id)objectForKey:(NSString *)key inCollection:(NSString *)collection; +- (int)incrementIntForKey:(NSString *)key inCollection:(NSString *)collection; +- (nullable NSDate *)dateForKey:(NSString *)key inCollection:(NSString *)collection; +- (void)setDate:(nonnull NSDate *)value forKey:(NSString *)key inCollection:(NSString *)collection; - (NSDictionary *)dictionaryForKey:(NSString *)key inCollection:(NSString *)collection; - (NSString *)stringForKey:(NSString *)key inCollection:(NSString *)collection; - (NSData *)dataForKey:(NSString *)key inCollection:(NSString *)collection; diff --git a/src/Storage/TSStorageManager.m b/src/Storage/TSStorageManager.m index 83303ec4b..b02951112 100644 --- a/src/Storage/TSStorageManager.m +++ b/src/Storage/TSStorageManager.m @@ -378,7 +378,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; - (void)removeObjectForKey:(NSString *)string inCollection:(NSString *)collection { [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [transaction removeObjectForKey:string inCollection:collection]; + [transaction removeObjectForKey:string inCollection:collection]; }]; } @@ -447,6 +447,32 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; [self setObject:[NSNumber numberWithInt:integer] forKey:key inCollection:collection]; } +- (int)incrementIntForKey:(NSString *)key inCollection:(NSString *)collection +{ + __block int value = 0; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + value = [[transaction objectForKey:key inCollection:collection] intValue]; + value++; + [transaction setObject:@(value) forKey:key inCollection:collection]; + }]; + return value; +} + +- (nullable NSDate *)dateForKey:(NSString *)key inCollection:(NSString *)collection +{ + NSNumber *value = [self objectForKey:key inCollection:collection]; + if (value) { + return [NSDate dateWithTimeIntervalSince1970:value.doubleValue]; + } else { + return nil; + } +} + +- (void)setDate:(nonnull NSDate *)value forKey:(NSString *)key inCollection:(NSString *)collection +{ + [self setObject:@(value.timeIntervalSince1970) forKey:key inCollection:collection]; +} + - (void)deleteThreadsAndMessages { [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [transaction removeAllObjectsInCollection:[TSThread collection]]; diff --git a/src/TSPrefix.h b/src/TSPrefix.h index 2bf20c5e9..b50c3222e 100644 --- a/src/TSPrefix.h +++ b/src/TSPrefix.h @@ -1,7 +1,13 @@ -#import -#import +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + #import "Asserts.h" +#import "Constraints.h" +#import "OWSAnalytics.h" #import "OWSDispatch.h" +#import +#import #ifdef DEBUG static const NSUInteger ddLogLevel = DDLogLevelAll; diff --git a/src/Util/OWSError.h b/src/Util/OWSError.h index c809828be..c3d400945 100644 --- a/src/Util/OWSError.h +++ b/src/Util/OWSError.h @@ -21,6 +21,7 @@ typedef NS_ENUM(NSInteger, OWSErrorCode) { OWSErrorCodeSignalServiceRateLimited = 1010, OWSErrorCodeUserError = 2001, OWSErrorCodeNoSuchSignalRecipient = 777404, + OWSErrorCodeMessageSendDisabledDueToPreKeyUpdateFailures = 777405, }; extern NSError *OWSErrorWithCodeDescription(OWSErrorCode code, NSString *description); @@ -28,5 +29,6 @@ extern NSError *OWSErrorMakeUnableToProcessServerResponseError(); extern NSError *OWSErrorMakeFailedToSendOutgoingMessageError(); extern NSError *OWSErrorMakeNoSuchSignalRecipientError(); extern NSError *OWSErrorMakeAssertionError(); +extern NSError *OWSErrorMakeMessageSendDisabledDueToPreKeyUpdateFailuresError(); NS_ASSUME_NONNULL_END diff --git a/src/Util/OWSError.m b/src/Util/OWSError.m index d721f9f63..e423070ee 100644 --- a/src/Util/OWSError.m +++ b/src/Util/OWSError.m @@ -40,4 +40,11 @@ NSError *OWSErrorMakeAssertionError() NSLocalizedString(@"ERROR_DESCRIPTION_UNKNOWN_ERROR", @"Worst case generic error message")); } +NSError *OWSErrorMakeMessageSendDisabledDueToPreKeyUpdateFailuresError() +{ + return OWSErrorWithCodeDescription(OWSErrorCodeMessageSendDisabledDueToPreKeyUpdateFailures, + NSLocalizedString(@"ERROR_DESCRIPTION_MESSAGE_SEND_DISABLED_PREKEY_UPDATE_FAILURES", + @"Error mesage indicating that message send is disabled due to prekey update failures")); +} + NS_ASSUME_NONNULL_END diff --git a/src/Util/constraints/BadState.m b/src/Util/constraints/BadState.m index 26a0e6451..a9ca6293e 100755 --- a/src/Util/constraints/BadState.m +++ b/src/Util/constraints/BadState.m @@ -1,4 +1,8 @@ -#import "Constraints.h" +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import "BadState.h" @implementation BadState +(void)raise:(NSString *)message { diff --git a/src/Util/constraints/OperationFailed.m b/src/Util/constraints/OperationFailed.m index b1b911f38..dc1f187e6 100755 --- a/src/Util/constraints/OperationFailed.m +++ b/src/Util/constraints/OperationFailed.m @@ -1,4 +1,8 @@ -#import "Constraints.h" +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import "OperationFailed.h" @implementation OperationFailed +(OperationFailed*) new:(NSString*)reason {