Merge branch 'release/0.26.8/master'

matrix-org · Apr 18, 2023 · a12a559 · a12a559
2 parents 79e3d20 + a61844a
commit a12a559
Show file tree

Hide file tree

Showing 20 changed files with 194 additions and 53 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,12 @@
+## Changes in 0.26.8 (2023-04-18)
+
+🙌 Improvements
+
+- Crypto: Update Crypto SDK ([#1767](https://github.com/matrix-org/matrix-ios-sdk/pull/1767))
+- Cross-signing: Ensure device signed after restoring cross-signing keys ([#1768](https://github.com/matrix-org/matrix-ios-sdk/pull/1768))
+- Crypto: Remove legacy crypto store ([#1769](https://github.com/matrix-org/matrix-ios-sdk/pull/1769))
+
+
 ## Changes in 0.26.7 (2023-04-12)
 
 🙌 Improvements

diff --git a/MatrixSDK.podspec b/MatrixSDK.podspec
@@ -1,7 +1,7 @@
 Pod::Spec.new do |s|
 
   s.name         = "MatrixSDK"
-  s.version      = "0.26.7"
+  s.version      = "0.26.8"
   s.summary      = "The iOS SDK to build apps compatible with Matrix (https://www.matrix.org)"
 
   s.description  = <<-DESC
@@ -45,7 +45,7 @@ Pod::Spec.new do |s|
       ss.dependency 'OLMKit', '~> 3.2.5'
       ss.dependency 'Realm', '10.27.0'
       ss.dependency 'libbase58', '~> 0.1.4'
-      ss.dependency 'MatrixSDKCrypto', '0.3.3', :configurations => ["DEBUG", "RELEASE"], :inhibit_warnings => true
+      ss.dependency 'MatrixSDKCrypto', '0.3.4', :configurations => ["DEBUG", "RELEASE"], :inhibit_warnings => true
   end
 
   s.subspec 'JingleCallStack' do |ss|

diff --git a/MatrixSDK/Crypto/CrossSigning/MXCrossSigning.h b/MatrixSDK/Crypto/CrossSigning/MXCrossSigning.h
@@ -137,11 +137,13 @@ typedef NS_ENUM(NSInteger, MXCrossSigningErrorCode)
  The operation requires to have the Self Signing Key in the local secret storage.
 
  @param deviceId the id of the device to cross-sign.
+ @param userId the user that owns the device.
 
  @param success A block object called when the operation succeeds.
  @param failure A block object called when the operation fails.
  */
 - (void)crossSignDeviceWithDeviceId:(NSString*)deviceId
+                             userId:(NSString*)userId
                             success:(void (^)(void))success
                             failure:(void (^)(NSError *error))failure;
 

diff --git a/MatrixSDK/Crypto/CrossSigning/MXCrossSigning.m b/MatrixSDK/Crypto/CrossSigning/MXCrossSigning.m
@@ -142,7 +142,7 @@ - (void)setupWithAuthParams:(NSDictionary*)authParams
             // Refresh our state so that we can cross-sign
             [self refreshStateWithSuccess:^(BOOL stateUpdated) {
                 // Expose this device to other users as signed by me
-                [self crossSignDeviceWithDeviceId:myCreds.deviceId success:^{
+                [self crossSignDeviceWithDeviceId:myCreds.deviceId userId:myCreds.userId success:^{
                     success();
                 } failure:failureBlock];
             } failure:failureBlock];
@@ -218,6 +218,7 @@ - (MXCrossSigningInfo *)createKeys:(NSDictionary<NSString *,NSData *> *__autorel
 }
 
 - (void)crossSignDeviceWithDeviceId:(NSString*)deviceId
+                            userId:(NSString *)userId
                             success:(void (^)(void))success
                             failure:(void (^)(NSError *error))failure
 {

diff --git a/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift b/MatrixSDK/Crypto/CrossSigning/MXCrossSigningV2.swift
@@ -144,10 +144,17 @@ class MXCrossSigningV2: NSObject, MXCrossSigning {
 
     func crossSignDevice(
         withDeviceId deviceId: String,
+        userId: String,
         success: @escaping () -> Void,
         failure: @escaping (Swift.Error) -> Void
     ) {
-        log.debug("->")
+        log.debug("Attempting to cross sign a device \(deviceId)")
+
+        if let device = crossSigning.device(userId: userId, deviceId: deviceId), device.crossSigningTrusted {
+            log.debug("Device is already cross-signing trusted, no need to verify")
+            success()
+            return
+        }
 
         Task {
             do {

diff --git a/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift b/MatrixSDK/Crypto/CryptoMachine/MXCryptoProtocols.swift
@@ -82,7 +82,7 @@ protocol MXCryptoRoomEventDecrypting: MXCryptoIdentity {
 }
 
 /// Cross-signing functionality
-protocol MXCryptoCrossSigning: MXCryptoUserIdentitySource {
+protocol MXCryptoCrossSigning: MXCryptoUserIdentitySource, MXCryptoDevicesSource {
     func refreshCrossSigningStatus() async throws
     func crossSigningStatus() -> CrossSigningStatus
     func bootstrapCrossSigning(authParams: [AnyHashable: Any]) async throws

diff --git a/MatrixSDK/Crypto/MXCrypto.m b/MatrixSDK/Crypto/MXCrypto.m
@@ -1092,7 +1092,7 @@ - (void)setDeviceVerification2:(MXDeviceVerification)verificationStatus forDevic
             {
                 // Cross-sign our own device
                 MXLogDebug(@"[MXCrypto] setDeviceVerificationForDevice: Mark device %@ as self verified", deviceId);
-                [self.crossSigning crossSignDeviceWithDeviceId:deviceId success:success failure:failure];
+                [self.crossSigning crossSignDeviceWithDeviceId:deviceId userId:userId success:success failure:failure];
 
                 // Wait the end of cross-sign before returning
                 return;

diff --git a/MatrixSDK/Crypto/MXCryptoV2.swift b/MatrixSDK/Crypto/MXCryptoV2.swift
@@ -197,13 +197,6 @@ class MXCryptoV2: NSObject, MXCrypto {
         }
 
         if deleteStore {
-            if let credentials = session?.credentials,
-               MXRealmCryptoStore.hasData(for: credentials) {
-                MXRealmCryptoStore.delete(with: credentials)
-            } else {
-                log.failure("Missing credentials, cannot delete store")
-            }
-
             do {
                 try machine.deleteAllData()
             } catch {

diff --git a/MatrixSDK/Crypto/MXCryptoV2Factory.swift b/MatrixSDK/Crypto/MXCryptoV2Factory.swift
@@ -91,6 +91,9 @@ import Foundation
 
         log.debug("Legacy crypto store exists")
         try migrateIfNecessary(legacyStore: legacyStore, updateProgress: updateProgress)
+
+        log.debug("Removing legacy crypto store entirely")
+        MXRealmCryptoStore.delete(with: credentials)
     }
 
     private func migrateIfNecessary(
@@ -123,8 +126,5 @@ import Foundation
             log.debug("Needs verification upgrade")
             MXSDKOptions.sharedInstance().cryptoSDKFeature?.needsVerificationUpgrade = true
         }
-
-        log.debug("Setting the latest deprecated version of legacy store")
-        legacyStore.cryptoVersion = lastDeprecatedVersion
     }
 }
diff --git a/MatrixSDK/Crypto/Recovery/MXRecoveryService.m b/MatrixSDK/Crypto/Recovery/MXRecoveryService.m
@@ -740,19 +740,14 @@ - (void)recoverCrossSigningWithSuccess:(void (^)(void))success
 
     [self.dependencies.crossSigning refreshStateWithSuccess:^(BOOL stateUpdated) {
 
-        // Check if the service really needs to be started
-        if (self.dependencies.crossSigning.canCrossSign)
-        {
-            MXLogDebug(@"[MXRecoveryService] recoverCrossSigning: Cross-signing is already up");
-            success();
-            return;
-        }
+        NSString *userId = self.dependencies.credentials.userId;
+        NSString *deviceId = self.dependencies.credentials.deviceId;
 
         // Mark our user MSK as verified locally
-        [self.delegate setUserVerification:YES forUser:self.dependencies.credentials.userId success:^{
+        [self.delegate setUserVerification:YES forUser:userId success:^{
 
             // Cross sign our current device
-            [self.dependencies.crossSigning crossSignDeviceWithDeviceId:self.dependencies.credentials.deviceId success:^{
+            [self.dependencies.crossSigning crossSignDeviceWithDeviceId:deviceId userId:userId success:^{
 
                 // And update the state
                 [self.dependencies.crossSigning refreshStateWithSuccess:^(BOOL stateUpdated) {

diff --git a/MatrixSDK/MatrixSDKVersion.m b/MatrixSDK/MatrixSDKVersion.m
@@ -16,4 +16,4 @@
 
 #import <Foundation/Foundation.h>
 
-NSString *const MatrixSDKVersion = @"0.26.7";
+NSString *const MatrixSDKVersion = @"0.26.8";
diff --git a/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift b/MatrixSDKTests/Crypto/Algorithms/RoomEvents/MXRoomEventDecryptionUnitTests.swift
@@ -102,7 +102,9 @@ class MXRoomEventDecryptionUnitTests: XCTestCase {
         let invalidEvent = MXEvent.fixture(id: 123)
 
         await decryptor.handlePossibleRoomKeyEvent(invalidEvent)
-        await waitForDecryption()
+        // We do not expect anything to be decrypted, so there is no notification or other signal to listen to
+        // We will simply wait an entire second and assert nothing was decrypted
+        try! await Task.sleep(nanoseconds: 1_000_000_000)
 
         XCTAssertNil(events[0].clear)
         XCTAssertNil(events[1].clear)
@@ -114,7 +116,7 @@ class MXRoomEventDecryptionUnitTests: XCTestCase {
         let roomKey = MXEvent.roomKeyFixture(sessionId: "123")
 
         await decryptor.handlePossibleRoomKeyEvent(roomKey)
-        await waitForDecryption()
+        await waitForDecryption(events: events)
 
         XCTAssertNotNil(events[0].clear)
         XCTAssertNil(events[1].clear)
@@ -126,7 +128,7 @@ class MXRoomEventDecryptionUnitTests: XCTestCase {
         let roomKey = MXEvent.forwardedRoomKeyFixture(sessionId: "123")
 
         await decryptor.handlePossibleRoomKeyEvent(roomKey)
-        await waitForDecryption()
+        await waitForDecryption(events: events)
 
         XCTAssertNotNil(events[0].clear)
         XCTAssertNil(events[1].clear)
@@ -139,7 +141,7 @@ class MXRoomEventDecryptionUnitTests: XCTestCase {
         let events = await prepareEventsForRedecryption()
 
         await decryptor.retryUndecryptedEvents(sessionIds: ["123", "456"])
-        await waitForDecryption()
+        await waitForDecryption(events: events)
 
         XCTAssertNotNil(events[0].clear)
         XCTAssertNotNil(events[1].clear)
@@ -188,10 +190,21 @@ class MXRoomEventDecryptionUnitTests: XCTestCase {
         return events
     }
 
-    private func waitForDecryption() async {
+    private func waitForDecryption(events: [MXEvent]) async {
         // When decrypting successfully, a notification will be triggered on the main thread, so we have to
         // make sure we wait until this happens. We cannot listen to notifications directly, because
         // repeated decryption failures will not trigger any. Instead simply wait a little while.
-        try! await Task.sleep(nanoseconds: 1_000_000)
+
+        // Maximum 100 attempts each pausing for a tenth of a second
+        for _ in 0 ..< 100 {
+
+            // As soon as at least one event is decrypted, we assume all are and return
+            if events.contains(where: { $0.clear != nil }) {
+                return
+            }
+
+            // Otherwise wait for a 0.1 second and run the next loop cycle
+            try! await Task.sleep(nanoseconds: 100_000_000)
+        }
     }
 }
diff --git a/MatrixSDKTests/Crypto/CrossSigning/MXCrossSigningV2UnitTests.swift b/MatrixSDKTests/Crypto/CrossSigning/MXCrossSigningV2UnitTests.swift
@@ -103,4 +103,60 @@ class MXCrossSigningV2UnitTests: XCTestCase {
             XCTAssertEqual(self.crossSigning.state, state, "Status: \(status)")
         }
     }
+
+    func test_crossSignDevice_verifiesUntrustedDevice() async throws {
+        let userId = "Alice"
+        let deviceId = "ABCD"
+        crypto.devices = [
+            userId: [
+                deviceId: .stub(crossSigningTrusted: false)
+            ]
+        ]
+
+        let before = crypto.device(userId: userId, deviceId: deviceId)
+        XCTAssertNotNil(before)
+        XCTAssertFalse(before!.crossSigningTrusted)
+        XCTAssertFalse(crypto.verifiedDevicesSpy.contains(deviceId))
+
+        try await crossSigning.crossSignDevice(deviceId: deviceId, userId: userId)
+
+        let after = crypto.device(userId: userId, deviceId: deviceId)
+        XCTAssertNotNil(after)
+        XCTAssertTrue(after!.crossSigningTrusted)
+        XCTAssertTrue(crypto.verifiedDevicesSpy.contains(deviceId))
+    }
+
+    func test_crossSignDevice_doesNotReverifyAlreadyTrustedDevice() async throws {
+        let userId = "Alice"
+        let deviceId = "ABCD"
+        crypto.devices = [
+            userId: [
+                deviceId: .stub(crossSigningTrusted: true)
+            ]
+        ]
+
+        let before = crypto.device(userId: userId, deviceId: deviceId)
+        XCTAssertNotNil(before)
+        XCTAssertTrue(before!.crossSigningTrusted)
+        XCTAssertFalse(crypto.verifiedDevicesSpy.contains(deviceId))
+
+        try await crossSigning.crossSignDevice(deviceId: deviceId, userId: userId)
+
+        let after = crypto.device(userId: userId, deviceId: deviceId)
+        XCTAssertNotNil(after)
+        XCTAssertTrue(after!.crossSigningTrusted)
+        XCTAssertFalse(crypto.verifiedDevicesSpy.contains(deviceId))
+    }
+}
+
+private extension MXCrossSigningV2 {
+    func crossSignDevice(deviceId: String, userId: String) async throws {
+        return try await withCheckedThrowingContinuation { continuation in
+            self.crossSignDevice(withDeviceId: deviceId, userId: userId) {
+                continuation.resume()
+            } failure: { error in
+                continuation.resume(throwing: error)
+            }
+        }
+    }
 }
diff --git a/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift b/MatrixSDKTests/Crypto/CryptoMachine/MXCryptoProtocolStubs.swift
@@ -72,6 +72,10 @@ class UserIdentitySourceStub: CryptoIdentityStub, MXCryptoUserIdentitySource {
 }
 
 class CryptoCrossSigningStub: CryptoIdentityStub, MXCryptoCrossSigning {
+    enum Error: Swift.Error {
+        case deviceMissing
+    }
+
     var stubbedStatus = CrossSigningStatus(
         hasMaster: false,
         hasSelfSigning: false,
@@ -115,11 +119,39 @@ class CryptoCrossSigningStub: CryptoIdentityStub, MXCryptoCrossSigning {
     func verifyUser(userId: String) async throws {
     }
 
+    var verifiedDevicesSpy = Set<String>()
     func verifyDevice(userId: String, deviceId: String) async throws {
+        guard let device = devices[userId]?[deviceId] else {
+            throw Error.deviceMissing
+        }
+
+        verifiedDevicesSpy.insert(deviceId)
+
+        devices[userId]?[deviceId] = Device(
+            userId: device.userId,
+            deviceId: device.deviceId,
+            keys: device.keys,
+            algorithms: device.algorithms,
+            displayName: device.displayName,
+            isBlocked: device.isBlocked,
+            locallyTrusted: device.locallyTrusted,
+            // Modify cross signing trusted
+            crossSigningTrusted: true
+        )
     }
 
     func setLocalTrust(userId: String, deviceId: String, trust: LocalTrust) throws {
     }
+
+    var devices = [String: [String: Device]]()
+
+    func device(userId: String, deviceId: String) -> Device? {
+        return devices[userId]?[deviceId]
+    }
+
+    func devices(userId: String) -> [Device] {
+        return devices[userId]?.map { $0.value } ?? []
+    }
 }
 
 class CryptoVerificationStub: CryptoIdentityStub {

diff --git a/MatrixSDKTests/Crypto/CryptoMachine/MXKeysQuerySchedulerUnitTests.swift b/MatrixSDKTests/Crypto/CryptoMachine/MXKeysQuerySchedulerUnitTests.swift
@@ -56,13 +56,17 @@ class MXKeysQuerySchedulerUnitTests: XCTestCase {
             }
 
             queryStartSpy?()
-            try await Task.sleep(nanoseconds: 1_000_000)
+
+            // Wait long enough to simulate expensive work
+            try await Task.sleep(nanoseconds: 100_000_000)
 
             return res
 
         case .failure(let error):
             queryStartSpy?()
-            try await Task.sleep(nanoseconds: 1_000_000)
+
+            // Wait long enough to simulate expensive work
+            try await Task.sleep(nanoseconds: 100_000_000)
             throw error
         }
     }
@@ -504,6 +508,6 @@ class MXKeysQuerySchedulerUnitTests: XCTestCase {
     // MARK: - Helpers
 
     private func XCTAssertQueriesCount(_ count: Int, file: StaticString = #file, line: UInt = #line) {
-        XCTAssertEqual(count, queryCounter, file: file, line: line)
+        XCTAssertEqual(queryCounter, count, file: file, line: line)
     }
 }