From 7cb01583313ab43abcca9b8072c9614e7ca8c9f8 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 5 Oct 2018 11:31:54 -0400 Subject: [PATCH 1/3] Apply UD access verifier. --- .../profiles/ProfileFetcherJob.swift | 33 ++++++++++++------- .../src/Messages/OWSMessageSend.swift | 4 ++- .../src/Messages/UD/OWSUDManager.swift | 7 ++-- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index f5c30ded9..c693610c7 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -173,18 +173,29 @@ public class ProfileFetcherJob: NSObject { } private func updateProfile(signalServiceProfile: SignalServiceProfile) { - verifyIdentityUpToDateAsync(recipientId: signalServiceProfile.recipientId, latestIdentityKey: signalServiceProfile.identityKey) - - profileManager.updateProfile(forRecipientId: signalServiceProfile.recipientId, - profileNameEncrypted: signalServiceProfile.profileNameEncrypted, - avatarUrlPath: signalServiceProfile.avatarUrlPath) + let recipientId = signalServiceProfile.recipientId + verifyIdentityUpToDateAsync(recipientId: recipientId, latestIdentityKey: signalServiceProfile.identityKey) + + profileManager.updateProfile(forRecipientId: recipientId, + profileNameEncrypted: signalServiceProfile.profileNameEncrypted, + avatarUrlPath: signalServiceProfile.avatarUrlPath) + + var supportsUnidentifiedDelivery = false + if let unidentifiedAccessVerifier = signalServiceProfile.unidentifiedAccessVerifier, + let udAccessKey = udManager.udAccessKeyForRecipient(recipientId) { + let dataToVerify = Data(count: 32) + if let expectedVerfier = Cryptography.computeSHA256HMAC(dataToVerify, withHMACKey: udAccessKey.keyData) { + supportsUnidentifiedDelivery = expectedVerfier == unidentifiedAccessVerifier + } else { + owsFailDebug("could not verify UD") + } + } // TODO: We may want to only call setSupportsUnidentifiedDelivery if // supportsUnidentifiedDelivery is true. - let supportsUnidentifiedDelivery = signalServiceProfile.unidentifiedAccessKey != nil - udManager.setSupportsUnidentifiedDelivery(supportsUnidentifiedDelivery, recipientId: signalServiceProfile.recipientId) + udManager.setSupportsUnidentifiedDelivery(supportsUnidentifiedDelivery, recipientId: recipientId) - udManager.setShouldAllowUnrestrictedAccess(recipientId: signalServiceProfile.recipientId, shouldAllowUnrestrictedAccess: signalServiceProfile.hasUnrestrictedUnidentifiedAccess) + udManager.setShouldAllowUnrestrictedAccess(recipientId: recipientId, shouldAllowUnrestrictedAccess: signalServiceProfile.hasUnrestrictedUnidentifiedAccess) } private func verifyIdentityUpToDateAsync(recipientId: String, latestIdentityKey: Data) { @@ -212,7 +223,7 @@ public class SignalServiceProfile: NSObject { public let identityKey: Data public let profileNameEncrypted: Data? public let avatarUrlPath: String? - public let unidentifiedAccessKey: Data? + public let unidentifiedAccessVerifier: Data? public let hasUnrestrictedUnidentifiedAccess: Bool init(recipientId: String, responseObject: Any?) throws { @@ -235,9 +246,7 @@ public class SignalServiceProfile: NSObject { let avatarUrlPath: String? = try params.optional(key: "avatar") self.avatarUrlPath = avatarUrlPath - // TODO: Should this key be "unidentifiedAccessKey" or "unidentifiedAccess"? - // The docs don't agree with the response from staging. - self.unidentifiedAccessKey = try params.optionalBase64EncodedData(key: "unidentifiedAccess") + self.unidentifiedAccessVerifier = try params.optionalBase64EncodedData(key: "unidentifiedAccess") self.hasUnrestrictedUnidentifiedAccess = try params.optional(key: "unrestrictedUnidentifiedAccess") ?? false } diff --git a/SignalServiceKit/src/Messages/OWSMessageSend.swift b/SignalServiceKit/src/Messages/OWSMessageSend.swift index 3e8af2d3d..7918ff904 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSend.swift +++ b/SignalServiceKit/src/Messages/OWSMessageSend.swift @@ -70,7 +70,9 @@ public class OWSMessageSend: NSObject { var udAccessKey: SMKUDAccessKey? var isLocalNumber: Bool = false if let recipientId = recipient.uniqueId { - udAccessKey = udManager.udAccessKeyForRecipient(recipientId) + udAccessKey = (udManager.supportsUnidentifiedDelivery(recipientId: recipientId) + ? udManager.udAccessKeyForRecipient(recipientId) + : nil) isLocalNumber = localNumber == recipientId } else { owsFailDebug("SignalRecipient missing recipientId") diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index 4efad614a..83841d72d 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -110,13 +110,10 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } } - // Returns the UD access key for a given recipient if they are - // a UD recipient and we have a valid profile key for them. + // Returns the UD access key for a given recipient + // if we have a valid profile key for them. @objc public func udAccessKeyForRecipient(_ recipientId: String) -> SMKUDAccessKey? { - guard supportsUnidentifiedDelivery(recipientId: recipientId) else { - return nil - } guard let profileKey = profileManager.profileKeyData(forRecipientId: recipientId) else { // Mark as "not a UD recipient". return nil From 01f63792f817c599eabc7433eef07f0392c11b9c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 5 Oct 2018 13:19:46 -0400 Subject: [PATCH 2/3] Respond to CR. --- SignalMessaging/profiles/ProfileFetcherJob.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index c693610c7..acd4c8afc 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -180,6 +180,13 @@ public class ProfileFetcherJob: NSObject { profileNameEncrypted: signalServiceProfile.profileNameEncrypted, avatarUrlPath: signalServiceProfile.avatarUrlPath) + // Recipients should be in "UD delivery mode" IFF: + // + // * Their profile includes a unidentifiedAccessVerifier. + // * The unidentifiedAccessVerifier matches the "expected" value derived + // from their profile key (if any). + // + // Recipients should be in "normal delivery mode" otherwise. var supportsUnidentifiedDelivery = false if let unidentifiedAccessVerifier = signalServiceProfile.unidentifiedAccessVerifier, let udAccessKey = udManager.udAccessKeyForRecipient(recipientId) { From 624ffdf9b5214c77687386182c0f48edc337053a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 10 Oct 2018 09:09:01 -0400 Subject: [PATCH 3/3] Update cocoapods. --- Podfile.lock | 2 +- Pods | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Podfile.lock b/Podfile.lock index c249934bd..d4cf61fc0 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -260,7 +260,7 @@ CHECKOUT OPTIONS: :commit: 8b8326cd50bc488663a3d3743f1a92b90f4d85b4 :git: https://github.com/signalapp/HKDFKit.git SignalCoreKit: - :commit: e5b6aa3c078d7c2fbc154e5dd806b8e55211697d + :commit: ff0b95770520133b83a4bd7b26bc2c90b51abc4d :git: https://github.com/signalapp/SignalCoreKit.git SignalMetadataKit: :commit: b0e664410dd3d709355bfdb9d464ae02644aeb74 diff --git a/Pods b/Pods index 706302e04..4f0b4bb0d 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 706302e046a89e743f903e02f36b4f73c007a0de +Subproject commit 4f0b4bb0d12438b39a5616cb4190cf656a82a3e2