From 471ee4a57358df22aa1ed5f7fabed2fe33bc8526 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Mon, 11 Sep 2023 22:18:37 -0500 Subject: [PATCH] Simplify sender certificate selection logic --- .../src/Messages/MessageSender.swift | 34 ++-- .../src/Messages/UD/OWSUDManager.swift | 70 +------ .../tests/Messages/OWSUDManagerTest.swift | 172 +----------------- 3 files changed, 23 insertions(+), 253 deletions(-) diff --git a/SignalServiceKit/src/Messages/MessageSender.swift b/SignalServiceKit/src/Messages/MessageSender.swift index b3a53b40d5..394796ce50 100644 --- a/SignalServiceKit/src/Messages/MessageSender.swift +++ b/SignalServiceKit/src/Messages/MessageSender.swift @@ -721,35 +721,35 @@ extension MessageSender { throw OWSAssertionError("Not registered.") } + let senderCertificate: SenderCertificate = { + switch udManager.phoneNumberSharingMode(tx: tx) { + case .everybody: + return senderCertificates.defaultCert + case .nobody: + return senderCertificates.uuidOnlyCert + } + }() + // 2. Gather "ud sending access". - var sendingAccessMap = [ServiceId: OWSUDSendingAccess]() - let phoneNumberSharingMode = udManager.phoneNumberSharingMode(tx: tx) + var udAccessMap = [ServiceId: OWSUDSendingAccess]() for serviceId in serviceIds { if localIdentifiers.contains(serviceId: serviceId) { continue } - sendingAccessMap[serviceId] = ( - message.isStorySend - ? udManager.storySendingAccess( - for: serviceId, - phoneNumberSharingMode: phoneNumberSharingMode, - senderCertificates: senderCertificates, - tx: tx - ) - : udManager.udSendingAccess( - for: serviceId, - phoneNumberSharingMode: phoneNumberSharingMode, - senderCertificates: senderCertificates, - tx: tx - ) + let udAccess = ( + message.isStorySend ? udManager.storyUdAccess() : udManager.udAccess(for: serviceId, tx: tx) ) + guard let udAccess else { + continue + } + udAccessMap[serviceId] = OWSUDSendingAccess(udAccess: udAccess, senderCertificate: senderCertificate) } return .sendMessage( serializedMessage: serializedMessage, thread: thread, serviceIds: serviceIds, - udAccess: sendingAccessMap, + udAccess: udAccessMap, localIdentifiers: localIdentifiers ) } diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index 37114fa2bf..a9c62379f3 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -122,19 +122,7 @@ public protocol OWSUDManager { func udAccess(for serviceId: ServiceId, tx: SDSAnyReadTransaction) -> OWSUDAccess? - func udSendingAccess( - for serviceId: ServiceId, - phoneNumberSharingMode: PhoneNumberSharingMode, - senderCertificates: SenderCertificates, - tx: SDSAnyReadTransaction - ) -> OWSUDSendingAccess? - - func storySendingAccess( - for serviceId: ServiceId, - phoneNumberSharingMode: PhoneNumberSharingMode, - senderCertificates: SenderCertificates, - tx: SDSAnyReadTransaction - ) -> OWSUDSendingAccess + func storyUdAccess() -> OWSUDAccess func fetchAllAciUakPairs(tx: SDSAnyReadTransaction) -> [Aci: SMKUDAccessKey] @@ -322,60 +310,8 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } } - // Returns the UD access key and appropriate sender certificate for sending to a given recipient - public func udSendingAccess( - for serviceId: ServiceId, - phoneNumberSharingMode: PhoneNumberSharingMode, - senderCertificates: SenderCertificates, - tx: SDSAnyReadTransaction - ) -> OWSUDSendingAccess? { - guard let udAccess = self.udAccess(for: serviceId, tx: tx) else { - return nil - } - return udSendingAccess( - for: serviceId, - udAccess: udAccess, - phoneNumberSharingMode: phoneNumberSharingMode, - senderCertificates: senderCertificates, - tx: tx - ) - } - - public func storySendingAccess( - for serviceId: ServiceId, - phoneNumberSharingMode: PhoneNumberSharingMode, - senderCertificates: SenderCertificates, - tx: SDSAnyReadTransaction - ) -> OWSUDSendingAccess { - let udAccess = OWSUDAccess(udAccessKey: randomUDAccessKey(), udAccessMode: .unrestricted, isRandomKey: true) - return udSendingAccess( - for: serviceId, - udAccess: udAccess, - phoneNumberSharingMode: phoneNumberSharingMode, - senderCertificates: senderCertificates, - tx: tx - ) - } - - private func udSendingAccess( - for serviceId: ServiceId, - udAccess: OWSUDAccess, - phoneNumberSharingMode: PhoneNumberSharingMode, - senderCertificates: SenderCertificates, - tx: SDSAnyReadTransaction - ) -> OWSUDSendingAccess { - let shouldSharePhoneNumber: Bool - switch phoneNumberSharingMode { - case .everybody: - shouldSharePhoneNumber = true - case .nobody: - let identityManager = DependenciesBridge.shared.identityManager - shouldSharePhoneNumber = identityManager.shouldSharePhoneNumber(with: serviceId, tx: tx.asV2Read) - } - return OWSUDSendingAccess( - udAccess: udAccess, - senderCertificate: shouldSharePhoneNumber ? senderCertificates.defaultCert : senderCertificates.uuidOnlyCert - ) + public func storyUdAccess() -> OWSUDAccess { + return OWSUDAccess(udAccessKey: randomUDAccessKey(), udAccessMode: .unrestricted, isRandomKey: true) } // MARK: - Sender Certificate diff --git a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift index fc4dd61029..4bad310c85 100644 --- a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift +++ b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift @@ -18,12 +18,9 @@ class OWSUDManagerTest: SSKBaseTestSwift { // MARK: - Setup/Teardown - let aliceE164 = "+13213214321" - let aliceAci = Aci.randomForTesting() - let trustRoot = IdentityKeyPair.generate() - lazy var aliceAddress = SignalServiceAddress(serviceId: aliceAci, phoneNumber: aliceE164) - lazy var defaultSenderCert = buildSenderCertificate(uuidOnly: false) - lazy var uuidOnlySenderCert = buildSenderCertificate(uuidOnly: true) + private let aliceE164 = "+13213214321" + private let aliceAci = Aci.randomForTesting() + private lazy var aliceAddress = SignalServiceAddress(serviceId: aliceAci, phoneNumber: aliceE164) override func setUp() { super.setUp() @@ -40,17 +37,11 @@ class OWSUDManagerTest: SSKBaseTestSwift { transaction: transaction ) } - - udManagerImpl.trustRoot = ECPublicKey(trustRoot.publicKey) - udManagerImpl.setSenderCertificate(uuidOnly: true, certificateData: Data(uuidOnlySenderCert.serialize())) - udManagerImpl.setSenderCertificate(uuidOnly: false, certificateData: Data(defaultSenderCert.serialize())) } // MARK: - Tests func testMode_noProfileKey() { - XCTAssert(udManagerImpl.hasSenderCertificates()) - XCTAssert(tsAccountManager.isRegistered) // Ensure UD is enabled by setting our own access level to enabled. @@ -95,8 +86,6 @@ class OWSUDManagerTest: SSKBaseTestSwift { } func testMode_withProfileKey() { - XCTAssert(udManagerImpl.hasSenderCertificates()) - XCTAssert(tsAccountManager.isRegistered) guard let localAddress = tsAccountManager.localAddress else { XCTFail("localAddress was unexpectedly nil") @@ -154,159 +143,4 @@ class OWSUDManagerTest: SSKBaseTestSwift { XCTAssert(udAccess.isRandomKey) } } - - func test_senderAccess() { - XCTAssert(udManagerImpl.hasSenderCertificates()) - - XCTAssert(tsAccountManager.isRegistered) - guard let localAddress = tsAccountManager.localAddress else { - XCTFail("localAddress was unexpectedly nil") - return - } - XCTAssert(localAddress.isValid) - - // Ensure UD is enabled by setting our own access level to enabled. - write { tx in - udManagerImpl.setUnidentifiedAccessMode(.enabled, for: aliceAci, tx: tx) - } - - let bobRecipientAci = Aci.randomForTesting() - write { transaction in - self.profileManager.setProfileKeyData( - OWSAES256Key.generateRandom().keyData, - for: SignalServiceAddress(bobRecipientAci), - userProfileWriter: .tests, - authedAccount: .implicit(), - transaction: transaction - ) - } - - let senderCertificates = SenderCertificates( - defaultCert: defaultSenderCert, - uuidOnlyCert: uuidOnlySenderCert - ) - - read { tx in - let sendingAccess = self.udManagerImpl.udSendingAccess( - for: bobRecipientAci, - phoneNumberSharingMode: .everybody, - senderCertificates: senderCertificates, - tx: tx - )! - XCTAssertEqual(.unknown, sendingAccess.udAccess.udAccessMode) - XCTAssertFalse(sendingAccess.udAccess.isRandomKey) - XCTAssertEqual(sendingAccess.senderCertificate.serialize(), defaultSenderCert.serialize()) - } - - read { tx in - let sendingAccess = self.udManagerImpl.udSendingAccess( - for: bobRecipientAci, - phoneNumberSharingMode: .nobody, - senderCertificates: senderCertificates, - tx: tx - )! - XCTAssertEqual(.unknown, sendingAccess.udAccess.udAccessMode) - XCTAssertFalse(sendingAccess.udAccess.isRandomKey) - XCTAssertEqual(sendingAccess.senderCertificate.serialize(), self.uuidOnlySenderCert.serialize()) - } - } - - func test_certificateChoiceWithPhoneNumberShared() { - XCTAssert(udManagerImpl.hasSenderCertificates()) - - XCTAssert(tsAccountManager.isRegistered) - guard let localAddress = tsAccountManager.localAddress else { - XCTFail("localAddress was unexpectedly nil") - return - } - XCTAssert(localAddress.isValid) - - let identityManager = DependenciesBridge.shared.identityManager - - // Ensure UD is enabled by setting our own access level to enabled. - write { tx in - udManagerImpl.setUnidentifiedAccessMode(.enabled, for: aliceAci, tx: tx) - } - - let bobAci = Aci.randomForTesting() - write { transaction in - self.profileManager.setProfileKeyData( - OWSAES256Key.generateRandom().keyData, - for: SignalServiceAddress(bobAci), - userProfileWriter: .tests, - authedAccount: .implicit(), - transaction: transaction - ) - identityManager.setShouldSharePhoneNumber(with: bobAci, tx: transaction.asV2Write) - } - - let senderCertificates = SenderCertificates( - defaultCert: defaultSenderCert, - uuidOnlyCert: uuidOnlySenderCert - ) - - read { tx in - let sendingAccess = udManagerImpl.udSendingAccess( - for: bobAci, - phoneNumberSharingMode: .everybody, - senderCertificates: senderCertificates, - tx: tx - )! - XCTAssertEqual(sendingAccess.senderCertificate.serialize(), defaultSenderCert.serialize()) - } - - read { tx in - let sendingAccess = udManagerImpl.udSendingAccess( - for: bobAci, - phoneNumberSharingMode: .nobody, - senderCertificates: senderCertificates, - tx: tx - )! - XCTAssertEqual(sendingAccess.senderCertificate.serialize(), defaultSenderCert.serialize()) - } - - // Make sure it resets on clear. - write { transaction in - self.profileManager.setProfileKeyData( - OWSAES256Key.generateRandom().keyData, - for: SignalServiceAddress(bobAci), - userProfileWriter: .tests, - authedAccount: .implicit(), - transaction: transaction - ) - identityManager.clearShouldSharePhoneNumber(with: bobAci, tx: transaction.asV2Write) - } - - read { tx in - let sendingAccess = udManagerImpl.udSendingAccess( - for: bobAci, - phoneNumberSharingMode: .nobody, - senderCertificates: senderCertificates, - tx: tx - )! - XCTAssertEqual(sendingAccess.senderCertificate.serialize(), uuidOnlySenderCert.serialize()) - } - } - - // MARK: - Util - - func buildSenderCertificate(uuidOnly: Bool) -> SenderCertificate { - let serverKeys = IdentityKeyPair.generate() - let serverCert = try! ServerCertificate(keyId: 1, - publicKey: serverKeys.publicKey, - trustRoot: trustRoot.privateKey) - - var senderAddress = try! SealedSenderAddress(e164: nil, aci: aliceAci, deviceId: 1) - if !uuidOnly { - senderAddress.e164 = aliceE164 - } - - let expires = NSDate.ows_millisecondTimeStamp() + kWeekInMs - let senderKeys = IdentityKeyPair.generate() - return try! SenderCertificate(sender: senderAddress, - publicKey: senderKeys.publicKey, - expiration: expires, - signerCertificate: serverCert, - signerKey: serverKeys.privateKey) - } }