Merge pull request #998 from matrix-org/3807_keys_not_sent_workaround

Workaround for users we did not send keys to

CHANGES.rst

@@ -9,6 +9,7 @@ Changes to be released in next version
 
 🐛 Bugfix
  * MXBackgroundSyncService: Clear the bg sync crypto db if needed (vector-im/element-ios/issues/3956).
+ * MXCrypto: Add a workaround when the megolm key is not shared to all members (vector-im/element-ios/issues/3907).
 
 ⚠️ API Changes
  * 

MatrixSDK/Crypto/Algorithms/Megolm/MXMegolmEncryption.m

@@ -456,7 +456,10 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId
  if (!deviceInfo)
  {
  NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Unknown device");
- failure(nil);
+ NSError *error = [NSError errorWithDomain:MXEncryptingErrorDomain
+ code:MXEncryptingErrorUnknownDeviceCode
+ userInfo:nil];
+ failure(error);
  return nil;
  }
 
@@ -465,8 +468,11 @@ - (MXHTTPOperation*)reshareKey:(NSString*)sessionId
  NSNumber *chainIndex = [obSessionInfo.sharedWithDevices objectForDevice:deviceId forUser:userId];
  if (!chainIndex)
  {
- NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Never share megolm with this device");
- failure(nil);
+ NSLog(@"[MXMegolmEncryption] reshareKey: ERROR: Never shared megolm key with this device");
+ NSError *error = [NSError errorWithDomain:MXEncryptingErrorDomain
+ code:MXEncryptingErrorReshareNotAllowedCode
+ userInfo:nil];
+ failure(error);
  return nil;
  }
 

MatrixSDK/Crypto/Data/MXCryptoConstants.h

@@ -41,7 +41,8 @@ FOUNDATION_EXPORT NSString *const MXEncryptingErrorDomain;
 typedef enum : NSUInteger
 {
  // Note: The list of unknown devices is passed into the MXEncryptingErrorUnknownDeviceDevicesKey key in userInfo
- MXEncryptingErrorUnknownDeviceCode
+ MXEncryptingErrorUnknownDeviceCode,
+ MXEncryptingErrorReshareNotAllowedCode
 } MXEncryptingErrorCode;
 
 FOUNDATION_EXPORT NSString* const MXEncryptingErrorUnknownDeviceReason;

MatrixSDK/Crypto/KeySharing/MXIncomingRoomKeyRequestManager.m

@@ -22,6 +22,10 @@
 
 #ifdef MX_CRYPTO
 
+
+NSTimeInterval kFixMissingUserInRoomRateLimit = 3600;
+
+
 @interface MXIncomingRoomKeyRequestManager ()
 {
  __weak MXCrypto *crypto;
@@ -30,6 +34,10 @@ @interface MXIncomingRoomKeyRequestManager ()
  // we received in the current sync.
  NSMutableArray<MXIncomingRoomKeyRequest*> *receivedRoomKeyRequests;
  NSMutableArray<MXIncomingRoomKeyRequestCancellation*> *receivedRoomKeyRequestCancellations;
+
+ // The list of rooms we fixed in the fixMissingUser:inRoom: method
+ // roomId -> Date of the last fix
+ NSMutableDictionary<NSString*, NSDate*> *roomsFixedForMissingUser;
 }
 
 @end
@@ -47,6 +55,8 @@ - (instancetype)initWithCrypto:(MXCrypto*)theCrypto
  // we received in the current sync.
  receivedRoomKeyRequests = [NSMutableArray array];
  receivedRoomKeyRequestCancellations = [NSMutableArray array];
+
+ roomsFixedForMissingUser = [NSMutableDictionary dictionary];
  }
  return self;
 }
@@ -150,7 +160,14 @@ - (void)processReceivedRoomKeyRequest:(MXIncomingRoomKeyRequest*)req
  [encryptor reshareKey:sessionId withUser:userId andDevice:deviceId senderKey:senderKey success:^{
 
  } failure:^(NSError *error) {
+ NSLog(@"[MXIncomingRoomKeyRequestManager] reshareKey failed. Error: %@", error);
 
+ if ([error.domain isEqualToString:MXEncryptingErrorDomain]
+ && (error.code == MXEncryptingErrorUnknownDeviceCode
+ || error.code == MXEncryptingErrorReshareNotAllowedCode))
+ {
+ [self fixMissingUser:userId inRoom:roomId];
+ }
  }];
  return;
  }
@@ -251,6 +268,45 @@ - (void)removePendingKeyRequest:(NSString*)requestId fromUser:(NSString*)userId
  return [crypto.store incomingRoomKeyRequests];
 }
 
+/**
+ Reset the flag that indicates that all room members in a room have been loaded.
+ 
+ @param userId the if of the user that failed to get the key.
+ @param roomid the room id.
+ */
+- (void)fixMissingUser:(NSString *)userId inRoom:(NSString *)roomId
+{
+ // TODO: Remove this method once the root issue is fixed
+
+ // This is a workaround for https:/vector-im/element-ios/issues/3807
+ // where the SDK seems to have a bad view of current members in a room. This make it "forget" to send
+ // the megolm key to all other users.
+
+ // If a user has this issue, their app will send a re-share request.
+ // The request will be rejected but this is the good time to attempt to reset the flag that indicates
+ // that all room members in the room have been loaded.
+
+ // On the next message encryption, the SDK will fetch all members again from the server and will share better the key.
+ // Next message should be decryptable for others.
+
+ // Rate limit the reset to 1h or one life cycle
+ NSDate *lastFixDate = roomsFixedForMissingUser[roomId];
+ if (lastFixDate
+ && [[NSDate date] timeIntervalSinceDate:lastFixDate] < kFixMissingUserInRoomRateLimit)
+ {
+ // To early to retry
+ NSLog(@"[MXIncomingRoomKeyRequestManager] fixMissingUser: %@ inRoom: %@ already requested at %@", userId, roomId, lastFixDate);
+ return;
+ }
+
+ NSLog(@"[MXIncomingRoomKeyRequestManager] fixMissingUser: %@ inRoom: %@", userId, roomId);
+ roomsFixedForMissingUser[roomId] = [NSDate date];
+
+ // Reset the flag
+ // This is ugly. We need to remove this workaround as soon as possible
+ [crypto.mxSession.store storeHasLoadedAllRoomMembersForRoom:roomId andValue:NO];
+}
+
 @end
 
 #endif

MatrixSDK/Data/MXRoom.m

@@ -214,6 +214,8 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  lazyLoadedMembers:(void (^)(MXRoomMembers *lazyLoadedMembers))lazyLoadedMembers
  failure:(void (^)(NSError *error))failure
 {
+ NSLog(@"[MXRoom] members: roomId: %@", _roomId);
+
  // Create an empty operation that will be mutated later
  MXHTTPOperation *operation = [[MXHTTPOperation alloc] init];
 
@@ -224,13 +226,19 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  // Return directly liveTimeline.state.members if we have already all of them
  if ([self.mxSession.store hasLoadedAllRoomMembersForRoom:self.roomId])
  {
+ NSLog(@"[MXRoom] members: All members are known. Return %@ joined, %@ invited",
+ @(liveTimeline.state.membersCount.joined), @(liveTimeline.state.membersCount.invited));
  success(liveTimeline.state.members);
  }
  else
  {
+ NSLog(@"[MXRoom] members: Currently known members: %@ joined, %@ invited",
+ @(liveTimeline.state.membersCount.joined), @(liveTimeline.state.membersCount.invited));
+
  // Return already lazy-loaded room members if requested
  if (lazyLoadedMembers)
  {
+ NSLog(@"[MXRoom] members: Call lazyLoadedMembers");
  lazyLoadedMembers(liveTimeline.state.members);
  }
 
@@ -249,13 +257,17 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  kMXMembersOfRoomParametersNotMembership: kMXMembershipStringLeave
  };
  }
+
+ NSLog(@"[MXRoom] members: Call /members with parameters: %@", parameters);
 
  MXWeakify(self);
  MXHTTPOperation *operation2 = [self.mxSession.matrixRestClient membersOfRoom:self.roomId
  withParameters:parameters
  success:^(NSArray *roomMemberEvents)
  {
  MXStrongifyAndReturnIfNil(self);
+
+ NSLog(@"[MXRoom] members: roomId: %@. /members returned %@ members", self.roomId, @(roomMemberEvents.count));
 
  // Manage the possible race condition where we could have received
  // update of members from the events stream (/sync) while the /members
@@ -289,6 +301,10 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  }
  }
 
+ NSLog(@"[MXRoom] members: roomId: %@. /members succeeded. Pending requesters: %@. Members: %@ joined, %@ invited ",
+ self.roomId, @(self->pendingMembersRequesters.count),
+ @(liveTimeline.state.membersCount.joined), @(liveTimeline.state.membersCount.invited));
+
  // Provide the members to pending requesters
  NSArray<void (^)(MXRoomMembers *)> *pendingMembersRequesters = [self->pendingMembersRequesters copy];
  self->pendingMembersRequesters = nil;
@@ -298,9 +314,12 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  {
  onRequesterComplete(liveTimeline.state.members);
  }
- NSLog(@"[MXRoom] members loaded. Pending requesters: %@", @(pendingMembersRequesters.count));
 
  } failure:^(NSError *error) {
+ MXStrongifyAndReturnIfNil(self);
+
+ NSLog(@"[MXRoom] members: roomId: %@. /members failed. Pending requesters: %@", self.roomId, @(self->pendingMembersFailureBlocks.count));
+
  // Notify the failure to the pending requesters
  NSArray<void (^)(NSError *)> *pendingRequesters = [self->pendingMembersFailureBlocks copy];
  self->pendingMembersRequesters = nil;
@@ -310,11 +329,14 @@ - (MXHTTPOperation*)members:(void (^)(MXRoomMembers *members))success
  {
  onFailure(error);
  }
- NSLog(@"[MXRoom] get members failed. Pending requesters: %@", @(pendingRequesters.count));
  }];
 
  [operation mutateTo:operation2];
  }
+ else
+ {
+ NSLog(@"[MXRoom] members: Request already pending for %@ requesters", @(self->pendingMembersRequesters.count));
+ }
 
  if (success)
  {