diff --git a/MatrixSDK/Crypto/Algorithms/RoomEvent/MXRoomEventDecryption.swift b/MatrixSDK/Crypto/Algorithms/RoomEvent/MXRoomEventDecryption.swift index bbae3238ef..d8bddbbebc 100644 --- a/MatrixSDK/Crypto/Algorithms/RoomEvent/MXRoomEventDecryption.swift +++ b/MatrixSDK/Crypto/Algorithms/RoomEvent/MXRoomEventDecryption.swift @@ -30,10 +30,10 @@ protocol MXRoomEventDecrypting: Actor { /// Note: room key could be contained in `m.room_key` or `m.forwarded_room_key` func handlePossibleRoomKeyEvent(_ event: MXEvent) - /// Retry decrypting all previously undecrypted events + /// Retry decrypting events with specific session ids /// /// Note: this may be useful if we have just imported keys from backup / file - func retryAllUndecryptedEvents() + func retryUndecryptedEvents(sessionIds: [String]) /// Reset the store of undecrypted events func resetUndecryptedEvents() @@ -46,7 +46,7 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { private let handler: MXCryptoRoomEventDecrypting private var undecryptedEvents: [SessionId: [EventId: MXEvent]] - private let log = MXNamedLog(name: "MXCryptoRoomEventDecryptor") + private let log = MXNamedLog(name: "MXRoomEventDecryption") init(handler: MXCryptoRoomEventDecrypting) { self.handler = handler @@ -82,14 +82,14 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { retryDecryption(events: events) } - func retryAllUndecryptedEvents() { - let allEvents = undecryptedEvents + func retryUndecryptedEvents(sessionIds: [String]) { + let events = sessionIds .flatMap { - $0.value.map { + undecryptedEvents[$0]?.map { $0.value - } + } ?? [] } - retryDecryption(events: allEvents) + retryDecryption(events: events) } func resetUndecryptedEvents() { @@ -100,11 +100,15 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { private func decrypt(event: MXEvent) -> MXEventDecryptionResult { guard - let sessionId = sessionId(for: event), - event.content?["algorithm"] as? String == kMXCryptoMegolmAlgorithm + event.isEncrypted && event.clear == nil, + event.content?["algorithm"] as? String == kMXCryptoMegolmAlgorithm, + let sessionId = sessionId(for: event) else { log.debug("Ignoring unencrypted or non-room event") - return MXEventDecryptionResult() + + let result = MXEventDecryptionResult() + result.clearEvent = event.clear?.jsonDictionary() + return result } do { @@ -117,8 +121,9 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { // hardcoded non-localized error message. Will be changed in future PR } catch DecryptionError.Megolm(message: "decryption failed because the room key is missing") { if undecryptedEvents[sessionId] == nil { - log.error("Failed to decrypt event due to missing room keys (further errors for the same key will be supressed)", context: [ - "session_id": sessionId + log.error("Failed to decrypt one or more events due to missing room keys", context: [ + "session_id": sessionId, + "details": "further errors for the same key will be supressed" ]) } @@ -178,7 +183,8 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { guard result.clearEvent != nil else { log.error("Event still not decryptable", context: [ "event_id": event.eventId ?? "unknown", - "session_id": sessionId(for: event) + "session_id": sessionId(for: event), + "error": result.error.localizedDescription ]) continue } @@ -207,8 +213,9 @@ actor MXRoomEventDecryption: MXRoomEventDecrypting { } private func sessionId(for event: MXEvent) -> String? { - guard event.isEncrypted, let sessionId = event.content["session_id"] as? String else { - log.error("Event is not encrypted or is missing session id") + let sessionId = event.content["session_id"] ?? event.wireContent["session_id"] + guard let sessionId = sessionId as? String else { + log.failure("Event is missing session id") return nil } return sessionId diff --git a/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift b/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift index 744f127836..78d6778b99 100644 --- a/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift +++ b/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift @@ -146,7 +146,7 @@ class MXCrossSigningV2: NSObject, MXCrossSigning { Task { do { - try await crossSigning.manuallyVerifyDevice(userId: crossSigning.userId, deviceId: deviceId) + try await crossSigning.verifyDevice(userId: crossSigning.userId, deviceId: deviceId) log.debug("Successfully cross-signed a device") await MainActor.run { @@ -170,7 +170,7 @@ class MXCrossSigningV2: NSObject, MXCrossSigning { Task { do { - try await crossSigning.manuallyVerifyUser(userId: userId) + try await crossSigning.verifyUser(userId: userId) log.debug("Successfully cross-signed a user") await MainActor.run { diff --git a/MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift b/MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift index ed189128c2..4af745ec15 100644 --- a/MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift +++ b/MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift @@ -108,11 +108,18 @@ class MXCryptoMachine { log.debug("Keys successfully uploaded") } - private static func storeURL(for userId: String) throws -> URL { + func deleteAllData() throws { + let url = try Self.storeURL(for: machine.userId()) + try FileManager.default.removeItem(at: url) + } +} + +extension MXCryptoMachine { + static func storeURL(for userId: String) throws -> URL { let container: URL if let sharedContainerURL = FileManager.default.applicationGroupContainerURL() { container = sharedContainerURL - } else if let url = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first { + } else if let url = platformDirectoryURL() { container = url } else { throw Error.invalidStorage @@ -123,9 +130,18 @@ class MXCryptoMachine { .appendingPathComponent(userId) } - func deleteAllData() throws { - let url = try Self.storeURL(for: machine.userId()) - try FileManager.default.removeItem(at: url) + private static func platformDirectoryURL() -> URL? { + #if os(OSX) + guard + let applicationSupport = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first, + let identifier = Bundle.main.bundleIdentifier + else { + return nil + } + return applicationSupport.appendingPathComponent(identifier) + #else + return FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first + #endif } } @@ -334,12 +350,12 @@ extension MXCryptoMachine: MXCryptoUserIdentitySource { ) } - func manuallyVerifyUser(userId: String) async throws { + func verifyUser(userId: String) async throws { let request = try machine.verifyIdentity(userId: userId) try await requests.uploadSignatures(request: request) } - func manuallyVerifyDevice(userId: String, deviceId: String) async throws { + func verifyDevice(userId: String, deviceId: String) async throws { let request = try machine.verifyDevice(userId: userId, deviceId: deviceId) try await requests.uploadSignatures(request: request) } diff --git a/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift b/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift index aec619d833..912bfcd3f6 100644 --- a/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift +++ b/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift @@ -54,8 +54,8 @@ protocol MXCryptoUserIdentitySource: MXCryptoIdentity { func isUserVerified(userId: String) -> Bool func isUserTracked(userId: String) -> Bool func downloadKeys(users: [String]) async throws - func manuallyVerifyUser(userId: String) async throws - func manuallyVerifyDevice(userId: String, deviceId: String) async throws + func verifyUser(userId: String) async throws + func verifyDevice(userId: String, deviceId: String) async throws func setLocalTrust(userId: String, deviceId: String, trust: LocalTrust) throws } diff --git a/MatrixSDK/Crypto/CryptoMachine/MXEventDecryptionResult+DecryptedEvent.swift b/MatrixSDK/Crypto/CryptoMachine/MXEventDecryptionResult+DecryptedEvent.swift index 922f0c41cc..3b61745d1e 100644 --- a/MatrixSDK/Crypto/CryptoMachine/MXEventDecryptionResult+DecryptedEvent.swift +++ b/MatrixSDK/Crypto/CryptoMachine/MXEventDecryptionResult+DecryptedEvent.swift @@ -37,11 +37,7 @@ extension MXEventDecryptionResult { senderCurve25519Key = event.senderCurve25519Key claimedEd25519Key = event.claimedEd25519Key forwardingCurve25519KeyChain = event.forwardingCurve25519Chain - - // Untrusted state not yet fully implemented, will equal to: - // `isUntrusted = event.verificationState == VerificationState.untrusted` - MXLog.warning("[MXEventDecryptionResult] trust not yet implemeneted") - isUntrusted = false + isUntrusted = event.verificationState == VerificationState.untrusted } } diff --git a/MatrixSDK/Crypto/Data/MXMegolmSessionData.m b/MatrixSDK/Crypto/Data/MXMegolmSessionData.m index f2ed48e9cd..56cbdab214 100644 --- a/MatrixSDK/Crypto/Data/MXMegolmSessionData.m +++ b/MatrixSDK/Crypto/Data/MXMegolmSessionData.m @@ -52,9 +52,21 @@ + (id)modelFromJSON:(NSDictionary *)JSONDictionary - (NSDictionary *)JSONDictionary { + if (!_senderKey || !_roomId || !_sessionId || !_sessionKey || !_algorithm) + { + NSDictionary *details = @{ + @"sender_key": _senderKey ?: @"unknown", + @"room_id": _roomId ?: @"unknown", + @"session_id": _sessionId ?: @"unknown", + @"algorithm": _algorithm ?: @"unknown", + }; + MXLogErrorDetails(@"[MXMegolmSessionData] JSONDictionary: some properties are missing", details); + return nil; + } + return @{ @"sender_key": _senderKey, - @"sender_claimed_keys": _senderClaimedKeys, + @"sender_claimed_keys": _senderClaimedKeys ?: @[], @"room_id": _roomId, @"session_id": _sessionId, @"session_key":_sessionKey, diff --git a/MatrixSDK/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngine.swift b/MatrixSDK/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngine.swift index bbccc8e110..4f70231530 100644 --- a/MatrixSDK/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngine.swift +++ b/MatrixSDK/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngine.swift @@ -297,7 +297,7 @@ class MXCryptoKeyBackupEngine: NSObject, MXKeyBackupEngine { do { let result = try backup.importDecryptedKeys(roomKeys: sessions, progressListener: self) - await roomEventDecryptor.retryAllUndecryptedEvents() + await roomEventDecryptor.retryUndecryptedEvents(sessionIds: sessions.map(\.sessionId)) let duration2 = Date().timeIntervalSince(date2) * 1000 log.debug("Successfully imported \(result.imported) out of \(result.total) sessions in \(duration2) ms") @@ -364,7 +364,14 @@ class MXCryptoKeyBackupEngine: NSObject, MXKeyBackupEngine { func importRoomKeys(_ data: Data, passphrase: String) async throws -> KeysImportResult { let result = try backup.importRoomKeys(data, passphrase: passphrase, progressListener: self) - await roomEventDecryptor.retryAllUndecryptedEvents() + let sessionIds = result.keys + .flatMap { (roomId, senders) in + senders.flatMap { (sender, sessionIds) in + sessionIds + } + } + + await roomEventDecryptor.retryUndecryptedEvents(sessionIds: sessionIds) return result } diff --git a/MatrixSDK/Crypto/MXCryptoV2.swift b/MatrixSDK/Crypto/MXCryptoV2.swift index ce5fdf02c9..34ba84aa2c 100644 --- a/MatrixSDK/Crypto/MXCryptoV2.swift +++ b/MatrixSDK/Crypto/MXCryptoV2.swift @@ -386,9 +386,11 @@ private class MXCryptoV2: NSObject, MXCrypto { func handle(_ syncResponse: MXSyncResponse, onComplete: @escaping () -> Void) { let toDeviceCount = syncResponse.toDevice?.events.count ?? 0 + let devicesChanged = syncResponse.deviceLists?.changed?.count ?? 0 + let devicesLeft = syncResponse.deviceLists?.left?.count ?? 0 MXLog.debug("[MXCryptoV2] --------------------------------") - log.debug("Handling new sync response with \(toDeviceCount) to-device event(s)") + log.debug("Handling new sync response with \(toDeviceCount) to-device event(s), \(devicesChanged) device(s) changed, \(devicesLeft) device(s) left") Task(priority: .medium) { do { @@ -447,7 +449,7 @@ private class MXCryptoV2: NSObject, MXCrypto { Task { do { - try await machine.manuallyVerifyDevice(userId: userId, deviceId: deviceId) + try await machine.verifyDevice(userId: userId, deviceId: deviceId) log.debug("Successfully marked device as verified") await MainActor.run { success?() @@ -486,22 +488,16 @@ private class MXCryptoV2: NSObject, MXCrypto { return } - log.debug("Setting user verification status manually") - - Task { - do { - try await machine.manuallyVerifyUser(userId: userId) - log.debug("Successfully marked user as verified") - await MainActor.run { - success?() - } - } catch { - log.error("Failed marking user as verified", context: error) - await MainActor.run { - failure?(error) - } + log.debug("Signing user") + crossSigning.signUser( + withUserId: userId, + success: { + success?() + }, + failure: { + failure?($0) } - } + ) } public func trustLevel(forUser userId: String) -> MXUserTrustLevel { @@ -726,8 +722,7 @@ private class MXCryptoV2: NSObject, MXCrypto { private func getRoomUserIds(for room: MXRoom) async throws -> [String] { return try await room.members()?.members - .compactMap(\.userId) - .filter { $0 != machine.userId } ?? [] + .compactMap(\.userId) ?? [] } private func crossSigningInfo(userIds: [String]) -> [String: MXCrossSigningInfo] { diff --git a/MatrixSDK/MXSession.m b/MatrixSDK/MXSession.m index 54984b3fcc..2f10aa7044 100644 --- a/MatrixSDK/MXSession.m +++ b/MatrixSDK/MXSession.m @@ -2082,7 +2082,7 @@ - (void)handleBackgroundSyncCacheIfRequiredWithCompletion:(void (^)(void))comple { BOOL isInValidState = _state == MXSessionStateStoreDataReady || _state == MXSessionStatePaused; if (!isInValidState) { - NSString *message = [NSString stringWithFormat:@"[MXSession] state %@ is not valid to handle background sync cache, investigate why the method was called", [MXTools readableSessionState:_state]]; + NSString *message = [NSString stringWithFormat:@"[MXSession] handleBackgroundSyncCacheIfRequired: state %@ is not valid to handle background sync cache, investigate why the method was called", [MXTools readableSessionState:_state]]; MXLogFailure(message); if (completion) { @@ -4928,6 +4928,12 @@ - (void)onDidDecryptEvent:(NSNotification *)notification MXRoomSummary *summary = [self roomSummaryWithRoomId:event.roomId]; if (summary) { + if (!summary.lastMessage) + { + [summary resetLastMessage:nil failure:nil commit:YES]; + return; + } + [self eventWithEventId:summary.lastMessage.eventId inRoom:summary.roomId success:^(MXEvent *lastEvent) { diff --git a/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift b/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift index df9bda8f9b..99a7937577 100644 --- a/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift +++ b/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift @@ -137,10 +137,10 @@ class MXRoomEventDecryptionUnitTests: XCTestCase { // MARK: - Retry all - func test_retryAllUndecryptedEvents() async { + func test_retryUndecryptedEvents() async { let events = await prepareEventsForRedecryption() - await roomDecryptor.retryAllUndecryptedEvents() + await roomDecryptor.retryUndecryptedEvents(sessionIds: ["123", "456"]) await waitForDecryption() XCTAssertNotNil(events[0].clear) diff --git a/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoMachineUnitTests.swift b/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoMachineUnitTests.swift index 7214953a3f..450f539d4c 100644 --- a/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoMachineUnitTests.swift +++ b/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoMachineUnitTests.swift @@ -23,13 +23,14 @@ import MatrixSDKCrypto class MXCryptoMachineUnitTests: XCTestCase { + var userId = "@alice:matrix.org" var restClient: MXRestClient! var machine: MXCryptoMachine! override func setUp() { restClient = MXRestClientStub() machine = try! MXCryptoMachine( - userId: "@alice:matrix.org", + userId: userId, deviceId: "ABCD", restClient: restClient, getRoomAction: { @@ -37,6 +38,18 @@ class MXCryptoMachineUnitTests: XCTestCase { }) } + override func tearDown() { + do { + let url = try MXCryptoMachine.storeURL(for: userId) + guard FileManager.default.fileExists(atPath: url.path) else { + return + } + try FileManager.default.removeItem(at: url) + } catch { + XCTFail("Cannot tear down test - \(error)") + } + } + func test_handleSyncResponse_canProcessEmptyResponse() throws { let result = try machine.handleSyncResponse( toDevice: nil, diff --git a/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift b/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift index cae79c1f4a..7330babed9 100644 --- a/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift +++ b/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift @@ -64,10 +64,10 @@ class UserIdentitySourceStub: CryptoIdentityStub, MXCryptoUserIdentitySource { func downloadKeys(users: [String]) async throws { } - func manuallyVerifyUser(userId: String) async throws { + func verifyUser(userId: String) async throws { } - func manuallyVerifyDevice(userId: String, deviceId: String) async throws { + func verifyDevice(userId: String, deviceId: String) async throws { } func setLocalTrust(userId: String, deviceId: String, trust: LocalTrust) throws { @@ -111,10 +111,10 @@ class CryptoCrossSigningStub: CryptoIdentityStub, MXCryptoCrossSigning { func downloadKeys(users: [String]) async throws { } - func manuallyVerifyUser(userId: String) async throws { + func verifyUser(userId: String) async throws { } - func manuallyVerifyDevice(userId: String, deviceId: String) async throws { + func verifyDevice(userId: String, deviceId: String) async throws { } func setLocalTrust(userId: String, deviceId: String, trust: LocalTrust) throws { diff --git a/MatrixSDKTests/Crypto/Data/MXMegolmSessionDataUnitTests.swift b/MatrixSDKTests/Crypto/Data/MXMegolmSessionDataUnitTests.swift index cbbb04308b..45f6ec5f37 100644 --- a/MatrixSDKTests/Crypto/Data/MXMegolmSessionDataUnitTests.swift +++ b/MatrixSDKTests/Crypto/Data/MXMegolmSessionDataUnitTests.swift @@ -71,7 +71,7 @@ class MXMegolmSessionDataUnitTests: XCTestCase { data.algorithm = "G" data.forwardingCurve25519KeyChain = ["H", "I"] - let json = data.jsonDictionary() as NSDictionary + let json = data.jsonDictionary() as? NSDictionary XCTAssertEqual(json, [ "sender_key": "A", @@ -85,4 +85,19 @@ class MXMegolmSessionDataUnitTests: XCTestCase { "untrusted": false ]) } + + func testInvalidJsonDictionary() { + let data = MXMegolmSessionData() + data.senderKey = nil + data.senderClaimedKeys = nil + data.roomId = nil + data.sessionId = nil + data.sessionKey = nil + data.algorithm = nil + data.forwardingCurve25519KeyChain = nil + + let json = data.jsonDictionary() as? NSDictionary + + XCTAssertNil(json) + } } diff --git a/MatrixSDKTests/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngineUnitTests.swift b/MatrixSDKTests/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngineUnitTests.swift index 28af1b5724..6c3ec3a150 100644 --- a/MatrixSDKTests/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngineUnitTests.swift +++ b/MatrixSDKTests/Crypto/KeyBackup/Engine/MXCryptoKeyBackupEngineUnitTests.swift @@ -22,7 +22,7 @@ import Foundation import MatrixSDKCrypto class MXCryptoKeyBackupEngineUnitTests: XCTestCase { - actor DecryptorDummy: MXRoomEventDecrypting { + actor DecryptorSpy: MXRoomEventDecrypting { func decrypt(events: [MXEvent]) -> [MXEventDecryptionResult] { return [] } @@ -30,20 +30,23 @@ class MXCryptoKeyBackupEngineUnitTests: XCTestCase { func handlePossibleRoomKeyEvent(_ event: MXEvent) { } - func retryAllUndecryptedEvents() { + var spySessionIds: [String] = [] + func retryUndecryptedEvents(sessionIds: [String]) { + spySessionIds = sessionIds } func resetUndecryptedEvents() { } } - + var decryptor: DecryptorSpy! var backup: CryptoBackupStub! var engine: MXCryptoKeyBackupEngine! override func setUp() { + decryptor = DecryptorSpy() backup = CryptoBackupStub() - engine = MXCryptoKeyBackupEngine(backup: backup, roomEventDecryptor: DecryptorDummy()) + engine = MXCryptoKeyBackupEngine(backup: backup, roomEventDecryptor: decryptor) } func test_createsBackupKeyFromVersion() { @@ -215,7 +218,11 @@ class MXCryptoKeyBackupEngineUnitTests: XCTestCase { XCTAssertEqual(Set(sessionIds), ["1", "2", "3"]) XCTAssertEqual(Set(roomIds), ["A", "B"]) - exp.fulfill() + Task { + let sessionIds = await self.decryptor.spySessionIds + XCTAssertEqual(Set(sessionIds), ["1", "2", "3"]) + exp.fulfill() + } }, failure: { XCTFail("Importing failed with error \($0)") diff --git a/changelog.d/pr-1630.change b/changelog.d/pr-1630.change new file mode 100644 index 0000000000..24ef74f350 --- /dev/null +++ b/changelog.d/pr-1630.change @@ -0,0 +1,2 @@ +CryptoV2: Bugfixes +