Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upload fallback keys with Crypto V2 #1697

Merged
merged 1 commit into from
Jan 27, 2023
Merged

Upload fallback keys with Crypto V2 #1697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions MatrixSDK.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1880,8 +1880,8 @@
ED55806E296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55806C296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift */; };
ED558070296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55806F296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift */; };
ED558071296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55806F296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift */; };
ED5580732970265A003443E3 /* MXCryptoMachineLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED5580722970265A003443E3 /* MXCryptoMachineLogger.swift */; };
ED5580742970265A003443E3 /* MXCryptoMachineLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED5580722970265A003443E3 /* MXCryptoMachineLogger.swift */; };
ED5580732970265A003443E3 /* MXCryptoSDKLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED5580722970265A003443E3 /* MXCryptoSDKLogger.swift */; };
ED5580742970265A003443E3 /* MXCryptoSDKLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED5580722970265A003443E3 /* MXCryptoSDKLogger.swift */; };
ED55807629709943003443E3 /* MatrixSDKTestsE2EData.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55807529709943003443E3 /* MatrixSDKTestsE2EData.swift */; };
ED55807729709943003443E3 /* MatrixSDKTestsE2EData.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55807529709943003443E3 /* MatrixSDKTestsE2EData.swift */; };
ED5580792970A879003443E3 /* MatrixSDKTestsData.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED5580782970A879003443E3 /* MatrixSDKTestsData.swift */; };
Expand Down Expand Up @@ -3096,7 +3096,7 @@
ED558067296F0361003443E3 /* MXCryptoMigrationStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationStore.swift; sourceTree = "<group>"; };
ED55806C296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationStoreUnitTests.swift; sourceTree = "<group>"; };
ED55806F296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationV2Tests.swift; sourceTree = "<group>"; };
ED5580722970265A003443E3 /* MXCryptoMachineLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMachineLogger.swift; sourceTree = "<group>"; };
ED5580722970265A003443E3 /* MXCryptoSDKLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoSDKLogger.swift; sourceTree = "<group>"; };
ED55807529709943003443E3 /* MatrixSDKTestsE2EData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MatrixSDKTestsE2EData.swift; sourceTree = "<group>"; };
ED5580782970A879003443E3 /* MatrixSDKTestsData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MatrixSDKTestsData.swift; sourceTree = "<group>"; };
ED5AE8C32816C8CF00105072 /* MXRoomSummaryCoreDataStore2.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = MXRoomSummaryCoreDataStore2.xcdatamodel; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5450,7 +5450,7 @@
children = (
ED5EF147297AB28600A5ADDA /* Extensions */,
ED2DD111286C450600F06731 /* MXCryptoMachine.swift */,
ED5580722970265A003443E3 /* MXCryptoMachineLogger.swift */,
ED5580722970265A003443E3 /* MXCryptoSDKLogger.swift */,
EDF154E0296C203E004D7FFE /* MXCryptoMachineStore.swift */,
ED8F1D3A2885BB2D00F897E7 /* MXCryptoProtocols.swift */,
ED2DD113286C450600F06731 /* MXCryptoRequests.swift */,
Expand Down Expand Up @@ -7335,7 +7335,7 @@
B1F04B132811E9D300103EBE /* MXBeaconInfoSummaryStoreProtocol.swift in Sources */,
183892802702F553003F0C4F /* MXRoomNameDefaultStringLocalizer.m in Sources */,
C6F9358B1E5B3BE600FC34BF /* MXJSONModels.swift in Sources */,
ED5580732970265A003443E3 /* MXCryptoMachineLogger.swift in Sources */,
ED5580732970265A003443E3 /* MXCryptoSDKLogger.swift in Sources */,
EC8A539725B1BC77004E0802 /* MXCallReplacesEventContent.m in Sources */,
B135066E27EA44C800BD3276 /* MXLocationServiceError.swift in Sources */,
32FA10C21FA1C9EE00E54233 /* MXOutgoingRoomKeyRequestManager.m in Sources */,
Expand Down Expand Up @@ -8000,7 +8000,7 @@
B1F04B142811E9D300103EBE /* MXBeaconInfoSummaryStoreProtocol.swift in Sources */,
EC8A53A225B1BC77004E0802 /* MXCallSelectAnswerEventContent.m in Sources */,
183892812702F553003F0C4F /* MXRoomNameDefaultStringLocalizer.m in Sources */,
ED5580742970265A003443E3 /* MXCryptoMachineLogger.swift in Sources */,
ED5580742970265A003443E3 /* MXCryptoSDKLogger.swift in Sources */,
B14EF2912397E90400758AF0 /* MXOutgoingRoomKeyRequestManager.m in Sources */,
B14EF2922397E90400758AF0 /* MXWellKnown.m in Sources */,
B1A026F926161EF5001AADFF /* MXSpaceChildSummaryResponse.m in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions MatrixSDK/Background/Crypto/MXBackgroundCryptoV2.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class MXBackgroundCryptoV2: MXBackgroundCrypto {
- to-device events : \(syncResponse.toDevice?.events.count ?? 0)
- devices changed : \(syncResponse.deviceLists?.changed?.count ?? 0)
- devices left : \(syncResponse.deviceLists?.left?.count ?? 0)
- one time keys : \(syncResponse.deviceOneTimeKeysCount?[kMXKeySignedCurve25519Type] ?? 0)
- fallback keys : \(syncResponse.unusedFallbackKeys ?? [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure its not, but double checking: Is this going to log keys that are sensitive?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those keys are public, after all they get uploaded to the server.

Copy link
Contributor Author

@Anderas Anderas Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, actually in this particular log we only print the count of keys (or type in the case of fallback keys), but I have also started logging the actual keys elsewhere so your question is valid (and it is safe as we only print the public keys)

"""
log.debug(details)

Expand Down
21 changes: 8 additions & 13 deletions MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,12 @@ class MXCryptoMachine {
case missingVerificationContent
case missingVerificationRequest
case missingVerification
case missingEmojis
case missingDecimals
case cannotCancelVerification
case cannotExportKeys
case cannotImportKeys
}

private let machine: OlmMachine
private let requests: MXCryptoRequests
private let queryScheduler: MXKeysQueryScheduler<MXKeysQueryResponse>
private let getRoomAction: GetRoomAction

private let sessionsQueue = MXTaskQueue()
Expand All @@ -77,7 +73,7 @@ class MXCryptoMachine {
restClient: MXRestClient,
getRoomAction: @escaping GetRoomAction
) throws {
MXCryptoMachineLogger.shared.log(logLine: "Starting logs")
MXCryptoSDKLogger.shared.log(logLine: "Starting logs")

let url = try MXCryptoMachineStore.createStoreURLIfNecessary(for: userId)
log.debug("Opening crypto store at \(url.path)/matrix-sdk-crypto.sqlite3") // Hardcoding path to db for debugging purpose
Expand All @@ -89,12 +85,7 @@ class MXCryptoMachine {
passphrase: nil
)

let requests = MXCryptoRequests(restClient: restClient)
self.requests = requests

queryScheduler = MXKeysQueryScheduler { users in
try await requests.queryKeys(users: users)
}
self.requests = MXCryptoRequests(restClient: restClient)
self.getRoomAction = getRoomAction

let details = """
Expand Down Expand Up @@ -123,6 +114,7 @@ class MXCryptoMachine {
return
}

log.debug("We have some keys to upload")
try await handleRequest(request)
log.debug("Keys successfully uploaded")
}
Expand Down Expand Up @@ -228,6 +220,8 @@ extension MXCryptoMachine: MXCryptoSyncing {
// MARK: - Private

private func handleRequest(_ request: Request) async throws {
log.debug("Handling request \(request)")

switch request {
case .toDevice(let requestId, let eventType, let body):
try await requests.sendToDevice(
Expand All @@ -242,8 +236,7 @@ extension MXCryptoMachine: MXCryptoSyncing {
try markRequestAsSent(requestId: requestId, requestType: .keysUpload, response: response.jsonString())

case .keysQuery(let requestId, let users):
// Key queries go through a scheduler layer instead of directly through the rest client
let response = try await queryScheduler.query(users: Set(users))
let response = try await requests.queryKeys(users: users)
try markRequestAsSent(requestId: requestId, requestType: .keysQuery, response: response.jsonString())

case .keysClaim(let requestId, let oneTimeKeys):
Expand Down Expand Up @@ -283,6 +276,8 @@ extension MXCryptoMachine: MXCryptoSyncing {
}

private func handleOutgoingRequests() async throws {
log.debug("->")

let requests = try machine.outgoingRequests()

try await withThrowingTaskGroup(of: Void.self) { [weak self] group in
Expand Down
29 changes: 18 additions & 11 deletions MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,22 @@ import MatrixSDKCrypto
/// to the native REST API client
struct MXCryptoRequests {
private let restClient: MXRestClient
private let queryScheduler: MXKeysQueryScheduler<MXKeysQueryResponse>

init(restClient: MXRestClient) {
self.restClient = restClient
self.queryScheduler = .init { users in
try await performCallbackRequest { completion in
_ = restClient.downloadKeysByChunk(
forUsers: users,
token: nil,
success: {
completion(.success($0))
}, failure: {
completion(.failure($0 ?? Error.unknownError))
})
}
}
}

func sendToDevice(request: ToDeviceRequest) async throws {
Expand All @@ -47,7 +61,7 @@ struct MXCryptoRequests {
restClient.uploadKeys(
request.deviceKeys,
oneTimeKeys: request.oneTimeKeys,
fallbackKeys: nil,
fallbackKeys: request.fallbackKeys,
forDevice: request.deviceId,
completion: $0
)
Expand Down Expand Up @@ -86,16 +100,7 @@ struct MXCryptoRequests {
}

func queryKeys(users: [String]) async throws -> MXKeysQueryResponse {
return try await performCallbackRequest { completion in
_ = restClient.downloadKeysByChunk(
forUsers: users,
token: nil,
success: {
completion(.success($0))
}, failure: {
completion(.failure($0 ?? Error.unknownError))
})
}
try await queryScheduler.query(users: Set(users))
}

func claimKeys(request: ClaimKeysRequest) async throws -> MXKeysClaimResponse {
Expand Down Expand Up @@ -160,6 +165,7 @@ extension MXCryptoRequests {
struct UploadKeysRequest {
let deviceKeys: [String: Any]?
let oneTimeKeys: [String: Any]?
let fallbackKeys: [String: Any]?
let deviceId: String

init(body: String, deviceId: String) throws {
Expand All @@ -169,6 +175,7 @@ extension MXCryptoRequests {

self.deviceKeys = json["device_keys"] as? [String : Any]
self.oneTimeKeys = json["one_time_keys"] as? [String : Any]
self.fallbackKeys = json["fallback_keys"] as? [String : Any]
self.deviceId = deviceId
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...CryptoMachine/MXCryptoMachineLogger.swift → ...pto/CryptoMachine/MXCryptoSDKLogger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import Foundation
import MatrixSDKCrypto

/// Redirects logs originating in `MatrixSDKCrypto` into `MXLog`
class MXCryptoMachineLogger: Logger {
static let shared = MXCryptoMachineLogger()
class MXCryptoSDKLogger: Logger {
static let shared = MXCryptoSDKLogger()

init() {
setLogger(logger: self)
Expand All @@ -35,7 +35,7 @@ class MXCryptoMachineLogger: Logger {
return
}

MXLog.debug("[MXCryptoMachine] \(logLine)")
MXLog.debug("[MXCryptoSDK] \(logLine)")
}
}

Expand Down
9 changes: 8 additions & 1 deletion MatrixSDK/Crypto/MXCryptoV2.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ class MXCryptoV2: NSObject, MXCrypto {
- to-device events : \(syncResponse.toDevice?.events.count ?? 0)
- devices changed : \(syncResponse.deviceLists?.changed?.count ?? 0)
- devices left : \(syncResponse.deviceLists?.left?.count ?? 0)
- one time keys : \(syncResponse.deviceOneTimeKeysCount?[kMXKeySignedCurve25519Type] ?? 0)
- fallback keys : \(syncResponse.unusedFallbackKeys ?? [])
"""
log.debug(details)

Expand All @@ -351,11 +353,16 @@ class MXCryptoV2: NSObject, MXCrypto {
unusedFallbackKeys: syncResponse.unusedFallbackKeys
)
await handle(toDeviceEvents: toDevice.events)
try await machine.processOutgoingRequests()
} catch {
log.error("Cannot handle sync", context: error)
}

do {
try await machine.processOutgoingRequests()
} catch {
log.error("Failed processing outgoing requests", context: error)
}

log.debug("Completed handling sync response `\(syncId)`")
await MainActor.run {
onComplete()
Expand Down
2 changes: 1 addition & 1 deletion MatrixSDK/Crypto/Migration/MXCryptoMigrationV2.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class MXCryptoMigrationV2: NSObject {

func migrateCrypto(updateProgress: @escaping (Double) -> Void) throws {
log.debug("Starting migration")
MXCryptoMachineLogger.shared.log(logLine: "Starting logs")
MXCryptoSDKLogger.shared.log(logLine: "Starting logs")

let startDate = Date()
updateProgress(0)
Expand Down
8 changes: 8 additions & 0 deletions MatrixSDKTests/Crypto/CryptoMachine/MXCryptoRequestsUnitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class MXCryptoRequestsUnitTests: XCTestCase {
"one_time_keys": [
"1": "C",
"2": "D",
],
"fallback_keys": [
"3": "E",
"4": "F",
]
]

Expand All @@ -65,6 +69,10 @@ class MXCryptoRequestsUnitTests: XCTestCase {
"1": "C",
"2": "D",
])
XCTAssertEqual(request.fallbackKeys as? [String: String], [
"3": "E",
"4": "F",
])
XCTAssertEqual(request.deviceId, "A")
} catch {
XCTFail("Failed creating upload keys request with error - \(error)")
Expand Down
1 change: 1 addition & 0 deletions changelog.d/pr-1697.change
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CryptoV2: Upload fallback keys