6.4.3-rc4 (#1195)

- Improve PLAIN-only warning message on first login

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)

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);

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)
     {

Monal/Classes/xmpp.m

@@ -2934,22 +2934,24 @@ -(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
         //or display a message pointing to the advanced account creation menu if sasl2 is pinned and login was NOT successful at least once
         //(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!");