diff --git a/Signal/src/ViewControllers/AppSettings/OWSLinkDeviceViewController.m b/Signal/src/ViewControllers/AppSettings/OWSLinkDeviceViewController.m index 79c01657b..273bbff63 100644 --- a/Signal/src/ViewControllers/AppSettings/OWSLinkDeviceViewController.m +++ b/Signal/src/ViewControllers/AppSettings/OWSLinkDeviceViewController.m @@ -193,9 +193,6 @@ NS_ASSUME_NONNULL_BEGIN // so all sync message sends will fail on the socket until it is cycled. [TSSocketManager.shared cycleSocket]; - [self.udManager setUnidentifiedAccessMode:UnidentifiedAccessModeUnknown - recipientId:self.tsAccountManager.localNumber]; - // Fetch the local profile to determine if all // linked devices support UD. [self.profileManager fetchLocalUsersProfile]; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 988b9db07..e846fff17 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -155,6 +155,13 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); return SSKEnvironment.shared.syncManager; } +- (id)udManager +{ + OWSAssertDebug(SSKEnvironment.shared.udManager); + + return SSKEnvironment.shared.udManager; +} + #pragma mark - User Profile Accessor - (void)ensureLocalProfileCached @@ -1006,7 +1013,9 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); [userProfile clearWithProfileKey:profileKey dbConnection:self.dbConnection completion:^{ - dispatch_async(dispatch_get_main_queue(), ^(void) { + dispatch_async(dispatch_get_main_queue(), ^{ + [self.udManager setUnidentifiedAccessMode:UnidentifiedAccessModeUnknown + recipientId:recipientId]; [self fetchProfileForRecipientId:recipientId]; }); }]; diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index d740e59cf..1261b3b0e 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -135,53 +135,15 @@ public class ProfileFetcherJob: NSObject { Logger.error("getProfile: \(recipientId)") - switch self.udManager.unidentifiedAccessMode(forRecipientId: recipientId) { - case .unknown: - if let udAccessKey = udManager.udAccessKey(forRecipientId: recipientId) { - // If we are in unknown mode and have a profile key, - // try using the profile key. - return self.requestProfile(recipientId: recipientId, - udAccessKey: udAccessKey, - canFailoverUDAuth: true) - } else { - // If we are in unknown mode and don't have a profile key, - // try using a random UD access key in case they support - // unrestricted access. - let randomUDAccessKey = self.udManager.randomUDAccessKey() - return requestProfile(recipientId: recipientId, - udAccessKey: randomUDAccessKey, - canFailoverUDAuth: true) - } - case .unrestricted: - let randomUDAccessKey = self.udManager.randomUDAccessKey() - return requestProfile(recipientId: recipientId, - udAccessKey: randomUDAccessKey, - canFailoverUDAuth: false) - .recover { (_: Error) -> Promise in - Logger.verbose("Failing over to non-random access.") - let udAccessKey = self.udManager.udAccessKey(forRecipientId: recipientId) - // This may fail over again to non-UD-auth. - return self.requestProfile(recipientId: recipientId, - udAccessKey: udAccessKey, - canFailoverUDAuth: true) - } - case .disabled: - // This may fail over to non-UD-auth. - return requestProfile(recipientId: recipientId, - udAccessKey: nil, - canFailoverUDAuth: true) - case .enabled: - // This may be nil if we don't have a profile key for them. - let udAccessKey = udManager.udAccessKey(forRecipientId: recipientId) - // This may fail over to non-UD-auth. - return requestProfile(recipientId: recipientId, - udAccessKey: udAccessKey, - canFailoverUDAuth: true) - } + let udAccess = udManager.udAccess(forRecipientId: recipientId, + requireSyncAccess: false) + return requestProfile(recipientId: recipientId, + udAccess: udAccess, + canFailoverUDAuth: true) } private func requestProfile(recipientId: String, - udAccessKey: SMKUDAccessKey?, + udAccess: OWSUDAccess?, canFailoverUDAuth: Bool) -> Promise { AssertIsOnMainThread() @@ -193,7 +155,7 @@ public class ProfileFetcherJob: NSObject { }, websocketFailureBlock: { // Do nothing }, recipientId: recipientId, - udAccessKey: udAccessKey, + udAccess: udAccess, canFailoverUDAuth: canFailoverUDAuth) return requestMaker.makeRequest() .map { (result: RequestMakerResult) -> SignalServiceProfile in @@ -233,13 +195,13 @@ public class ProfileFetcherJob: NSObject { } let dataToVerify = Data(count: 32) - guard let expectedVerfier = Cryptography.computeSHA256HMAC(dataToVerify, withHMACKey: udAccessKey.keyData) else { + guard let expectedVerifier = Cryptography.computeSHA256HMAC(dataToVerify, withHMACKey: udAccessKey.keyData) else { owsFailDebug("could not compute verification") udManager.setUnidentifiedAccessMode(.disabled, recipientId: recipientId) return } - guard expectedVerfier == verifier else { + guard expectedVerifier.ows_constantTimeIsEqual(to: verifier) else { Logger.verbose("verifier mismatch, new profile key?") udManager.setUnidentifiedAccessMode(.disabled, recipientId: recipientId) return diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.m b/SignalServiceKit/src/Contacts/SignalRecipient.m index 170a1c7ef..3ce3bebb3 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.m +++ b/SignalServiceKit/src/Contacts/SignalRecipient.m @@ -148,11 +148,7 @@ NS_ASSUME_NONNULL_BEGIN // Device changes dispatch_async(dispatch_get_main_queue(), ^{ // Device changes can affect the UD access mode for a recipient, - // so we need to: - // - // * Mark the UD access mode as "unknown". - // * Fetch the profile for this user to update UD access mode. - [self.udManager setUnidentifiedAccessMode:UnidentifiedAccessModeUnknown recipientId:self.recipientId]; + // so we need to fetch the profile for this user to update UD access mode. [self.profileManager fetchProfileForRecipientId:self.recipientId]; }); } diff --git a/SignalServiceKit/src/Devices/OWSDevice.m b/SignalServiceKit/src/Devices/OWSDevice.m index 8fec95a7f..4800f4da5 100644 --- a/SignalServiceKit/src/Devices/OWSDevice.m +++ b/SignalServiceKit/src/Devices/OWSDevice.m @@ -217,12 +217,7 @@ NSString *const kOWSPrimaryStorage_MayHaveLinkedDevices = @"kTSStorageManager_Ma if (didAddOrRemove) { dispatch_async(dispatch_get_main_queue(), ^{ // Device changes can affect the UD access mode for a recipient, - // so we need to: - // - // * Mark the UD access mode as "unknown". - // * Fetch the profile for this user to update UD access mode. - [self.udManager setUnidentifiedAccessMode:UnidentifiedAccessModeUnknown - recipientId:self.tsAccountManager.localNumber]; + // so we need to fetch the profile for this user to update UD access mode. [self.profileManager fetchLocalUsersProfile]; }); return YES; diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 06a2a5213..1247a4a20 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -156,6 +156,11 @@ NS_ASSUME_NONNULL_BEGIN return SSKEnvironment.shared.tsAccountManager; } +- (id)profileManager +{ + return SSKEnvironment.shared.profileManager; +} + #pragma mark - - (void)startObserving @@ -564,11 +569,6 @@ NS_ASSUME_NONNULL_BEGIN }]; } -- (id)profileManager -{ - return SSKEnvironment.shared.profileManager; -} - - (void)handleIncomingEnvelope:(SSKProtoEnvelope *)envelope withReceiptMessage:(SSKProtoReceiptMessage *)receiptMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -1453,6 +1453,8 @@ NS_ASSUME_NONNULL_BEGIN return; } + // Consult the device list cache we use for message sending + // whether or not we know about this linked device. SignalRecipient *_Nullable recipient = [SignalRecipient registeredRecipientForRecipientId:localNumber transaction:transaction]; if (!recipient) { @@ -1469,6 +1471,8 @@ NS_ASSUME_NONNULL_BEGIN } } + // Consult the device list cache we use for the "linked device" UI + // whether or not we know about this linked device. NSMutableSet *deviceIdSet = [NSMutableSet new]; for (OWSDevice *device in [OWSDevice currentDevicesWithTransaction:transaction]) { [deviceIdSet addObject:@(device.deviceId)]; @@ -1479,6 +1483,7 @@ NS_ASSUME_NONNULL_BEGIN (unsigned long) envelope.sourceDevice); [OWSDevicesService refreshDevices]; + [self.profileManager fetchLocalUsersProfile]; } } diff --git a/SignalServiceKit/src/Messages/OWSMessageSend.swift b/SignalServiceKit/src/Messages/OWSMessageSend.swift index 29b7f8b2c..366e5b99c 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSend.swift +++ b/SignalServiceKit/src/Messages/OWSMessageSend.swift @@ -32,7 +32,7 @@ public class OWSMessageSend: NSObject { public var hasWebsocketSendFailed = false @objc - public var udAccessKey: SMKUDAccessKey? + public var udAccess: OWSUDAccess? @objc public var senderCertificate: SMKSenderCertificate? @@ -54,7 +54,7 @@ public class OWSMessageSend: NSObject { thread: TSThread?, recipient: SignalRecipient, senderCertificate: SMKSenderCertificate?, - udAccessKey: SMKUDAccessKey?, + udAccess: OWSUDAccess?, localNumber: String, success: @escaping () -> Void, failure: @escaping (Error) -> Void) { @@ -63,7 +63,7 @@ public class OWSMessageSend: NSObject { self.recipient = recipient self.localNumber = localNumber self.senderCertificate = senderCertificate - self.udAccessKey = udAccessKey + self.udAccess = udAccess if let recipientId = recipient.uniqueId { self.isLocalNumber = localNumber == recipientId @@ -78,13 +78,13 @@ public class OWSMessageSend: NSObject { @objc public var isUDSend: Bool { - return udAccessKey != nil && senderCertificate != nil + return udAccess != nil && senderCertificate != nil } @objc public func disableUD() { Logger.verbose("\(recipient.recipientId)") - udAccessKey = nil + udAccess = nil } @objc diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 81b6953f3..c65d29bc0 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -579,7 +579,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; message:(TSOutgoingMessage *)message thread:(nullable TSThread *)thread senderCertificate:(nullable SMKSenderCertificate *)senderCertificate - selfUDAccessKey:(nullable SMKUDAccessKey *)selfUDAccessKey + selfUDAccess:(nullable OWSUDAccess *)selfUDAccess sendErrors:(NSMutableArray *)sendErrors { OWSAssertDebug(recipients.count > 0); @@ -591,16 +591,16 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; for (SignalRecipient *recipient in recipients) { // Use chained promises to make the code more readable. AnyPromise *sendPromise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { - SMKUDAccessKey *_Nullable theirUDAccessKey; - if (senderCertificate != nil && selfUDAccessKey != nil) { - theirUDAccessKey = [self.udManager udSendAccessKeyForRecipientId:recipient.recipientId]; + OWSUDAccess *_Nullable theirUDAccess; + if (senderCertificate != nil && selfUDAccess != nil) { + theirUDAccess = [self.udManager udAccessForRecipientId:recipient.recipientId requireSyncAccess:YES]; } OWSMessageSend *messageSend = [[OWSMessageSend alloc] initWithMessage:message thread:thread recipient:recipient senderCertificate:senderCertificate - udAccessKey:theirUDAccessKey + udAccess:theirUDAccess localNumber:self.tsAccountManager.localNumber success:^{ // The value doesn't matter, we just need any non-NSError value. @@ -631,16 +631,16 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; { AssertIsOnSendingQueue(); - SMKUDAccessKey *_Nullable selfUDAccessKey; + OWSUDAccess *_Nullable selfUDAccess; if (senderCertificate) { - selfUDAccessKey = [self.udManager udSendAccessKeyForRecipientId:self.tsAccountManager.localNumber]; + selfUDAccess = [self.udManager udAccessForRecipientId:self.tsAccountManager.localNumber requireSyncAccess:YES]; } void (^successHandler)(void) = ^() { dispatch_async([OWSDispatch sendingQueue], ^{ [self handleMessageSentLocally:message senderCertificate:senderCertificate - selfUDAccessKey:selfUDAccessKey + selfUDAccess:selfUDAccess success:^{ successHandlerParam(); } @@ -658,7 +658,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; dispatch_async([OWSDispatch sendingQueue], ^{ [self handleMessageSentLocally:message senderCertificate:senderCertificate - selfUDAccessKey:selfUDAccessKey + selfUDAccess:selfUDAccess success:^{ failureHandlerParam(error); } @@ -740,7 +740,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; message:message thread:thread senderCertificate:senderCertificate - selfUDAccessKey:selfUDAccessKey + selfUDAccess:selfUDAccess sendErrors:sendErrors] .then(^(id value) { successHandler(); @@ -1076,8 +1076,15 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; OWSLogWarn(@"Sending a message with no device messages."); } - // NOTE: canFailoverUDAuth is NO because UD-auth and Non-UD-auth requests - // use different device lists. + // NOTE: canFailoverUDAuth depends on whether or not we're sending a + // sync message because sync messages use different device lists + // for UD-auth and Non-UD-auth requests. + // + // Therefore, for sync messages, we can't use OWSRequestMaker's + // retry/failover logic; we need to use the message sender's retry + // logic that will build a new set of device messages. + BOOL isSyncMessageSend = messageSend.isLocalNumber; + BOOL canFailoverUDAuth = !isSyncMessageSend; OWSRequestMaker *requestMaker = [[OWSRequestMaker alloc] initWithLabel:@"Message Send" requestFactoryBlock:^(SMKUDAccessKey *_Nullable udAccessKey) { return [OWSRequestFactory submitMessageRequestWithRecipient:recipient.recipientId @@ -1086,14 +1093,18 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; udAccessKey:udAccessKey]; } udAuthFailureBlock:^{ + // Note the UD auth failure so subsequent retries + // to this recipient also use basic auth. [messageSend setHasUDAuthFailed]; } websocketFailureBlock:^{ + // Note the websocket failure so subsequent retries + // to this recipient also use REST. messageSend.hasWebsocketSendFailed = YES; } recipientId:recipient.recipientId - udAccessKey:messageSend.udAccessKey - canFailoverUDAuth:NO]; + udAccess:messageSend.udAccess + canFailoverUDAuth:canFailoverUDAuth]; [[requestMaker makeRequestObjc] .then(^(OWSRequestMakerResult *result) { dispatch_async([OWSDispatch sendingQueue], ^{ @@ -1341,7 +1352,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; - (void)handleMessageSentLocally:(TSOutgoingMessage *)message senderCertificate:(nullable SMKSenderCertificate *)senderCertificate - selfUDAccessKey:(nullable SMKUDAccessKey *)selfUDAccessKey + selfUDAccess:(nullable OWSUDAccess *)selfUDAccess success:(void (^)(void))success failure:(RetryableFailureHandler)failure { @@ -1358,7 +1369,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [self sendSyncTranscriptForMessage:message senderCertificate:senderCertificate - selfUDAccessKey:selfUDAccessKey + selfUDAccess:selfUDAccess success:^{ // TODO: We might send to a recipient, then to another recipient on retry. // To ensure desktop receives all "delivery status" info, we might @@ -1375,7 +1386,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; - (void)sendSyncTranscriptForMessage:(TSOutgoingMessage *)message senderCertificate:(nullable SMKSenderCertificate *)senderCertificate - selfUDAccessKey:(nullable SMKUDAccessKey *)selfUDAccessKey + selfUDAccess:(nullable OWSUDAccess *)selfUDAccess success:(void (^)(void))success failure:(RetryableFailureHandler)failure { @@ -1392,7 +1403,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; thread:message.thread recipient:recipient senderCertificate:senderCertificate - udAccessKey:selfUDAccessKey + udAccess:selfUDAccess localNumber:self.tsAccountManager.localNumber success:^{ OWSLogInfo(@"Successfully sent sync transcript."); @@ -1578,13 +1589,17 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; udAccessKey:udAccessKey]; } udAuthFailureBlock:^{ + // Note the UD auth failure so subsequent retries + // to this recipient also use basic auth. [messageSend setHasUDAuthFailed]; } websocketFailureBlock:^{ + // Note the websocket failure so subsequent retries + // to this recipient also use REST. messageSend.hasWebsocketSendFailed = YES; } recipientId:recipientId - udAccessKey:messageSend.udAccessKey + udAccess:messageSend.udAccess canFailoverUDAuth:YES]; [[requestMaker makeRequestObjc] .then(^(OWSRequestMakerResult *result) { diff --git a/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift b/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift index ad0478e2e..6a9b7b88b 100644 --- a/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift +++ b/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift @@ -47,7 +47,7 @@ public class RequestMaker: NSObject { private let udAuthFailureBlock: UDAuthFailureBlock private let websocketFailureBlock: WebsocketFailureBlock private let recipientId: String - private let udAccessKey: SMKUDAccessKey? + private let udAccess: OWSUDAccess? private let canFailoverUDAuth: Bool @objc @@ -56,14 +56,14 @@ public class RequestMaker: NSObject { udAuthFailureBlock : @escaping UDAuthFailureBlock, websocketFailureBlock : @escaping WebsocketFailureBlock, recipientId: String, - udAccessKey: SMKUDAccessKey?, + udAccess: OWSUDAccess?, canFailoverUDAuth: Bool) { self.label = label self.requestFactoryBlock = requestFactoryBlock self.udAuthFailureBlock = udAuthFailureBlock self.websocketFailureBlock = websocketFailureBlock self.recipientId = recipientId - self.udAccessKey = udAccessKey + self.udAccess = udAccess self.canFailoverUDAuth = canFailoverUDAuth } @@ -81,6 +81,10 @@ public class RequestMaker: NSObject { return SSKEnvironment.shared.udManager } + private var profileManager: ProfileManagerProtocol { + return SSKEnvironment.shared.profileManager + } + // MARK: - @objc @@ -104,38 +108,42 @@ public class RequestMaker: NSObject { } private func makeRequestInternal(skipUD: Bool, skipWebsocket: Bool) -> Promise { - var udAccessKeyForRequest: SMKUDAccessKey? + var udAccessForRequest: OWSUDAccess? if !skipUD { - udAccessKeyForRequest = udAccessKey + udAccessForRequest = udAccess } - let isUDSend = udAccessKeyForRequest != nil - let request = requestFactoryBlock(udAccessKeyForRequest) - let webSocketType: OWSWebSocketType = (isUDSend ? .UD : .default) + let isUDRequest: Bool = udAccessForRequest != nil + let request: TSRequest = requestFactoryBlock(udAccessForRequest?.udAccessKey) + let webSocketType: OWSWebSocketType = (isUDRequest ? .UD : .default) let canMakeWebsocketRequests = (socketManager.canMakeRequests(of: webSocketType) && !skipWebsocket) if canMakeWebsocketRequests { return Promise { resolver in socketManager.make(request, webSocketType: webSocketType, success: { (responseObject: Any?) in if self.udManager.isUDVerboseLoggingEnabled() { - if isUDSend { + if isUDRequest { Logger.debug("UD websocket request '\(self.label)' succeeded.") } else { Logger.debug("Non-UD websocket request '\(self.label)' succeeded.") } } - resolver.fulfill(RequestMakerResult(responseObject: responseObject, wasSentByUD: isUDSend)) + self.requestSucceeded(udAccess: udAccessForRequest) + + resolver.fulfill(RequestMakerResult(responseObject: responseObject, wasSentByUD: isUDRequest)) }) { (statusCode: Int, responseData: Data?, error: Error) in resolver.reject(RequestMakerError.websocketRequestError(statusCode: statusCode, responseData: responseData, underlyingError: error)) } }.recover { (error: Error) -> Promise in switch error { case RequestMakerError.websocketRequestError(let statusCode, _, _): - if isUDSend && (statusCode == 401 || statusCode == 403) { - // If a UD send fails due to service response (as opposed to network + if isUDRequest && (statusCode == 401 || statusCode == 403) { + // If a UD request fails due to service response (as opposed to network // failure), mark recipient as _not_ in UD mode, then retry. self.udManager.setUnidentifiedAccessMode(.disabled, recipientId: self.recipientId) + self.profileManager.fetchProfile(forRecipientId: self.recipientId) self.udAuthFailureBlock() + if self.canFailoverUDAuth { Logger.info("UD websocket request '\(self.label)' auth failed; failing over to non-UD websocket request.") return self.makeRequestInternal(skipUD: true, skipWebsocket: skipWebsocket) @@ -157,24 +165,28 @@ public class RequestMaker: NSObject { return self.networkManager.makePromise(request: request) .map { (networkManagerResult: TSNetworkManager.NetworkManagerResult) -> RequestMakerResult in if self.udManager.isUDVerboseLoggingEnabled() { - if isUDSend { + if isUDRequest { Logger.debug("UD REST request '\(self.label)' succeeded.") } else { Logger.debug("Non-UD REST request '\(self.label)' succeeded.") } } + self.requestSucceeded(udAccess: udAccessForRequest) + // Unwrap the network manager promise into a request maker promise. - return RequestMakerResult(responseObject: networkManagerResult.responseObject, wasSentByUD: isUDSend) + return RequestMakerResult(responseObject: networkManagerResult.responseObject, wasSentByUD: isUDRequest) }.recover { (error: Error) -> Promise in switch error { case NetworkManagerError.taskError(let task, _): let statusCode = task.statusCode() - if isUDSend && (statusCode == 401 || statusCode == 403) { - // If a UD send fails due to service response (as opposed to network + if isUDRequest && (statusCode == 401 || statusCode == 403) { + // If a UD request fails due to service response (as opposed to network // failure), mark recipient as _not_ in UD mode, then retry. self.udManager.setUnidentifiedAccessMode(.disabled, recipientId: self.recipientId) + self.profileManager.fetchProfile(forRecipientId: self.recipientId) self.udAuthFailureBlock() + if self.canFailoverUDAuth { Logger.info("UD REST request '\(self.label)' auth failed; failing over to non-UD REST request.") return self.makeRequestInternal(skipUD: true, skipWebsocket: skipWebsocket) @@ -193,4 +205,26 @@ public class RequestMaker: NSObject { } } } + + private func requestSucceeded(udAccess: OWSUDAccess?) { + // If this was a UD request... + guard let udAccess = udAccess else { + return + } + // ...made for a user in "unknown" UD access mode... + guard udAccess.udAccessMode == .unknown else { + return + } + + if udAccess.isRandomKey { + // If a UD request succeeds for an unknown user with a random key, + // mark recipient as .unrestricted. + udManager.setUnidentifiedAccessMode(.unrestricted, recipientId: recipientId) + } else { + // If a UD request succeeds for an unknown user with a non-random key, + // mark recipient as .enabled. + udManager.setUnidentifiedAccessMode(.enabled, recipientId: recipientId) + } + profileManager.fetchProfile(forRecipientId: recipientId) + } } diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index ce6686121..012c03eed 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -33,14 +33,33 @@ private func string(forUnidentifiedAccessMode mode: UnidentifiedAccessMode) -> S } } +@objc +public class OWSUDAccess: NSObject { + @objc + public let udAccessKey: SMKUDAccessKey + + @objc + public let udAccessMode: UnidentifiedAccessMode + + @objc + public let isRandomKey: Bool + + @objc + public required init(udAccessKey: SMKUDAccessKey, + udAccessMode: UnidentifiedAccessMode, + isRandomKey: Bool) { + self.udAccessKey = udAccessKey + self.udAccessMode = udAccessMode + self.isRandomKey = isRandomKey + } +} + @objc public protocol OWSUDManager: class { @objc func setup() @objc func trustRoot() -> ECPublicKey - @objc func isUDEnabled() -> Bool - @objc func isUDVerboseLoggingEnabled() -> Bool // MARK: - Recipient State @@ -48,9 +67,6 @@ private func string(forUnidentifiedAccessMode mode: UnidentifiedAccessMode) -> S @objc func setUnidentifiedAccessMode(_ mode: UnidentifiedAccessMode, recipientId: String) - @objc - func randomUDAccessKey() -> SMKUDAccessKey - @objc func unidentifiedAccessMode(forRecipientId recipientId: RecipientIdentifier) -> UnidentifiedAccessMode @@ -58,7 +74,8 @@ private func string(forUnidentifiedAccessMode mode: UnidentifiedAccessMode) -> S func udAccessKey(forRecipientId recipientId: RecipientIdentifier) -> SMKUDAccessKey? @objc - func udSendAccessKey(forRecipientId recipientId: RecipientIdentifier) -> SMKUDAccessKey? + func udAccess(forRecipientId recipientId: RecipientIdentifier, + requireSyncAccess: Bool) -> OWSUDAccess? // MARK: Sender Certificate @@ -129,17 +146,6 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { // MARK: - - @objc - public func isUDEnabled() -> Bool { - // Only enable UD if UD is supported by all linked devices, - // so that sync messages can also be sent via UD. - guard let localNumber = tsAccountManager.localNumber() else { - return false - } - let ourAccessMode = unidentifiedAccessMode(forRecipientId: localNumber) - return ourAccessMode == .enabled || ourAccessMode == .unrestricted - } - @objc public func isUDVerboseLoggingEnabled() -> Bool { return true @@ -163,35 +169,45 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } private func unidentifiedAccessMode(forRecipientId recipientId: RecipientIdentifier, + isLocalNumber: Bool, transaction: YapDatabaseReadTransaction) -> UnidentifiedAccessMode { + let defaultValue: UnidentifiedAccessMode = isLocalNumber ? .enabled : .unknown guard let existingRawValue = transaction.object(forKey: recipientId, inCollection: kUnidentifiedAccessCollection) as? Int else { - return .unknown + return defaultValue } guard let existingValue = UnidentifiedAccessMode(rawValue: existingRawValue) else { - return .unknown + owsFailDebug("Couldn't parse mode value.") + return defaultValue } return existingValue } @objc public func unidentifiedAccessMode(forRecipientId recipientId: RecipientIdentifier) -> UnidentifiedAccessMode { + var isLocalNumber = false + if let localNumber = tsAccountManager.localNumber() { + isLocalNumber = recipientId == localNumber + } + var mode: UnidentifiedAccessMode = .unknown dbConnection.read { (transaction) in - mode = self.unidentifiedAccessMode(forRecipientId: recipientId, transaction: transaction) + mode = self.unidentifiedAccessMode(forRecipientId: recipientId, isLocalNumber: isLocalNumber, transaction: transaction) } return mode } @objc public func setUnidentifiedAccessMode(_ mode: UnidentifiedAccessMode, recipientId: String) { + var isLocalNumber = false if let localNumber = tsAccountManager.localNumber() { if recipientId == localNumber { Logger.info("Setting local UD access mode: \(string(forUnidentifiedAccessMode: mode))") + isLocalNumber = true } } dbConnection.readWrite { (transaction) in - let oldMode = self.unidentifiedAccessMode(forRecipientId: recipientId, transaction: transaction) + let oldMode = self.unidentifiedAccessMode(forRecipientId: recipientId, isLocalNumber: isLocalNumber, transaction: transaction) transaction.setObject(mode.rawValue as Int, forKey: recipientId, inCollection: self.kUnidentifiedAccessCollection) @@ -220,46 +236,69 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { // Returns the UD access key for sending to a given recipient. @objc - public func udSendAccessKey(forRecipientId recipientId: RecipientIdentifier) -> SMKUDAccessKey? { - // This check is currently redundant with the "send access key for local number" - // check below, but behavior of isUDEnabled() may change. - guard isUDEnabled() else { - if isUDVerboseLoggingEnabled() { - Logger.info("UD Send disabled for \(recipientId), UD disabled.") + public func udAccess(forRecipientId recipientId: RecipientIdentifier, + requireSyncAccess: Bool) -> OWSUDAccess? { + if requireSyncAccess { + guard let localNumber = tsAccountManager.localNumber() else { + if isUDVerboseLoggingEnabled() { + Logger.info("UD disabled for \(recipientId), no local number.") + } + owsFailDebug("Missing local number.") + return nil + } + if localNumber != recipientId { + let selfAccessMode = unidentifiedAccessMode(forRecipientId: localNumber) + guard selfAccessMode != .disabled else { + if isUDVerboseLoggingEnabled() { + Logger.info("UD disabled for \(recipientId), UD disabled for sync messages.") + } + return nil + } } - return nil } - guard let localNumber = tsAccountManager.localNumber() else { + + let accessMode = unidentifiedAccessMode(forRecipientId: recipientId) + switch accessMode { + case .unrestricted: + // Unrestricted users should use a random key. if isUDVerboseLoggingEnabled() { - Logger.info("UD Send disabled for \(recipientId), no local number.") + Logger.info("UD enabled for \(recipientId) with random key.") } - return nil - } - if localNumber != recipientId { - guard udSendAccessKey(forRecipientId: localNumber) != nil else { + let udAccessKey = randomUDAccessKey() + return OWSUDAccess(udAccessKey: udAccessKey, udAccessMode: accessMode, isRandomKey: true) + case .unknown: + // Unknown users should use a derived key if possible, + // and otherwise use a random key. + if let udAccessKey = udAccessKey(forRecipientId: recipientId) { + if isUDVerboseLoggingEnabled() { + Logger.info("UD unknown for \(recipientId); trying derived key.") + } + return OWSUDAccess(udAccessKey: udAccessKey, udAccessMode: accessMode, isRandomKey: false) + } else { + if isUDVerboseLoggingEnabled() { + Logger.info("UD unknown for \(recipientId); trying random key.") + } + let udAccessKey = randomUDAccessKey() + return OWSUDAccess(udAccessKey: udAccessKey, udAccessMode: accessMode, isRandomKey: true) + } + case .enabled: + guard let udAccessKey = udAccessKey(forRecipientId: recipientId) else { if isUDVerboseLoggingEnabled() { - Logger.info("UD Send disabled for \(recipientId), UD disabled for sync messages.") + Logger.info("UD disabled for \(recipientId), no profile key for this recipient.") } + owsFailDebug("Couldn't find profile key for UD-enabled user.") return nil } - } - let accessMode = unidentifiedAccessMode(forRecipientId: recipientId) - if accessMode == .unrestricted { if isUDVerboseLoggingEnabled() { - Logger.info("UD Send enabled for \(recipientId) with random key.") + Logger.info("UD enabled for \(recipientId).") } - return randomUDAccessKey() - } - guard accessMode == .enabled else { + return OWSUDAccess(udAccessKey: udAccessKey, udAccessMode: accessMode, isRandomKey: false) + case .disabled: if isUDVerboseLoggingEnabled() { - Logger.info("UD Send disabled for \(recipientId), UD not enabled for this recipient.") + Logger.info("UD disabled for \(recipientId), UD not enabled for this recipient.") } return nil } - if isUDVerboseLoggingEnabled() { - Logger.info("UD Send enabled for \(recipientId).") - } - return udAccessKey(forRecipientId: recipientId) } // MARK: - Sender Certificate diff --git a/SignalServiceKit/src/Network/WebSockets/OWSWebSocket.m b/SignalServiceKit/src/Network/WebSockets/OWSWebSocket.m index ad7757151..b2f514781 100644 --- a/SignalServiceKit/src/Network/WebSockets/OWSWebSocket.m +++ b/SignalServiceKit/src/Network/WebSockets/OWSWebSocket.m @@ -1045,11 +1045,6 @@ NSString *NSStringFromOWSWebSocketType(OWSWebSocketType type) } #endif - if (!self.udManager.isUDEnabled && self.webSocketType == OWSWebSocketTypeUD) { - OWSLogWarn(@"Suppressing UD socket in prod."); - return; - } - if (!AppReadiness.isAppReady) { static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ diff --git a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift index c6cb8fc11..bc302d941 100644 --- a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift +++ b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift @@ -77,24 +77,24 @@ class OWSUDManagerTest: SSKBaseTestSwift { let aliceRecipientId = "+13213214321" - XCTAssert(UnidentifiedAccessMode.unknown == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: aliceRecipientId)) + XCTAssert(UnidentifiedAccessMode.enabled == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: aliceRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.unknown, recipientId: aliceRecipientId) XCTAssert(UnidentifiedAccessMode.unknown == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: aliceRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: aliceRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.disabled, recipientId: aliceRecipientId) XCTAssert(UnidentifiedAccessMode.disabled == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: aliceRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: aliceRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.enabled, recipientId: aliceRecipientId) XCTAssert(UnidentifiedAccessMode.enabled == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) - XCTAssertNotNil(udManager.udSendAccessKey(forRecipientId: aliceRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: aliceRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.unrestricted, recipientId: aliceRecipientId) XCTAssert(UnidentifiedAccessMode.unrestricted == udManager.unidentifiedAccessMode(forRecipientId: aliceRecipientId)) - XCTAssertNotNil(udManager.udSendAccessKey(forRecipientId: aliceRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: aliceRecipientId, requireSyncAccess: false)) } func testMode_noProfileKey() { @@ -111,24 +111,24 @@ class OWSUDManagerTest: SSKBaseTestSwift { XCTAssertNotEqual(bobRecipientId, tsAccountManager.localNumber()!) XCTAssertEqual(UnidentifiedAccessMode.unknown, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.unknown, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.unknown, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.disabled, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.disabled, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.enabled, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.enabled, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) // Bob should work in unrestricted mode, even if he doesn't have a profile key. udManager.setUnidentifiedAccessMode(.unrestricted, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.unrestricted, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNotNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) } func testMode_withProfileKey() { @@ -145,22 +145,22 @@ class OWSUDManagerTest: SSKBaseTestSwift { profileManager.setProfileKeyData(OWSAES256Key.generateRandom().keyData, forRecipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.unknown, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.unknown, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.unknown, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.disabled, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.disabled, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.enabled, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.enabled, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNotNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) udManager.setUnidentifiedAccessMode(.unrestricted, recipientId: bobRecipientId) XCTAssertEqual(UnidentifiedAccessMode.unrestricted, udManager.unidentifiedAccessMode(forRecipientId: bobRecipientId)) - XCTAssertNotNil(udManager.udSendAccessKey(forRecipientId: bobRecipientId)) + XCTAssertNotNil(udManager.udAccess(forRecipientId: bobRecipientId, requireSyncAccess: false)) } }