Bug 1334201 - Ignore window.opener for insecure form warnings. r=MattN a=jcristau
authorJohann Hofmann <jhofmann@mozilla.com>
Tue, 31 Jan 2017 10:34:06 +0100
changeset 375857 1b1e50873a309d820b87daebb2f69adcb9858a5d
parent 375856 09e755555ba5d2ca4be8a0c754d941b8e2ef3633
child 375858 cd8688f42f8a32f24906ef5de60000e2759546ea
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, jcristau
bugs1334201
milestone53.0a2
Bug 1334201 - Ignore window.opener for insecure form warnings. r=MattN a=jcristau MozReview-Commit-ID: BLuwIAWEYHi
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_insecureLoginForms.js
browser/base/content/test/general/insecure_opener.html
toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
toolkit/components/passwordmgr/LoginManagerContent.jsm
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -321,16 +321,17 @@ support-files = fxa_profile_handler.sjs
 [browser_fxa_web_channel.js]
 [browser_gestureSupport.js]
 skip-if = e10s # Bug 863514 - no gesture support.
 [browser_getshortcutoruri.js]
 [browser_hide_removing.js]
 [browser_homeDrop.js]
 [browser_identity_UI.js]
 [browser_insecureLoginForms.js]
+support-files = insecure_opener.html
 [browser_invalid_uri_back_forward_manipulation.js]
 [browser_keywordBookmarklets.js]
 [browser_keywordSearch.js]
 [browser_keywordSearch_postData.js]
 [browser_lastAccessedTab.js]
 skip-if = toolkit == "windows" # Disabled on Windows due to frequent failures (bug 969405)
 [browser_menuButtonFitts.js]
 skip-if = os != "win" # The Fitts Law menu button is only supported on Windows (bug 969376)
--- a/browser/base/content/test/general/browser_insecureLoginForms.js
+++ b/browser/base/content/test/general/browser_insecureLoginForms.js
@@ -98,8 +98,65 @@ add_task(function* test_mixedcontent() {
   ]);
 
   assertMixedContentBlockingState(browser, { activeLoaded: true,
                                              activeBlocked: false,
                                              passiveLoaded: false });
 
   gBrowser.removeTab(tab);
 });
+
+/**
+ * Checks that insecure window.opener does not trigger a warning.
+ */
+add_task(function* test_ignoring_window_opener() {
+  let newTabURL = "https://example.com" + TEST_URL_PATH + "form_basic.html";
+  let path = getRootDirectory(gTestPath)
+    .replace("chrome://mochitests/content", "http://example.com");
+  let url = path + "insecure_opener.html";
+
+  yield BrowserTestUtils.withNewTab(url, function*(browser) {
+    // Clicking the link will spawn a new tab.
+    let loaded = BrowserTestUtils.waitForNewTab(gBrowser, newTabURL);
+    yield ContentTask.spawn(browser, {}, function() {
+      content.document.getElementById("link").click();
+    });
+    let tab = yield loaded;
+    browser = tab.linkedBrowser;
+    yield waitForInsecureLoginFormsStateChange(browser, 2);
+
+    // Open the identity popup.
+    let { gIdentityHandler } = gBrowser.ownerGlobal;
+    gIdentityHandler._identityBox.click();
+    document.getElementById("identity-popup-security-expander").click();
+
+    ok(is_visible(document.getElementById("connection-icon")),
+       "Connection icon is visible");
+
+    // Assert that the identity indicators are still "secure".
+    let connectionIconImage = gBrowser.ownerGlobal
+          .getComputedStyle(document.getElementById("connection-icon"))
+          .getPropertyValue("list-style-image");
+    let securityViewBG = gBrowser.ownerGlobal
+          .getComputedStyle(document.getElementById("identity-popup-securityView"))
+          .getPropertyValue("background-image");
+    let securityContentBG = gBrowser.ownerGlobal
+          .getComputedStyle(document.getElementById("identity-popup-security-content"))
+          .getPropertyValue("background-image");
+    is(connectionIconImage,
+       "url(\"chrome://browser/skin/connection-secure.svg\")",
+       "Using expected icon image in the identity block");
+    is(securityViewBG,
+       "url(\"chrome://browser/skin/controlcenter/connection.svg#connection-secure\")",
+       "Using expected icon image in the Control Center main view");
+    is(securityContentBG,
+       "url(\"chrome://browser/skin/controlcenter/connection.svg#connection-secure\")",
+       "Using expected icon image in the Control Center subview");
+
+    ok(Array.every(document.querySelectorAll("[when-loginforms=insecure]"),
+                   element => is_hidden(element)),
+       "All messages should be hidden.");
+
+    gIdentityHandler._identityPopup.hidden = true;
+
+    yield BrowserTestUtils.removeTab(tab);
+  });
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/insecure_opener.html
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html dir="ltr" xml:lang="en-US" lang="en-US">
+  <head>
+    <meta charset="utf8">
+  </head>
+  <body>
+    <a id="link" target="_blank" href="https://example.com/browser/toolkit/components/passwordmgr/test/browser/form_basic.html">Click me, I'm "secure".</a>
+  </body>
+</html>
--- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
+++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
@@ -17,16 +17,31 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "nsIContentSecurityManager");
 XPCOMUtils.defineLazyServiceGetter(this, "gScriptSecurityManager",
                                    "@mozilla.org/scriptsecuritymanager;1",
                                    "nsIScriptSecurityManager");
 XPCOMUtils.defineLazyGetter(this, "WebConsoleUtils", () => {
   return this.devtools.require("devtools/server/actors/utils/webconsole-utils").WebConsoleUtils;
 });
 
+/*
+ * A module that provides utility functions for form security.
+ *
+ * Note:
+ *  This module uses isSecureContextIfOpenerIgnored instead of isSecureContext.
+ *
+ *  We don't want to expose JavaScript APIs in a non-Secure Context even if
+ *  the context is only insecure because the windows has an insecure opener.
+ *  Doing so prevents sites from implementing postMessage workarounds to enable
+ *  an insecure opener to gain access to Secure Context-only APIs. However,
+ *  in the case of form fields such as password fields we don't need to worry
+ *  about whether the opener is secure or not. In fact to flag a password
+ *  field as insecure in such circumstances would unnecessarily confuse our
+ *  users.
+ */
 this.InsecurePasswordUtils = {
   _formRootsWarned: new WeakMap(),
   _sendWebConsoleMessage(messageTag, domDoc) {
     let windowId = WebConsoleUtils.getInnerWindowId(domDoc.defaultView);
     let category = "Insecure Password Field";
     // All web console messages are warnings for now.
     let flag = Ci.nsIScriptError.warningFlag;
     let bundle = Services.strings.createBundle(STRINGS_URI);
@@ -70,24 +85,17 @@ this.InsecurePasswordUtils = {
    * Checks if there are insecure password fields present on the form's document
    * i.e. passwords inside forms with http action, inside iframes with http src,
    * or on insecure web pages.
    *
    * @param {FormLike} aForm A form-like object. @See {LoginFormFactory}
    * @return {boolean} whether the form is secure
    */
   isFormSecure(aForm) {
-    // We don't want to expose JavaScript APIs in a non-Secure Context even if
-    // the context is only insecure because the windows has an insecure opener.
-    // Doing so prevents sites from implementing postMessage workarounds to enable
-    // an insecure opener to gain access to Secure Context-only APIs. However,
-    // in the case of form fields such as password fields we don't need to worry
-    // about whether the opener is secure or not. In fact to flag a password
-    // field as insecure in such circumstances would unnecessarily confuse our
-    // users. Hence we use isSecureContextIfOpenerIgnored here.
+    // Ignores window.opener, see top level documentation.
     let isSafePage = aForm.ownerDocument.defaultView.isSecureContextIfOpenerIgnored;
     let { isFormSubmitSecure, isFormSubmitHTTP } = this._checkFormSecurity(aForm);
 
     return isSafePage && (isFormSubmitSecure || !isFormSubmitHTTP);
   },
 
   /**
    * Report insecure password fields in a form to the web console to warn developers.
@@ -96,17 +104,18 @@ this.InsecurePasswordUtils = {
    */
   reportInsecurePasswords(aForm) {
     if (this._formRootsWarned.has(aForm.rootElement) ||
         this._formRootsWarned.get(aForm.rootElement)) {
       return;
     }
 
     let domDoc = aForm.ownerDocument;
-    let isSafePage = domDoc.defaultView.isSecureContext;
+    // Ignores window.opener, see top level documentation.
+    let isSafePage = domDoc.defaultView.isSecureContextIfOpenerIgnored;
 
     let { isFormSubmitHTTP, isFormSubmitSecure } = this._checkFormSecurity(aForm);
 
     if (!isSafePage) {
       if (domDoc.defaultView == domDoc.defaultView.parent) {
         this._sendWebConsoleMessage("InsecurePasswordsPresentOnPage", domDoc);
       } else {
         this._sendWebConsoleMessage("InsecurePasswordsPresentOnIframe", domDoc);
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -454,17 +454,19 @@ var LoginManagerContent = {
    */
   _detectInsecureFormLikes(topWindow) {
     log("_detectInsecureFormLikes", topWindow.location.href);
 
     // Returns true if this window or any subframes have insecure login forms.
     let hasInsecureLoginForms = (thisWindow) => {
       let doc = thisWindow.document;
       let hasLoginForm = this.stateForDocument(doc).loginFormRootElements.size > 0;
-      return (hasLoginForm && !thisWindow.isSecureContext) ||
+      // Ignores window.opener, because it's not relevant for indicating
+      // form security. See InsecurePasswordUtils docs for details.
+      return (hasLoginForm && !thisWindow.isSecureContextIfOpenerIgnored) ||
              Array.some(thisWindow.frames,
                         frame => hasInsecureLoginForms(frame));
     };
 
     let messageManager = messageManagerFromWindow(topWindow);
     messageManager.sendAsyncMessage("RemoteLogins:insecureLoginFormPresent", {
       hasInsecureLoginForms: hasInsecureLoginForms(topWindow),
     });