Skip to content

Commit b960b87

Browse files
6.4.19-b2 (#1587)
- Fixed endless loop adding/removing devices from omemo devicelist - Some more small fixes/improvements
2 parents 1f77096 + 93722f3 commit b960b87

File tree

4 files changed

+51
-8
lines changed

4 files changed

+51
-8
lines changed

Monal/Classes/MLCall.m

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ -(void) offerSDP
899899
self.localSDP = sdpIQ;
900900
}
901901
[self.account sendIq:sdpIQ withResponseHandler:^(XMPPIQ* result) {
902-
DDLogDebug(@"Received SDP response for offer: %@", result);
902+
DDLogDebug(@"Received response iq for SDP offer: %@", result);
903903
} andErrorHandler:^(XMPPIQ* error) {
904904
DDLogError(@"Got error for SDP offer: %@", error);
905905
}];
@@ -1741,7 +1741,11 @@ -(void) processIncomingSDP:(NSNotification*) notification
17411741
@"action": @"session-accept",
17421742
@"sid": self.jmiid,
17431743
} andChildren:children andData:nil]];
1744-
[self.account send:sdpIQ];
1744+
[self.account sendIq:sdpIQ withResponseHandler:^(XMPPIQ* result) {
1745+
DDLogDebug(@"Received response iq for SDP asnwer: %@", result);
1746+
} andErrorHandler:^(XMPPIQ* error) {
1747+
DDLogError(@"Got error for SDP answer: %@", error);
1748+
}];
17451749

17461750
@synchronized(self.candidateQueueLock) {
17471751
self.localSDP = sdpIQ;

Monal/Classes/MLIQProcessor.m

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ +(void) processResultIq:(XMPPIQ*) iqNode forAccount:(xmpp*) account
173173
//WARNING: be careful adding stateless result handlers here (those can impose security risks!)
174174

175175
DDLogWarn(@"Got unhandled result IQ: %@", iqNode);
176-
[self respondWithErrorTo:iqNode onAccount:account];
177176
}
178177

179178
+(void) processErrorIq:(XMPPIQ*) iqNode forAccount:(xmpp*) account

Monal/Classes/MLOMEMO.m

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ -(void) repairQueuedSessions
286286
[self checkBundleFetchCount];
287287

288288
DDLogVerbose(@"New state: %@", self.state);
289+
290+
[self cleanupOwnOldDevices];
289291
}
290292

291293
-(void) retriggerKeyTransportElementsForJid:(NSString*) jid
@@ -308,6 +310,8 @@ -(void) retriggerKeyTransportElementsForJid:(NSString*) jid
308310
[self.state.queuedKeyTransportElements removeObjectForKey:jid];
309311
[self sendKeyTransportElement:jid forRids:rids];
310312
}
313+
314+
[self cleanupOwnOldDevices];
311315
}
312316

313317
$$instance_handler(devicelistHandler, account.omemo, $$ID(xmpp*, account), $$ID(NSString*, node), $$ID(NSString*, jid), $$ID(NSString*, type), $_ID((NSDictionary<NSString*, MLXMLNode*>*), data))
@@ -562,8 +566,30 @@ -(void) handleOwnDevicelistUpdate:(NSSet<NSNumber*>*) receivedDevices
562566

563567
-(void) cleanupOwnOldDevices
564568
{
565-
NSMutableArray* deletedDevices = [NSMutableArray new];
566569
NSString* jid = self.account.connectionProperties.identity.jid;
570+
571+
//don't try to clean up in all these cases
572+
@synchronized(self.state.queuedKeyTransportElements) {
573+
if(self.state.queuedKeyTransportElements[jid] != nil && [self.state.queuedKeyTransportElements[jid] count] > 0)
574+
{
575+
DDLogWarn(@"Not cleaning up own old devices: key transport elements still queued!");
576+
return;
577+
}
578+
}
579+
@synchronized(self.state.queuedSessionRepairs) {
580+
if(self.state.queuedSessionRepairs[jid] != nil && [self.state.queuedSessionRepairs[jid] count] > 0)
581+
{
582+
DDLogWarn(@"Not cleaning up own old devices: session repairs still queued!");
583+
return;
584+
}
585+
}
586+
if(self.state.openBundleFetches[jid] != nil && self.state.openBundleFetches[jid].count > 0)
587+
{
588+
DDLogWarn(@"Not cleaning up own old devices: bundle fetches still pending!");
589+
return;
590+
}
591+
592+
NSMutableArray* deletedDevices = [NSMutableArray new];
567593
for(NSNumber* device in [self.ownDeviceList copy])
568594
{
569595
SignalAddress* address = [[SignalAddress alloc] initWithName:jid deviceId:(uint32_t)device.unsignedIntValue];
@@ -580,13 +606,17 @@ -(void) cleanupOwnOldDevices
580606
//this is what conversations does, too
581607
if(trust == MLOmemoToFUButNoMsgSeenInTime || trust == MLOmemoTrustedButNoMsgSeenInTime)
582608
{
609+
DDLogWarn(@"Device %@ is old, trust=%d --> deleting...", device, trust);
583610
[self bulkDeleteDeviceForSource:jid andRid:device];
584611
[deletedDevices addObject:device];
585612
}
586613
}
587614
//publish our changes, but only if we actually changed someting
588615
if(deletedDevices.count > 0)
616+
{
617+
DDLogInfo(@"Deleted at least one device in our own devicelist, publishing the cleaned up devicelist now...");
589618
[self publishOwnDeviceList];
619+
}
590620
}
591621

592622
-(void) publishOwnDeviceList
@@ -1430,8 +1460,10 @@ -(NSString* _Nullable) decryptOmemoEnvelope:(MLXMLNode*) envelope forSenderJid:(
14301460
}
14311461

14321462
//make sure the dh ratchet always advances, even on "receive only" devices
1433-
//--> force a key transport message with 1% probability (~ every 100th message)
1434-
if(arc4random_uniform(100)==42)
1463+
//--> force a key transport message with 1% probability
1464+
//that means after 100 messages there is a 87% probability we sent at least one key transport message
1465+
//and after 200 messages the probability for at least one key transport message is 98% (its 99,9% after 341 messages)
1466+
if(arc4random_uniform(50)==42)
14351467
[self sendKeyTransportElement:senderJid forRids:[NSSet setWithArray:@[sid]]];
14361468

14371469
//some clients have the auth parameter in the ciphertext?

Monal/Classes/MLXMPPManager.m

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,16 @@ -(void) connectAccountWithDictionary:(NSDictionary*) account
497497
}
498498
DDLogInfo(@"existing account, calling unfreeze");
499499
[existing unfreeze];
500-
DDLogInfo(@"existing account, just pinging.");
501-
[existing sendPing:SHORT_PING]; //short ping timeout to quickly check if connectivity is still okay
500+
if(existing.accountState<kStateReconnecting)
501+
{
502+
DDLogInfo(@"disconnected existing account, reconnecting.");
503+
[existing connect];
504+
}
505+
else
506+
{
507+
DDLogInfo(@"connected(?) existing account, just pinging.");
508+
[existing sendPing:SHORT_PING]; //short ping timeout to quickly check if connectivity is still okay
509+
}
502510
return;
503511
}
504512
DDLogVerbose(@"connecting account %@@%@",[account objectForKey:kUsername], [account objectForKey:kDomain]);

0 commit comments

Comments
 (0)