From f3f1c30ea427ff4174780e44757cd481a55eae71 Mon Sep 17 00:00:00 2001 From: shindli Date: Thu, 28 Nov 2019 16:12:45 +0200 Subject: [PATCH] Backed out changeset 68ff34ec8e96 (bug 1597704) for causing perma bc3 failures in browser/extensions/formautofill/test/browser/browser_autocomplete_footer.js CLOSED TREE --- caps/BasePrincipal.cpp | 16 ----------- caps/BasePrincipal.h | 1 - caps/NullPrincipal.cpp | 5 ---- caps/NullPrincipal.h | 1 - caps/SystemPrincipal.cpp | 6 ----- caps/SystemPrincipal.h | 1 - caps/nsIPrincipal.idl | 12 --------- dom/base/nsContentUtils.cpp | 27 ++++++++++++++----- dom/base/nsGlobalWindowOuter.cpp | 15 ++++++++--- .../security/nsIContentSecurityManager.idl | 11 ++++++++ dom/presentation/PresentationRequest.cpp | 8 +++++- dom/security/nsContentSecurityManager.cpp | 27 +++++++++++++++++++ dom/security/test/gtest/TestSecureContext.cpp | 18 +++++++++---- .../test_isOriginPotentiallyTrustworthy.js | 10 +++++-- .../clearsitedata/ClearSiteData.cpp | 5 +++- .../xpcshell/test_ext_trustworthy_origin.js | 5 +++- 16 files changed, 107 insertions(+), 61 deletions(-) diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index a26298ce9771..49cfcbfd6ad8 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -24,7 +24,6 @@ #include "mozilla/dom/BlobURLProtocolHandler.h" #include "mozilla/dom/ChromeUtils.h" #include "mozilla/dom/ToJSValue.h" -#include "mozilla/dom/nsMixedContentBlocker.h" #include "json/json.h" #include "nsSerializationHelper.h" @@ -499,21 +498,6 @@ BasePrincipal::IsURIInPrefList(const char* aPref, bool* aResult) { return NS_OK; } -NS_IMETHODIMP -BasePrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) { - MOZ_ASSERT(NS_IsMainThread()); - *aResult = false; - - nsCOMPtr uri; - nsresult rv = GetURI(getter_AddRefs(uri)); - if (NS_FAILED(rv) || !uri) { - return NS_OK; - } - - *aResult = nsMixedContentBlocker::IsPotentiallyTrustworthyOrigin(uri); - return NS_OK; -} - NS_IMETHODIMP BasePrincipal::GetAboutModuleFlags(uint32_t* flags) { *flags = 0; diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index 00a936b40b4e..132c53fc347a 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -133,7 +133,6 @@ class BasePrincipal : public nsJSPrincipals { NS_IMETHOD GetSiteOrigin(nsACString& aOrigin) override; NS_IMETHOD IsThirdPartyURI(nsIURI* uri, bool* aRes) override; NS_IMETHOD IsThirdPartyPrincipal(nsIPrincipal* uri, bool* aRes) override; - NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override; nsresult ToJSON(nsACString& aJSON); static already_AddRefed FromJSON(const nsACString& aJSON); diff --git a/caps/NullPrincipal.cpp b/caps/NullPrincipal.cpp index cf3d9468bb26..ca29d12e3257 100644 --- a/caps/NullPrincipal.cpp +++ b/caps/NullPrincipal.cpp @@ -136,11 +136,6 @@ NullPrincipal::GetURI(nsIURI** aURI) { uri.forget(aURI); return NS_OK; } -NS_IMETHODIMP -NullPrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) { - *aResult = false; - return NS_OK; -} NS_IMETHODIMP NullPrincipal::GetDomain(nsIURI** aDomain) { diff --git a/caps/NullPrincipal.h b/caps/NullPrincipal.h index 60c17a12df62..09f09581bed4 100644 --- a/caps/NullPrincipal.h +++ b/caps/NullPrincipal.h @@ -51,7 +51,6 @@ class NullPrincipal final : public BasePrincipal { NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override; uint32_t GetHashValue() override; NS_IMETHOD GetURI(nsIURI** aURI) override; - NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override; NS_IMETHOD GetDomain(nsIURI** aDomain) override; NS_IMETHOD SetDomain(nsIURI* aDomain) override; NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override; diff --git a/caps/SystemPrincipal.cpp b/caps/SystemPrincipal.cpp index b5c3c9337013..cb9c5aca97ef 100644 --- a/caps/SystemPrincipal.cpp +++ b/caps/SystemPrincipal.cpp @@ -51,12 +51,6 @@ SystemPrincipal::GetURI(nsIURI** aURI) { return NS_OK; } -NS_IMETHODIMP -SystemPrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) { - *aResult = true; - return NS_OK; -} - NS_IMETHODIMP SystemPrincipal::GetDomain(nsIURI** aDomain) { *aDomain = nullptr; diff --git a/caps/SystemPrincipal.h b/caps/SystemPrincipal.h index 4853632f348b..63bbf692b625 100644 --- a/caps/SystemPrincipal.h +++ b/caps/SystemPrincipal.h @@ -44,7 +44,6 @@ class SystemPrincipal final : public BasePrincipal { NS_IMETHOD SetDomain(nsIURI* aDomain) override; NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override; NS_IMETHOD GetAddonId(nsAString& aAddonId) override; - NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override; virtual nsresult GetScriptLocation(nsACString& aStr) override; diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index 3d4ce3859e00..2a8d3a34d9a3 100644 --- a/caps/nsIPrincipal.idl +++ b/caps/nsIPrincipal.idl @@ -237,18 +237,6 @@ interface nsIPrincipal : nsISerializable */ bool IsURIInPrefList(in string pref); - /** - * Implementation of - * https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy - * - * The value returned by this method feeds into the the Secure Context - * algorithm that determins the value of Window.isSecureContext and - * WorkerGlobalScope.isSecureContext. - * - * This method returns false instead of throwing upon errors. - */ - readonly attribute bool IsOriginPotentiallyTrustworthy; - /** * Returns the Flags of the Principals * associated AboutModule, in case there is one. diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index ce95f984eee3..e82892721366 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -8891,9 +8891,18 @@ bool nsContentUtils::HttpsStateIsModern(Document* aDocument) { MOZ_ASSERT(principal->GetIsContentPrincipal()); - bool isTrustworthyOrigin = false; - principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin); - return isTrustworthyOrigin; + nsCOMPtr csm = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + NS_WARNING_ASSERTION(csm, "csm is null"); + if (csm) { + bool isTrustworthyOrigin = false; + csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin); + if (isTrustworthyOrigin) { + return true; + } + } + + return false; } /* static */ @@ -8923,9 +8932,15 @@ bool nsContentUtils::ComputeIsSecureContext(nsIChannel* aChannel) { return false; } - bool isTrustworthyOrigin = false; - principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin); - return isTrustworthyOrigin; + nsCOMPtr csm = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + NS_WARNING_ASSERTION(csm, "csm is null"); + if (csm) { + bool isTrustworthyOrigin = false; + csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin); + return isTrustworthyOrigin; + } + return true; } /* static */ diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index dfd6b53fb83c..ecb216e65492 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -1693,9 +1693,18 @@ bool nsGlobalWindowOuter::ComputeIsSecureContext(Document* aDocument, } } - bool isTrustworthyOrigin = false; - principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin); - return isTrustworthyOrigin; + nsCOMPtr csm = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + NS_WARNING_ASSERTION(csm, "csm is null"); + if (csm) { + bool isTrustworthyOrigin = false; + csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin); + if (isTrustworthyOrigin) { + return true; + } + } + + return false; } // We need certain special behavior for remote XUL whitelisted domains, but we diff --git a/dom/interfaces/security/nsIContentSecurityManager.idl b/dom/interfaces/security/nsIContentSecurityManager.idl index 5cd2feffad19..1a1c91021751 100644 --- a/dom/interfaces/security/nsIContentSecurityManager.idl +++ b/dom/interfaces/security/nsIContentSecurityManager.idl @@ -42,4 +42,15 @@ interface nsIContentSecurityManager : nsISupports nsIStreamListener performSecurityCheck(in nsIChannel aChannel, in nsIStreamListener aStreamListener); + /** + * Implementation of + * https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy + * + * The value returned by this method feeds into the the Secure Context + * algorithm that determins the value of Window.isSecureContext and + * WorkerGlobalScope.isSecureContext. + * + * This method returns false instead of throwing upon errors. + */ + boolean isOriginPotentiallyTrustworthy(in nsIPrincipal aPrincipal); }; diff --git a/dom/presentation/PresentationRequest.cpp b/dom/presentation/PresentationRequest.cpp index fd0c887429dd..fd2f1fc08851 100644 --- a/dom/presentation/PresentationRequest.cpp +++ b/dom/presentation/PresentationRequest.cpp @@ -509,8 +509,14 @@ bool PresentationRequest::IsPrioriAuthenticatedURL(const nsAString& aUrl) { return false; } + nsCOMPtr csm = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + if (NS_WARN_IF(!csm)) { + return false; + } + bool isTrustworthyOrigin = false; - principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin); + csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin); return isTrustworthyOrigin; } diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index 2f7274248fcc..09ca5b688d70 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -1051,3 +1051,30 @@ nsContentSecurityManager::PerformSecurityCheck( inAndOutListener.forget(outStreamListener); return NS_OK; } + +NS_IMETHODIMP +nsContentSecurityManager::IsOriginPotentiallyTrustworthy( + nsIPrincipal* aPrincipal, bool* aIsTrustWorthy) { + MOZ_ASSERT(NS_IsMainThread()); + NS_ENSURE_ARG_POINTER(aPrincipal); + NS_ENSURE_ARG_POINTER(aIsTrustWorthy); + + if (aPrincipal->IsSystemPrincipal()) { + *aIsTrustWorthy = true; + return NS_OK; + } + *aIsTrustWorthy = false; + if (aPrincipal->GetIsNullPrincipal()) { + return NS_OK; + } + + MOZ_ASSERT(aPrincipal->GetIsContentPrincipal(), + "Nobody is expected to call us with an nsIExpandedPrincipal"); + + nsCOMPtr uri; + nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri)); + NS_ENSURE_SUCCESS(rv, rv); + *aIsTrustWorthy = nsMixedContentBlocker::IsPotentiallyTrustworthyOrigin(uri); + + return NS_OK; +} diff --git a/dom/security/test/gtest/TestSecureContext.cpp b/dom/security/test/gtest/TestSecureContext.cpp index f971963d9e7d..fa6678dc9396 100644 --- a/dom/security/test/gtest/TestSecureContext.cpp +++ b/dom/security/test/gtest/TestSecureContext.cpp @@ -70,7 +70,8 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) rv = nsScriptSecurityManager::GetScriptSecurityManager() ->CreateContentPrincipalFromOrigin(uri, getter_AddRefs(prin)); bool isPotentiallyTrustworthy = false; - rv = prin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy); + rv = csManager->IsOriginPotentiallyTrustworthy(prin, + &isPotentiallyTrustworthy); ASSERT_EQ(NS_OK, rv); ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult); } @@ -81,10 +82,14 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithSystemPrincipal) RefPtr ssManager = nsScriptSecurityManager::GetScriptSecurityManager(); ASSERT_TRUE(!!ssManager); + nsCOMPtr csManager = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + ASSERT_TRUE(!!csManager); + nsCOMPtr sysPrin = nsContentUtils::GetSystemPrincipal(); bool isPotentiallyTrustworthy; - nsresult rv = - sysPrin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy); + nsresult rv = csManager->IsOriginPotentiallyTrustworthy( + sysPrin, &isPotentiallyTrustworthy); ASSERT_EQ(rv, NS_OK); ASSERT_TRUE(isPotentiallyTrustworthy); } @@ -94,12 +99,15 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithNullPrincipal) RefPtr ssManager = nsScriptSecurityManager::GetScriptSecurityManager(); ASSERT_TRUE(!!ssManager); + nsCOMPtr csManager = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + ASSERT_TRUE(!!csManager); RefPtr nullPrin = NullPrincipal::CreateWithoutOriginAttributes(); bool isPotentiallyTrustworthy; - nsresult rv = - nullPrin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy); + nsresult rv = csManager->IsOriginPotentiallyTrustworthy( + nullPrin, &isPotentiallyTrustworthy); ASSERT_EQ(rv, NS_OK); ASSERT_TRUE(!isPotentiallyTrustworthy); } diff --git a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js index b88c6ed83048..e5bc3f7dcb87 100644 --- a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js +++ b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js @@ -49,12 +49,18 @@ add_task(async function test_isOriginPotentiallyTrustworthy() { ]) { let uri = NetUtil.newURI(uriSpec); let principal = gScriptSecurityManager.createContentPrincipal(uri, {}); - Assert.equal(principal.IsOriginPotentiallyTrustworthy, expectedResult); + Assert.equal( + gContentSecurityManager.isOriginPotentiallyTrustworthy(principal), + expectedResult + ); } // And now let's test whether .onion sites are properly treated when // whitelisted, see bug 1382359. Services.prefs.setBoolPref("dom.securecontext.whitelist_onions", true); let uri = NetUtil.newURI("http://1234567890abcdef.onion/"); let principal = gScriptSecurityManager.createContentPrincipal(uri, {}); - Assert.equal(principal.IsOriginPotentiallyTrustworthy, true); + Assert.equal( + gContentSecurityManager.isOriginPotentiallyTrustworthy(principal), + true + ); }); diff --git a/toolkit/components/clearsitedata/ClearSiteData.cpp b/toolkit/components/clearsitedata/ClearSiteData.cpp index e2760fc83265..e056648b7468 100644 --- a/toolkit/components/clearsitedata/ClearSiteData.cpp +++ b/toolkit/components/clearsitedata/ClearSiteData.cpp @@ -162,8 +162,11 @@ void ClearSiteData::ClearDataFromChannel(nsIHttpChannel* aChannel) { return; } + nsCOMPtr csm = + do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID); + bool secure; - rv = principal->GetIsOriginPotentiallyTrustworthy(&secure); + rv = csm->IsOriginPotentiallyTrustworthy(principal, &secure); if (NS_WARN_IF(NS_FAILED(rv)) || !secure) { return; } diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_trustworthy_origin.js b/toolkit/components/extensions/test/xpcshell/test_ext_trustworthy_origin.js index 5597ccd6ab62..0c6dd0b6f393 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_trustworthy_origin.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_trustworthy_origin.js @@ -6,13 +6,16 @@ add_task( function test_isOriginPotentiallyTrustworthnsIContentSecurityManagery() { + let contentSecManager = Cc[ + "@mozilla.org/contentsecuritymanager;1" + ].getService(Ci.nsIContentSecurityManager); let uri = NetUtil.newURI("moz-extension://foobar/something.html"); let principal = Services.scriptSecurityManager.createContentPrincipal( uri, {} ); Assert.equal( - principal.IsOriginPotentiallyTrustworthy(), + contentSecManager.isOriginPotentiallyTrustworthy(principal), true, "it is potentially trustworthy" );