From 3edf3ed19986e7dc5d96454cae71020548473754 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 19 Nov 2018 15:57:25 -0500 Subject: [PATCH 1/3] Don't use UD for "self" profile fetches. --- SignalMessaging/profiles/ProfileFetcherJob.swift | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index 1261b3b0e..fae0485e5 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -62,6 +62,10 @@ public class ProfileFetcherJob: NSObject { return SignalServiceRestClient() } + private var tsAccountManager: TSAccountManager { + return SSKEnvironment.shared.tsAccountManager + } + // MARK: - public func run(recipientIds: [String]) { @@ -135,8 +139,13 @@ public class ProfileFetcherJob: NSObject { Logger.error("getProfile: \(recipientId)") - let udAccess = udManager.udAccess(forRecipientId: recipientId, - requireSyncAccess: false) + // Don't use UD for "self" profile fetches. + var udAccess: OWSUDAccess? + if recipientId != tsAccountManager.localNumber() { + udAccess = udManager.udAccess(forRecipientId: recipientId, + requireSyncAccess: false) + } + return requestProfile(recipientId: recipientId, udAccess: udAccess, canFailoverUDAuth: true) From 4ce0b68a8614fb7a7154cd42b0eccee6271caa78 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 19 Nov 2018 16:04:09 -0500 Subject: [PATCH 2/3] Discard sender certificates after 24 hours. --- .../src/Messages/UD/OWSUDManager.swift | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index fbe0b3aa5..4a821ff48 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -104,6 +104,8 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { private let kUDCollection = "kUDCollection" private let kUDCurrentSenderCertificateKey_Production = "kUDCurrentSenderCertificateKey_Production" private let kUDCurrentSenderCertificateKey_Staging = "kUDCurrentSenderCertificateKey_Staging" + private let kUDCurrentSenderCertificateDateKey_Production = "kUDCurrentSenderCertificateDateKey_Production" + private let kUDCurrentSenderCertificateDateKey_Staging = "kUDCurrentSenderCertificateDateKey_Staging" private let kUDUnrestrictedAccessKey = "kUDUnrestrictedAccessKey" // MARK: Recipient State @@ -134,6 +136,10 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { selector: #selector(registrationStateDidChange), name: .RegistrationStateDidChange, object: nil) + NotificationCenter.default.addObserver(self, + selector: #selector(didBecomeActive), + name: NSNotification.Name.OWSApplicationDidBecomeActive, + object: nil) } @objc @@ -144,6 +150,19 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { ensureSenderCertificate().retainUntilComplete() } + @objc func didBecomeActive() { + AssertIsOnMainThread() + + AppReadiness.runNowOrWhenAppDidBecomeReady { + guard TSAccountManager.isRegistered() else { + return + } + + // Any error is silently ignored on startup. + self.ensureSenderCertificate().retainUntilComplete() + } + } + // MARK: - @objc @@ -313,6 +332,14 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { #endif private func senderCertificate() -> SMKSenderCertificate? { + guard let certificateDate = dbConnection.object(forKey: senderCertificateDateKey(), inCollection: kUDCollection) as? Date else { + return nil + } + guard certificateDate.timeIntervalSinceNow < kDayInterval else { + // Discard certificates that we obtained more than 24 hours ago. + return nil + } + guard let certificateData = dbConnection.object(forKey: senderCertificateKey(), inCollection: kUDCollection) as? Data else { return nil } @@ -333,6 +360,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } func setSenderCertificate(_ certificateData: Data) { + dbConnection.setObject(Date(), forKey: senderCertificateDateKey(), inCollection: kUDCollection) dbConnection.setObject(certificateData, forKey: senderCertificateKey(), inCollection: kUDCollection) } @@ -340,6 +368,10 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { return IsUsingProductionService() ? kUDCurrentSenderCertificateKey_Production : kUDCurrentSenderCertificateKey_Staging } + private func senderCertificateDateKey() -> String { + return IsUsingProductionService() ? kUDCurrentSenderCertificateDateKey_Production : kUDCurrentSenderCertificateDateKey_Staging + } + @objc public func ensureSenderCertificate(success:@escaping (SMKSenderCertificate) -> Void, failure:@escaping (Error) -> Void) { From 4126b35a2782074a9bb7bbfa199f002e111db6b6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 26 Nov 2018 09:42:41 -0500 Subject: [PATCH 3/3] Respond to CR. --- .../src/Messages/UD/OWSUDManager.swift | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index 4a821ff48..0a01222a6 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -12,6 +12,15 @@ public enum OWSUDError: Error { case invalidData(description: String) } +@objc +public enum OWSUDCertificateExpirationPolicy: Int { + // We want to try to rotate the sender certificate + // on a frequent basis, but we don't want to block + // sending on this. + case strict + case permissive +} + @objc public enum UnidentifiedAccessMode: Int { case unknown @@ -130,7 +139,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } // Any error is silently ignored on startup. - self.ensureSenderCertificate().retainUntilComplete() + self.ensureSenderCertificate(certificateExpirationPolicy: .strict).retainUntilComplete() } NotificationCenter.default.addObserver(self, selector: #selector(registrationStateDidChange), @@ -147,7 +156,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { AssertIsOnMainThread() // Any error is silently ignored - ensureSenderCertificate().retainUntilComplete() + ensureSenderCertificate(certificateExpirationPolicy: .strict).retainUntilComplete() } @objc func didBecomeActive() { @@ -159,7 +168,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } // Any error is silently ignored on startup. - self.ensureSenderCertificate().retainUntilComplete() + self.ensureSenderCertificate(certificateExpirationPolicy: .strict).retainUntilComplete() } } @@ -327,17 +336,19 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { #if DEBUG @objc public func hasSenderCertificate() -> Bool { - return senderCertificate() != nil + return senderCertificate(certificateExpirationPolicy: .permissive) != nil } #endif - private func senderCertificate() -> SMKSenderCertificate? { - guard let certificateDate = dbConnection.object(forKey: senderCertificateDateKey(), inCollection: kUDCollection) as? Date else { - return nil - } - guard certificateDate.timeIntervalSinceNow < kDayInterval else { - // Discard certificates that we obtained more than 24 hours ago. - return nil + private func senderCertificate(certificateExpirationPolicy: OWSUDCertificateExpirationPolicy) -> SMKSenderCertificate? { + if certificateExpirationPolicy == .strict { + guard let certificateDate = dbConnection.object(forKey: senderCertificateDateKey(), inCollection: kUDCollection) as? Date else { + return nil + } + guard certificateDate.timeIntervalSinceNow < kDayInterval else { + // Discard certificates that we obtained more than 24 hours ago. + return nil + } } guard let certificateData = dbConnection.object(forKey: senderCertificateKey(), inCollection: kUDCollection) as? Data else { @@ -375,8 +386,16 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { @objc public func ensureSenderCertificate(success:@escaping (SMKSenderCertificate) -> Void, failure:@escaping (Error) -> Void) { + return ensureSenderCertificate(certificateExpirationPolicy: .permissive, + success: success, + failure: failure) + } + + private func ensureSenderCertificate(certificateExpirationPolicy: OWSUDCertificateExpirationPolicy, + success:@escaping (SMKSenderCertificate) -> Void, + failure:@escaping (Error) -> Void) { firstly { - ensureSenderCertificate() + ensureSenderCertificate(certificateExpirationPolicy: certificateExpirationPolicy) }.map { certificate in success(certificate) }.catch { error in @@ -384,9 +403,11 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { }.retainUntilComplete() } - public func ensureSenderCertificate() -> Promise { + public func ensureSenderCertificate(certificateExpirationPolicy: OWSUDCertificateExpirationPolicy) -> Promise { // If there is a valid cached sender certificate, use that. - if let certificate = senderCertificate() { + // + // NOTE: We use a "strict" expiration policy. + if let certificate = senderCertificate(certificateExpirationPolicy: certificateExpirationPolicy) { return Promise.value(certificate) }