From 94ac3c82fde9dd701b7ca24c35211d583934e71d Mon Sep 17 00:00:00 2001 From: manuroe Date: Thu, 6 Feb 2020 17:46:00 +0100 Subject: [PATCH 1/6] Crypto: Restart broken Olm sessions: Create the test --- MatrixSDKTests/MXCryptoTests.m | 129 +++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/MatrixSDKTests/MXCryptoTests.m b/MatrixSDKTests/MXCryptoTests.m index 35d72efe10..3bc161b677 100644 --- a/MatrixSDKTests/MXCryptoTests.m +++ b/MatrixSDKTests/MXCryptoTests.m @@ -1828,6 +1828,135 @@ - (void)testLateRoomKey }]; } +// Test the restart of broken Olm sessions (https://github.com/vector-im/riot-ios/issues/2129) +// Inspired from https://github.com/poljar/matrix-nio/blob/0.7.1/tests/encryption_test.py#L872 +// +// - Alice & Bob in a e2e room +// - Alice sends a 1st message with a 1st megolm session +// - Store the olm session between A&B devices +// - Alice sends a 2nd message with a 2nd megolm session +// - Simulate Alice using a backup of her OS and make her crypto state like after the first message +// - Alice sends a 3rd message with a 3rd megolm session but a wedged olm session +// +// What Bob must see: +// -> No issue with the 2 first messages +// -> The third event must fail to decrypt at first because Bob the olm session is wedged +// -> This is automatically fixed after SDKs restarted the olm session +- (void)testOlmSessionUnwedging +{ + // - Alice & Bob have messages in a room + [matrixSDKTestsE2EData doE2ETestWithAliceAndBobInARoom:self cryptedBob:YES warnOnUnknowDevices:NO aliceStore:[[MXFileStore alloc] init] bobStore:[[MXFileStore alloc] init] readyToTest:^(MXSession *aliceSession, MXSession *bobSession, NSString *roomId, XCTestExpectation *expectation) { + + // - Alice sends a 1st message with a 1st megolm session + MXRoom *roomFromAlicePOV = [aliceSession roomWithRoomId:roomId]; + [roomFromAlicePOV sendTextMessage:@"0" success:^(NSString *eventId) { + + // - Store the olm session between A&B devices + // Let us pickle our session with bob here so we can later unpickle it + // and wedge our session. + MXOlmSession *olmSession = [aliceSession.crypto.store sessionsWithDevice:bobSession.crypto.deviceCurve25519Key].firstObject; + + // Relaunch Alice + // This forces her to use a new megolm session for sending message "11" + // This will move the olm session ratchet to share this new megolm session + MXSession *aliceSession1 = [[MXSession alloc] initWithMatrixRestClient:aliceSession.matrixRestClient]; + [aliceSession close]; + [aliceSession1 setStore:[[MXFileStore alloc] init] success:^{ + [aliceSession1 start:^{ + aliceSession1.crypto.warnOnUnknowDevices = NO; + + // - Alice sends a 2nd message with a 2nd megolm session + MXRoom *roomFromAlicePOV1 = [aliceSession1 roomWithRoomId:roomId]; + [roomFromAlicePOV1 sendTextMessage:@"11" success:^(NSString *eventId) { + + + // - Simulate Alice using a backup of her OS and make her crypto state like after the first message + // Relaunch again alice + MXSession *aliceSession2 = [[MXSession alloc] initWithMatrixRestClient:aliceSession1.matrixRestClient]; + [aliceSession1 close]; + [aliceSession2 setStore:[[MXFileStore alloc] init] success:^{ + [aliceSession2 start:^{ + aliceSession2.crypto.warnOnUnknowDevices = NO; + + // Let us wedge the session now. Set crypto state like after the first message + [aliceSession2.crypto.store storeSession:olmSession forDevice:bobSession.crypto.deviceCurve25519Key]; + + // - Alice sends a 3rd message with a 3rd megolm session but a wedged olm session + MXRoom *roomFromAlicePOV2 = [aliceSession2 roomWithRoomId:roomId]; + [roomFromAlicePOV2 sendTextMessage:@"222" success:nil failure:^(NSError *error) { + XCTFail(@"Cannot set up intial test conditions - error: %@", error); + [expectation fulfill]; + }]; + } failure:nil]; + } failure:nil]; + + + + } failure:^(NSError *error) { + XCTFail(@"Cannot set up intial test conditions - error: %@", error); + [expectation fulfill]; + }]; + } failure:nil]; + } failure:nil]; + + } failure:^(NSError *error) { + XCTFail(@"Cannot set up intial test conditions - error: %@", error); + [expectation fulfill]; + }]; + + + // What Bob must see: + __block NSUInteger messageCount = 0; + [bobSession listenToEventsOfTypes:@[kMXEventTypeStringRoomMessage, kMXEventTypeStringRoomEncrypted] onEvent:^(MXEvent *event, MXTimelineDirection direction, id customObject) { + + switch (messageCount++) + { + case 0: + case 1: + { + // -> No issue with the 2 first messages + // The 2 first events can be decrypted. They just use different megolm session + XCTAssertTrue(event.isEncrypted); + XCTAssertNotNil(event.clearEvent); + XCTAssertEqual(event.eventType, MXEventTypeRoomMessage); + + break; + } + + case 2: + { + // -> The third event must fail to decrypt at first because Bob the olm session is wedged + XCTAssertTrue(event.isEncrypted); + XCTAssertNil(event.clearEvent); + + observer = [[NSNotificationCenter defaultCenter] addObserverForName:kMXEventDidDecryptNotification object:event queue:[NSOperationQueue mainQueue] usingBlock:^(NSNotification *note) { + + // -> This is automatically fixed after SDKs restarted the olm session + MXEvent *event2 = note.object; + + XCTAssertTrue(event2.isEncrypted); + XCTAssertNotNil(event2.clearEvent); + XCTAssertEqual(event2.eventType, MXEventTypeRoomMessage); + + [expectation fulfill]; + }]; + + if (event.clearEvent) + { + XCTAssert(NO, @"The scenario went wrong. Escape now to avoid to wait forever"); + [expectation fulfill]; + } + + break; + } + + default: + break; + } + }]; + }]; +} + #pragma mark - Tests for reproducing bugs From 4b93c65a20a4cf943422db9965cdcab3c317af9a Mon Sep 17 00:00:00 2001 From: manuroe Date: Thu, 6 Feb 2020 18:05:02 +0100 Subject: [PATCH 2/6] Crypto: Restart broken Olm sessions Implement MSC1719: https://github.com/uhoreg/matrix-doc/blob/olm_unwedging/proposals/1719-olm_unwedging.md --- CHANGES.rst | 1 + MatrixSDK/Crypto/Algorithms/MXEncrypting.h | 21 ++++ .../Algorithms/Megolm/MXMegolmDecryption.m | 1 + .../Algorithms/Megolm/MXMegolmEncryption.m | 104 +++++++++++++++++- .../Crypto/Algorithms/Olm/MXOlmEncryption.m | 11 ++ .../MXIncomingRoomKeyRequestManager.m | 26 ++++- MatrixSDK/Crypto/MXCrypto.m | 84 +++++++++++++- MatrixSDK/Crypto/MXCrypto_Private.h | 10 ++ 8 files changed, 249 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index daa6cbf3f6..75d3cc5aed 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,6 +9,7 @@ Improvements: * MXSession: Add createRoomWithParameters with a MXRoomCreationParameters model class. * MXRoomCreationParameters: Support the initial_state parameter and allow e2e on room creation (vector-im/riot-ios/issues/2943). * MXCrypto: Expose devicesForUser. + * MXCrypto: Restart broken Olm sessions ([MSC1719](https://github.com/matrix-org/matrix-doc/pull/1719)) (vector-im/riot-ios/issues/2129). * MXRoomSummary: Add the trust property to indicate trust in other users and devices in the room (vector-im/riot-ios/issues/2906). * MXStore: Add a method to get related events for a specific event. diff --git a/MatrixSDK/Crypto/Algorithms/MXEncrypting.h b/MatrixSDK/Crypto/Algorithms/MXEncrypting.h index 62a50508e8..dfd625aed5 100644 --- a/MatrixSDK/Crypto/Algorithms/MXEncrypting.h +++ b/MatrixSDK/Crypto/Algorithms/MXEncrypting.h @@ -64,4 +64,25 @@ success:(void (^)(NSObject *sessionInfo))success failure:(void (^)(NSError *error))failure; +/** + Re-shares a session key with devices if the key has already been + sent to them. + + @param sessionId The id of the outbound session to share. + @param userId The id of the user who owns the target device. + @param deviceId he id of the target device. + @param senderKey The key of the originating device for the session. + + @param success A block object called when the operation succeeds. + @param failure A block object called when the operation fails. + + @return a MXHTTPOperation instance. + */ +- (MXHTTPOperation*)reshareKey:(NSString*)sessionId + withUser:(NSString*)userId + andDevice:(NSString*)deviceId + senderKey:(NSString*)senderKey + success:(void (^)(void))success + failure:(void (^)(NSError *error))failure; + @end diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m index 732ed9ef82..98df2a3886 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m @@ -297,6 +297,7 @@ - (MXHTTPOperation*)shareKeysWithDevice:(MXIncomingRoomKeyRequest*)keyRequest operation = [crypto ensureOlmSessionsForDevices:@{ userId: @[deviceInfo] } + force:NO success:^(MXUsersDevicesMap *results) { MXStrongifyAndReturnIfNil(self); diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m index 5415da0047..66525c5dd2 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m @@ -82,6 +82,10 @@ @interface MXMegolmEncryption () // that even if this is non-null, it may not be ready for use (in which // case outboundSession.shareOperation will be non-nill.) MXOutboundSessionInfo *outboundSession; + + // Map of outbound sessions by sessions ID. Used if we need a particular + // session. + NSMutableDictionary *outboundSessions; NSMutableArray *pendingEncryptions; @@ -112,6 +116,7 @@ - (instancetype)initWithCrypto:(MXCrypto *)theCrypto andRoom:(NSString *)theRoom roomId = theRoomId; deviceId = crypto.store.deviceId; + outboundSessions = [NSMutableDictionary dictionary]; pendingEncryptions = [NSMutableArray array]; // Default rotation periods @@ -285,6 +290,7 @@ - (MXHTTPOperation *)ensureOutboundSession:(MXUsersDevicesMap *) if (!session) { outboundSession = session = [self prepareNewSession]; + outboundSessions[outboundSession.sessionId] = outboundSession; } if (session.shareOperation) @@ -372,7 +378,7 @@ - (MXHTTPOperation*)shareKey:(MXOutboundSessionInfo*)session MXHTTPOperation *operation; MXWeakify(self); - operation = [crypto ensureOlmSessionsForDevices:devicesByUser success:^(MXUsersDevicesMap *results) { + operation = [crypto ensureOlmSessionsForDevices:devicesByUser force:NO success:^(MXUsersDevicesMap *results) { MXStrongifyAndReturnIfNil(self); NSLog(@"[MXMegolmEncryption] shareKey: ensureOlmSessionsForDevices result (users: %tu - devices: %tu): %@", results.map.count, results.count, results); @@ -462,6 +468,102 @@ - (MXHTTPOperation*)shareKey:(MXOutboundSessionInfo*)session return operation; } +- (MXHTTPOperation*)reshareKey:(NSString*)sessionId + withUser:(NSString*)userId + andDevice:(NSString*)deviceId + senderKey:(NSString*)senderKey + success:(void (^)(void))success + failure:(void (^)(NSError *error))failure +{ + NSLog(@"[MXMegolmEncryption] reshareKey: %@ to %@:%@", sessionId, userId, deviceId); + + MXDeviceInfo *deviceInfo = [crypto.store deviceWithDeviceId:deviceId forUser:userId]; + if (!deviceInfo) + { + NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Unknown device"); + failure(nil); + return nil; + } + + // Get the chain index of the key we previously sent this device + MXOutboundSessionInfo *obSessionInfo = outboundSessions[sessionId]; + NSNumber *chainIndexNumber = [obSessionInfo.sharedWithDevices objectForDevice:deviceId forUser:userId]; + if (!chainIndexNumber) + { + NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Never share megolm with this device"); + failure(nil); + return nil; + } + + NSUInteger chainIndex = [chainIndexNumber unsignedIntegerValue]; + + MXHTTPOperation *operation; + MXWeakify(self); + operation = [crypto ensureOlmSessionsForDevices:@{ + userId: @[deviceInfo] + } + force:NO + success:^(MXUsersDevicesMap *results) + { + MXStrongifyAndReturnIfNil(self); + + MXOlmSessionResult *olmSessionResult = [results objectForDevice:deviceId forUser:userId]; + if (!olmSessionResult.sessionId) + { + // no session with this device, probably because there + // were no one-time keys. + // + // ensureOlmSessionsForUsers has already done the logging, + // so just skip it. + if (success) + { + success(); + } + return; + } + + MXDeviceInfo *deviceInfo = olmSessionResult.device; + + NSLog(@"[MXMegolmEncryption] reshareKey: sharing keys for session %@|%@:%@ with device %@:%@", senderKey, sessionId, @(chainIndex), userId, deviceId); + + NSDictionary *payload = [self buildKeyForwardingMessage:self->roomId senderKey:senderKey sessionId:sessionId]; + + + MXUsersDevicesMap *contentMap = [[MXUsersDevicesMap alloc] init]; + [contentMap setObject:[self->crypto encryptMessage:payload forDevices:@[deviceInfo]] + forUser:userId andDevice:deviceId]; + + MXHTTPOperation *operation2 = [self->crypto.matrixRestClient sendToDevice:kMXEventTypeStringRoomEncrypted contentMap:contentMap txnId:nil success:success failure:failure]; + [operation mutateTo:operation2]; + + } failure:failure]; + + return operation; +} + +- (NSDictionary*)buildKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId +{ + NSDictionary *key = [crypto.olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId]; + if (key) + { + return @{ + @"type": kMXEventTypeStringRoomForwardedKey, + @"content": @{ + @"algorithm": kMXCryptoMegolmAlgorithm, + @"room_id": roomId, + @"sender_key": senderKey, + @"sender_claimed_ed25519_key": key[@"sender_claimed_ed25519_key"], + @"session_id": sessionId, + @"session_key": key[@"key"], + @"chain_index": key[@"chain_index"], + @"forwarding_curve25519_key_chain": key[@"forwarding_curve25519_key_chain"] + } + }; + } + + return nil; +} + - (void)processPendingEncryptionsInSession:(MXOutboundSessionInfo*)session withError:(NSError*)error { if (session) diff --git a/MatrixSDK/Crypto/Algorithms/Olm/MXOlmEncryption.m b/MatrixSDK/Crypto/Algorithms/Olm/MXOlmEncryption.m index 571380e04a..ca372f439e 100644 --- a/MatrixSDK/Crypto/Algorithms/Olm/MXOlmEncryption.m +++ b/MatrixSDK/Crypto/Algorithms/Olm/MXOlmEncryption.m @@ -122,6 +122,17 @@ - (MXHTTPOperation*)ensureSessionForUsers:(NSArray*)users return operation; } +- (MXHTTPOperation*)reshareKey:(NSString*)sessionId + withUser:(NSString*)userId + andDevice:(NSString*)deviceId + senderKey:(NSString*)senderKey + success:(void (^)(void))success + failure:(void (^)(NSError *error))failure +{ + // No need for olm + return nil; +} + @end #endif diff --git a/MatrixSDK/Crypto/KeySharing/MXIncomingRoomKeyRequestManager.m b/MatrixSDK/Crypto/KeySharing/MXIncomingRoomKeyRequestManager.m index 78f343fdd0..06e17d19af 100644 --- a/MatrixSDK/Crypto/KeySharing/MXIncomingRoomKeyRequestManager.m +++ b/MatrixSDK/Crypto/KeySharing/MXIncomingRoomKeyRequestManager.m @@ -131,9 +131,27 @@ - (void)processReceivedRoomKeyRequest:(MXIncomingRoomKeyRequest*)req if (![userId isEqualToString:crypto.matrixRestClient.credentials.userId]) { - // TODO: determine if we sent this device the keys already: in - // which case we can do so again. - NSLog(@"[MXIncomingRoomKeyRequestManager] Ignoring room key request from other user for now"); + NSString *senderKey, *sessionId; + MXJSONModelSetString(senderKey, body[@"sender_key"]); + MXJSONModelSetString(sessionId, body[@"session_id"]); + + if (!senderKey && !sessionId) + { + return; + } + + id encryptor = [crypto getRoomEncryptor:roomId algorithm:alg]; + if (!encryptor) + { + NSLog(@"[MXIncomingRoomKeyRequestManager] room key request for unknown alg %@ in room %@", alg, roomId); + return; + } + + [encryptor reshareKey:sessionId withUser:userId andDevice:deviceId senderKey:senderKey success:^{ + + } failure:^(NSError *error) { + + }]; return; } @@ -170,7 +188,7 @@ - (void)processReceivedRoomKeyRequest:(MXIncomingRoomKeyRequest*)req NSLog(@"[MXIncomingRoomKeyRequestManager] Already have this key request, ignoring"); return; } - + // Add it to pending key requests [crypto.store storeIncomingRoomKeyRequest:req]; diff --git a/MatrixSDK/Crypto/MXCrypto.m b/MatrixSDK/Crypto/MXCrypto.m index 620ade759b..46c63082b3 100644 --- a/MatrixSDK/Crypto/MXCrypto.m +++ b/MatrixSDK/Crypto/MXCrypto.m @@ -480,6 +480,12 @@ - (MXEventDecryptionResult *)decryptEvent:(MXEvent *)event inTimeline:(NSString* if (error && *error) { NSLog(@"[MXCrypto] decryptEvent: Error for %@: %@\nEvent: %@", event.eventId, *error, event.JSONDictionary); + + if ([(*error).domain isEqualToString:MXDecryptingErrorDomain] + && (*error).code == MXDecryptingErrorBadEncryptedMessageCode) + { + [self markOlmSessionForUnwedgingInEvent:event]; + } } } }); @@ -1675,10 +1681,11 @@ - (MXHTTPOperation*)ensureOlmSessionsForUsers:(NSArray*)users } } - return [self ensureOlmSessionsForDevices:devicesByUser success:success failure:failure]; + return [self ensureOlmSessionsForDevices:devicesByUser force:NO success:success failure:failure]; } - (MXHTTPOperation*)ensureOlmSessionsForDevices:(NSDictionary*>*)devicesByUser + force:(BOOL)force success:(void (^)(MXUsersDevicesMap *results))success failure:(void (^)(NSError *error))failure @@ -1698,7 +1705,7 @@ - (MXHTTPOperation*)ensureOlmSessionsForDevices:(NSDictionary)getRoomEncryptor:(NSString*)roomId algorithm:(NSString*)algorithm +{ + if (![algorithm isEqualToString:kMXCryptoMegolmAlgorithm]) + { + return nil; + } + return roomEncryptors[roomId]; +} + - (NSDictionary*)signObject:(NSDictionary*)object { return @{ @@ -2430,6 +2446,66 @@ - (MXHTTPOperation *)uploadOneTimeKeys:(void (^)(MXKeysUploadResponse *keysUploa }]; } + +#pragma mark Wedged olm sessions + +- (void)markOlmSessionForUnwedgingInEvent:(MXEvent*)event +{ + NSString *sender = event.sender; + NSString *deviceKey, *algorithm; + MXJSONModelSetString(deviceKey, event.content[@"sender_key"]); + MXJSONModelSetString(algorithm, event.content[@"algorithm"]); + + NSLog(@"[MXCrypto] markOlmSessionForUnwedging from %@:%@", sender, deviceKey); + + if (!sender || !deviceKey || !algorithm) + { + return; + } + + // check when we last forced a new session with this device: if we've already done so + // recently, don't do it again. + // @TODO: Manu + + // Establish a new olm session with this device since we're failing to decrypt messages + // on a current session. + MXDeviceInfo *device = [_store deviceWithIdentityKey:deviceKey]; + if (!device) + { + NSLog(@"[MXCrypto] markOlmSessionForUnwedgingInEvent: Couldn't find device for identity key %@: not re-establishing session", deviceKey); + return; + } + + NSLog(@"[MXCrypto] markOlmSessionForUnwedging from %@:%@", sender, device.deviceId); + + NSDictionary *userDevice = @{ + sender: @[device] + }; + [self ensureOlmSessionsForDevices:userDevice force:YES success:^(MXUsersDevicesMap *results) { + + // Now send a blank message on that session so the other side knows about it. + // (The keyshare request is sent in the clear so that won't do) + // We send this first such that, as long as the toDevice messages arrive in the + // same order we sent them, the other end will get this first, set up the new session, + // then get the keyshare request and send the key over this new session (because it + // is the session it has most recently received a message on). + NSDictionary *encryptedContent = [self encryptMessage:@{ + @"type": @"m.dummy" + } + forDevices:@[device]]; + + MXUsersDevicesMap *contentMap = [MXUsersDevicesMap new]; + [contentMap setObject:encryptedContent forUser:sender andDevice:device.deviceId]; + + [self.matrixRestClient sendToDevice:kMXEventTypeStringRoomEncrypted contentMap:contentMap txnId:nil success:nil failure:^(NSError *error) { + NSLog(@"[MXCrypto] markOlmSessionForUnwedgingInEvent: ERROR for sendToDevice: %@", error); + }]; + + } failure:^(NSError *error) { + NSLog(@"[MXCrypto] markOlmSessionForUnwedgingInEvent: ERROR for ensureOlmSessionsForDevices: %@", error); + }]; +} + #endif @end diff --git a/MatrixSDK/Crypto/MXCrypto_Private.h b/MatrixSDK/Crypto/MXCrypto_Private.h index ff2249e581..fbfde71150 100644 --- a/MatrixSDK/Crypto/MXCrypto_Private.h +++ b/MatrixSDK/Crypto/MXCrypto_Private.h @@ -131,6 +131,7 @@ @param failure A block object called when the operation fails. */ - (MXHTTPOperation*)ensureOlmSessionsForDevices:(NSDictionary*>*)devicesByUser + force:(BOOL)force success:(void (^)(MXUsersDevicesMap *results))success failure:(void (^)(NSError *error))failure; @@ -156,6 +157,15 @@ */ - (id)getRoomDecryptor:(NSString*)roomId algorithm:(NSString*)algorithm; +/** + Get the encryptor for a given room and algorithm. + + @param roomId room id for encryptor. + @param algorithm the crypto algorithm. + @return the decryptor. + */ +- (id)getRoomEncryptor:(NSString*)roomId algorithm:(NSString*)algorithm; + /** Sign the given object with our ed25519 key. From f08260021937e8cced1821db2576123087c6bdab Mon Sep 17 00:00:00 2001 From: manuroe Date: Thu, 6 Feb 2020 18:29:15 +0100 Subject: [PATCH 3/6] Crypto: Some factorisation --- .../Algorithms/Megolm/MXMegolmDecryption.m | 25 +----------------- .../Algorithms/Megolm/MXMegolmEncryption.m | 26 ++----------------- MatrixSDK/Crypto/MXCrypto.m | 22 ++++++++++++++++ MatrixSDK/Crypto/MXCrypto_Private.h | 3 +++ 4 files changed, 28 insertions(+), 48 deletions(-) diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m index 98df2a3886..994a83e0e3 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m @@ -324,7 +324,7 @@ - (MXHTTPOperation*)shareKeysWithDevice:(MXIncomingRoomKeyRequest*)keyRequest NSLog(@"[MXMegolmDecryption] shareKeysWithDevice: sharing keys for session %@|%@ with device %@:%@", senderKey, sessionId, userId, deviceId); - NSDictionary *payload = [self buildKeyForwardingMessage:roomId senderKey:senderKey sessionId:sessionId]; + NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:roomId senderKey:senderKey sessionId:sessionId]; MXDeviceInfo *deviceInfo = olmSessionResult.device; @@ -464,29 +464,6 @@ - (void)requestKeysForEvent:(MXEvent*)event } } -- (NSDictionary*)buildKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId -{ - NSDictionary *key = [olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId]; - if (key) - { - return @{ - @"type": kMXEventTypeStringRoomForwardedKey, - @"content": @{ - @"algorithm": kMXCryptoMegolmAlgorithm, - @"room_id": roomId, - @"sender_key": senderKey, - @"sender_claimed_ed25519_key": key[@"sender_claimed_ed25519_key"], - @"session_id": sessionId, - @"session_key": key[@"key"], - @"chain_index": key[@"chain_index"], - @"forwarding_curve25519_key_chain": key[@"forwarding_curve25519_key_chain"] - } - }; - } - - return nil; -} - @end #endif diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m index 66525c5dd2..39df838f53 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m @@ -495,6 +495,7 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId return nil; } + // @TODO: Manu: To use NSUInteger chainIndex = [chainIndexNumber unsignedIntegerValue]; MXHTTPOperation *operation; @@ -526,7 +527,7 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId NSLog(@"[MXMegolmEncryption] reshareKey: sharing keys for session %@|%@:%@ with device %@:%@", senderKey, sessionId, @(chainIndex), userId, deviceId); - NSDictionary *payload = [self buildKeyForwardingMessage:self->roomId senderKey:senderKey sessionId:sessionId]; + NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:self->roomId senderKey:senderKey sessionId:sessionId]; MXUsersDevicesMap *contentMap = [[MXUsersDevicesMap alloc] init]; @@ -541,29 +542,6 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId return operation; } -- (NSDictionary*)buildKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId -{ - NSDictionary *key = [crypto.olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId]; - if (key) - { - return @{ - @"type": kMXEventTypeStringRoomForwardedKey, - @"content": @{ - @"algorithm": kMXCryptoMegolmAlgorithm, - @"room_id": roomId, - @"sender_key": senderKey, - @"sender_claimed_ed25519_key": key[@"sender_claimed_ed25519_key"], - @"session_id": sessionId, - @"session_key": key[@"key"], - @"chain_index": key[@"chain_index"], - @"forwarding_curve25519_key_chain": key[@"forwarding_curve25519_key_chain"] - } - }; - } - - return nil; -} - - (void)processPendingEncryptionsInSession:(MXOutboundSessionInfo*)session withError:(NSError*)error { if (session) diff --git a/MatrixSDK/Crypto/MXCrypto.m b/MatrixSDK/Crypto/MXCrypto.m index 46c63082b3..77f977df91 100644 --- a/MatrixSDK/Crypto/MXCrypto.m +++ b/MatrixSDK/Crypto/MXCrypto.m @@ -1941,6 +1941,28 @@ - (void)cancelRoomKeyRequest:(NSDictionary*)requestBody [outgoingRoomKeyRequestManager cancelRoomKeyRequest:requestBody]; } +- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId +{ + NSDictionary *key = [self.olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId]; + if (key) + { + return @{ + @"type": kMXEventTypeStringRoomForwardedKey, + @"content": @{ + @"algorithm": kMXCryptoMegolmAlgorithm, + @"room_id": roomId, + @"sender_key": senderKey, + @"sender_claimed_ed25519_key": key[@"sender_claimed_ed25519_key"], + @"session_id": sessionId, + @"session_key": key[@"key"], + @"chain_index": key[@"chain_index"], + @"forwarding_curve25519_key_chain": key[@"forwarding_curve25519_key_chain"] + } + }; + } + + return nil; +} #pragma mark - Private methods /** diff --git a/MatrixSDK/Crypto/MXCrypto_Private.h b/MatrixSDK/Crypto/MXCrypto_Private.h index fbfde71150..5477036081 100644 --- a/MatrixSDK/Crypto/MXCrypto_Private.h +++ b/MatrixSDK/Crypto/MXCrypto_Private.h @@ -209,6 +209,9 @@ */ - (void)cancelRoomKeyRequest:(NSDictionary*)requestBody; +// Create a message to forward a megolm session +- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId; + @end #endif From cae0a72779aaa5a6c38fc9201db1faa9af6570d3 Mon Sep 17 00:00:00 2001 From: manuroe Date: Thu, 6 Feb 2020 18:34:57 +0100 Subject: [PATCH 4/6] MXRealCryptoStore: Fix log --- .../Crypto/Data/Store/MXRealmCryptoStore/MXRealmCryptoStore.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MatrixSDK/Crypto/Data/Store/MXRealmCryptoStore/MXRealmCryptoStore.m b/MatrixSDK/Crypto/Data/Store/MXRealmCryptoStore/MXRealmCryptoStore.m index 6763f6fc44..a57fefc92b 100644 --- a/MatrixSDK/Crypto/Data/Store/MXRealmCryptoStore/MXRealmCryptoStore.m +++ b/MatrixSDK/Crypto/Data/Store/MXRealmCryptoStore/MXRealmCryptoStore.m @@ -656,7 +656,7 @@ - (MXRealmRoomAlgorithm *)realmRoomAlgorithmForRoom:(NSString*)roomId inRealm:(R - (void)storeSession:(MXOlmSession*)session forDevice:(NSString*)deviceKey { - BOOL isNew = NO; + __block BOOL isNew = NO; NSDate *startDate = [NSDate date]; RLMRealm *realm = self.realm; @@ -671,6 +671,7 @@ - (void)storeSession:(MXOlmSession*)session forDevice:(NSString*)deviceKey else { // Create it + isNew = YES; realmOlmSession = [[MXRealmOlmSession alloc] initWithValue:@{ @"sessionId": session.session.sessionIdentifier, @"deviceKey": deviceKey, From fffef5eb11eccd0936560e6cee812ce769137807 Mon Sep 17 00:00:00 2001 From: manuroe Date: Fri, 7 Feb 2020 09:21:53 +0100 Subject: [PATCH 5/6] MXCrypto: Share megolm keys from the right chain index --- .../Crypto/Algorithms/Megolm/MXMegolmDecryption.m | 2 +- .../Crypto/Algorithms/Megolm/MXMegolmEncryption.m | 11 ++++------- MatrixSDK/Crypto/MXCrypto.m | 4 ++-- MatrixSDK/Crypto/MXCrypto_Private.h | 2 +- MatrixSDK/Crypto/MXOlmDevice.h | 4 +++- MatrixSDK/Crypto/MXOlmDevice.m | 12 ++++++++---- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m index 994a83e0e3..a2cc7f0d93 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmDecryption.m @@ -324,7 +324,7 @@ - (MXHTTPOperation*)shareKeysWithDevice:(MXIncomingRoomKeyRequest*)keyRequest NSLog(@"[MXMegolmDecryption] shareKeysWithDevice: sharing keys for session %@|%@ with device %@:%@", senderKey, sessionId, userId, deviceId); - NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:roomId senderKey:senderKey sessionId:sessionId]; + NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:roomId senderKey:senderKey sessionId:sessionId chainIndex:nil]; MXDeviceInfo *deviceInfo = olmSessionResult.device; diff --git a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m index 39df838f53..d204af37ea 100644 --- a/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m +++ b/MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m @@ -487,16 +487,13 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId // Get the chain index of the key we previously sent this device MXOutboundSessionInfo *obSessionInfo = outboundSessions[sessionId]; - NSNumber *chainIndexNumber = [obSessionInfo.sharedWithDevices objectForDevice:deviceId forUser:userId]; - if (!chainIndexNumber) + NSNumber *chainIndex = [obSessionInfo.sharedWithDevices objectForDevice:deviceId forUser:userId]; + if (!chainIndex) { NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Never share megolm with this device"); failure(nil); return nil; } - - // @TODO: Manu: To use - NSUInteger chainIndex = [chainIndexNumber unsignedIntegerValue]; MXHTTPOperation *operation; MXWeakify(self); @@ -525,9 +522,9 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId MXDeviceInfo *deviceInfo = olmSessionResult.device; - NSLog(@"[MXMegolmEncryption] reshareKey: sharing keys for session %@|%@:%@ with device %@:%@", senderKey, sessionId, @(chainIndex), userId, deviceId); + NSLog(@"[MXMegolmEncryption] reshareKey: sharing keys for session %@|%@:%@ with device %@:%@", senderKey, sessionId, chainIndex, userId, deviceId); - NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:self->roomId senderKey:senderKey sessionId:sessionId]; + NSDictionary *payload = [self->crypto buildMegolmKeyForwardingMessage:self->roomId senderKey:senderKey sessionId:sessionId chainIndex:chainIndex]; MXUsersDevicesMap *contentMap = [[MXUsersDevicesMap alloc] init]; diff --git a/MatrixSDK/Crypto/MXCrypto.m b/MatrixSDK/Crypto/MXCrypto.m index 77f977df91..8ab278ade5 100644 --- a/MatrixSDK/Crypto/MXCrypto.m +++ b/MatrixSDK/Crypto/MXCrypto.m @@ -1941,9 +1941,9 @@ - (void)cancelRoomKeyRequest:(NSDictionary*)requestBody [outgoingRoomKeyRequestManager cancelRoomKeyRequest:requestBody]; } -- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId +- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId chainIndex:(NSNumber*)chainIndex { - NSDictionary *key = [self.olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId]; + NSDictionary *key = [self.olmDevice getInboundGroupSessionKey:roomId senderKey:senderKey sessionId:sessionId chainIndex:chainIndex]; if (key) { return @{ diff --git a/MatrixSDK/Crypto/MXCrypto_Private.h b/MatrixSDK/Crypto/MXCrypto_Private.h index 5477036081..5dcc41e7f6 100644 --- a/MatrixSDK/Crypto/MXCrypto_Private.h +++ b/MatrixSDK/Crypto/MXCrypto_Private.h @@ -210,7 +210,7 @@ - (void)cancelRoomKeyRequest:(NSDictionary*)requestBody; // Create a message to forward a megolm session -- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId; +- (NSDictionary*)buildMegolmKeyForwardingMessage:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId chainIndex:(NSNumber*)chainIndex; @end diff --git a/MatrixSDK/Crypto/MXOlmDevice.h b/MatrixSDK/Crypto/MXOlmDevice.h index ced9ea06b7..ae558e5335 100644 --- a/MatrixSDK/Crypto/MXOlmDevice.h +++ b/MatrixSDK/Crypto/MXOlmDevice.h @@ -278,6 +278,8 @@ Determine if an incoming messages is a prekey message matching an existing sessi @param roomId the room in which the message was received. @param senderKey the base64-encoded curve25519 key of the sender. @param sessionId the session identifier. + @param chainIndex The chain index at which to export the session. + If nil, export at the first index we know about. @return a dictinary { chain_index: number, @@ -286,7 +288,7 @@ Determine if an incoming messages is a prekey message matching an existing sessi sender_claimed_ed25519_key: string } details of the session key. The key is a base64-encoded megolm key in export format. */ -- (NSDictionary*)getInboundGroupSessionKey:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId; +- (NSDictionary*)getInboundGroupSessionKey:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId chainIndex:(NSNumber*)chainIndex; #pragma mark - Utilities diff --git a/MatrixSDK/Crypto/MXOlmDevice.m b/MatrixSDK/Crypto/MXOlmDevice.m index af8d694545..25f9c53027 100644 --- a/MatrixSDK/Crypto/MXOlmDevice.m +++ b/MatrixSDK/Crypto/MXOlmDevice.m @@ -542,7 +542,7 @@ - (BOOL)hasInboundSessionKeys:(NSString*)roomId senderKey:(NSString*)senderKey s return YES; } -- (NSDictionary*)getInboundGroupSessionKey:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId +- (NSDictionary*)getInboundGroupSessionKey:(NSString*)roomId senderKey:(NSString*)senderKey sessionId:(NSString*)sessionId chainIndex:(NSNumber*)chainIndex { NSDictionary *inboundGroupSessionKey; @@ -550,16 +550,20 @@ - (NSDictionary*)getInboundGroupSessionKey:(NSString*)roomId senderKey:(NSString MXOlmInboundGroupSession *session = [self inboundGroupSessionWithId:sessionId senderKey:senderKey roomId:roomId error:&error]; if (session) { - NSUInteger messageIndex = session.session.firstKnownIndex; + NSNumber *messageIndex = chainIndex; + if (!messageIndex) + { + messageIndex = @(session.session.firstKnownIndex); + } NSDictionary *claimedKeys = session.keysClaimed; NSString *senderEd25519Key = claimedKeys[@"ed25519"]; - MXMegolmSessionData *sessionData = [session exportSessionDataAtMessageIndex:messageIndex]; + MXMegolmSessionData *sessionData = [session exportSessionDataAtMessageIndex:[messageIndex unsignedIntegerValue]]; NSArray *forwardingCurve25519KeyChain = sessionData.forwardingCurve25519KeyChain; inboundGroupSessionKey = @{ - @"chain_index": @(messageIndex), + @"chain_index": messageIndex, @"key": sessionData.sessionKey, @"forwarding_curve25519_key_chain": forwardingCurve25519KeyChain ? forwardingCurve25519KeyChain : @[], @"sender_claimed_ed25519_key": senderEd25519Key ? senderEd25519Key : [NSNull null] From e09dde62f7702d1c81717849d06e5a108beda689 Mon Sep 17 00:00:00 2001 From: manuroe Date: Fri, 7 Feb 2020 15:56:29 +0100 Subject: [PATCH 6/6] MXCrypto: Rate limit reset of olm sessions to one every hour max --- MatrixSDK/Crypto/MXCrypto.m | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/MatrixSDK/Crypto/MXCrypto.m b/MatrixSDK/Crypto/MXCrypto.m index 8ab278ade5..a8164b7f78 100644 --- a/MatrixSDK/Crypto/MXCrypto.m +++ b/MatrixSDK/Crypto/MXCrypto.m @@ -54,6 +54,7 @@ // Frequency with which to check & upload one-time keys NSTimeInterval kMXCryptoUploadOneTimeKeysPeriod = 60.0; // one minute +NSTimeInterval kMXCryptoMinForceSessionPeriod = 3600.0; // one hour @interface MXCrypto () { @@ -89,6 +90,10 @@ @interface MXCrypto () // The manager for incoming room key requests MXIncomingRoomKeyRequestManager *incomingRoomKeyRequestManager; + + // The date of the last time we forced establishment + // of a new session for each user:device. + MXUsersDevicesMap *lastNewSessionForcedDates; } @end @@ -1549,6 +1554,8 @@ - (instancetype)initWithMatrixSession:(MXSession*)matrixSession cryptoQueue:(dis _crossSigning = [[MXCrossSigning alloc] initWithCrypto:self]; + lastNewSessionForcedDates = [MXUsersDevicesMap new]; + [self registerEventHandlers]; } @@ -2485,9 +2492,14 @@ - (void)markOlmSessionForUnwedgingInEvent:(MXEvent*)event return; } - // check when we last forced a new session with this device: if we've already done so + // Check when we last forced a new session with this device: if we've already done so // recently, don't do it again. - // @TODO: Manu + NSDate *lastNewSessionForcedDate = [lastNewSessionForcedDates objectForDevice:deviceKey forUser:sender]; + if ([lastNewSessionForcedDate timeIntervalSinceNow] < -kMXCryptoMinForceSessionPeriod) + { + NSLog(@"[MXCrypto] markOlmSessionForUnwedging: New session already forced with device at %@. Not forcing another", lastNewSessionForcedDate); + return; + } // Establish a new olm session with this device since we're failing to decrypt messages // on a current session. @@ -2500,6 +2512,8 @@ - (void)markOlmSessionForUnwedgingInEvent:(MXEvent*)event NSLog(@"[MXCrypto] markOlmSessionForUnwedging from %@:%@", sender, device.deviceId); + [lastNewSessionForcedDates setObject:[NSDate date] forUser:sender andDevice:deviceKey]; + NSDictionary *userDevice = @{ sender: @[device] };