Bug 1410364 - Don't consider opener when calculating IsSecureContext. r=bz, r=dveditz

Per w3c/webappsec-secure-contexts#42, the
section considering the window opener when calculating secure context is
to be dropped. Firefox already uses "isSecureContextIfOpenerIgnored" in
most places as this is the actual behavior we want. This patch aligns
with the upcoming spec changes by ignoring the window opener. We also no
longer have to keep information about whether our opener was secure as
that no longer factors in our calculations.

--HG--
extra : rebase_source : 3d7fa73976571f357e84e369093aecfc10c5872e
extra : amend_source : ca86714f357b653577f3186b6312bfa00f1f45b9

Author: Kate McKinley

dom/base/nsGlobalWindowInner.cpp

@@ -584,7 +584,6 @@ nsGlobalWindowInner::nsGlobalWindowInner(nsGlobalWindowOuter *aOuterWindow)
     mInClose(false),
     mHavePendingClose(false),
     mHadOriginalOpener(false),
-    mOriginalOpenerWasSecureContext(false),
     mIsPopupSpam(false),
     mBlockScriptedClosingFlag(false),
     mWasOffline(false),
@@ -2055,12 +2054,6 @@ nsPIDOMWindowInner::IsSecureContext() const
   return nsGlobalWindowInner::Cast(this)->IsSecureContext();
 }
 
-bool
-nsPIDOMWindowInner::IsSecureContextIfOpenerIgnored() const
-{
-  return nsGlobalWindowInner::Cast(this)->IsSecureContextIfOpenerIgnored();
-}
-
 void
 nsPIDOMWindowInner::Suspend()
 {
@@ -7407,14 +7400,6 @@ nsGlobalWindowInner::IsSecureContext() const
   return JS_GetIsSecureContext(js::GetObjectCompartment(GetWrapperPreserveColor()));
 }
 
-bool
-nsGlobalWindowInner::IsSecureContextIfOpenerIgnored() const
-{
-  MOZ_RELEASE_ASSERT(IsInnerWindow());
-
-  return mIsSecureContextIfOpenerIgnored;
-}
-
 already_AddRefed<External>
 nsGlobalWindowInner::GetExternal(ErrorResult& aRv)
 {

dom/base/nsGlobalWindowInner.h

@@ -760,7 +760,6 @@ class nsGlobalWindowInner : public mozilla::dom::EventTarget,
 
   // https://w3c.github.io/webappsec-secure-contexts/#dom-window-issecurecontext
   bool IsSecureContext() const;
-  bool IsSecureContextIfOpenerIgnored() const;
 
   void GetSidebar(mozilla::dom::OwningExternalOrWindowProxy& aResult,
                   mozilla::ErrorResult& aRv);
@@ -1371,8 +1370,6 @@ class nsGlobalWindowInner : public mozilla::dom::EventTarget,
   // event posted.  If this is set, just ignore window.close() calls.
   bool                          mHavePendingClose : 1;
   bool                          mHadOriginalOpener : 1;
-  bool                          mOriginalOpenerWasSecureContext : 1;
-  bool                          mIsSecureContextIfOpenerIgnored : 1;
   bool                          mIsPopupSpam : 1;
 
   // Indicates whether scripts are allowed to close this window.

dom/base/nsGlobalWindowOuter.cpp

@@ -628,7 +628,6 @@ nsGlobalWindowOuter::nsGlobalWindowOuter()
     mInClose(false),
     mHavePendingClose(false),
     mHadOriginalOpener(false),
-    mOriginalOpenerWasSecureContext(false),
     mIsPopupSpam(false),
     mBlockScriptedClosingFlag(false),
     mWasOffline(false),
@@ -1561,15 +1560,7 @@ nsGlobalWindowOuter::ComputeIsSecureContext(nsIDocument* aDocument, SecureContex
     MOZ_ASSERT(parentWin ==
                nsGlobalWindowInner::Cast(parentOuterWin->GetCurrentInnerWindow()),
                "Creator window mismatch while setting Secure Context state");
-    if (aFlags != SecureContextFlags::eIgnoreOpener) {
-      hadNonSecureContextCreator = !parentWin->IsSecureContext();
-    } else {
-      hadNonSecureContextCreator = !parentWin->IsSecureContextIfOpenerIgnored();
-    }
-  } else if (mHadOriginalOpener) {
-    if (aFlags != SecureContextFlags::eIgnoreOpener) {
-      hadNonSecureContextCreator = !mOriginalOpenerWasSecureContext;
-    }
+    hadNonSecureContextCreator = !parentWin->IsSecureContext();
   }
 
   if (hadNonSecureContextCreator) {
@@ -1781,8 +1772,6 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument,
       NS_ASSERTION(NS_SUCCEEDED(rv) && newInnerGlobal &&
                    newInnerWindow->GetWrapperPreserveColor() == newInnerGlobal,
                    "Failed to get script global");
-      newInnerWindow->mIsSecureContextIfOpenerIgnored =
-        ComputeIsSecureContext(aDocument, SecureContextFlags::eIgnoreOpener);
 
       mCreatingInnerWindow = false;
       createdInnerWindow = true;
@@ -2243,8 +2232,6 @@ nsGlobalWindowOuter::SetOpenerWindow(nsPIDOMWindowOuter* aOpener,
     MOZ_ASSERT(!mHadOriginalOpener,
                "Probably too late to call ComputeIsSecureContext again");
     mHadOriginalOpener = true;
-    mOriginalOpenerWasSecureContext =
-      aOpener->GetCurrentInnerWindow()->IsSecureContext();
   }
 
 #ifdef DEBUG

dom/base/nsGlobalWindowOuter.h

@@ -1191,8 +1191,6 @@ class nsGlobalWindowOuter : public mozilla::dom::EventTarget,
   // event posted.  If this is set, just ignore window.close() calls.
   bool                          mHavePendingClose : 1;
   bool                          mHadOriginalOpener : 1;
-  bool                          mOriginalOpenerWasSecureContext : 1;
-  bool                          mIsSecureContextIfOpenerIgnored : 1;
   bool                          mIsPopupSpam : 1;
 
   // Indicates whether scripts are allowed to close this window.

dom/base/nsPIDOMWindow.h

@@ -863,7 +863,6 @@ class nsPIDOMWindowInner : public nsPIDOMWindow<mozIDOMWindow>
    * Check whether this window is a secure context.
    */
   bool IsSecureContext() const;
-  bool IsSecureContextIfOpenerIgnored() const;
 
   // Calling suspend should prevent any asynchronous tasks from
   // executing javascript for this window.  This means setTimeout,

dom/geolocation/nsGeolocation.cpp

@@ -1195,7 +1195,7 @@ Geolocation::ShouldBlockInsecureRequests() const
     return false;
   }
 
-  if (!nsGlobalWindowInner::Cast(win)->IsSecureContextIfOpenerIgnored()) {
+  if (!nsGlobalWindowInner::Cast(win)->IsSecureContext()) {
     nsContentUtils::ReportToConsole(nsIScriptError::errorFlag,
                                     NS_LITERAL_CSTRING("DOM"), doc,
                                     nsContentUtils::eDOM_PROPERTIES,

dom/webidl/Window.webidl

@@ -491,19 +491,6 @@ dictionary IdleRequestOptions {
 
 callback IdleRequestCallback = void (IdleDeadline deadline);
 
-/**
- * Similar to |isSecureContext|, but doesn't pay attention to whether the
- * window's opener (if any) is a secure context or not.
- *
- * WARNING: Do not use this unless you are familiar with the issues that
- * taking opener state into account is designed to address (or else you may
- * introduce security issues).  If in doubt, use |isSecureContext|.  In
- * particular do not use this to gate access to JavaScript APIs.
- */
-partial interface Window {
-  [ChromeOnly] readonly attribute boolean isSecureContextIfOpenerIgnored;
-};
-
 partial interface Window {
   /**
    * Returns a list of locales that the internationalization components

toolkit/components/passwordmgr/InsecurePasswordUtils.jsm

@@ -29,17 +29,6 @@ XPCOMUtils.defineLazyGetter(this, "log", () => {
 /*
  * 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(),
@@ -120,8 +109,7 @@ this.InsecurePasswordUtils = {
    * @return {boolean} whether the form is secure
    */
   isFormSecure(aForm) {
-    // Ignores window.opener, see top level documentation.
-    let isSafePage = aForm.ownerDocument.defaultView.isSecureContextIfOpenerIgnored;
+    let isSafePage = aForm.ownerDocument.defaultView.isSecureContext;
 
     // Ignore insecure documents with URLs that are local IP addresses.
     // This is done because the vast majority of routers and other devices
@@ -156,8 +144,7 @@ this.InsecurePasswordUtils = {
     }
 
     let domDoc = aForm.ownerDocument;
-    // Ignores window.opener, see top level documentation.
-    let isSafePage = domDoc.defaultView.isSecureContextIfOpenerIgnored;
+    let isSafePage = domDoc.defaultView.isSecureContext;
 
     let { isFormSubmitHTTP, isFormSubmitSecure } = this._checkFormSecurity(aForm);
 

toolkit/components/passwordmgr/LoginManagerContent.jsm

@@ -492,9 +492,7 @@ var LoginManagerContent = {
     let hasInsecureLoginForms = (thisWindow) => {
       let doc = thisWindow.document;
       let hasLoginForm = this.stateForDocument(doc).loginFormRootElements.size > 0;
-      // Ignores window.opener, because it's not relevant for indicating
-      // form security. See InsecurePasswordUtils docs for details.
-      return (hasLoginForm && !thisWindow.isSecureContextIfOpenerIgnored) ||
+      return (hasLoginForm && !thisWindow.isSecureContext) ||
              Array.some(thisWindow.frames,
                         frame => hasInsecureLoginForms(frame));
     };

toolkit/components/passwordmgr/test/browser/browser.ini

@@ -51,6 +51,8 @@ support-files =
 [browser_hasInsecureLoginForms.js]
 [browser_hasInsecureLoginForms_streamConverter.js]
 [browser_http_autofill.js]
+support-files =
+  form_cross_origin_insecure_action.html
 [browser_insecurePasswordConsoleWarning.js]
 support-files =
   form_cross_origin_insecure_action.html

toolkit/components/passwordmgr/test/browser/browser_http_autofill.js

@@ -75,4 +75,3 @@ add_task(async function test_http_action_autofill() {
     gBrowser.removeTab(tab);
   }
 });
-