From 96d8ee8908179c2e8f9cebf33975d20f7ae4566f Mon Sep 17 00:00:00 2001 From: Mihai Alexandru Michis Date: Fri, 14 Aug 2020 18:43:43 +0300 Subject: [PATCH] Backed out changeset 6b495a62f535 (bug 1658594) for causing failures in test_cors_mixedcontent.html CLOSED TREE --- dom/security/nsHTTPSOnlyUtils.cpp | 12 --- dom/security/nsHTTPSOnlyUtils.h | 8 -- dom/security/nsMixedContentBlocker.cpp | 5 +- .../test/https-only/file_insecure_fetch.html | 30 ------- dom/security/test/https-only/mochitest.ini | 15 ++-- .../https-only/test_cors_mixedcontent.html | 84 ------------------- netwerk/protocol/http/nsCORSListenerProxy.cpp | 6 +- 7 files changed, 14 insertions(+), 146 deletions(-) delete mode 100644 dom/security/test/https-only/file_insecure_fetch.html delete mode 100644 dom/security/test/https-only/test_cors_mixedcontent.html diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp index bb5c2ed0f1d0..5c32d49e917f 100644 --- a/dom/security/nsHTTPSOnlyUtils.cpp +++ b/dom/security/nsHTTPSOnlyUtils.cpp @@ -268,18 +268,6 @@ void nsHTTPSOnlyUtils::TestSitePermissionAndPotentiallyAddExemption( loadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); } -/* static */ -bool nsHTTPSOnlyUtils::IsSafeToAcceptCORSOrMixedContent( - nsILoadInfo* aLoadInfo) { - // Check if the request is exempt from upgrades - if ((aLoadInfo->GetHttpsOnlyStatus() & nsILoadInfo::HTTPS_ONLY_EXEMPT)) { - return false; - } - // Check if HTTPS-Only Mode is enabled for this request - bool isPrivateWin = aLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; - return nsHTTPSOnlyUtils::IsHttpsOnlyModeEnabled(isPrivateWin); -} - /* ------ Logging ------ */ /* static */ diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h index 477c35fbe705..864c17dd8a72 100644 --- a/dom/security/nsHTTPSOnlyUtils.h +++ b/dom/security/nsHTTPSOnlyUtils.h @@ -79,14 +79,6 @@ class nsHTTPSOnlyUtils { static void TestSitePermissionAndPotentiallyAddExemption( nsIChannel* aChannel); - /** - * Checks whether CORS or mixed content requests are safe because they'll get - * upgraded to HTTPS - * @param aLoadInfo nsILoadInfo of request - * @return true if it's safe to accept - */ - static bool IsSafeToAcceptCORSOrMixedContent(nsILoadInfo* aLoadInfo); - private: /** * Logs localized message to either content console or browser console diff --git a/dom/security/nsMixedContentBlocker.cpp b/dom/security/nsMixedContentBlocker.cpp index 03272205ecde..dad6b36e8353 100644 --- a/dom/security/nsMixedContentBlocker.cpp +++ b/dom/security/nsMixedContentBlocker.cpp @@ -645,8 +645,9 @@ nsresult nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect, return NS_OK; } - // Check if https-only mode upgrades this later anyway - if (nsHTTPSOnlyUtils::IsSafeToAcceptCORSOrMixedContent(aLoadInfo)) { + // If https-only mode is enabled we'll upgrade this later anyway + bool isPrivateWin = aLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; + if (nsHTTPSOnlyUtils::IsHttpsOnlyModeEnabled(isPrivateWin)) { *aDecision = ACCEPT; return NS_OK; } diff --git a/dom/security/test/https-only/file_insecure_fetch.html b/dom/security/test/https-only/file_insecure_fetch.html deleted file mode 100644 index 002aa934d0b0..000000000000 --- a/dom/security/test/https-only/file_insecure_fetch.html +++ /dev/null @@ -1,30 +0,0 @@ - - - - - - - - - - diff --git a/dom/security/test/https-only/mochitest.ini b/dom/security/test/https-only/mochitest.ini index d759c03be590..72b80a7ca91c 100644 --- a/dom/security/test/https-only/mochitest.ini +++ b/dom/security/test/https-only/mochitest.ini @@ -1,15 +1,14 @@ -[test_resource_upgrade.html] -scheme=https +[DEFAULT] support-files = + file_redirect.sjs file_upgrade_insecure.html file_upgrade_insecure_server.sjs file_upgrade_insecure_wsh.py +prefs = + security.mixed_content.upgrade_display_content=false + +[test_resource_upgrade.html] +scheme=https [test_redirect_upgrade.html] scheme=https fail-if = xorigin -support-files = - file_redirect.sjs -[test_cors_mixedcontent.html] -scheme=https -support-files = - file_insecure_fetch.html diff --git a/dom/security/test/https-only/test_cors_mixedcontent.html b/dom/security/test/https-only/test_cors_mixedcontent.html deleted file mode 100644 index f1c7f0b29ae5..000000000000 --- a/dom/security/test/https-only/test_cors_mixedcontent.html +++ /dev/null @@ -1,84 +0,0 @@ - - - - - - HTTPS-Only Mode - CORS & Mixed Content - - - - - - -

HTTPS-Only Mode

-

CORS and Mixed Content blocking tests

-

Bug 1658594

- - - - - - - diff --git a/netwerk/protocol/http/nsCORSListenerProxy.cpp b/netwerk/protocol/http/nsCORSListenerProxy.cpp index 0da61b71d1ee..7af59acc2b5f 100644 --- a/netwerk/protocol/http/nsCORSListenerProxy.cpp +++ b/netwerk/protocol/http/nsCORSListenerProxy.cpp @@ -892,9 +892,11 @@ nsresult nsCORSListenerProxy::UpdateChannel(nsIChannel* aChannel, // then the xhr request will be upgraded to https before it fetches any data // from the netwerk, hence we shouldn't require CORS in that specific case. if (CheckInsecureUpgradePreventsCORS(mRequestingPrincipal, aChannel)) { - // Check if https-only mode upgrades this later anyway + // Check if HTTPS-Only Mode is enabled nsCOMPtr loadinfo = aChannel->LoadInfo(); - if (nsHTTPSOnlyUtils::IsSafeToAcceptCORSOrMixedContent(loadinfo)) { + bool isPrivateWin = loadinfo->GetOriginAttributes().mPrivateBrowsingId > 0; + if (!(loadInfo->GetHttpsOnlyStatus() & nsILoadInfo::HTTPS_ONLY_EXEMPT) && + nsHTTPSOnlyUtils::IsHttpsOnlyModeEnabled(isPrivateWin)) { return NS_OK; } // Check if 'upgrade-insecure-requests' is used