Bug 377496 - Improve auth dialog blocking heuristics. r=MattN

The fix in bug 1312243 introduced a maximum of three consecutive cancelations (controlled by a pref) that a user could perform until Firefox would prevent the page from showing more dialogs.

This, in my opinion, is a great idea. The implementation, however, has a major fallacy: It checks the inner window id in the well-meaning attempt to find user navigation or reloads and clears its internal counter when that window id changes. Unfortunately this also clears the counter on non-user-initiated navigations and reloads. I believe that the true intention of the patch was to cancel the auth dialog after 3 attempts, except if:

- The user reloads the page on their own terms
- The user navigates to a different site on their own

Which is what I plan to implement, using the same pattern we applied to implement temporarily blocked site permissions:

- Temporarily store basic auth counter state on the browser object, as a map from baseDomain (eTLD+1) to number of cancellations
- Reset this state only on user initiated reload
- Reset the counter for a domain if the user has entered login data into the dialog and submitted

This would mitigate the DOS issue while hopefully not breaking any sites that rely on basic auth.

Differential Revision: https://phabricator.services.mozilla.com/D18019

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Johann Hofmann 2019-02-03 19:42:19 +00:00
Родитель ecc4cd8db6
Коммит d59d079e47
4 изменённых файлов: 40 добавлений и 19 удалений

Просмотреть файл

@ -3275,6 +3275,8 @@ function BrowserReloadWithFlags(reloadFlags) {
// 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;
}
PanelMultiView.hidePopup(gIdentityHandler._identityPopup);

Просмотреть файл

@ -3525,6 +3525,8 @@ window._gBrowser = {
// 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;
PanelMultiView.hidePopup(gIdentityHandler._identityPopup);
browser.reload();
},

Просмотреть файл

@ -850,6 +850,13 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
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;
}
let params = {
postData,
allowThirdPartyFixup: true,

Просмотреть файл

@ -96,17 +96,25 @@ LoginManagerPromptFactory.prototype = {
return;
}
// Allow only a limited number of authentication dialogs when they are all
// canceled by the user.
var cancelationCounter = (prompter._browser && prompter._browser.canceledAuthenticationPromptCounter) || { count: 0, id: 0 };
if (prompt.channel) {
var httpChannel = prompt.channel.QueryInterface(Ci.nsIHttpChannel);
if (httpChannel) {
var windowId = httpChannel.topLevelContentWindowId;
if (windowId != cancelationCounter.id) {
// window has been reloaded or navigated, reset the counter
cancelationCounter = { count: 0, id: windowId };
}
// 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;
}
}
@ -136,13 +144,14 @@ LoginManagerPromptFactory.prototype = {
prompt.inProgress = false;
self._asyncPromptInProgress = false;
if (ok) {
cancelationCounter.count = 0;
} else {
cancelationCounter.count++;
}
if (prompter._browser) {
prompter._browser.canceledAuthenticationPromptCounter = cancelationCounter;
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;
}
}
}
@ -168,8 +177,9 @@ LoginManagerPromptFactory.prototype = {
var cancelDialogLimit = Services.prefs.getIntPref("prompts.authentication_dialog_abuse_limit");
let cancelationCounter = browser.canceledAuthenticationPromptCounter[baseDomain];
this.log("cancelationCounter =", cancelationCounter);
if (cancelDialogLimit && cancelationCounter.count >= cancelDialogLimit) {
if (cancelDialogLimit && cancelationCounter >= cancelDialogLimit) {
this.log("Blocking auth dialog, due to exceeding dialog bloat limit");
delete this._asyncPrompts[hashKey];