From 6d2a972dfeb09fe8737e41674bf125b3aa698f45 Mon Sep 17 00:00:00 2001 From: shindli Date: Wed, 22 Nov 2017 22:17:02 +0200 Subject: [PATCH] Backed out 1 changesets (bug 1410364) Backed out changeset 681fece780ae (bug 1410364) for failing in /secure-contexts/basic-popup-and-iframe-tests.html r=backout a=backout on a CLOSED TREE --- dom/base/nsGlobalWindowInner.cpp | 15 +++++++++++++++ dom/base/nsGlobalWindowInner.h | 3 +++ dom/base/nsGlobalWindowOuter.cpp | 15 ++++++++++++++- dom/base/nsGlobalWindowOuter.h | 2 ++ dom/base/nsPIDOMWindow.h | 1 + dom/geolocation/nsGeolocation.cpp | 2 +- dom/webidl/Window.webidl | 13 +++++++++++++ .../passwordmgr/InsecurePasswordUtils.jsm | 17 +++++++++++++++-- .../passwordmgr/LoginManagerContent.jsm | 4 +++- .../passwordmgr/test/browser/browser.ini | 2 -- .../test/browser/browser_http_autofill.js | 1 + 11 files changed, 68 insertions(+), 7 deletions(-) diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index ed5c0c099fdd..2ece33e9581b 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -584,6 +584,7 @@ nsGlobalWindowInner::nsGlobalWindowInner(nsGlobalWindowOuter *aOuterWindow) mInClose(false), mHavePendingClose(false), mHadOriginalOpener(false), + mOriginalOpenerWasSecureContext(false), mIsPopupSpam(false), mBlockScriptedClosingFlag(false), mWasOffline(false), @@ -2054,6 +2055,12 @@ nsPIDOMWindowInner::IsSecureContext() const return nsGlobalWindowInner::Cast(this)->IsSecureContext(); } +bool +nsPIDOMWindowInner::IsSecureContextIfOpenerIgnored() const +{ + return nsGlobalWindowInner::Cast(this)->IsSecureContextIfOpenerIgnored(); +} + void nsPIDOMWindowInner::Suspend() { @@ -7400,6 +7407,14 @@ nsGlobalWindowInner::IsSecureContext() const return JS_GetIsSecureContext(js::GetObjectCompartment(GetWrapperPreserveColor())); } +bool +nsGlobalWindowInner::IsSecureContextIfOpenerIgnored() const +{ + MOZ_RELEASE_ASSERT(IsInnerWindow()); + + return mIsSecureContextIfOpenerIgnored; +} + already_AddRefed nsGlobalWindowInner::GetExternal(ErrorResult& aRv) { diff --git a/dom/base/nsGlobalWindowInner.h b/dom/base/nsGlobalWindowInner.h index e24e0c343427..39e89261e2d1 100644 --- a/dom/base/nsGlobalWindowInner.h +++ b/dom/base/nsGlobalWindowInner.h @@ -760,6 +760,7 @@ public: // 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); @@ -1370,6 +1371,8 @@ protected: // 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. diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index 3d8bed549098..59ddbbcca657 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -628,6 +628,7 @@ nsGlobalWindowOuter::nsGlobalWindowOuter() mInClose(false), mHavePendingClose(false), mHadOriginalOpener(false), + mOriginalOpenerWasSecureContext(false), mIsPopupSpam(false), mBlockScriptedClosingFlag(false), mWasOffline(false), @@ -1560,7 +1561,15 @@ nsGlobalWindowOuter::ComputeIsSecureContext(nsIDocument* aDocument, SecureContex MOZ_ASSERT(parentWin == nsGlobalWindowInner::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) { + if (aFlags != SecureContextFlags::eIgnoreOpener) { + hadNonSecureContextCreator = !mOriginalOpenerWasSecureContext; + } } if (hadNonSecureContextCreator) { @@ -1772,6 +1781,8 @@ 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; @@ -2232,6 +2243,8 @@ nsGlobalWindowOuter::SetOpenerWindow(nsPIDOMWindowOuter* aOpener, MOZ_ASSERT(!mHadOriginalOpener, "Probably too late to call ComputeIsSecureContext again"); mHadOriginalOpener = true; + mOriginalOpenerWasSecureContext = + aOpener->GetCurrentInnerWindow()->IsSecureContext(); } #ifdef DEBUG diff --git a/dom/base/nsGlobalWindowOuter.h b/dom/base/nsGlobalWindowOuter.h index ac4c38d8e53a..8d83755e88b5 100644 --- a/dom/base/nsGlobalWindowOuter.h +++ b/dom/base/nsGlobalWindowOuter.h @@ -1191,6 +1191,8 @@ protected: // 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. diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h index f58738dbcd13..e75e5746c8cf 100644 --- a/dom/base/nsPIDOMWindow.h +++ b/dom/base/nsPIDOMWindow.h @@ -863,6 +863,7 @@ public: * 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, diff --git a/dom/geolocation/nsGeolocation.cpp b/dom/geolocation/nsGeolocation.cpp index 915f5f3d57e3..c987b53ae71d 100644 --- a/dom/geolocation/nsGeolocation.cpp +++ b/dom/geolocation/nsGeolocation.cpp @@ -1195,7 +1195,7 @@ Geolocation::ShouldBlockInsecureRequests() const return false; } - if (!nsGlobalWindowInner::Cast(win)->IsSecureContext()) { + if (!nsGlobalWindowInner::Cast(win)->IsSecureContextIfOpenerIgnored()) { nsContentUtils::ReportToConsole(nsIScriptError::errorFlag, NS_LITERAL_CSTRING("DOM"), doc, nsContentUtils::eDOM_PROPERTIES, diff --git a/dom/webidl/Window.webidl b/dom/webidl/Window.webidl index c326613463dc..c51646feba83 100644 --- a/dom/webidl/Window.webidl +++ b/dom/webidl/Window.webidl @@ -491,6 +491,19 @@ 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 diff --git a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm index b986607d1c21..6bcc5371a232 100644 --- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm +++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm @@ -29,6 +29,17 @@ 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(), @@ -109,7 +120,8 @@ this.InsecurePasswordUtils = { * @return {boolean} whether the form is secure */ isFormSecure(aForm) { - let isSafePage = aForm.ownerDocument.defaultView.isSecureContext; + // Ignores window.opener, see top level documentation. + let isSafePage = aForm.ownerDocument.defaultView.isSecureContextIfOpenerIgnored; // Ignore insecure documents with URLs that are local IP addresses. // This is done because the vast majority of routers and other devices @@ -144,7 +156,8 @@ this.InsecurePasswordUtils = { } 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); diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index 5222ad106148..f1fc9bdf800a 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -492,7 +492,9 @@ var LoginManagerContent = { 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)); }; diff --git a/toolkit/components/passwordmgr/test/browser/browser.ini b/toolkit/components/passwordmgr/test/browser/browser.ini index d2b4748af447..672f95aa972e 100644 --- a/toolkit/components/passwordmgr/test/browser/browser.ini +++ b/toolkit/components/passwordmgr/test/browser/browser.ini @@ -51,8 +51,6 @@ 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 diff --git a/toolkit/components/passwordmgr/test/browser/browser_http_autofill.js b/toolkit/components/passwordmgr/test/browser/browser_http_autofill.js index fc5be0c192d6..60d306554560 100644 --- a/toolkit/components/passwordmgr/test/browser/browser_http_autofill.js +++ b/toolkit/components/passwordmgr/test/browser/browser_http_autofill.js @@ -75,3 +75,4 @@ add_task(async function test_http_action_autofill() { gBrowser.removeTab(tab); } }); +