Bug 1261234 - Use isURIPotentiallyTrustworthy to verify the form action.; r?MattN draft
authorSean Lee <selee@mozilla.com>
Sun, 03 Apr 2016 01:07:55 +0800
changeset 363660 4daf7d4669369ef9bdc1f8d4e2ae0acc09649622
parent 362943 0a25833062a880f369e6f9f622413a94cc671bf4
child 520107 13e42d711ffece0693ed01b0e50b20c61ac1787a
push id17277
push userbmo:selee@mozilla.com
push dateThu, 05 May 2016 08:29:32 +0000
reviewersMattN
bugs1261234
milestone49.0a1
Bug 1261234 - Use isURIPotentiallyTrustworthy to verify the form action.; r?MattN MozReview-Commit-ID: CqkG54Qj9mm
toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
toolkit/components/passwordmgr/test/browser/browser.ini
toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
toolkit/components/passwordmgr/test/browser/form_cross_origin_insecure_action.html
toolkit/components/passwordmgr/test/browser/form_cross_origin_secure_action.html
toolkit/components/passwordmgr/test/browser/form_same_origin_action.html
--- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
+++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
@@ -9,16 +9,22 @@ const STRINGS_URI = "chrome://global/loc
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "devtools",
                                   "resource://devtools/shared/Loader.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LoginManagerContent",
                                   "resource://gre/modules/LoginManagerContent.jsm");
+XPCOMUtils.defineLazyServiceGetter(this, "gContentSecurityManager",
+                                   "@mozilla.org/contentsecuritymanager;1",
+                                   "nsIContentSecurityManager");
+XPCOMUtils.defineLazyServiceGetter(this, "gScriptSecurityManager",
+                                   "@mozilla.org/scriptsecuritymanager;1",
+                                   "nsIScriptSecurityManager");
 XPCOMUtils.defineLazyGetter(this, "WebConsoleUtils", () => {
   return this.devtools.require("devtools/shared/webconsole/utils").Utils;
 });
 XPCOMUtils.defineLazyGetter(this, "l10n", () => {
   return new this.WebConsoleUtils.L10n(STRINGS_URI);
 });
 
 this.InsecurePasswordUtils = {
@@ -87,42 +93,48 @@ this.InsecurePasswordUtils = {
     // Check if we are on an iframe with insecure src, or inside another
     // insecure iframe or document.
     if (this._checkForInsecureNestedDocuments(domDoc)) {
       this._sendWebConsoleMessage("InsecurePasswordsPresentOnIframe", domDoc);
       this._formRootsWarned.set(aForm.rootElement, true);
       isSafePage = false;
     }
 
-    let isFormSubmitHTTP = false, isFormSubmitHTTPS = false;
+    let isFormSubmitHTTP = false, isFormSubmitSecure = false;
     if (aForm.rootElement instanceof Ci.nsIDOMHTMLFormElement) {
-      // Note that aForm.action can be a relative path (e.g. "", "/login", "//example.com", etc.)
-      // but we don't warn about those since we would have already warned about the form's document
-      // not being safe above.
-      if (aForm.action.match(/^http:\/\//)) {
-        this._sendWebConsoleMessage("InsecureFormActionPasswordsPresent", domDoc);
-        this._formRootsWarned.set(aForm.rootElement, true);
+      let uri = Services.io.newURI(aForm.rootElement.action, null, null);
+      let principal = gScriptSecurityManager.getCodebasePrincipal(uri);
+      let host = uri.host;
+
+      if (uri.schemeIs("http")) {
         isFormSubmitHTTP = true;
-      } else if (aForm.action.match(/^https:\/\//)) {
-        isFormSubmitHTTPS = true;
+        if (gContentSecurityManager.isOriginPotentiallyTrustworthy(principal)) {
+          isFormSubmitSecure = true;
+        } else if (isSafePage) {
+          // Only warn about the action if we didn't already warn about the form being insecure.
+          this._sendWebConsoleMessage("InsecureFormActionPasswordsPresent", domDoc);
+          this._formRootsWarned.set(aForm.rootElement, true);
+        }
+      } else {
+        isFormSubmitSecure = true;
       }
     }
 
     // The safety of a password field determined by the form action and the page protocol
     let passwordSafety;
     if (isSafePage) {
-      if (isFormSubmitHTTPS) {
+      if (isFormSubmitSecure) {
         passwordSafety = 0;
       } else if (isFormSubmitHTTP) {
         passwordSafety = 1;
       } else {
         passwordSafety = 2;
       }
     } else {
-      if (isFormSubmitHTTPS) {
+      if (isFormSubmitSecure) {
         passwordSafety = 3;
       } else if (isFormSubmitHTTP) {
         passwordSafety = 4;
       } else {
         passwordSafety = 5;
       }
     }
 
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -1,14 +1,17 @@
 [DEFAULT]
 support-files =
   ../formsubmit.sjs
   authenticate.sjs
   form_basic.html
   formless_basic.html
+  form_same_origin_action.html
+  form_cross_origin_insecure_action.html
+  form_cross_origin_secure_action.html
   head.js
   insecure_test.html
   insecure_test_subframe.html
   multiple_forms.html
   streamConverter_content.sjs
 
 [browser_capture_doorhanger.js]
 support-files =
--- a/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
@@ -31,52 +31,64 @@ add_task(function* testInsecurePasswordW
     }
   }
   Services.console.registerListener(messageHandler);
   registerCleanupFunction(function() {
     Services.console.unregisterListener(messageHandler);
   });
 
   for (let [origin, testFile, expectWarnings] of [
-    // Form action at 127.0.0.1/localhost is considered as a secure case.
-    // There should be no INSECURE_FORM_ACTION warning at 127.0.0.1/localhost.
-    // This will be fixed at Bug 1261234.
-    ["http://127.0.0.1", "form_basic.html", ["INSECURE_FORM_ACTION"]],
+    ["http://127.0.0.1", "form_basic.html", []],
     ["http://127.0.0.1", "formless_basic.html", []],
-    ["http://example.com", "form_basic.html", ["INSECURE_FORM_ACTION", "INSECURE_PAGE"]],
+    ["http://example.com", "form_basic.html", ["INSECURE_PAGE"]],
     ["http://example.com", "formless_basic.html", ["INSECURE_PAGE"]],
     ["https://example.com", "form_basic.html", []],
     ["https://example.com", "formless_basic.html", []],
+
+    // For a form with customized action link in the same origin.
+    ["http://127.0.0.1", "form_same_origin_action.html", []],
+    ["http://example.com", "form_same_origin_action.html", ["INSECURE_PAGE"]],
+    ["https://example.com", "form_same_origin_action.html", []],
+
+    // For a form with an insecure (http) customized action link.
+    ["http://127.0.0.1", "form_cross_origin_insecure_action.html", ["INSECURE_FORM_ACTION"]],
+    ["http://example.com", "form_cross_origin_insecure_action.html", ["INSECURE_PAGE"]],
+    ["https://example.com", "form_cross_origin_insecure_action.html", ["INSECURE_FORM_ACTION"]],
+
+    // For a form with a secure (https) customized action link.
+    ["http://127.0.0.1", "form_cross_origin_secure_action.html", []],
+    ["http://example.com", "form_cross_origin_secure_action.html", ["INSECURE_PAGE"]],
+    ["https://example.com", "form_cross_origin_secure_action.html", []],
   ]) {
-    let testUrlPath = origin + TEST_URL_PATH + testFile;
-    var promiseConsoleMessages = new Promise(resolve => {
+    let testURL = origin + TEST_URL_PATH + testFile;
+    let promiseConsoleMessages = new Promise(resolve => {
       warningPatternHandler = function (warning, originMessage) {
         ok(warning, "Handling a warning pattern");
-        let fullMessage = `[${warning.msg} {file: "${testUrlPath}" line: 0 column: 0 source: "0"}]`;
+        let fullMessage = `[${warning.msg} {file: "${testURL}" line: 0 column: 0 source: "0"}]`;
         is(originMessage, fullMessage, "Message full matched:" + originMessage);
 
         let index = expectWarnings.indexOf(warning.key);
-        isnot(index, -1, "Found warning: " + warning.key + " for URL:" + testUrlPath);
+        isnot(index, -1, "Found warning: " + warning.key + " for URL:" + testURL);
         if (index !== -1) {
           // Remove the shown message.
           expectWarnings.splice(index, 1);
         }
         if (expectWarnings.length === 0) {
-          info("All warnings are shown. URL:" + testUrlPath);
+          info("All warnings are shown for URL:" + testURL);
           resolve();
         }
-      }
+      };
     });
 
     yield BrowserTestUtils.withNewTab({
       gBrowser,
-      url: testUrlPath
+      url: testURL
     }, function*() {
       if (expectWarnings.length === 0) {
-        info("All warnings are shown. URL:" + testUrlPath);
+        info("All warnings are shown for URL:" + testURL);
         return Promise.resolve();
       }
       return promiseConsoleMessages;
     });
 
     // Remove warningPatternHandler to stop handling the matched warning pattern
     // and the task should not get any warning anymore.
     warningPatternHandler = null;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_cross_origin_insecure_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="http://another.domain/custom_action.html">
+  <input id="form-basic-username" name="username">
+  <input id="form-basic-password" name="password" type="password">
+  <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_cross_origin_secure_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="https://another.domain/custom_action.html">
+  <input id="form-basic-username" name="username">
+  <input id="form-basic-password" name="password" type="password">
+  <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_same_origin_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="./custom_action.html">
+  <input id="form-basic-username" name="username">
+  <input id="form-basic-password" name="password" type="password">
+  <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>