Bug 126975 - Refuse to LOGIN when hasCapability("LOGINDISABLED")
Summary: Refuse to LOGIN when hasCapability("LOGINDISABLED")
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: imap (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-08 19:08 UTC by Magnus Holmgren
Modified: 2007-09-14 12:17 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Abort login if disabled (1.06 KB, patch)
2006-05-08 20:22 UTC, Magnus Holmgren
Details
Abort login if disabled (1.06 KB, patch)
2006-09-20 14:56 UTC, Magnus Holmgren
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Holmgren 2006-05-08 19:08:48 UTC
Version:            (using KDE KDE 3.5.2)
Installed from:    Debian testing/unstable Packages

When the server response to CAPABILITY includes LOGINDISABLED, LOGIN should not be attempted. LOGINDISABLED is often sent when the connection is unencrypted and TLS has not yet been negotiated, and neglecting it in that situation will cause the password to be sent in the clear (to no avail).

If TLS hasn't been enabled for some reason, or LOGINDISABLED is present for any other reason, and AUTHENTICATE (SASL) isn't used, then IMAP4Protocol::makeLogin() should return false before imapParser::clientLogin() is called.

(It can be noted that before AUTHENTICATE is used, the capabilities string is checked for AUTH=<mechanism>. Checking that LOGINDISABLED isn't there before using LOGIN is analogous.)
Comment 1 Magnus Holmgren 2006-05-08 19:21:44 UTC
In addition, RFC 3501 section 7.2.1 states:

An IMAP client MUST NOT issue the LOGIN command if the server advertises the  LOGINDISABLED capability.
Comment 2 Magnus Holmgren 2006-05-08 20:22:22 UTC
Created attachment 15983 [details]
Abort login if disabled

And here is my proposed, but yet-to-be-tested solution.
Comment 3 Carsten Burghardt 2006-05-10 22:52:24 UTC
Good idea but this can only be included in kde 4 as it introduces a new 
string.
Comment 4 Magnus Holmgren 2006-09-18 17:00:53 UTC
Just one more reminder and request for a core developer to check this bug and patch out (and in) before the string unfreeze ends today, please.
Comment 5 Allen Winter 2006-09-18 18:19:09 UTC
SVN commit 586077 by winterz:

Fix for "Refuse to LOGIN when hasCapability("LOGINDISABLED")"

Patch provided by Magnus, and approved by Carsten.
And also Till says it looks ok too.

BUGS: 126975


 M  +33 -25    imap4.cc  


--- branches/KDE/3.5/kdepim/kioslaves/imap4/imap4.cc #586076:586077
@@ -152,7 +152,7 @@
 #ifdef HAVE_LIBSASL2
   sasl_done();
 #endif
-  
+
   return 0;
 }
 
@@ -747,7 +747,7 @@
       }
 
       readBufferLen -= copyLen;
-      if (readBufferLen) 
+      if (readBufferLen)
         memmove(readBuffer, &readBuffer[copyLen], readBufferLen);
       if (buffer[buffer.size() - 1] == '\n') return TRUE;
     }
@@ -945,10 +945,10 @@
   if (type == ITYPE_BOX)
   {
     bool ask = ( aInfo.find( "ASKUSER" ) != -1 );
-    if ( ask && 
+    if ( ask &&
         messageBox(QuestionYesNo,
           i18n("The following folder will be created on the server: %1 "
-               "What do you want to store in this folder?").arg( aBox ), 
+               "What do you want to store in this folder?").arg( aBox ),
           i18n("Create Folder"),
           i18n("&Messages"), i18n("&Subfolders")) == KMessageBox::No )
     {
@@ -1212,7 +1212,7 @@
  * No-op: data = 'N'
  * Namespace: data = 'n'. Result shipped in infoMessage() signal
  *                        The format is: section=namespace=delimiter
- *                        Note that the namespace can be empty               
+ *                        Note that the namespace can be empty
  * Unsubscribe: data = 'U' + URL (KURL)
  * Subscribe: data = 'u' + URL (KURL)
  * Change the status: data = 'S' + URL (KURL) + Flags (QCString)
@@ -1264,7 +1264,7 @@
     break;
   }
   case 'n':
-  { 
+  {
     // namespace in the form "section=namespace=delimiter"
     // entries are separated by ,
     infoMessage( imapNamespaces.join(",") );
@@ -1313,7 +1313,7 @@
     finished();
     break;
   }
-  case 'A': 
+  case 'A':
   {
     // acl
     int cmd;
@@ -1325,7 +1325,7 @@
     }
     break;
   }
-  case 'M': 
+  case 'M':
   {
     // annotatemore
     int cmd;
@@ -1337,7 +1337,7 @@
     }
     break;
   }
-  case 'S': 
+  case 'S':
   {
     // status
     KURL _url;
@@ -1374,7 +1374,7 @@
     finished();
     break;
   }
-  case 'E': 
+  case 'E':
   {
     // search
     specialSearchCommand( stream );
@@ -1665,7 +1665,7 @@
   QString aBox, aSequence, aLType, aSection, aValidity, aDelimiter, aInfo;
   // parseURL with caching
   enum IMAP_TYPE aType =
-    parseURL (_url, aBox, aSection, aLType, aSequence, aValidity, aDelimiter, 
+    parseURL (_url, aBox, aSection, aLType, aSequence, aValidity, aDelimiter,
         aInfo, true);
 
   UDSEntry entry;
@@ -1924,16 +1924,24 @@
       } else completeQueue.removeRef(cmd);
     }
 
-    if (!myAuth.isEmpty () && myAuth != "*"
-        && !hasCapability (QString ("AUTH=") + myAuth))
-    {
-      error (ERR_COULD_NOT_LOGIN, i18n("The authentication method %1 is not "
-        "supported by the server.").arg(myAuth));
-      closeConnection();
-      return false;
+    if ( myAuth.isEmpty () || myAuth == "*" ) {
+      if ( hasCapability( QString( "LOGINDISABLED" ) ) ) {
+        error( ERR_COULD_NOT_LOGIN, i18n("LOGIN is disabled by the server.") );
+        closeConnection();
+        return false;
+      }
+      else {
+        if ( !hasCapability( QString( "AUTH=" ) + myAuth ) ) {
+          error( ERR_COULD_NOT_LOGIN,
+                 i18n("The authentication method %1 is not "
+                      "supported by the server.").arg( myAuth ) );
+          closeConnection();
+          return false;
+        }
+      }
     }
 
-    if (  greeting.contains(  QRegExp(  "Cyrus IMAP4 v2.1" ) ) ) {
+    if ( greeting.contains(  QRegExp(  "Cyrus IMAP4 v2.1" ) ) ) {
       removeCapability( "ANNOTATEMORE" );
     }
 
@@ -1961,7 +1969,7 @@
     }
     else
     {
-#ifdef HAVE_LIBSASL2      
+#ifdef HAVE_LIBSASL2
       if (!clientAuthenticate (this, authInfo, myHost, myAuth, mySSL, resultInfo))
         error(KIO::ERR_COULD_NOT_LOGIN, i18n("Unable to authenticate via %1.\n"
 	"The server %2 replied:\n%3").arg(myAuth).arg(myHost).arg(resultInfo));
@@ -2002,12 +2010,12 @@
       if ( it != listResponses.end() )
       {
         namespaceToDelimiter[QString::null] = (*it).hierarchyDelimiter();
-        kdDebug(7116) << "makeLogin - delimiter for empty ns='" << 
+        kdDebug(7116) << "makeLogin - delimiter for empty ns='" <<
           (*it).hierarchyDelimiter() << "'" << endl;
         if ( !hasCapability("NAMESPACE") )
         {
           // server does not support namespaces
-          QString nsentry = QString::number( 0 ) + "==" 
+          QString nsentry = QString::number( 0 ) + "=="
             + (*it).hierarchyDelimiter();
           imapNamespaces.append( nsentry );
         }
@@ -2336,7 +2344,7 @@
               }
             }
             // if we got no list response for the box see if it's a prefix
-            if ( retVal == ITYPE_UNKNOWN && 
+            if ( retVal == ITYPE_UNKNOWN &&
                  namespaceToDelimiter.contains(_box) ) {
               retVal = ITYPE_DIR;
             }
@@ -2354,7 +2362,7 @@
     else
       kdDebug(7116) << "IMAP4::parseURL: no login!" << endl;
 
-  } 
+  }
   else // empty box
   {
     // the root is just a dir
@@ -2383,7 +2391,7 @@
   if ( _hierarchyDelimiter.isEmpty() &&
        (_type == "LIST" || _type == "LSUB" || _type == "LSUBNOCHECK") )
   {
-    // this shouldn't happen but when the delimiter is really empty 
+    // this shouldn't happen but when the delimiter is really empty
     // we try to reconstruct it from the URL
     if (!_box.isEmpty())
     {
Comment 6 Allen Winter 2006-09-20 14:13:20 UTC
Magnus,

I will need to revert this patch unless you can give me a fix for your fix very quickly (today).

During my testing I am unable to access a Kolab server.  I continually get the message "The authentication method * is not supported by the server."  And then I am prompted for a username and password.  But access fails anyway.
Comment 7 Magnus Holmgren 2006-09-20 14:53:30 UTC
The patch as you applied it differs from my patch which in turn differs from the changes I actually made to that file (and nobody spotted the unbalanced braces?).

The relevant block should of course read

    if (myAuth.isEmpty () || myAuth == "*") {
      if (hasCapability (QString ("LOGINDISABLED"))) {
        error (ERR_COULD_NOT_LOGIN, i18n("LOGIN is disabled by the server."));
        closeConnection();
        return false;
      }
    }
    else {
      if (!hasCapability (QString ("AUTH=") + myAuth)) {
        error (ERR_COULD_NOT_LOGIN, i18n("The authentication method %1 is not "
          "supported by the server.").arg(myAuth));
        closeConnection();
        return false;
      }
    }
Comment 8 Magnus Holmgren 2006-09-20 14:56:13 UTC
Created attachment 17852 [details]
Abort login if disabled
Comment 9 Allen Winter 2006-09-20 15:20:36 UTC
SVN commit 586730 by winterz:

fix my silly typos to Magnus' patch.  Lucky I tested.

BUGS: 126975


 M  +10 -11    imap4.cc  


--- branches/KDE/3.5/kdepim/kioslaves/imap4/imap4.cc #586729:586730
@@ -1924,20 +1924,19 @@
       } else completeQueue.removeRef(cmd);
     }
 
-    if ( myAuth.isEmpty () || myAuth == "*" ) {
-      if ( hasCapability( QString( "LOGINDISABLED" ) ) ) {
-        error( ERR_COULD_NOT_LOGIN, i18n("LOGIN is disabled by the server.") );
+    if (myAuth.isEmpty () || myAuth == "*") {
+      if (hasCapability (QString ("LOGINDISABLED"))) {
+        error (ERR_COULD_NOT_LOGIN, i18n("LOGIN is disabled by the server."));
         closeConnection();
         return false;
       }
-      else {
-        if ( !hasCapability( QString( "AUTH=" ) + myAuth ) ) {
-          error( ERR_COULD_NOT_LOGIN,
-                 i18n("The authentication method %1 is not "
-                      "supported by the server.").arg( myAuth ) );
-          closeConnection();
-          return false;
-        }
+    }
+    else {
+      if (!hasCapability (QString ("AUTH=") + myAuth)) {
+        error (ERR_COULD_NOT_LOGIN, i18n("The authentication method %1 is not "
+          "supported by the server.").arg(myAuth));
+        closeConnection();
+        return false;
       }
     }