From c907721a184ee68f3a79fc6622e5d32670dcbff6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Oct 2018 08:55:20 -0400 Subject: [PATCH 1/5] Rotate profile key if blocklist intersects profile whitelist. --- Signal.xcodeproj/project.pbxproj | 1 - .../ViewControllers/DebugUI/DebugUIMessages.m | 12 +- .../ViewControllers/DebugUI/DebugUIStress.m | 2 +- .../ViewControllers/NewGroupViewController.m | 2 +- Signal/test/util/SearcherTest.swift | 4 +- SignalMessaging/environment/AppSetup.m | 6 +- .../OWS111UDAttributesMigration.swift | 6 - SignalMessaging/profiles/OWSProfileManager.h | 4 +- SignalMessaging/profiles/OWSProfileManager.m | 204 ++++++++++++++++-- SignalMessaging/profiles/OWSUserProfile.h | 4 + SignalServiceKit/src/Messages/TSGroupModel.h | 2 + SignalServiceKit/src/Messages/TSGroupModel.m | 2 + 12 files changed, 212 insertions(+), 37 deletions(-) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 483e2523f1..1edd42b87f 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -1627,7 +1627,6 @@ 4598198D204E2F28009414F2 /* OWS108CallLoggingPreference.m */, 34D5872E208E2C4100D2255A /* OWS109OutgoingMessageState.h */, 34D5872D208E2C4100D2255A /* OWS109OutgoingMessageState.m */, - 4C858A552130CBEC001B45D3 /* OWS110SortIdMigration.swift */, 349EA07B2162AEA700F7B17F /* OWS111UDAttributesMigration.swift */, 346129931FD1E30000532771 /* OWSDatabaseMigration.h */, 346129941FD1E30000532771 /* OWSDatabaseMigration.m */, diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 938b2c36f8..d269fd0434 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -239,9 +239,9 @@ NS_ASSUME_NONNULL_BEGIN itemWithTitle:@"Request Bogus group info" actionBlock:^{ OWSLogInfo(@"Requesting bogus group info for thread: %@", thread); - OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = - [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread - groupId:[Randomness generateRandomBytes:16]]; + OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = [[OWSSyncGroupsRequestMessage alloc] + initWithThread:thread + groupId:[Randomness generateRandomBytes:kGroupIdLength]]; [SSKEnvironment.shared.messageSender enqueueMessage:syncGroupsRequestMessage success:^{ OWSLogWarn(@"Successfully sent Request Group Info message."); @@ -3816,7 +3816,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac recipientId, [TSAccountManager localNumber], ] mutableCopy]; - NSData *groupId = [Randomness generateRandomBytes:16]; + NSData *groupId = [Randomness generateRandomBytes:kGroupIdLength]; TSGroupModel *groupModel = [[TSGroupModel alloc] initWithTitle:groupName memberIds:recipientIds image:nil groupId:groupId]; @@ -4309,7 +4309,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac recipientId, [TSAccountManager localNumber], ] mutableCopy]; - NSData *groupId = [Randomness generateRandomBytes:16]; + NSData *groupId = [Randomness generateRandomBytes:kGroupIdLength]; TSGroupModel *groupModel = [[TSGroupModel alloc] initWithTitle:groupName memberIds:recipientIds image:nil groupId:groupId]; @@ -4348,7 +4348,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac recipientId, [TSAccountManager localNumber], ] mutableCopy]; - NSData *groupId = [Randomness generateRandomBytes:16]; + NSData *groupId = [Randomness generateRandomBytes:kGroupIdLength]; TSGroupModel *groupModel = [[TSGroupModel alloc] initWithTitle:groupName memberIds:recipientIds image:nil groupId:groupId]; diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m index 49d4700a52..c47cd87a29 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m @@ -506,7 +506,7 @@ NS_ASSUME_NONNULL_BEGIN [[TSGroupModel alloc] initWithTitle:[groupThread.groupModel.groupName stringByAppendingString:@" Copy"] memberIds:groupThread.groupModel.groupMemberIds image:groupThread.groupModel.groupImage - groupId:[Randomness generateRandomBytes:16]]; + groupId:[Randomness generateRandomBytes:kGroupIdLength]]; thread = [TSGroupThread getOrCreateThreadWithGroupModel:groupModel transaction:transaction]; }]; OWSAssertDebug(thread); diff --git a/Signal/src/ViewControllers/NewGroupViewController.m b/Signal/src/ViewControllers/NewGroupViewController.m index 30f3e91844..33bda1c696 100644 --- a/Signal/src/ViewControllers/NewGroupViewController.m +++ b/Signal/src/ViewControllers/NewGroupViewController.m @@ -86,7 +86,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)commonInit { - _groupId = [Randomness generateRandomBytes:16]; + _groupId = [Randomness generateRandomBytes:kGroupIdLength]; _messageSender = SSKEnvironment.shared.messageSender; _contactsViewHelper = [[ContactsViewHelper alloc] initWithDelegate:self]; diff --git a/Signal/test/util/SearcherTest.swift b/Signal/test/util/SearcherTest.swift index 5c2334246b..c2bcf8914d 100644 --- a/Signal/test/util/SearcherTest.swift +++ b/Signal/test/util/SearcherTest.swift @@ -80,11 +80,11 @@ class ConversationSearcherTest: SignalBaseTest { SSKEnvironment.shared.contactsManager = ConversationSearcherContactsManager() self.dbConnection.readWrite { transaction in - let bookModel = TSGroupModel(title: "Book Club", memberIds: [aliceRecipientId, bobRecipientId], image: nil, groupId: Randomness.generateRandomBytes(16)) + let bookModel = TSGroupModel(title: "Book Club", memberIds: [aliceRecipientId, bobRecipientId], image: nil, groupId: Randomness.generateRandomBytes(kGroupIdLength)) let bookClubGroupThread = TSGroupThread.getOrCreateThread(with: bookModel, transaction: transaction) self.bookClubThread = ThreadViewModel(thread: bookClubGroupThread, transaction: transaction) - let snackModel = TSGroupModel(title: "Snack Club", memberIds: [aliceRecipientId], image: nil, groupId: Randomness.generateRandomBytes(16)) + let snackModel = TSGroupModel(title: "Snack Club", memberIds: [aliceRecipientId], image: nil, groupId: Randomness.generateRandomBytes(kGroupIdLength)) let snackClubGroupThread = TSGroupThread.getOrCreateThread(with: snackModel, transaction: transaction) self.snackClubThread = ThreadViewModel(thread: snackClubGroupThread, transaction: transaction) diff --git a/SignalMessaging/environment/AppSetup.m b/SignalMessaging/environment/AppSetup.m index 225be50947..23a04b611e 100644 --- a/SignalMessaging/environment/AppSetup.m +++ b/SignalMessaging/environment/AppSetup.m @@ -53,11 +53,7 @@ NS_ASSUME_NONNULL_BEGIN OWSContactsManager *contactsManager = [[OWSContactsManager alloc] initWithPrimaryStorage:primaryStorage]; ContactsUpdater *contactsUpdater = [ContactsUpdater new]; OWSMessageSender *messageSender = [[OWSMessageSender alloc] initWithPrimaryStorage:primaryStorage]; - - OWSProfileManager *profileManager = [[OWSProfileManager alloc] initWithPrimaryStorage:primaryStorage - messageSender:messageSender - networkManager:networkManager]; - + OWSProfileManager *profileManager = [[OWSProfileManager alloc] initWithPrimaryStorage:primaryStorage]; OWSMessageManager *messageManager = [[OWSMessageManager alloc] initWithPrimaryStorage:primaryStorage]; OWSBlockingManager *blockingManager = [[OWSBlockingManager alloc] initWithPrimaryStorage:primaryStorage]; OWSIdentityManager *identityManager = [[OWSIdentityManager alloc] initWithPrimaryStorage:primaryStorage]; diff --git a/SignalMessaging/environment/migrations/OWS111UDAttributesMigration.swift b/SignalMessaging/environment/migrations/OWS111UDAttributesMigration.swift index 98f35617a5..3c5180db7d 100644 --- a/SignalMessaging/environment/migrations/OWS111UDAttributesMigration.swift +++ b/SignalMessaging/environment/migrations/OWS111UDAttributesMigration.swift @@ -8,12 +8,6 @@ import SignalServiceKit @objc public class OWS111UDAttributesMigration: OWSDatabaseMigration { - // MARK: - Singletons - - private var networkManager: TSNetworkManager { - return SSKEnvironment.shared.networkManager - } - // MARK: - // increment a similar constant for each migration. diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index 9cfa1ddd7b..533b36b380 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -22,9 +22,7 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; - (instancetype)init NS_UNAVAILABLE; -- (instancetype)initWithPrimaryStorage:(OWSPrimaryStorage *)primaryStorage - messageSender:(OWSMessageSender *)messageSender - networkManager:(TSNetworkManager *)networkManager; +- (instancetype)initWithPrimaryStorage:(OWSPrimaryStorage *)primaryStorage; + (instancetype)sharedManager; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 64470bc6b3..2ab8adb804 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -11,9 +11,11 @@ #import #import #import +#import #import #import #import +#import #import #import #import @@ -27,6 +29,7 @@ #import #import #import +#import NS_ASSUME_NONNULL_BEGIN @@ -42,10 +45,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; @interface OWSProfileManager () -@property (nonatomic, readonly) OWSMessageSender *messageSender; @property (nonatomic, readonly) YapDatabaseConnection *dbConnection; -@property (nonatomic, readonly) TSNetworkManager *networkManager; -@property (nonatomic, readonly) OWSIdentityManager *identityManager; // This property can be accessed on any thread, while synchronized on self. @property (atomic, readonly) OWSUserProfile *localUserProfile; @@ -72,8 +72,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } - (instancetype)initWithPrimaryStorage:(OWSPrimaryStorage *)primaryStorage - messageSender:(OWSMessageSender *)messageSender - networkManager:(TSNetworkManager *)networkManager { self = [super init]; @@ -83,18 +81,18 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssertIsOnMainThread(); OWSAssertDebug(primaryStorage); - OWSAssertDebug(messageSender); - OWSAssertDebug(messageSender); - _messageSender = messageSender; _dbConnection = primaryStorage.newDatabaseConnection; - _networkManager = networkManager; _profileAvatarImageCache = [NSCache new]; _currentAvatarDownloads = [NSMutableSet new]; OWSSingletonAssert(); + [AppReadiness runNowOrWhenAppIsReady:^{ + [self rotateLocalProfileKeyIfNecessarySync]; + }]; + return self; } @@ -109,6 +107,10 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; selector:@selector(applicationDidBecomeActive:) name:OWSApplicationDidBecomeActiveNotification object:nil]; + [[NSNotificationCenter defaultCenter] addObserver:self + selector:@selector(blockListDidChange:) + name:kNSNotificationName_BlockListDidChange + object:nil]; } #pragma mark - Dependencies @@ -124,7 +126,21 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (OWSIdentityManager *)identityManager { - return [OWSIdentityManager sharedManager]; + return SSKEnvironment.shared.identityManager; +} + +- (OWSMessageSender *)messageSender { + OWSAssertDebug(SSKEnvironment.shared.messageSender); + + return SSKEnvironment.shared.messageSender; +} + +- (TSNetworkManager *)networkManager { + return SSKEnvironment.shared.networkManager; +} + +- (OWSBlockingManager *)blockingManager { + return SSKEnvironment.shared.blockingManager; } #pragma mark - User Profile Accessor @@ -504,6 +520,154 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; [ProfileFetcherJob runWithRecipientId:localNumber ignoreThrottling:YES]; } +#pragma mark - Profile Key Rotation + +- (nullable NSString *)groupKeyForGroupId:(NSData *)groupId { + NSString *groupIdKey = [groupId hexadecimalString]; + return groupIdKey; +} + +- (nullable NSData *)groupIdForGroupKey:(NSString *)groupKey { + NSMutableData *groupId = [NSMutableData new]; + + for (NSUInteger i = 0; i + 2 <= groupKey.length; i += 2) { + NSString *_Nullable byteString = [groupKey substringWithRange:NSMakeRange(i, 2)]; + if (!byteString) { + OWSFailDebug(@"Couldn't slice group key."); + return nil; + } + unsigned byteValue; + if (![[NSScanner scannerWithString:byteString] scanHexInt:&byteValue]) { + OWSFailDebug(@"Couldn't parse hex byte: %@.", byteString); + return nil; + } + if (byteValue > 0xff) { + OWSFailDebug(@"Invalid hex byte: %@ (%d).", byteString, byteValue); + return nil; + } + uint8_t byte = (uint8_t)(0xff & byteValue); + [groupId appendBytes:&byte length:1]; + } + if (groupId.length != kGroupIdLength) { + OWSFailDebug(@"Parsed group id has unexpected length: %@ (%lu)", + groupId.hexadecimalString, + (unsigned long)groupId.length); + return nil; + } + return [groupId copy]; +} + +- (void)rotateLocalProfileKeyIfNecessarySync { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + [self rotateLocalProfileKeyIfNecessarySync]; + }); +} + +- (void)rotateLocalProfileKeyIfNecessaryAsync { + OWSAssertDebug(AppReadiness.isAppReady); + + NSMutableSet *whitelistedRecipientIds = [NSMutableSet new]; + NSMutableSet *whitelistedGroupIds = [NSMutableSet new]; + [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + [whitelistedRecipientIds + addObjectsFromArray:[transaction allKeysInCollection:kOWSProfileManager_UserWhitelistCollection]]; + + NSArray *whitelistedGroupKeys = + [transaction allKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]; + for (NSString *groupKey in whitelistedGroupKeys) { + NSData *_Nullable groupId = [self groupIdForGroupKey:groupKey]; + if (!groupId) { + continue; + } + + [whitelistedGroupIds addObject:groupId]; + + TSGroupThread *_Nullable thread = [TSGroupThread threadWithGroupId:groupId transaction:transaction]; + if (!thread) { + OWSLogInfo(@"Could not find whitelisted thread: %@", groupId.hexadecimalString); + continue; + } + + [whitelistedRecipientIds addObjectsFromArray:thread.recipientIdentifiers]; + } + }]; + + [whitelistedRecipientIds removeObject:[TSAccountManager localNumber]]; + + NSSet *blockedRecipientIds = [NSSet setWithArray:self.blockingManager.blockedPhoneNumbers]; + NSSet *blockedGroupIds = [NSSet setWithArray:self.blockingManager.blockedGroupIds]; + + // Find the users and groups which are both a) blocked b) may have our current profile key. + NSMutableSet *intersectingRecipientIds = [blockedRecipientIds mutableCopy]; + [intersectingRecipientIds intersectSet:whitelistedRecipientIds]; + NSMutableSet *intersectingGroupIds = [blockedGroupIds mutableCopy]; + [intersectingGroupIds intersectSet:whitelistedGroupIds]; + + BOOL isProfileKeySharedWithBlocked = (intersectingRecipientIds.count > 0 || intersectingGroupIds.count > 0); + if (!isProfileKeySharedWithBlocked) { + // No need to rotate the profile key. + return; + } + + // Rotate the profile key + + // Make copies of the current local profile state. + OWSUserProfile *localUserProfile = self.localUserProfile; + NSString *_Nullable oldProfileName = localUserProfile.profileName; + NSString *_Nullable oldAvatarFileName = localUserProfile.avatarFileName; + NSData *_Nullable oldAvatarData = [self profileAvatarDataForRecipientId:self.tsAccountManager.localNumber]; + + // Rotate the stored profile key. + // + // This will always succeed. + [self.localUserProfile + updateWithProfileKey:[OWSAES256Key generateRandomKey] + dbConnection:self.dbConnection + completion:^{ + // Remove blocked users and groups from profile whitelist. + // + // This will always succeed. + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [transaction removeObjectsForKeys:intersectingRecipientIds.allObjects + inCollection:kOWSProfileManager_UserWhitelistCollection]; + for (NSData *groupId in intersectingGroupIds) { + NSString *groupIdKey = [self groupKeyForGroupId:groupId]; + [transaction removeObjectForKey:groupIdKey + inCollection:kOWSProfileManager_GroupWhitelistCollection]; + } + }]; + + // Try to re-upload our profile name and avatar, if any. + // + // This may fail. + if (oldProfileName.length > 0) { + [self updateServiceWithProfileName:oldProfileName + success:^{ + OWSLogInfo(@"Update to profile name after profile key rotation succeeded."); + } + failure:^{ + OWSLogInfo(@"Update to profile name after profile key rotation failed."); + }]; + } + + if (oldAvatarData.length > 0 && oldAvatarFileName.length > 0) { + [self uploadAvatarToService:oldAvatarData + success:^(NSString *_Nullable avatarUrlPath) { + OWSLogInfo(@"Update to profile avatar after profile key rotation succeeded."); + // We need to update the local profile with the avatar state since + // it is cleared during the "avatar update" process. + [self.localUserProfile updateWithAvatarUrlPath:avatarUrlPath + avatarFileName:oldAvatarFileName + dbConnection:self.dbConnection + completion:nil]; + } + failure:^{ + OWSLogInfo(@"Update to profile avatar after profile key rotation failed."); + }]; + } + }]; +} + #pragma mark - Profile Whitelist - (void)clearProfileWhitelist @@ -581,6 +745,10 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssertDebug(recipientId.length > 0); + if ([self.blockingManager isRecipientIdBlocked:recipientId]) { + return NO; + } + __block BOOL result = NO; [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { NSNumber *_Nullable oldValue = @@ -594,7 +762,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssertDebug(groupId.length > 0); - NSString *groupIdKey = [groupId hexadecimalString]; + NSString *groupIdKey = [self groupKeyForGroupId:groupId]; __block BOOL didChange = NO; [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { @@ -645,7 +813,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssertDebug(groupId.length > 0); - NSString *groupIdKey = [groupId hexadecimalString]; + if ([self.blockingManager isGroupIdBlocked:groupId]) { + return NO; + } + + NSString *groupIdKey = [self groupKeyForGroupId:groupId]; __block BOOL result = NO; [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { @@ -1173,6 +1345,14 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // TODO: Sync if necessary. } +- (void)blockListDidChange:(NSNotification *)notification { + OWSAssertIsOnMainThread(); + + [AppReadiness runNowOrWhenAppIsReady:^{ + [self rotateLocalProfileKeyIfNecessarySync]; + }]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h index 4453a5dd0a..e0e298180a 100644 --- a/SignalMessaging/profiles/OWSUserProfile.h +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -61,6 +61,10 @@ extern NSString *const kLocalProfileUniqueId; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; +- (void)updateWithProfileKey:(OWSAES256Key *)profileKey + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion; + - (void)clearWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; diff --git a/SignalServiceKit/src/Messages/TSGroupModel.h b/SignalServiceKit/src/Messages/TSGroupModel.h index f9f7c48fb3..2872096878 100644 --- a/SignalServiceKit/src/Messages/TSGroupModel.h +++ b/SignalServiceKit/src/Messages/TSGroupModel.h @@ -7,6 +7,8 @@ NS_ASSUME_NONNULL_BEGIN +extern const NSUInteger kGroupIdLength; + @interface TSGroupModel : TSYapDatabaseObject @property (nonatomic) NSArray *groupMemberIds; diff --git a/SignalServiceKit/src/Messages/TSGroupModel.m b/SignalServiceKit/src/Messages/TSGroupModel.m index 1665c8133e..377bd23f16 100644 --- a/SignalServiceKit/src/Messages/TSGroupModel.m +++ b/SignalServiceKit/src/Messages/TSGroupModel.m @@ -8,6 +8,8 @@ NS_ASSUME_NONNULL_BEGIN +const NSUInteger kGroupIdLength = 16; + @interface TSGroupModel () @property (nullable, nonatomic) NSString *groupName; From 0ce2e4d4d9cc05b01df7932c49b7621125124641 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Oct 2018 08:59:47 -0400 Subject: [PATCH 2/5] Rotate profile key if blocklist intersects profile whitelist. --- SignalMessaging/profiles/OWSProfileManager.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 2ab8adb804..8a137d1066 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -530,6 +530,10 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (nullable NSData *)groupIdForGroupKey:(NSString *)groupKey { NSMutableData *groupId = [NSMutableData new]; + if (groupKey.length % 2 != 0) { + OWSFailDebug(@"Group key has unexpected length: %@ (%lu)", groupKey, (unsigned long)groupKey.length); + return nil; + } for (NSUInteger i = 0; i + 2 <= groupKey.length; i += 2) { NSString *_Nullable byteString = [groupKey substringWithRange:NSMakeRange(i, 2)]; if (!byteString) { From f7827cda7b5e7e03490f356ccfa0221dd337ad7e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Oct 2018 15:38:29 -0400 Subject: [PATCH 3/5] Respond to CR. --- SignalMessaging/profiles/OWSProfileManager.m | 240 +++++++++++-------- SignalServiceKit/src/Util/OWSError.h | 1 + 2 files changed, 147 insertions(+), 94 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 8a137d1066..8c1e21f91c 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -5,6 +5,7 @@ #import "OWSProfileManager.h" #import "Environment.h" #import "OWSUserProfile.h" +#import #import #import #import @@ -90,7 +91,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSSingletonAssert(); [AppReadiness runNowOrWhenAppIsReady:^{ - [self rotateLocalProfileKeyIfNecessarySync]; + [self rotateLocalProfileKeyIfNecessary]; }]; return self; @@ -561,115 +562,166 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; return [groupId copy]; } -- (void)rotateLocalProfileKeyIfNecessarySync { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - [self rotateLocalProfileKeyIfNecessarySync]; - }); +- (void)rotateLocalProfileKeyIfNecessary { + [self + rotateLocalProfileKeyIfNecessaryWithSuccess:^{ + } + failure:^ { + }]; } -- (void)rotateLocalProfileKeyIfNecessaryAsync { +- (void)rotateLocalProfileKeyIfNecessaryWithSuccess:(dispatch_block_t)success failure:(dispatch_block_t)failure { OWSAssertDebug(AppReadiness.isAppReady); - NSMutableSet *whitelistedRecipientIds = [NSMutableSet new]; - NSMutableSet *whitelistedGroupIds = [NSMutableSet new]; - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - [whitelistedRecipientIds - addObjectsFromArray:[transaction allKeysInCollection:kOWSProfileManager_UserWhitelistCollection]]; + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + NSMutableSet *whitelistedRecipientIds = [NSMutableSet new]; + NSMutableSet *whitelistedGroupIds = [NSMutableSet new]; + [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + [whitelistedRecipientIds + addObjectsFromArray:[transaction allKeysInCollection:kOWSProfileManager_UserWhitelistCollection]]; - NSArray *whitelistedGroupKeys = - [transaction allKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]; - for (NSString *groupKey in whitelistedGroupKeys) { - NSData *_Nullable groupId = [self groupIdForGroupKey:groupKey]; - if (!groupId) { - continue; + NSArray *whitelistedGroupKeys = + [transaction allKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]; + for (NSString *groupKey in whitelistedGroupKeys) { + NSData *_Nullable groupId = [self groupIdForGroupKey:groupKey]; + if (!groupId) { + OWSFailDebug(@"Couldn't parse group key: %@.", groupKey); + continue; + } + + [whitelistedGroupIds addObject:groupId]; + + TSGroupThread *_Nullable thread = [TSGroupThread threadWithGroupId:groupId transaction:transaction]; + if (!thread) { + OWSLogInfo(@"Could not find whitelisted thread: %@", groupId.hexadecimalString); + continue; + } + + [whitelistedRecipientIds addObjectsFromArray:thread.recipientIdentifiers]; } + }]; - [whitelistedGroupIds addObject:groupId]; - - TSGroupThread *_Nullable thread = [TSGroupThread threadWithGroupId:groupId transaction:transaction]; - if (!thread) { - OWSLogInfo(@"Could not find whitelisted thread: %@", groupId.hexadecimalString); - continue; - } - - [whitelistedRecipientIds addObjectsFromArray:thread.recipientIdentifiers]; + NSString *_Nullable localNumber = [TSAccountManager localNumber]; + if (localNumber) { + [whitelistedRecipientIds removeObject:localNumber]; + } else { + OWSFailDebug(@"Missing localNumber"); } - }]; - [whitelistedRecipientIds removeObject:[TSAccountManager localNumber]]; + NSSet *blockedRecipientIds = [NSSet setWithArray:self.blockingManager.blockedPhoneNumbers]; + NSSet *blockedGroupIds = [NSSet setWithArray:self.blockingManager.blockedGroupIds]; - NSSet *blockedRecipientIds = [NSSet setWithArray:self.blockingManager.blockedPhoneNumbers]; - NSSet *blockedGroupIds = [NSSet setWithArray:self.blockingManager.blockedGroupIds]; + // Find the users and groups which are both a) blocked b) may have our current profile key. + NSMutableSet *intersectingRecipientIds = [blockedRecipientIds mutableCopy]; + [intersectingRecipientIds intersectSet:whitelistedRecipientIds]; + NSMutableSet *intersectingGroupIds = [blockedGroupIds mutableCopy]; + [intersectingGroupIds intersectSet:whitelistedGroupIds]; - // Find the users and groups which are both a) blocked b) may have our current profile key. - NSMutableSet *intersectingRecipientIds = [blockedRecipientIds mutableCopy]; - [intersectingRecipientIds intersectSet:whitelistedRecipientIds]; - NSMutableSet *intersectingGroupIds = [blockedGroupIds mutableCopy]; - [intersectingGroupIds intersectSet:whitelistedGroupIds]; + BOOL isProfileKeySharedWithBlocked = (intersectingRecipientIds.count > 0 || intersectingGroupIds.count > 0); + if (!isProfileKeySharedWithBlocked) { + // No need to rotate the profile key. + return; + } - BOOL isProfileKeySharedWithBlocked = (intersectingRecipientIds.count > 0 || intersectingGroupIds.count > 0); - if (!isProfileKeySharedWithBlocked) { - // No need to rotate the profile key. - return; - } + // Rotate the profile key - // Rotate the profile key + // Make copies of the current local profile state. + OWSUserProfile *localUserProfile = self.localUserProfile; + NSString *_Nullable oldProfileName = localUserProfile.profileName; + NSString *_Nullable oldAvatarFileName = localUserProfile.avatarFileName; + NSData *_Nullable oldAvatarData = [self profileAvatarDataForRecipientId:self.tsAccountManager.localNumber]; - // Make copies of the current local profile state. - OWSUserProfile *localUserProfile = self.localUserProfile; - NSString *_Nullable oldProfileName = localUserProfile.profileName; - NSString *_Nullable oldAvatarFileName = localUserProfile.avatarFileName; - NSData *_Nullable oldAvatarData = [self profileAvatarDataForRecipientId:self.tsAccountManager.localNumber]; + // Rotate the stored profile key. + AnyPromise *promise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { + [self.localUserProfile updateWithProfileKey:[OWSAES256Key generateRandomKey] + dbConnection:self.dbConnection + completion:^{ + // The value doesn't matter, we just need any non-NSError value. + resolve(@(1)); + }]; + }]; - // Rotate the stored profile key. - // - // This will always succeed. - [self.localUserProfile - updateWithProfileKey:[OWSAES256Key generateRandomKey] - dbConnection:self.dbConnection - completion:^{ - // Remove blocked users and groups from profile whitelist. - // - // This will always succeed. - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [transaction removeObjectsForKeys:intersectingRecipientIds.allObjects - inCollection:kOWSProfileManager_UserWhitelistCollection]; - for (NSData *groupId in intersectingGroupIds) { - NSString *groupIdKey = [self groupKeyForGroupId:groupId]; - [transaction removeObjectForKey:groupIdKey - inCollection:kOWSProfileManager_GroupWhitelistCollection]; - } - }]; + // Try to re-upload our profile name, if any. + // + // This may fail. + promise = promise.then(^(id value) { + if (oldProfileName.length < 1) { + return [AnyPromise promiseWithValue:@(1)]; + } + return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { + [self updateServiceWithProfileName:oldProfileName + success:^{ + OWSLogInfo(@"Update to profile name succeeded."); - // Try to re-upload our profile name and avatar, if any. - // - // This may fail. - if (oldProfileName.length > 0) { - [self updateServiceWithProfileName:oldProfileName - success:^{ - OWSLogInfo(@"Update to profile name after profile key rotation succeeded."); - } - failure:^{ - OWSLogInfo(@"Update to profile name after profile key rotation failed."); - }]; - } + // The value doesn't matter, we just need any non-NSError value. + resolve(@(1)); + } + failure:^{ + NSError *error = OWSErrorWithCodeDescription( + OWSErrorCodeProfileUpdateFailed, @"Update to profile name failed."); + resolve(error); + }]; + }]; + }); - if (oldAvatarData.length > 0 && oldAvatarFileName.length > 0) { - [self uploadAvatarToService:oldAvatarData - success:^(NSString *_Nullable avatarUrlPath) { - OWSLogInfo(@"Update to profile avatar after profile key rotation succeeded."); - // We need to update the local profile with the avatar state since - // it is cleared during the "avatar update" process. - [self.localUserProfile updateWithAvatarUrlPath:avatarUrlPath - avatarFileName:oldAvatarFileName - dbConnection:self.dbConnection - completion:nil]; - } - failure:^{ - OWSLogInfo(@"Update to profile avatar after profile key rotation failed."); - }]; - } - }]; + // Try to re-upload our profile avatar, if any. + // + // This may fail. + promise = promise.then(^(id value) { + if (oldAvatarData.length < 1 || oldAvatarFileName.length < 1) { + return [AnyPromise promiseWithValue:@(1)]; + } + return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { + [self uploadAvatarToService:oldAvatarData + success:^(NSString *_Nullable avatarUrlPath) { + OWSLogInfo(@"Update to profile avatar after profile key rotation succeeded."); + // We need to update the local profile with the avatar state since + // it is cleared during the "avatar update" process. + [self.localUserProfile updateWithAvatarUrlPath:avatarUrlPath + avatarFileName:oldAvatarFileName + dbConnection:self.dbConnection + completion:^{ + // The value doesn't matter, we just need any + // non-NSError value. + resolve(@(1)); + }]; + } + failure:^{ + OWSLogInfo(@"Update to profile avatar after profile key rotation failed."); + NSError *error = OWSErrorWithCodeDescription( + OWSErrorCodeProfileUpdateFailed, @"Update to profile avatar failed."); + resolve(error); + }]; + }]; + }); + + // Try to re-upload our profile avatar, if any. + // + // This may fail. + promise = promise.then(^(id value) { + // Remove blocked users and groups from profile whitelist. + // + // This will always succeed. + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [transaction removeObjectsForKeys:intersectingRecipientIds.allObjects + inCollection:kOWSProfileManager_UserWhitelistCollection]; + for (NSData *groupId in intersectingGroupIds) { + NSString *groupIdKey = [self groupKeyForGroupId:groupId]; + [transaction removeObjectForKey:groupIdKey + inCollection:kOWSProfileManager_GroupWhitelistCollection]; + } + }]; + return @(1); + }); + + promise = promise.then(^(id value) { + success(); + }); + promise = promise.catch(^(id error) { + failure(); + }); + [promise retainUntilComplete]; + }); } #pragma mark - Profile Whitelist @@ -1353,7 +1405,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssertIsOnMainThread(); [AppReadiness runNowOrWhenAppIsReady:^{ - [self rotateLocalProfileKeyIfNecessarySync]; + [self rotateLocalProfileKeyIfNecessary]; }]; } diff --git a/SignalServiceKit/src/Util/OWSError.h b/SignalServiceKit/src/Util/OWSError.h index 5f1f76a2fe..831f2060ba 100644 --- a/SignalServiceKit/src/Util/OWSError.h +++ b/SignalServiceKit/src/Util/OWSError.h @@ -48,6 +48,7 @@ typedef NS_ENUM(NSInteger, OWSErrorCode) { OWSErrorCodeMessageRequestFailed = 777421, OWSErrorCodeMessageResponseFailed = 777422, OWSErrorCodeInvalidMessage = 777423, + OWSErrorCodeProfileUpdateFailed = 777424, }; extern NSString *const OWSErrorRecipientIdentifierKey; From f00f60883313b3f54732e48caece9eb3a6524cd9 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 11 Oct 2018 09:25:21 -0400 Subject: [PATCH 4/5] Respond to CR. --- SignalMessaging/profiles/OWSProfileManager.m | 87 ++++++++++---------- SignalServiceKit/src/Util/OWSError.h | 2 + 2 files changed, 46 insertions(+), 43 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 8c1e21f91c..6751f3ec2b 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -44,6 +44,8 @@ NSString *const kOWSProfileManager_GroupWhitelistCollection = @"kOWSProfileManag const NSUInteger kOWSProfileManager_NameDataLength = 26; const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; +typedef void (^ProfileManagerFailureBlock)(NSError *error); + @interface OWSProfileManager () @property (nonatomic, readonly) YapDatabaseConnection *dbConnection; @@ -244,7 +246,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; successBlock(); }]; } - failure:^{ + failure:^(NSError *error) { failureBlock(); }]; }; @@ -275,11 +277,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; success:^(NSString *_Nullable avatarUrlPath) { tryToUpdateService(avatarUrlPath, fileName); } - failure:^{ + failure:^(NSError *error) { failureBlock(); }]; } - failure:^{ + failure:^(NSError *error) { failureBlock(); }]; } @@ -289,7 +291,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; success:^(NSString *_Nullable avatarUrlPath) { tryToUpdateService(nil, nil); } - failure:^{ + failure:^(NSError *error) { failureBlock(); }]; } else { @@ -300,8 +302,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (void)writeAvatarToDisk:(UIImage *)avatar success:(void (^)(NSData *data, NSString *fileName))successBlock - failure:(void (^)(void))failureBlock -{ + failure:(ProfileManagerFailureBlock)failureBlock { OWSAssertDebug(avatar); OWSAssertDebug(successBlock); OWSAssertDebug(failureBlock); @@ -316,12 +317,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; BOOL success = [data writeToFile:filePath atomically:YES]; OWSAssertDebug(success); if (success) { - successBlock(data, fileName); - return; + return successBlock(data, fileName); } } } - failureBlock(); + failureBlock(OWSErrorWithCodeDescription(OWSErrorCodeAvatarWriteFailed, @"Avatar write failed.")); }); } @@ -351,8 +351,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // If avatarData is nil, we are clearing the avatar. - (void)uploadAvatarToService:(NSData *_Nullable)avatarData success:(void (^)(NSString *_Nullable avatarUrlPath))successBlock - failure:(void (^)(void))failureBlock -{ + failure:(ProfileManagerFailureBlock)failureBlock { OWSAssertDebug(successBlock); OWSAssertDebug(failureBlock); OWSAssertDebug(avatarData == nil || avatarData.length > 0); @@ -388,8 +387,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; if (![formResponseObject isKindOfClass:[NSDictionary class]]) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidResponse]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSDictionary *responseMap = formResponseObject; OWSLogError(@"responseObject: %@", formResponseObject); @@ -397,44 +396,44 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; NSString *formAcl = responseMap[@"acl"]; if (![formAcl isKindOfClass:[NSString class]] || formAcl.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidAcl]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formKey = responseMap[@"key"]; if (![formKey isKindOfClass:[NSString class]] || formKey.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidKey]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formPolicy = responseMap[@"policy"]; if (![formPolicy isKindOfClass:[NSString class]] || formPolicy.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidPolicy]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formAlgorithm = responseMap[@"algorithm"]; if (![formAlgorithm isKindOfClass:[NSString class]] || formAlgorithm.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidAlgorithm]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formCredential = responseMap[@"credential"]; if (![formCredential isKindOfClass:[NSString class]] || formCredential.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidCredential]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formDate = responseMap[@"date"]; if (![formDate isKindOfClass:[NSString class]] || formDate.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidDate]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } NSString *formSignature = responseMap[@"signature"]; if (![formSignature isKindOfClass:[NSString class]] || formSignature.length < 1) { OWSProdFail([OWSAnalyticsEvents profileManagerErrorAvatarUploadFormInvalidSignature]); - failureBlock(); - return; + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); } [self.avatarHTTPManager POST:@"" @@ -472,7 +471,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } failure:^(NSURLSessionDataTask *_Nullable uploadTask, NSError *_Nonnull error) { OWSLogError(@"uploading avatar failed with error: %@", error); - failureBlock(); + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); }]; } failure:^(NSURLSessionDataTask *task, NSError *error) { @@ -483,15 +483,15 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } OWSLogError(@"Failed to get profile avatar upload form: %@", error); - failureBlock(); + return failureBlock( + OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); }]; }); } - (void)updateServiceWithProfileName:(nullable NSString *)localProfileName success:(void (^)(void))successBlock - failure:(void (^)(void))failureBlock -{ + failure:(ProfileManagerFailureBlock)failureBlock { OWSAssertDebug(successBlock); OWSAssertDebug(failureBlock); @@ -505,7 +505,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } failure:^(NSURLSessionDataTask *task, NSError *error) { OWSLogError(@"Failed to update profile with error: %@", error); - failureBlock(); + failureBlock(error); }]; }); } @@ -566,11 +566,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; [self rotateLocalProfileKeyIfNecessaryWithSuccess:^{ } - failure:^ { + failure:^(NSError *error) { }]; } -- (void)rotateLocalProfileKeyIfNecessaryWithSuccess:(dispatch_block_t)success failure:(dispatch_block_t)failure { +- (void)rotateLocalProfileKeyIfNecessaryWithSuccess:(dispatch_block_t)success + failure:(ProfileManagerFailureBlock)failure { OWSAssertDebug(AppReadiness.isAppReady); dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ @@ -620,7 +621,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; BOOL isProfileKeySharedWithBlocked = (intersectingRecipientIds.count > 0 || intersectingGroupIds.count > 0); if (!isProfileKeySharedWithBlocked) { // No need to rotate the profile key. - return; + return success(); } // Rotate the profile key @@ -656,9 +657,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // The value doesn't matter, we just need any non-NSError value. resolve(@(1)); } - failure:^{ - NSError *error = OWSErrorWithCodeDescription( - OWSErrorCodeProfileUpdateFailed, @"Update to profile name failed."); + failure:^(NSError *error) { resolve(error); }]; }]; @@ -686,10 +685,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; resolve(@(1)); }]; } - failure:^{ + failure:^(NSError *error) { OWSLogInfo(@"Update to profile avatar after profile key rotation failed."); - NSError *error = OWSErrorWithCodeDescription( - OWSErrorCodeProfileUpdateFailed, @"Update to profile avatar failed."); resolve(error); }]; }]; @@ -717,8 +714,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; promise = promise.then(^(id value) { success(); }); - promise = promise.catch(^(id error) { - failure(); + promise = promise.catch(^(NSError *error) { + if ([error isKindOfClass:[NSError class]]) { + failure(error); + } else { + failure(OWSErrorWithCodeDescription(OWSErrorCodeProfileUpdateFailed, @"Profile key rotation failed.")); + } }); [promise retainUntilComplete]; }); diff --git a/SignalServiceKit/src/Util/OWSError.h b/SignalServiceKit/src/Util/OWSError.h index 831f2060ba..238ab6b6d1 100644 --- a/SignalServiceKit/src/Util/OWSError.h +++ b/SignalServiceKit/src/Util/OWSError.h @@ -49,6 +49,8 @@ typedef NS_ENUM(NSInteger, OWSErrorCode) { OWSErrorCodeMessageResponseFailed = 777422, OWSErrorCodeInvalidMessage = 777423, OWSErrorCodeProfileUpdateFailed = 777424, + OWSErrorCodeAvatarWriteFailed = 777425, + OWSErrorCodeAvatarUploadFailed = 777426, }; extern NSString *const OWSErrorRecipientIdentifierKey; From 794914353d24c2a32703f8a5c7da24fcc73ce149 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 11 Oct 2018 13:07:25 -0400 Subject: [PATCH 5/5] Respond to CR. --- SignalMessaging/profiles/OWSProfileManager.m | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 6751f3ec2b..9143ac88e1 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -469,10 +469,9 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); OWSLogInfo(@"successfully uploaded avatar with key: %@", formKey); successBlock(formKey); } - failure:^(NSURLSessionDataTask *_Nullable uploadTask, NSError *_Nonnull error) { + failure:^(NSURLSessionDataTask *_Nullable uploadTask, NSError *error) { OWSLogError(@"uploading avatar failed with error: %@", error); - return failureBlock( - OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); + return failureBlock(error); }]; } failure:^(NSURLSessionDataTask *task, NSError *error) { @@ -483,8 +482,7 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); } OWSLogError(@"Failed to get profile avatar upload form: %@", error); - return failureBlock( - OWSErrorWithCodeDescription(OWSErrorCodeAvatarUploadFailed, @"Avatar upload failed.")); + return failureBlock(error); }]; }); } @@ -718,7 +716,7 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); if ([error isKindOfClass:[NSError class]]) { failure(error); } else { - failure(OWSErrorWithCodeDescription(OWSErrorCodeProfileUpdateFailed, @"Profile key rotation failed.")); + failure(OWSErrorMakeAssertionError(@"Profile key rotation failure missing error.")); } }); [promise retainUntilComplete];