Bug 1613623 - For IMAP, no longer allow STARTTLS when server sends PREAUTH greeting. (relanding) r=mkmelin DONTBUILD
authorGene Smith <gds@chartertn.net>
Mon, 18 May 2020 13:35:16 +0300
changeset 39172 70726ff4284b49e1e8d28626d5a0b45896c19454
parent 39171 67525d68c3a975cb81f8e04424c3d6a3383bb2c4
child 39173 ea113591641e95077f801b17482f2e6b96960389
push id402
push userclokep@gmail.com
push dateMon, 29 Jun 2020 20:48:04 +0000
reviewersmkmelin
bugs1613623
Bug 1613623 - For IMAP, no longer allow STARTTLS when server sends PREAUTH greeting. (relanding) r=mkmelin DONTBUILD
mailnews/imap/src/nsImapProtocol.cpp
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -1520,35 +1520,54 @@ void nsImapProtocol::EstablishServerConn
                            ESC_CAPABILITY_STAR_LEN] = '\0';
         // Tell the response parser that we just issued a "CAPABILITY" and
         // got the following back.
         GetServerStateParser().ParseIMAPServerResponse("1 CAPABILITY", true,
                                                        fakeServerResponse);
       }
     }
   } else if (!PL_strncasecmp(serverResponse, ESC_PREAUTH, ESC_PREAUTH_LEN)) {
-    // we've been pre-authenticated.
-    // we can skip the whole password step, right into the
-    // kAuthenticated state
-    GetServerStateParser().PreauthSetAuthenticatedState();
-
-    if (GetServerStateParser().GetCapabilityFlag() == kCapabilityUndefined)
-      Capability();
-
-    if (!(GetServerStateParser().GetCapabilityFlag() &
-          (kIMAP4Capability | kIMAP4rev1Capability | kIMAP4other))) {
-      // AlertUserEvent_UsingId(MK_MSG_IMAP_SERVER_NOT_IMAP4);
+    // PREAUTH greeting received. We've been pre-authenticated by the server.
+    // We can skip sending a password and transition right into the
+    // kAuthenticated state; but we won't if the user has configured STARTTLS.
+    // (STARTTLS can only occur with the server in non-authenticated state.)
+    if (!(m_socketType == nsMsgSocketType::alwaysSTARTTLS ||
+          m_socketType == nsMsgSocketType::trySTARTTLS)) {
+      GetServerStateParser().PreauthSetAuthenticatedState();
+
+      if (GetServerStateParser().GetCapabilityFlag() == kCapabilityUndefined)
+        Capability();
+
+      if (!(GetServerStateParser().GetCapabilityFlag() &
+            (kIMAP4Capability | kIMAP4rev1Capability | kIMAP4other))) {
+        // AlertUserEventUsingId(MK_MSG_IMAP_SERVER_NOT_IMAP4);
+        SetConnectionStatus(NS_ERROR_FAILURE);  // stop netlib
+      } else {
+        // let's record the user as authenticated.
+        m_imapServerSink->SetUserAuthenticated(true);
+
+        ProcessAfterAuthenticated();
+        // the connection was a success
+        SetConnectionStatus(NS_OK);
+      }
+    } else {
+      // STARTTLS is configured so don't transition to authenticated state. Just
+      // alert the user, log the error and drop the connection. This may
+      // indicate a man-in-the middle attack if the user is not expecting
+      // PREAUTH. The user must change the connection security setting to other
+      // than STARTTLS to allow PREAUTH to be accepted on subsequent IMAP
+      // connections.
+      AlertUserEventUsingName("imapServerDisconnected");
+      const nsCString &hostName = GetImapHostName();
+      MOZ_LOG(
+          IMAP, LogLevel::Error,
+          ("PREAUTH received from IMAP server %s because STARTTLS selected. "
+           "Connection dropped",
+           hostName.get()));
       SetConnectionStatus(NS_ERROR_FAILURE);  // stop netlib
-    } else {
-      // let's record the user as authenticated.
-      m_imapServerSink->SetUserAuthenticated(true);
-
-      ProcessAfterAuthenticated();
-      // the connection was a success
-      SetConnectionStatus(NS_OK);
     }
   }
 
   PR_Free(serverResponse);  // we don't care about the greeting yet...
 
 #undef ESC_LENGTH
 #undef ESC_OK
 #undef ESC_OK_LEN