Bug 1525267 - Move auth prompt rate limiting to nsIAuthPrompt2.promptAuth. r=MattN
authorJohann Hofmann <jhofmann@mozilla.com>
Fri, 01 Mar 2019 15:24:19 +0000
changeset 519841 6933acee1ad9aba811d05eb9d820d9397f45bd2a
parent 519840 72820387d7082b7fd88d53be1e896f73ac596e00
child 519842 e4bda8b288b31ebcf537a1764c6f166cee5c3fb5
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1525267
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1525267 - Move auth prompt rate limiting to nsIAuthPrompt2.promptAuth. r=MattN This also changes the name of 'canceledAuthenticationPromptCounter' to account for the fact that we no longer count up when the prompt was cancelled, but when it was shown. Differential Revision: https://phabricator.services.mozilla.com/D21680
browser/base/content/browser.js
browser/base/content/tabbrowser.js
browser/base/content/urlbarBindings.xml
browser/components/urlbar/UrlbarInput.jsm
toolkit/components/passwordmgr/LoginManagerPrompter.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3330,17 +3330,17 @@ function BrowserReloadWithFlags(reloadFl
   }
 
   // Reset temporary permissions on the remaining tabs to reload.
   // This is done here because we only want to reset
   // permissions on user reload.
   for (let tab of unchangedRemoteness) {
     SitePermissions.clearTemporaryPermissions(tab.linkedBrowser);
     // Also reset DOS mitigations for the basic auth prompt on reload.
-    delete tab.linkedBrowser.canceledAuthenticationPromptCounter;
+    delete tab.linkedBrowser.authPromptAbuseCounter;
   }
   PanelMultiView.hidePopup(gIdentityHandler._identityPopup);
 
 
   let handlingUserInput = window.windowUtils.isHandlingUserInput;
 
   for (let tab of unchangedRemoteness) {
     if (tab.linkedPanel) {
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -3543,17 +3543,17 @@ window._gBrowser = {
   },
 
   reloadTab(aTab) {
     let browser = this.getBrowserForTab(aTab);
     // Reset temporary permissions on the current tab. This is done here
     // because we only want to reset permissions on user reload.
     SitePermissions.clearTemporaryPermissions(browser);
     // Also reset DOS mitigations for the basic auth prompt on reload.
-    delete browser.canceledAuthenticationPromptCounter;
+    delete browser.authPromptAbuseCounter;
     PanelMultiView.hidePopup(gIdentityHandler._identityPopup);
     browser.reload();
   },
 
   addProgressListener(aListener) {
     if (arguments.length != 1) {
       Cu.reportError("gBrowser.addProgressListener was " +
         "called with a second argument, " +
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -855,17 +855,17 @@ file, You can obtain one at http://mozil
             // but don't let that interfere with the loading of the url.
             Cu.reportError(ex);
           }
 
           // Reset DOS mitigations for the basic auth prompt.
           // TODO: When bug 1498553 is resolved, we should be able to
           // remove the !triggeringPrincipal condition here.
           if (!triggeringPrincipal || triggeringPrincipal.isSystemPrincipal) {
-            delete browser.canceledAuthenticationPromptCounter;
+            delete browser.authPromptAbuseCounter;
           }
 
           let params = {
             postData,
             allowThirdPartyFixup: true,
             triggeringPrincipal,
           };
           if (openUILinkWhere == "current") {
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -874,17 +874,17 @@ class UrlbarInput {
       Cu.reportError(ex);
     }
 
     // Reset DOS mitigations for the basic auth prompt.
     // TODO: When bug 1498553 is resolved, we should be able to
     // remove the !triggeringPrincipal condition here.
     if (!params.triggeringPrincipal ||
         params.triggeringPrincipal.isSystemPrincipal) {
-      delete browser.canceledAuthenticationPromptCounter;
+      delete browser.authPromptAbuseCounter;
     }
 
     params.allowThirdPartyFixup = true;
 
     if (openUILinkWhere == "current") {
       params.targetBrowser = browser;
       params.indicateErrorPageLoad = true;
       params.allowPinnedTabHostChange = true;
--- a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
@@ -21,16 +21,69 @@ const BRAND_BUNDLE = "chrome://branding/
  * Mirrored in mobile/android/components/LoginManagerPrompter.js */
 const PROMPT_DISPLAYED = 0;
 
 const PROMPT_ADD_OR_UPDATE = 1;
 const PROMPT_NOTNOW = 2;
 const PROMPT_NEVER = 3;
 
 /**
+ * A helper module to prevent modal auth prompt abuse.
+ */
+const PromptAbuseHelper = {
+  getBaseDomainOrFallback(hostname) {
+    try {
+      return Services.eTLD.getBaseDomainFromHost(hostname);
+    } catch (e) {
+      return hostname;
+    }
+  },
+
+  incrementPromptAbuseCounter(baseDomain, browser) {
+    if (!browser) {
+      return;
+    }
+
+    if (!browser.authPromptAbuseCounter) {
+      browser.authPromptAbuseCounter = {};
+    }
+
+    if (!browser.authPromptAbuseCounter[baseDomain]) {
+      browser.authPromptAbuseCounter[baseDomain] = 0;
+    }
+
+    browser.authPromptAbuseCounter[baseDomain] += 1;
+  },
+
+  resetPromptAbuseCounter(baseDomain, browser) {
+    if (!browser || !browser.authPromptAbuseCounter) {
+      return;
+    }
+
+    browser.authPromptAbuseCounter[baseDomain] = 0;
+  },
+
+  hasReachedAbuseLimit(baseDomain, browser) {
+    if (!browser || !browser.authPromptAbuseCounter) {
+      return false;
+    }
+
+    let abuseCounter = browser.authPromptAbuseCounter[baseDomain];
+    // Allow for setting -1 to turn the feature off.
+    if (this.abuseLimit < 0) {
+      return false;
+    }
+    return !!abuseCounter && abuseCounter >= this.abuseLimit;
+  },
+};
+
+XPCOMUtils.defineLazyPreferenceGetter(PromptAbuseHelper, "abuseLimit",
+                                      "prompts.authentication_dialog_abuse_limit");
+
+/**
  * Implements nsIPromptFactory
  *
  * Invoked by [toolkit/components/prompts/src/nsPrompter.js]
  */
 function LoginManagerPromptFactory() {
   Services.obs.addObserver(this, "quit-application-granted", true);
   Services.obs.addObserver(this, "passwordmgr-crypto-login", true);
   Services.obs.addObserver(this, "passwordmgr-crypto-loginCanceled", true);
@@ -91,38 +144,16 @@ LoginManagerPromptFactory.prototype = {
       let httpHostname = hostname.replace(/^https:\/\//, "http://");
       hasLogins = (Services.logins.countLogins(httpHostname, null, httpRealm) > 0);
     }
     if (hasLogins && Services.logins.uiBusy) {
       this.log("_doAsyncPrompt:run bypassed, master password UI busy");
       return;
     }
 
-    // Set up a counter for ensuring that the basic auth prompt can not
-    // be abused for DOS-style attacks. With this counter, each eTLD+1
-    // per browser will get a limited number of times a user can
-    // cancel the prompt until we stop showing it.
-    let browser = prompter._browser;
-    let baseDomain = null;
-    if (browser) {
-      try {
-        baseDomain = Services.eTLD.getBaseDomainFromHost(hostname);
-      } catch (e) {
-        baseDomain = hostname;
-      }
-
-      if (!browser.canceledAuthenticationPromptCounter) {
-        browser.canceledAuthenticationPromptCounter = {};
-      }
-
-      if (!browser.canceledAuthenticationPromptCounter[baseDomain]) {
-        browser.canceledAuthenticationPromptCounter[baseDomain] = 0;
-      }
-    }
-
     var self = this;
 
     var runnable = {
       cancel: false,
       run() {
         var ok = false;
         if (!this.cancel) {
           try {
@@ -138,26 +169,16 @@ LoginManagerPromptFactory.prototype = {
               Cu.reportError("LoginManagerPrompter: " +
                              "_doAsyncPrompt:run: " + e + "\n");
             }
           }
 
           delete self._asyncPrompts[hashKey];
           prompt.inProgress = false;
           self._asyncPromptInProgress = false;
-
-          if (browser) {
-            // Reset the counter state if the user replied to a prompt and actually
-            // tried to login (vs. simply clicking any button to get out).
-            if (ok && (prompt.authInfo.username || prompt.authInfo.password)) {
-              browser.canceledAuthenticationPromptCounter[baseDomain] = 0;
-            } else {
-              browser.canceledAuthenticationPromptCounter[baseDomain] += 1;
-            }
-          }
         }
 
         for (var consumer of prompt.consumers) {
           if (!consumer.callback) {
           // Not having a callback means that consumer didn't provide it
           // or canceled the notification
             continue;
           }
@@ -170,30 +191,18 @@ LoginManagerPromptFactory.prototype = {
               consumer.callback.onAuthCancelled(consumer.context, !this.cancel);
             }
           } catch (e) { /* Throw away exceptions caused by callback */ }
         }
         self._doAsyncPrompt();
       },
     };
 
-    var cancelDialogLimit = Services.prefs.getIntPref("prompts.authentication_dialog_abuse_limit");
-
-    let cancelationCounter = browser.canceledAuthenticationPromptCounter[baseDomain];
-    this.log("cancelationCounter =", cancelationCounter);
-    if (cancelDialogLimit && cancelationCounter >= cancelDialogLimit) {
-      this.log("Blocking auth dialog, due to exceeding dialog bloat limit");
-      delete this._asyncPrompts[hashKey];
-
-      // just make the runnable cancel all consumers
-      runnable.cancel = true;
-    } else {
-      this._asyncPromptInProgress = true;
-      prompt.inProgress = true;
-    }
+    this._asyncPromptInProgress = true;
+    prompt.inProgress = true;
 
     Services.tm.dispatchToMainThread(runnable);
     this.log("_doAsyncPrompt:run dispatched");
   },
 
 
   _cancelPendingPrompts() {
     this.log("Canceling all pending prompts...");
@@ -618,37 +627,57 @@ LoginManagerPrompter.prototype = {
     } catch (e) {
       // Ignore any errors and display the prompt anyway.
       epicfail = true;
       Cu.reportError("LoginManagerPrompter: " +
           "Epic fail in promptAuth: " + e + "\n");
     }
 
     var ok = canAutologin;
+    let browser = this._browser;
+    let baseDomain = PromptAbuseHelper.getBaseDomainOrFallback(hostname);
+
     if (!ok) {
+      if (PromptAbuseHelper.hasReachedAbuseLimit(baseDomain, browser)) {
+        this.log("Blocking auth dialog, due to exceeding dialog bloat limit");
+        return false;
+      }
+
+      // Set up a counter for ensuring that the basic auth prompt can not
+      // be abused for DOS-style attacks. With this counter, each eTLD+1
+      // per browser will get a limited number of times a user can
+      // cancel the prompt until we stop showing it.
+      PromptAbuseHelper.incrementPromptAbuseCounter(baseDomain, browser);
+
       if (this._chromeWindow) {
         PromptUtils.fireDialogEvent(this._chromeWindow, "DOMWillOpenModalDialog", this._browser);
       }
       ok = Services.prompt.promptAuth(this._chromeWindow,
                                       aChannel, aLevel, aAuthInfo,
                                       checkboxLabel, checkbox);
     }
 
+    let [username, password] = this._GetAuthInfo(aAuthInfo);
+
+    // Reset the counter state if the user replied to a prompt and actually
+    // tried to login (vs. simply clicking any button to get out).
+    if (ok && (username || password)) {
+      PromptAbuseHelper.resetPromptAbuseCounter(baseDomain, browser);
+    }
+
     // If there's a notification prompt, use it to allow the user to
     // determine if the login should be saved. If there isn't a
     // notification prompt, only save the login if the user set the
     // checkbox to do so.
     var rememberLogin = notifyObj ? canRememberLogin : checkbox.value;
     if (!ok || !rememberLogin || epicfail) {
       return ok;
     }
 
     try {
-      var [username, password] = this._GetAuthInfo(aAuthInfo);
-
       if (!password) {
         this.log("No password entered, so won't offer to save.");
         return ok;
       }
 
       // XXX We can't prompt with multiple logins yet (bug 227632), so
       // the entered login might correspond to an existing login
       // other than the one we originally selected.