Bug 1329940 - Ignore window.opener for password manager warnings r?mattn,jwatt,baku draft
authorKate McKinley <kmckinley@mozilla.com>
Thu, 12 Jan 2017 16:57:44 -0800
changeset 466277 4fb030381a8397e85005acd82d433dd5ea6f01d7
parent 466276 902da67acc3c6fe79decd8762de2bdba7a21a5bb
child 543386 8a39119d4bd1912247b63c4c6970d8b7f3e4a0ba
push id42857
push userbmo:kmckinley@mozilla.com
push dateWed, 25 Jan 2017 19:28:39 +0000
reviewersmattn, jwatt, baku
bugs1329940
milestone54.0a1
Bug 1329940 - Ignore window.opener for password manager warnings r?mattn,jwatt,baku MozReview-Commit-ID: KiIR6WEddgO
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/base/nsPIDOMWindow.h
dom/webidl/Window.webidl
toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -2379,26 +2379,27 @@ InitializeLegacyNetscapeObject(JSContext
   /* Define PrivilegeManager object with the necessary "static" methods. */
   obj = JS_DefineObject(aCx, obj, "PrivilegeManager", nullptr);
   NS_ENSURE_TRUE(obj, false);
 
   return JS_DefineFunctions(aCx, obj, EnablePrivilegeSpec);
 }
 
 bool
-nsGlobalWindow::ComputeIsSecureContext(nsIDocument* aDocument)
+nsGlobalWindow::ComputeIsSecureContext(nsIDocument* aDocument, SecureContextFlags aFlags)
 {
   MOZ_ASSERT(IsOuterWindow());
 
   nsCOMPtr<nsIPrincipal> principal = aDocument->NodePrincipal();
   if (nsContentUtils::IsSystemPrincipal(principal)) {
     return true;
   }
 
   // Implement https://w3c.github.io/webappsec-secure-contexts/#settings-object
+  // With some modifications to allow for aFlags.
 
   bool hadNonSecureContextCreator = false;
 
   nsPIDOMWindowOuter* parentOuterWin = GetScriptableParent();
   MOZ_ASSERT(parentOuterWin, "How can we get here? No docShell somehow?");
   if (nsGlobalWindow::Cast(parentOuterWin) != this) {
     // There may be a small chance that parentOuterWin has navigated in
     // the time that it took us to start loading this sub-document.  If that
@@ -2414,19 +2415,25 @@ nsGlobalWindow::ComputeIsSecureContext(n
     nsGlobalWindow* parentWin =
       nsGlobalWindow::Cast(creatorDoc->GetInnerWindow());
     if (!parentWin) {
       return false; // we must be tearing down
     }
     MOZ_ASSERT(parentWin ==
                nsGlobalWindow::Cast(parentOuterWin->GetCurrentInnerWindow()),
                "Creator window mismatch while setting Secure Context state");
-    hadNonSecureContextCreator = !parentWin->IsSecureContext();
+    if (aFlags != SecureContextFlags::eIgnoreOpener) {
+      hadNonSecureContextCreator = !parentWin->IsSecureContext();
+    } else {
+      hadNonSecureContextCreator = !parentWin->IsSecureContextIfOpenerIgnored();
+    }
   } else if (mHadOriginalOpener) {
-    hadNonSecureContextCreator = !mOriginalOpenerWasSecureContext;
+    if (aFlags != SecureContextFlags::eIgnoreOpener) {
+      hadNonSecureContextCreator = !mOriginalOpenerWasSecureContext;
+    }
   }
 
   if (hadNonSecureContextCreator) {
     return false;
   }
 
   if (nsContentUtils::HttpsStateIsModern(aDocument)) {
     return true;
@@ -2721,16 +2728,18 @@ nsGlobalWindow::SetNewDocument(nsIDocume
       rv = CreateNativeGlobalForInner(cx, newInnerWindow,
                                       aDocument->GetDocumentURI(),
                                       aDocument->NodePrincipal(),
                                       &newInnerGlobal,
                                       ComputeIsSecureContext(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;
 
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     if (currentInner && currentInner->GetWrapperPreserveColor()) {
@@ -3862,16 +3871,22 @@ nsPIDOMWindowInner::CreatePerformanceObj
 }
 
 bool
 nsPIDOMWindowInner::IsSecureContext() const
 {
   return nsGlobalWindow::Cast(this)->IsSecureContext();
 }
 
+bool
+nsPIDOMWindowInner::IsSecureContextIfOpenerIgnored() const
+{
+  return nsGlobalWindow::Cast(this)->IsSecureContextIfOpenerIgnored();
+}
+
 void
 nsPIDOMWindowInner::Suspend()
 {
   nsGlobalWindow::Cast(this)->Suspend();
 }
 
 void
 nsPIDOMWindowInner::Resume()
@@ -13822,16 +13837,24 @@ nsGlobalWindow::GetConsole(ErrorResult& 
 bool
 nsGlobalWindow::IsSecureContext() const
 {
   MOZ_RELEASE_ASSERT(IsInnerWindow());
 
   return JS_GetIsSecureContext(js::GetObjectCompartment(GetWrapperPreserveColor()));
 }
 
+bool
+nsGlobalWindow::IsSecureContextIfOpenerIgnored() const
+{
+  MOZ_RELEASE_ASSERT(IsInnerWindow());
+
+  return mIsSecureContextIfOpenerIgnored;
+}
+
 already_AddRefed<External>
 nsGlobalWindow::GetExternal(ErrorResult& aRv)
 {
   MOZ_RELEASE_ASSERT(IsInnerWindow());
 
 #ifdef HAVE_SIDEBAR
   if (!mExternal) {
     AutoJSContext cx;
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -906,16 +906,17 @@ public:
 #if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_GONK)
   int16_t Orientation(mozilla::dom::CallerType aCallerType) const;
 #endif
 
   mozilla::dom::Console* GetConsole(mozilla::ErrorResult& aRv);
 
   // 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);
   already_AddRefed<mozilla::dom::External> GetExternal(mozilla::ErrorResult& aRv);
 
   // Exposed only for testing
   static bool
   TokenizeDialogOptions(nsAString& aToken, nsAString::const_iterator& aIter,
@@ -1748,19 +1749,26 @@ protected:
   void CheckForDPIChange();
 
 private:
   // Fire the JS engine's onNewGlobalObject hook.  Only used on inner windows.
   void FireOnNewGlobalObject();
 
   void DisconnectEventTargetObjects();
 
+
+  enum class SecureContextFlags {
+    eDefault,
+    eIgnoreOpener
+  };
   // Called only on outer windows to compute the value that will be returned by
   // IsSecureContext() for the inner window that corresponds to aDocument.
-  bool ComputeIsSecureContext(nsIDocument* aDocument);
+  bool ComputeIsSecureContext(nsIDocument* aDocument,
+                              SecureContextFlags aFlags =
+                                SecureContextFlags::eDefault);
 
   // nsPIDOMWindow<T> should be able to see these helper methods.
   friend class nsPIDOMWindow<mozIDOMWindowProxy>;
   friend class nsPIDOMWindow<mozIDOMWindow>;
   friend class nsPIDOMWindow<nsISupports>;
 
   mozilla::dom::TabGroup* TabGroupInner();
   mozilla::dom::TabGroup* TabGroupOuter();
@@ -1787,16 +1795,17 @@ protected:
   bool                          mIsClosed : 1;
   bool                          mInClose : 1;
   // mHavePendingClose means we've got a termination function set to
   // close us when the JS stops executing or that we have a close
   // 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.
   bool                          mBlockScriptedClosingFlag : 1;
 
   // Window offline status. Checked to see if we need to fire offline event
   bool                          mWasOffline : 1;
 
--- a/dom/base/nsPIDOMWindow.h
+++ b/dom/base/nsPIDOMWindow.h
@@ -846,16 +846,17 @@ public:
   {
     return mInnerObjectsFreed;
   }
 
   /**
    * 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,
   // requestAnimationFrame, and events should not be fired. Suspending
   // a window also suspends its children and workers.  Workers may
   // continue to perform computations in the background.  A window
   // can have Suspend() called multiple times and will only resume after
   // a matching number of Resume() calls.
--- a/dom/webidl/Window.webidl
+++ b/dom/webidl/Window.webidl
@@ -509,8 +509,21 @@ partial interface Window {
   void          cancelIdleCallback(unsigned long handle);
 };
 
 dictionary IdleRequestOptions {
   unsigned long timeout;
 };
 
 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;
+};
--- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
+++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
@@ -70,17 +70,25 @@ 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) {
-    let isSafePage = aForm.ownerDocument.defaultView.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. Hence we use isSecureContextIfOpenerIgnored here.
+    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.
    *