Modernize CDSv2 lookups for updated rollout plan

This commit is contained in:
Max Radermacher 2023-08-02 17:30:47 -05:00 committed by GitHub
parent 7642a67d84
commit 97dc481d44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 166 additions and 123 deletions

View File

@ -148,6 +148,7 @@ public class AppSetup {
recipientFetcher: dependenciesBridge.recipientFetcher,
recipientMerger: dependenciesBridge.recipientMerger,
tsAccountManager: tsAccountManager,
udManager: udManager,
websocketFactory: webSocketFactory
)
let messageSendLog = MessageSendLog(

View File

@ -104,6 +104,7 @@ public final class ContactDiscoveryManagerImpl: NSObject, ContactDiscoveryManage
recipientFetcher: RecipientFetcher,
recipientMerger: RecipientMerger,
tsAccountManager: TSAccountManager,
udManager: OWSUDManager,
websocketFactory: WebSocketFactory
) {
self.init(
@ -112,6 +113,7 @@ public final class ContactDiscoveryManagerImpl: NSObject, ContactDiscoveryManage
recipientFetcher: recipientFetcher,
recipientMerger: recipientMerger,
tsAccountManager: tsAccountManager,
udManager: udManager,
websocketFactory: websocketFactory
)
)

View File

@ -16,6 +16,7 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue {
private let recipientFetcher: RecipientFetcher
private let recipientMerger: RecipientMerger
private let tsAccountManager: TSAccountManager
private let udManager: OWSUDManager
private let websocketFactory: WebSocketFactory
init(
@ -23,12 +24,14 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue {
recipientFetcher: RecipientFetcher,
recipientMerger: RecipientMerger,
tsAccountManager: TSAccountManager,
udManager: OWSUDManager,
websocketFactory: WebSocketFactory
) {
self.db = db
self.recipientFetcher = recipientFetcher
self.recipientMerger = recipientMerger
self.tsAccountManager = tsAccountManager
self.udManager = udManager
self.websocketFactory = websocketFactory
}
@ -46,41 +49,29 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue {
)
return firstly {
Self.createContactDiscoveryOperation(
for: e164s,
ContactDiscoveryV2Operation(
e164sToLookup: e164s,
mode: mode,
tryToReturnAcisWithoutUaks: RemoteConfig.tryToReturnAcisWithoutUaks,
udManager: ContactDiscoveryV2Operation.Wrappers.UDManager(db: db, udManager: udManager),
websocketFactory: websocketFactory
).perform(on: workQueue)
}.map(on: workQueue) { (discoveredContacts: Set<DiscoveredContactInfo>) -> Set<SignalRecipient> in
try self.processResults(requestedPhoneNumbers: e164s, discoveryResults: discoveredContacts)
}.map(on: workQueue) { (discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult]) -> Set<SignalRecipient> in
try self.processResults(requestedPhoneNumbers: e164s, discoveryResults: discoveryResults)
}
}
private static func createContactDiscoveryOperation(
for e164s: Set<E164>,
mode: ContactDiscoveryMode,
websocketFactory: WebSocketFactory
) -> ContactDiscoveryOperation {
return ContactDiscoveryV2CompatibilityOperation(
e164sToLookup: e164s,
mode: mode,
websocketFactory: websocketFactory
)
}
private func processResults(
requestedPhoneNumbers: Set<E164>,
discoveryResults: Set<DiscoveredContactInfo>
discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult]
) throws -> Set<SignalRecipient> {
let undiscoverableE164s = requestedPhoneNumbers.subtracting(discoveryResults.lazy.map { $0.e164 })
return try db.write { tx in
guard let localIdentifiers = tsAccountManager.localIdentifiers(transaction: SDSDB.shimOnlyBridge(tx)) else {
throw OWSAssertionError("Not registered.")
}
return storeResults(
discoveredContacts: discoveryResults,
undiscoverableE164s: undiscoverableE164s,
requestedPhoneNumbers: requestedPhoneNumbers,
discoveryResults: discoveryResults,
localIdentifiers: localIdentifiers,
tx: tx
)
@ -88,45 +79,56 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue {
}
private func storeResults(
discoveredContacts: Set<DiscoveredContactInfo>,
undiscoverableE164s: Set<E164>,
requestedPhoneNumbers: Set<E164>,
discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult],
localIdentifiers: LocalIdentifiers,
tx: DBWriteTransaction
) -> Set<SignalRecipient> {
let registeredRecipients = Set(discoveredContacts.map { discoveredContact -> SignalRecipient in
var registeredRecipients = Set<SignalRecipient>()
for discoveryResult in discoveryResults {
// PNI TODO: Pass the PNI into the merging logic.
guard let aci = discoveryResult.aci else {
continue
}
let recipient = recipientMerger.applyMergeFromContactDiscovery(
localIdentifiers: localIdentifiers,
aci: UntypedServiceId(discoveredContact.uuid),
phoneNumber: discoveredContact.e164,
aci: aci.untypedServiceId,
phoneNumber: discoveryResult.e164,
tx: tx
)
recipient.markAsRegisteredAndSave(tx: SDSDB.shimOnlyBridge(tx))
return recipient
})
for undiscoverableE164 in undiscoverableE164s {
let address = SignalServiceAddress(undiscoverableE164)
// We process all the results that we were provided, but we only return the
// recipients that were specifically requested as part of this operation.
if requestedPhoneNumbers.contains(discoveryResult.e164) {
registeredRecipients.insert(recipient)
}
}
// It's possible we have an undiscoverable address that has a UUID in a
// number of scenarios, such as (but not exclusive to) the following:
let undiscoverablePhoneNumbers = requestedPhoneNumbers.subtracting(discoveryResults.lazy.map { $0.e164 })
for phoneNumber in undiscoverablePhoneNumbers {
// It's possible we have an undiscoverable phone number that already has an
// ACI or PNI in a number of scenarios, such as (but not exclusive to) the
// following:
//
// * You do "find by phone number" for someone you've previously interacted
// with (and had a UUID for) who is no longer registered.
// with (and had an ACI or PNI for) who is no longer registered.
//
// * You do an intersection to look up someone who has shared their phone
// number with you (via message send) but has chosen to be undiscoverable
// by CDS lookups.
// number with you (via message send) but has chosen to be undiscoverable
// by CDS lookups.
//
// When any of these scenarios occur, we cannot know with certainty if the
// user is unregistered or has only turned off discoverability, so we
// *only* mark the addresses without any UUIDs as unregistered. Everything
// else we ignore; we will identify their current registration status
// either when attempting to send a message or when fetching their profile.
guard address.uuid == nil else {
let finder = AnySignalRecipientFinder()
let recipient = finder.signalRecipientForPhoneNumber(phoneNumber.stringValue, transaction: SDSDB.shimOnlyBridge(tx))
// PNI TODO: Also check for PNIs here.
guard let recipient, recipient.serviceId == nil else {
continue
}
let recipient = recipientFetcher.fetchOrCreate(phoneNumber: undiscoverableE164, tx: tx)
recipient.markAsUnregisteredAndSave(tx: SDSDB.shimOnlyBridge(tx))
}

View File

@ -5,6 +5,8 @@
import GRDB
import Foundation
import LibSignalClient
import SignalCoreKit
// MARK: -
@ -14,49 +16,6 @@ private enum Constant {
// MARK: -
/// Runs a CDSv1-compatible operation against the CDSv2 backend.
final class ContactDiscoveryV2CompatibilityOperation: ContactDiscoveryOperation {
let e164sToLookup: Set<E164>
let mode: ContactDiscoveryMode
let websocketFactory: WebSocketFactory
init(e164sToLookup: Set<E164>, mode: ContactDiscoveryMode, websocketFactory: WebSocketFactory) {
self.e164sToLookup = e164sToLookup
self.mode = mode
self.websocketFactory = websocketFactory
}
func perform(on queue: DispatchQueue) -> Promise<Set<DiscoveredContactInfo>> {
let operation = ContactDiscoveryV2Operation(
e164sToLookup: e164sToLookup,
mode: mode,
compatibilityMode: .fetchAllACIs,
websocketFactory: websocketFactory
)
return firstly {
operation.perform(on: queue)
}.map(on: queue) { discoveryResults in
var results = Set<DiscoveredContactInfo>()
for discoveryResult in discoveryResults {
guard self.e164sToLookup.contains(discoveryResult.e164) else {
// In v2, we get back results for previous lookups as well. The API
// contract expects only those that were explicitly requested, so filter to
// include only those.
continue
}
guard let aci = discoveryResult.aci else {
owsFailDebug("CDSv2: All discovery results should have an ACI")
continue
}
results.insert(DiscoveredContactInfo(e164: discoveryResult.e164, uuid: aci))
}
return results
}
}
}
// MARK: -
protocol ContactDiscoveryV2PersistentState {
/// Load the token & e164s represented by the token.
///
@ -86,14 +45,7 @@ final class ContactDiscoveryV2Operation {
let e164sToLookup: Set<E164>
let compatibilityMode: CompatibilityMode
enum CompatibilityMode {
/// Send a request that returns a CDSv1-equivalent response.
case fetchAllACIs
/// Send a PNP-aware request. Requires full support for PNI-only contacts.
case fetchKnownACIs
}
let tryToReturnAcisWithoutUaks: Bool
/// If non-nil, requests will include prevE164s & a token, so we'll only
/// consume quota for new E164s.
@ -104,24 +56,29 @@ final class ContactDiscoveryV2Operation {
/// consume too much quota without the user's consent.
let persistentState: ContactDiscoveryV2PersistentState?
let udManager: Shims.UDManager
let connectionFactory: SgxWebsocketConnectionFactory
init(
e164sToLookup: Set<E164>,
compatibilityMode: CompatibilityMode,
tryToReturnAcisWithoutUaks: Bool,
persistentState: ContactDiscoveryV2PersistentState?,
udManager: Shims.UDManager,
connectionFactory: SgxWebsocketConnectionFactory
) {
self.e164sToLookup = e164sToLookup
self.compatibilityMode = compatibilityMode
self.tryToReturnAcisWithoutUaks = tryToReturnAcisWithoutUaks
self.persistentState = persistentState
self.udManager = udManager
self.connectionFactory = connectionFactory
}
convenience init(
e164sToLookup: Set<E164>,
mode: ContactDiscoveryMode,
compatibilityMode: CompatibilityMode,
tryToReturnAcisWithoutUaks: Bool,
udManager: Shims.UDManager,
websocketFactory: WebSocketFactory
) {
let persistentState: ContactDiscoveryV2PersistentState?
@ -132,8 +89,9 @@ final class ContactDiscoveryV2Operation {
}
self.init(
e164sToLookup: e164sToLookup,
compatibilityMode: compatibilityMode,
tryToReturnAcisWithoutUaks: tryToReturnAcisWithoutUaks,
persistentState: persistentState,
udManager: udManager,
connectionFactory: SgxWebsocketConnectionFactoryImpl(websocketFactory: websocketFactory)
)
}
@ -207,14 +165,15 @@ final class ContactDiscoveryV2Operation {
request.token = prevToken
request.prevE164S = prevE164s
request.newE164S = ContactDiscoveryE164Collection(newE164s).encodedValues
switch compatibilityMode {
case .fetchAllACIs:
request.returnAcisWithoutUaks = true
case .fetchKnownACIs:
request.returnAcisWithoutUaks = false
// TODO: Fetch all ACI-UAK pairs.
}
request.returnAcisWithoutUaks = tryToReturnAcisWithoutUaks
request.aciUakPairs = { () -> Data in
var result = Data()
for (aci, uak) in udManager.fetchAllAciUakPairsWithSneakyTransaction() {
result.append(contentsOf: aci.wrappedAciValue.serviceIdBinary)
result.append(uak.keyData)
}
return result
}()
return InitialRequest(
hasToken: !prevToken.isEmpty,
@ -357,11 +316,11 @@ final class ContactDiscoveryV2Operation {
/// If the lookup succeeds, we'll get back a PNI. If it doesn't succeed, the
/// user with a particular e164 may not be registered, or they may have
/// chosen to hide their phone number.
var pni: UUID
var pni: Pni
/// If we provide the correct ACI-UAK pair, we'll also get back the ACI
/// associated with the e164/PNI.
var aci: UUID?
var aci: Aci?
}
static func decodePniAciResult(_ data: Data) throws -> [DiscoveryResult] {
@ -383,17 +342,17 @@ final class ContactDiscoveryV2Operation {
}
remainingData = remainingData.dropFirst(rawE164Count)
guard let (pni, pniCount) = UUID.from(data: remainingData) else {
guard let (pniUuid, pniCount) = UUID.from(data: remainingData) else {
throw ContactDiscoveryError.assertionError(description: "malformed e164/aci/pni triples")
}
remainingData = remainingData.dropFirst(pniCount)
guard let (aci, aciCount) = UUID.from(data: remainingData) else {
guard let (aciUuid, aciCount) = UUID.from(data: remainingData) else {
throw ContactDiscoveryError.assertionError(description: "malformed e164/aci/pni triples")
}
remainingData = remainingData.dropFirst(aciCount)
guard pni != UUID.allZeros else {
guard pniUuid != UUID.allZeros else {
return nil
}
guard let e164 = E164("+\(rawE164)") else {
@ -401,8 +360,8 @@ final class ContactDiscoveryV2Operation {
}
return DiscoveryResult(
e164: e164,
pni: pni,
aci: aci == UUID.allZeros ? nil : aci
pni: Pni(fromUUID: pniUuid),
aci: aciUuid == UUID.allZeros ? nil : Aci(fromUUID: aciUuid)
)
}
}
@ -485,3 +444,33 @@ extension UUID {
Self(data: Data(count: 16))!
}()
}
// MARK: - Shims
extension ContactDiscoveryV2Operation {
enum Shims {
typealias UDManager = _ContactDiscoveryV2Operation_UDManagerShim
}
enum Wrappers {
typealias UDManager = _ContactDiscoveryV2Operation_UDManagerWrapper
}
}
protocol _ContactDiscoveryV2Operation_UDManagerShim {
func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey]
}
class _ContactDiscoveryV2Operation_UDManagerWrapper: _ContactDiscoveryV2Operation_UDManagerShim {
private let db: DB
private let udManager: OWSUDManager
init(db: DB, udManager: OWSUDManager) {
self.db = db
self.udManager = udManager
}
func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey] {
db.read { tx in udManager.fetchAllAciUakPairs(tx: SDSDB.shimOnlyBridge(tx)) }
}
}

View File

@ -159,6 +159,9 @@ public protocol OWSUDManager: AnyObject {
@objc
func storySendingAccess(for serviceId: UntypedServiceIdObjC, senderCertificates: SenderCertificates) -> OWSUDSendingAccess
@objc
func fetchAllAciUakPairs(tx: SDSAnyReadTransaction) -> [AciObjC: SMKUDAccessKey]
// MARK: Sender Certificate
// We use completion handlers instead of a promise so that message sending
@ -224,6 +227,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager {
//
// TODO: We might not want to use comprehensive caches here.
private var phoneNumberAccessCache = [String: UnidentifiedAccessMode]()
// PNI TODO: Change this type to Aci or ServiceId.
private var uuidAccessCache = [UUID: UnidentifiedAccessMode]()
@objc
@ -428,21 +432,37 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager {
}
}
public func fetchAllAciUakPairs(tx: SDSAnyReadTransaction) -> [AciObjC: SMKUDAccessKey] {
let acis = self.unfairLock.withLock {
self.uuidAccessCache.compactMap { (serviceId, mode) -> Aci? in
switch mode {
case .enabled, .unrestricted, .unknown:
return Aci(fromUUID: serviceId)
case .disabled:
return nil
}
}
}
var result = [AciObjC: SMKUDAccessKey]()
for aci in acis {
result[AciObjC(aci)] = udAccessKey(for: SignalServiceAddress(aci), tx: tx)
}
return result
}
// Returns the UD access key for a given recipient
// if we have a valid profile key for them.
@objc
public func udAccessKey(forAddress address: SignalServiceAddress) -> SMKUDAccessKey? {
let profileKeyData = databaseStorage.read { transaction in
return self.profileManager.profileKeyData(for: address,
transaction: transaction)
}
guard let profileKey = profileKeyData else {
// Mark as "not a UD recipient".
return databaseStorage.read { tx in udAccessKey(for: address, tx: tx) }
}
private func udAccessKey(for address: SignalServiceAddress, tx: SDSAnyReadTransaction) -> SMKUDAccessKey? {
guard let profileKey = profileManager.profileKeyData(for: address, transaction: tx) else {
return nil
}
do {
let udAccessKey = try SMKUDAccessKey(profileKey: profileKey)
return udAccessKey
return try SMKUDAccessKey(profileKey: profileKey)
} catch {
Logger.error("Could not determine udAccessKey: \(error)")
return nil

View File

@ -143,6 +143,7 @@ public class MockSSKEnvironment: SSKEnvironment {
recipientFetcher: dependenciesBridge.recipientFetcher,
recipientMerger: dependenciesBridge.recipientMerger,
tsAccountManager: tsAccountManager,
udManager: udManager,
websocketFactory: webSocketFactory
)
let messageSendLog = MessageSendLog(databaseStorage: databaseStorage, dateProvider: { Date() })

View File

@ -256,6 +256,22 @@ public class RemoteConfig: BaseFlags {
return .mirroring
}
static var tryToReturnAcisWithoutUaks: Bool {
if !FeatureFlags.phoneNumberIdentifiers {
return true
}
if TSConstants.isUsingProductionService {
return true
}
if OWSIsDebugBuild() {
return true
}
if !isEnabled(.cdsDisableCompatibilityMode) {
return true
}
return false
}
// MARK: UInt values
private static func getUIntValue(
@ -555,6 +571,7 @@ private struct Flags {
case ringrtcNwPathMonitorTrialKillSwitch
case stopMirroringToSVR2Override
case exclusiveSVR2
case cdsDisableCompatibilityMode
}
// Values defined in this array remain set once they are
@ -640,6 +657,7 @@ private extension FlagType {
case "minNicknameLength": return "global.nicknames.min"
case "maxNicknameLength": return "global.nicknames.max"
case "safetyNumberAci": return "global.safetyNumberAci"
case "cdsDisableCompatibilityMode": return "cds.disableCompatibilityMode"
default: return Flags.prefix + rawValue
}
}

View File

@ -3,13 +3,19 @@
// SPDX-License-Identifier: AGPL-3.0-only
//
import LibSignalClient
import XCTest
@testable import SignalServiceKit
final class ContactDiscoveryV2OperationTest: XCTestCase {
// MARK: - Mocks
private class MockUDManager: ContactDiscoveryV2Operation.Shims.UDManager {
func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey] { return [:] }
}
private class MockContactDiscoveryV2PersistentState: ContactDiscoveryV2PersistentState {
var token: Data?
var prevE164s = Set<E164>()
@ -41,13 +47,14 @@ final class ContactDiscoveryV2OperationTest: XCTestCase {
/// In .oneOffUserRequest mode, we should disregard tokens entirely.
func testOneOffRequest() throws {
let aci = UUID()
let pni = UUID()
let aci = Aci.randomForTesting()
let pni = Pni.randomForTesting()
let operation = ContactDiscoveryV2Operation(
e164sToLookup: [try XCTUnwrap(E164("+16505550100"))],
compatibilityMode: .fetchAllACIs,
tryToReturnAcisWithoutUaks: true,
persistentState: nil,
udManager: MockUDManager(),
connectionFactory: connectionFactory
)
@ -68,7 +75,7 @@ final class ContactDiscoveryV2OperationTest: XCTestCase {
XCTAssertTrue(request.tokenAck)
var response = CDSI_ClientResponse()
response.e164PniAciTriples = newE164s! + pni.data + aci.data
response.e164PniAciTriples = newE164s! + pni.rawUUID.data + aci.rawUUID.data
return .value([response])
}
connectionFactory.setOnConnectAndPerformHandshake({ _ in
@ -94,8 +101,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase {
func testNotDiscoverable() throws {
let operation = ContactDiscoveryV2Operation(
e164sToLookup: [try XCTUnwrap(E164("+16505550100"))],
compatibilityMode: .fetchAllACIs,
tryToReturnAcisWithoutUaks: true,
persistentState: nil,
udManager: MockUDManager(),
connectionFactory: connectionFactory
)
@ -132,8 +140,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase {
func testRateLimitError() throws {
let operation = ContactDiscoveryV2Operation(
e164sToLookup: [try XCTUnwrap(E164("+16505550100"))],
compatibilityMode: .fetchAllACIs,
tryToReturnAcisWithoutUaks: true,
persistentState: persistentState,
udManager: MockUDManager(),
connectionFactory: connectionFactory
)
@ -177,8 +186,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase {
func testInvalidTokenError() throws {
let operation = ContactDiscoveryV2Operation(
e164sToLookup: [try XCTUnwrap(E164("+16505550100"))],
compatibilityMode: .fetchAllACIs,
tryToReturnAcisWithoutUaks: true,
persistentState: persistentState,
udManager: MockUDManager(),
connectionFactory: connectionFactory
)