From 9b0d5a0cfd32b57397f4aaf1a6593a80bd6c6b4c Mon Sep 17 00:00:00 2001 From: Thilo Molitor Date: Mon, 5 Aug 2024 02:00:28 +0200 Subject: [PATCH] Allow PLAIN auth after account registration --- Monal/Classes/MLXMPPManager.m | 20 ++++++++++++++------ Monal/Classes/RegisterAccount.swift | 9 ++++++++- Monal/Classes/XMPPEdit.m | 3 ++- Monal/Classes/xmpp.m | 13 ++++++++++--- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Monal/Classes/MLXMPPManager.m b/Monal/Classes/MLXMPPManager.m index c316366767..1aa9fa13a6 100644 --- a/Monal/Classes/MLXMPPManager.m +++ b/Monal/Classes/MLXMPPManager.m @@ -706,16 +706,16 @@ -(void) sendChatState:(BOOL) isTyping toContact:(MLContact*) contact //this will NOT set plain_activated to YES, only using the advanced account creation ui can do this -(NSNumber*) login:(NSString*) jid password:(NSString*) password { - //if it is a JID + //check if it is a JID NSArray* elements = [jid componentsSeparatedByString:@"@"]; MLAssert([elements count] > 1, @"Got invalid jid", (@{@"jid": nilWrapper(jid), @"elements": elements})); NSString* domain; NSString* user; - user = [elements objectAtIndex:0]; - domain = [elements objectAtIndex:1]; + user = ((NSString*)[elements objectAtIndex:0]).lowercaseString; + domain = ((NSString*)[elements objectAtIndex:1]).lowercaseString; - if([[DataLayer sharedInstance] doesAccountExistUser:user.lowercaseString andDomain:domain.lowercaseString]) + if([[DataLayer sharedInstance] doesAccountExistUser:user andDomain:domain]) { [[MLNotificationQueue currentQueue] postNotificationName:kXMPPError object:nil userInfo:@{ @"title": NSLocalizedString(@"Duplicate Account", @""), @@ -725,11 +725,19 @@ -(NSNumber*) login:(NSString*) jid password:(NSString*) password } NSMutableDictionary* dic = [NSMutableDictionary new]; - [dic setObject:domain.lowercaseString forKey:kDomain]; - [dic setObject:user.lowercaseString forKey:kUsername]; + [dic setObject:domain forKey:kDomain]; + [dic setObject:user forKey:kUsername]; [dic setObject:[HelperTools encodeRandomResource] forKey:kResource]; [dic setObject:@YES forKey:kEnabled]; [dic setObject:@NO forKey:kDirectTLS]; + //we don't want to set kPlainActivated (not even according to our preload list) and default to plain_activated=false, + //because the error message will warn the user and direct them to the advanced account creation menu to activate PLAIN + //if they still want to connect to this server + //only exception: yax.im --> we don't want to suggest a server during account creation that has a scary warning + //when logging in using another device afterwards + //TODO: to be removed once yax.im supports SASL2 and SSDP!! + //TODO: use preload list and allow PLAIN for all others once enough domains are on this list + [dic setObject:([domain isEqualToString:@"yax.im"] ? @YES : @NO) forKey:kPlainActivated]; NSNumber* accountNo = [[DataLayer sharedInstance] addAccountWithDictionary:dic]; if(accountNo == nil) diff --git a/Monal/Classes/RegisterAccount.swift b/Monal/Classes/RegisterAccount.swift index 6a29d81238..70cfc3c9a5 100644 --- a/Monal/Classes/RegisterAccount.swift +++ b/Monal/Classes/RegisterAccount.swift @@ -201,7 +201,14 @@ struct RegisterAccount: View { kUsername: self.username, kResource: HelperTools.encodeRandomResource(), kEnabled: true, - kDirectTLS: false + kDirectTLS: false, + //creating an account involves transfering the password in cleartext only secured by TLS + //--> logging in directly afterwards using PLAIN doesn't make the situation any worse ==> allow it + //conversations.im already supports sasl2 and scram ## TODO: use SCRAM preload list + //using the preload list in this case won't solve the situation, but increase the attack cost because + //stripping off SASL2 won't suffice anymore (the attacker will have to use the password sniffed during account creation + //to fake the SCRAM HMAC sent to both client and server) + kPlainActivated: self.actualServer == "conversations.im" ? false : true, ] as [String : Any] let accountNo = DataLayer.sharedInstance().addAccount(with: dic); diff --git a/Monal/Classes/XMPPEdit.m b/Monal/Classes/XMPPEdit.m index 5643fc753f..92b0b585e6 100644 --- a/Monal/Classes/XMPPEdit.m +++ b/Monal/Classes/XMPPEdit.m @@ -339,7 +339,8 @@ -(IBAction) save:(id) sender if(self.statusMessage) [dic setObject:[self.statusMessage stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]] forKey:@"statusMessage"]; - [dic setObject:[NSNumber numberWithBool:self.plainActivated] forKey:kPlainActivated]; + //conversations.im already supports sasl2 and scram ## TODO: use SCRAM preload list + [dic setObject:([domain.lowercaseString isEqualToString:@"conversations.im"] ? @NO : @(self.plainActivated)) forKey:kPlainActivated]; if(!self.editMode) { diff --git a/Monal/Classes/xmpp.m b/Monal/Classes/xmpp.m index 3ca7727354..79f930b923 100644 --- a/Monal/Classes/xmpp.m +++ b/Monal/Classes/xmpp.m @@ -2934,7 +2934,7 @@ -(void) handleFeaturesBeforeAuth:(MLXMLNode*) parsedStanza withForceSasl2:(BOOL) }; //called below, if neither SASL1 nor SASL2 could be used to negotiate a valid SASL mechanism monal_void_block_t noAuthSupported = ^{ - DDLogWarn(@"No supported auth mechanism!"); + DDLogWarn(@"No supported auth mechanism: %@", self->_supportedSaslMechanisms); //sasl2 will be pinned if we saw sasl2 support and PLAIN was NOT allowed by creating this account using the advanced account creation menu //display scary warning message if sasl2 is pinned and login was successful at least once @@ -2942,14 +2942,16 @@ -(void) handleFeaturesBeforeAuth:(MLXMLNode*) parsedStanza withForceSasl2:(BOOL) //(e.g. we are trying to create this account just now) if(![[DataLayer sharedInstance] isPlainActivatedForAccount:self.accountNo]) { + DDLogDebug(@"Plain is not activated for this account..."); if(self->_loggedInOnce) { clearPipelineCacheOrReportSevereError(NSLocalizedString(@"Server suddenly lacks support for SASL2-SCRAM, ongoing MITM attack highly likely, aborting authentication and disabling account to limit damage. You should try to reenable your account once you are in a clean networking environment again.", @"")); return; } - else if([self->_supportedSaslMechanisms containsObject:@"PLAIN"]) + //_supportedSaslMechanisms==nil indicates SASL1 support only + else if([self->_supportedSaslMechanisms containsObject:@"PLAIN"] || self->_supportedSaslMechanisms == nil) { - clearPipelineCacheOrReportSevereError(NSLocalizedString(@"Server only supports authentication methods not safe against man-in-the-middle attacks! Use the advanced account creation menu if you absolutely must use this server.", @"")); + clearPipelineCacheOrReportSevereError(NSLocalizedString(@"This server isn't additionally hardened against man-in-the-middle attacks on the TLS encryption layer by using authentication methods that are secure against such attacks! This indicates an ongoing attack if the server is supposed to support SASL2 and SCRAM and is harmless otherwise. Use the advanced account creation menu and turn on the PLAIN switch there if you still want to log in to this server.", @"")); return; } } @@ -2979,6 +2981,8 @@ -(void) handleFeaturesBeforeAuth:(MLXMLNode*) parsedStanza withForceSasl2:(BOOL) //prefer SASL2 over SASL1 else if([parsedStanza check:@"{urn:xmpp:sasl:2}authentication/mechanism"] && (![[DataLayer sharedInstance] isPlainActivatedForAccount:self.accountNo] || forceSasl2)) { + DDLogDebug(@"Trying SASL2..."); + weakify(self); _blockToCallOnTCPOpen = ^{ strongify(self); @@ -3075,6 +3079,7 @@ -(void) handleFeaturesBeforeAuth:(MLXMLNode*) parsedStanza withForceSasl2:(BOOL) //SASL1 is fallback only if SASL2 isn't supported with something better than PLAIN else if([parsedStanza check:@"{urn:ietf:params:xml:ns:xmpp-sasl}mechanisms/mechanism"] && [[DataLayer sharedInstance] isPlainActivatedForAccount:self.accountNo]) { + DDLogDebug(@"Trying SASL1..."); //extract menchanisms presented NSSet* supportedSaslMechanisms = [NSSet setWithArray:[parsedStanza find:@"{urn:ietf:params:xml:ns:xmpp-sasl}mechanisms/mechanism#"]]; @@ -3116,6 +3121,8 @@ -(void) handleFeaturesBeforeAuth:(MLXMLNode*) parsedStanza withForceSasl2:(BOOL) } else { + DDLogDebug(@"Neither SASL2 nor SASL1 worked..."); + //this is not a downgrade but something weird going on, log it as such if(![parsedStanza check:@"{urn:xmpp:sasl:2}authentication/mechanism"] && ![parsedStanza check:@"{urn:ietf:params:xml:ns:xmpp-sasl}mechanisms/mechanism"]) DDLogError(@"Something weird happened: neither SASL1 nor SASL2 auth supported by this server!");