Bug 971347 - Fix autoconfig vulnerable to active MITM attacks for all domains (including the ones in ISPDB)

Tag #1273

4 files changed, 153 insertions, 77 deletions

diff --git a/mailnews/base/prefs/content/accountcreation/emailWizard.js b/mailnews/base/prefs/content/accountcreation/emailWizard.js
index 389feab6c..51c2bf1f5 100644
--- a/mailnews/base/prefs/content/accountcreation/emailWizard.js
+++ b/mailnews/base/prefs/content/accountcreation/emailWizard.js
@@ -393,8 +393,8 @@ EmailConfigWizard.prototype =
   /**
    * Start from beginning with possibly new email address.
    */
-  onStartOver : function()
-  {
+  onStartOver : function() {
+    this._currentConfig = null;
     if (this._abortable) {
       this.onStop();
     }
@@ -640,8 +640,9 @@ EmailConfigWizard.prototype =
     self._abortable = guessConfig(domain,
       function(type, hostname, port, ssl, done, config) // progress
       {
-        gEmailWizardLogger.info("progress callback host " + hostname +
-                                " port " +  port + " type " + type);
+        var msg = hostname + ":" + port + " ssl=" + ssl + " " +
+                  type + ": progress callback";
+        gEmailWizardLogger.info(msg);
       },
       function(config) // success
       {
diff --git a/mailnews/base/prefs/content/accountcreation/fetchConfig.js b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
index 07a9f5586..2fe604e69 100644
--- a/mailnews/base/prefs/content/accountcreation/fetchConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
@@ -63,52 +63,49 @@ function fetchConfigFromISP(domain, emailAddress, successCallback,
     return;
   }
 
-  let url1 = "http://autoconfig." + sanitize.hostname(domain) +
-             "/mail/config-v1.1.xml";
+  let conf1 = "autoconfig." + sanitize.hostname(domain) +
+              "/mail/config-v1.1.xml";
   // .well-known/ <http://tools.ietf.org/html/draft-nottingham-site-meta-04>
-  let url2 = "http://" + sanitize.hostname(domain) +
-             "/.well-known/autoconfig/mail/config-v1.1.xml";
+  let conf2 = sanitize.hostname(domain) +
+              "/.well-known/autoconfig/mail/config-v1.1.xml";
+  // This list is sorted by decreasing priority
+  var urls = ["https://" + conf1, "https://" + conf2];
+  if (!Services.prefs.getBoolPref("mailnews.auto_config.fetchFromISP.sslOnly")) {
+    urls.push("http://" + conf1, "http://" + conf2);
+  }
   let sucAbortable = new SuccessiveAbortable();
-  var time = Date.now();
   var urlArgs = { emailaddress: emailAddress };
   if (!Services.prefs.getBoolPref(
       "mailnews.auto_config.fetchFromISP.sendEmailAddress")) {
     delete urlArgs.emailaddress;
   }
-  let fetch1 = new FetchHTTP(url1, urlArgs, false,
-    function(result)
-    {
-      successCallback(readFromXML(result));
-    },
-    function(e1) // fetch1 failed
-    {
-      ddump("fetchisp 1 <" + url1 + "> took " + (Date.now() - time) +
-          "ms and failed with " + e1);
-      time = Date.now();
-      if (e1 instanceof CancelledException)
-      {
-        errorCallback(e1);
-        return;
-      }
-
-      let fetch2 = new FetchHTTP(url2, urlArgs, false,
-        function(result)
-        {
-          successCallback(readFromXML(result));
-        },
-        function(e2)
-        {
-          ddump("fetchisp 2 <" + url2 + "> took " + (Date.now() - time) +
-              "ms and failed with " + e2);
-          // return the error for the primary call,
-          // unless the fetch was cancelled
-          errorCallback(e2 instanceof CancelledException ? e2 : e1);
-        });
-      sucAbortable.current = fetch2;
-      fetch2.start();
-    });
-  sucAbortable.current = fetch1;
-  fetch1.start();
+  var time;
+
+  let success = function(result) {
+    successCallback(readFromXML(result));
+  };
+
+  let error = function(i, e) {
+    ddump("fetchisp " + i + " <" + urls[i] + "> took " +
+          (Date.now() - time) + "ms and failed with " + e);
+
+    if (i == urls.length - 1 || // implies all fetches failed
+        e instanceof CancelledException) {
+      errorCallback(e);
+      return;
+    }
+    let fetch = new FetchHTTP(urls[i + 1], urlArgs, false, success,
+                              function(e) { error(i + 1, e) });
+    sucAbortable.current = fetch;
+    time = Date.now();
+    fetch.start();
+  };
+
+  let fetch = new FetchHTTP(urls[0], urlArgs, false, success,
+                            function(e) { error(0, e) });
+  sucAbortable.current = fetch;
+  time = Date.now();
+  fetch.start();
   return sucAbortable;
 }
 
diff --git a/mailnews/base/prefs/content/accountcreation/guessConfig.js b/mailnews/base/prefs/content/accountcreation/guessConfig.js
index 9a44f7904..6ba9f1dfb 100644
--- a/mailnews/base/prefs/content/accountcreation/guessConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/guessConfig.js
@@ -6,8 +6,6 @@
 Cu.import("resource:///modules/gloda/log4moz.js");
 Cu.import("resource://gre/modules/Services.jsm");
 
-var TIMEOUT = 10; // in seconds
-
 // This is a bit ugly - we set outgoingDone to false
 // when emailWizard.js cancels the outgoing probe because the user picked
 // an outoing server. It does this by poking the probeAbortable object,
@@ -411,6 +409,7 @@ HostDetector.prototype =
         { "imap" : IMAP, "pop3" : POP, "smtp" : SMTP }, UNKNOWN);
     if (!port)
       port = UNKNOWN;
+    var ssl_only = Services.prefs.getBoolPref("mailnews.auto_config.guess.sslOnly");
     var ssl = ConvertSocketTypeToSSL(socketType);
     this._cancel = false;
     this._log.info("doing auto detect for protocol " + protocol +
@@ -434,8 +433,13 @@ HostDetector.prototype =
       for (let j = 0; j < hostEntries.length; j++)
       {
         let hostTry = hostEntries[j]; // from getHostEntry()
+        if (ssl_only && hostTry.ssl == NONE)
+          continue;
         hostTry.hostname = hostname;
         hostTry.status = kNotTried;
+        hostTry.desc = hostTry.hostname + ":" + hostTry.port +
+                       " ssl=" + hostTry.ssl + " " +
+                       protocolToString(hostTry.protocol);
         this._hostsToTry.push(hostTry);
       }
     }
@@ -453,21 +457,25 @@ HostDetector.prototype =
     if (this._cancel)
       return;
     var me = this;
-    for (let i = 0; i < this._hostsToTry.length; i++)
-    {
-      let thisTry = this._hostsToTry[i]; // {HostTry}
-      if (thisTry.status != kNotTried)
-        continue;
-      this._log.info("poking at " + thisTry.hostname + " port " +
-          thisTry.port + " ssl "+ thisTry.ssl + " protocol " +
-          protocolToString(thisTry.protocol));
-      if (i == 0) // showing 50 servers at once is pointless
-        this.mProgressCallback(thisTry);
-
-      thisTry.abortable = SocketUtil(
+    var timeout = Services.prefs.getIntPref("mailnews.auto_config.guess.timeout");
+    // We assume we'll resolve the same proxy for all tries, and
+    // proceed to use the first resolved proxy for all tries. This
+    // assumption is generally sound, but not always: mechanisms like
+    // the pref network.proxy.no_proxies_on can make imap.domain and
+    // pop.domain resolve differently.
+    doProxy(this._hostsToTry[0].hostname, function(proxy) {
+      for (let i = 0; i < me._hostsToTry.length; i++) {
+        let thisTry = me._hostsToTry[i]; // {HostTry}
+        if (thisTry.status != kNotTried)
+          continue;
+        me._log.info(thisTry.desc + ": initializing probe...");
+        if (i == 0) // showing 50 servers at once is pointless
+          me.mProgressCallback(thisTry);
+
+        thisTry.abortable = SocketUtil(
           thisTry.hostname, thisTry.port, thisTry.ssl,
-          thisTry.commands, TIMEOUT,
-          new SSLErrorHandler(thisTry, this._log),
+          thisTry.commands, timeout, proxy,
+          new SSLErrorHandler(thisTry, me._log),
           function(wiredata) // result callback
           {
             if (me._cancel)
@@ -480,12 +488,14 @@ HostDetector.prototype =
           {
             if (me._cancel)
               return; // who set cancel to true already called mErrorCallback()
-            me._log.warn(e);
+            me._log.warn(thisTry.desc + ": " + e);
             thisTry.status = kFailed;
             me._checkFinished();
-          });
-      thisTry.status = kOngoing;
-    }
+          }
+        );
+        thisTry.status = kOngoing;
+      }
+    });
   },
 
   /**
@@ -507,7 +517,7 @@ HostDetector.prototype =
       if (thisTry._gotCertError == Ci.nsICertOverrideService.ERROR_UNTRUSTED ||
           thisTry._gotCertError == Ci.nsICertOverrideService.ERROR_TIME)
       {
-        this._log.info("TRYING AGAIN, hopefully with exception recorded");
+        this._log.info(thisTry.desc + ": TRYING AGAIN, hopefully with exception recorded");
         thisTry._gotCertError = 0;
         thisTry.selfSignedCert = true; // _next_ run gets this exception
         thisTry.status = kNotTried; // try again (with exception)
@@ -518,22 +528,20 @@ HostDetector.prototype =
 
     if (wiredata == null || wiredata === undefined)
     {
-      this._log.info("no data");
+      this._log.info(thisTry.desc + ": no data");
       thisTry.status = kFailed;
       return;
     }
-    this._log.info("wiredata: " + wiredata.join(""));
+    this._log.info(thisTry.desc + ": wiredata: " + wiredata.join(""));
     thisTry.authMethods =
         this._advertisesAuthMethods(thisTry.protocol, wiredata);
     if (thisTry.ssl == TLS && !this._hasTLS(thisTry, wiredata))
     {
-      this._log.info("STARTTLS wanted, but not offered");
+      this._log.info(thisTry.desc + ": STARTTLS wanted, but not offered");
       thisTry.status = kFailed;
       return;
     }
-    this._log.info("success with " + thisTry.hostname + ":" +
-        thisTry.port + " " + protocolToString(thisTry.protocol) +
-        " ssl " + thisTry.ssl +
+    this._log.info(thisTry.desc + ": success" +
         (thisTry.selfSignedCert ? " (selfSignedCert)" : ""));
     thisTry.status = kSuccess;
 
@@ -542,7 +550,8 @@ HostDetector.prototype =
       // earlier we get into an infinite loop, probably because the cert
       // remembering is temporary and the next try gets a new connection which
       // isn't covered by that temporariness.
-      this._log.info("clearing validity override for " + thisTry.hostname);
+      this._log.info(thisTry.desc + ": clearing validity override for " +
+                     thisTry.hostname);
       Cc["@mozilla.org/security/certoverride;1"]
         .getService(Ci.nsICertOverrideService)
         .clearValidityOverride(thisTry.hostname, thisTry.port);
@@ -1016,13 +1025,14 @@ SSLErrorHandler.prototype =
  * @param commands {Array of String}: protocol commands
  *          to send to the server.
  * @param timeout {Integer} seconds to wait for a server response, then cancel.
+ * @param proxy {nsIProxyInfo} The proxy to use (or null to not use any).
  * @param sslErrorHandler {SSLErrorHandler}
  * @param resultCallback {function(wiredata)} This function will
  *            be called with the result string array from the server
  *            or null if no communication occurred.
  * @param errorCallback {function(e)}
  */
-function SocketUtil(hostname, port, ssl, commands, timeout,
+function SocketUtil(hostname, port, ssl, commands, timeout, proxy,
                     sslErrorHandler, resultCallback, errorCallback)
 {
   assert(commands && commands.length, "need commands");
@@ -1061,7 +1071,7 @@ function SocketUtil(hostname, port, ssl, commands, timeout,
   var socketTypeName = ssl == SSL ? "ssl" : (ssl == TLS ? "starttls" : null);
   var transport = transportService.createTransport([socketTypeName],
                                                    ssl == NONE ? 0 : 1,
-                                                   hostname, port, null);
+                                                   hostname, port, proxy);
 
   transport.setTimeout(Ci.nsISocketTransport.TIMEOUT_CONNECT, timeout);
   transport.setTimeout(Ci.nsISocketTransport.TIMEOUT_READ_WRITE, timeout);
@@ -1145,3 +1155,38 @@ SocketAbortable.prototype.cancel = function(ex)
   }
 }
 
+/**
+ * Resolve a proxy for some domain and expose it via a callback.
+ *
+ * @param hostname {String} The hostname which a proxy will be resolved for
+ * @param resultCallback {function(proxyInfo)}
+ *   Called after the proxy has been resolved for hostname.
+ *   proxy {nsIProxyInfo} The resolved proxy, or null if none were found
+ *         for hostname
+ */
+function doProxy(hostname, resultCallback) {
+  // This implements the nsIProtocolProxyCallback interface:
+  function ProxyResolveCallback() { }
+  ProxyResolveCallback.prototype = {
+    onProxyAvailable(req, uri, proxy, status) {
+      // Anything but a SOCKS proxy will be unusable for email.
+      if (proxy != null && proxy.type != "socks" &&
+          proxy.type != "socks4") {
+        proxy = null;
+      }
+      resultCallback(proxy);
+    },
+  };
+  var proxyService = Cc["@mozilla.org/network/protocol-proxy-service;1"]
+                     .getService(Ci.nsIProtocolProxyService);
+  // Use some arbitrary scheme just because it is required...
+  var uri = Services.io.newURI("http://" + hostname);
+  // ... we'll ignore it any way. We prefer SOCKS since that's the
+  // only thing we can use for email protocols.
+  var proxyFlags = Ci.nsIProtocolProxyService.RESOLVE_IGNORE_URI_SCHEME |
+                   Ci.nsIProtocolProxyService.RESOLVE_PREFER_SOCKS_PROXY;
+  if (Services.prefs.getBoolPref("network.proxy.socks_remote_dns")) {
+    proxyFlags |= Ci.nsIProtocolProxyService.RESOLVE_ALWAYS_TUNNEL;
+  }
+  proxyService.asyncResolve(uri, proxyFlags, new ProxyResolveCallback());
+}
diff --git a/mailnews/mailnews.js b/mailnews/mailnews.js
index 11aa5ab2e..705a0a08a 100644
--- a/mailnews/mailnews.js
+++ b/mailnews/mailnews.js
@@ -870,13 +870,46 @@ pref("mailnews.emptyTrash.dontAskAgain", false);
 pref("mailnews.auto_config_url", "https://live.mozillamessaging.com/autoconfig/v1.1/");
 // Added in bug 551519. Remove when bug 545866 is fixed.
 pref("mailnews.mx_service_url", "https://live.mozillamessaging.com/dns/mx/");
-// Allow to contact ISP (email address domain)
-// This happens via insecure means (HTTP), so the config cannot be trusted,
-// and also contains the email address
+// Allow to contact the ISP (email address domain).
+// This may happen via insecure means (HTTP) susceptible to eavesdropping
+// and MitM (see mailnews.auto_config.fetchFromISP.sslOnly below).
 pref("mailnews.auto_config.fetchFromISP.enabled", true);
-// Allow the fetch from ISP via HTTP, but not the email address
+// Allow the username to be sent to the ISP when fetching.
+// Note that the username will leak in plaintext if a non-SSL fetch is
+// performed.
 pref("mailnews.auto_config.fetchFromISP.sendEmailAddress", true);
+// Allow only SSL channels when fetching config from ISP.
+// If false, an active attacker can block SSL fetches and then
+// MITM the HTTP fetch, determining the config that is shown to the user.
+// However:
+// 1. The user still needs to explicitly approve the false config.
+// 2. Most hosters that offer this ISP config do so on HTTP and not on HTTPS.
+//    That's because they direct customer domains (HTTP) to their provider
+//    config (HTTPS). If you set this to true, you simply break this mechanism.
+//    You will simply not get most configs.
+// 3. There are guess config and AutoDiscover config mechanisms which
+//    have the exact same problem. In order to mitigate those additional
+//    vectors, set the following prefs accordingly:
+//     * mailnews.auto_config.guess.sslOnly = true
+//     * mailnews.auto_config.fetchFromExchange.enabled = false
+// Not all mail servers support SSL so enabling this option might lock
+// you out from your ISP. This especially affect internal mail servers.
+pref("mailnews.auto_config.fetchFromISP.sslOnly", false);
+// Whether we will attempt to guess the account configuration based on
+// protocol default ports and common domain practices
+// (e.g. {mail,pop,imap,smtp}.<email-domain>).
 pref("mailnews.auto_config.guess.enabled", true);
+// Allow only SSL configs when guessing.
+// An attacker could block SSL to force plaintext and thus be able to
+// eavesdrop. Compared to mailnews.auto_config.fetchFromISP.sslOnly
+// the attacker cannot determine the config, just pick which one it
+// likes best among those Thunderbird generates for the user based on
+// the email address.
+// Not all mail servers support SSL so enabling this option might lock
+// you out from your ISP. This especially affect internal mail servers.
+pref("mailnews.auto_config.guess.sslOnly", false);
+// The timeout (in seconds) for each guess
+pref("mailnews.auto_config.guess.timeout", 10);
 
 // -- Summary Database options
 // dontPreserveOnCopy: a space separated list of properties that are not