This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-91.13.0esr-11.5-1 in repository tor-browser.
commit 59ba109bb0c8a3967e80d047569beef159f73db3 Author: lyavor lyavor@mozilla.com AuthorDate: Mon Oct 11 13:51:53 2021 +0000
Bug 1722489 - Evaluate HSTS before https-only in NS_ShouldSecureUpgrade. r=ckerschb,necko-reviewers,kershaw
Differential Revision: https://phabricator.services.mozilla.com/D126238 --- dom/security/test/https-only/browser.ini | 3 + dom/security/test/https-only/browser_hsts_host.js | 111 +++++++ dom/security/test/https-only/hsts_headers.sjs | 24 ++ netwerk/base/nsNetUtil.cpp | 354 ++++++++++++---------- 4 files changed, 338 insertions(+), 154 deletions(-)
diff --git a/dom/security/test/https-only/browser.ini b/dom/security/test/https-only/browser.ini index bfe3b05140614..5797ace1adb1d 100644 --- a/dom/security/test/https-only/browser.ini +++ b/dom/security/test/https-only/browser.ini @@ -16,3 +16,6 @@ support-files = [browser_user_gesture.js] support-files = file_user_gesture.html +[browser_hsts_host.js] +support-files = + hsts_headers.sjs diff --git a/dom/security/test/https-only/browser_hsts_host.js b/dom/security/test/https-only/browser_hsts_host.js new file mode 100644 index 0000000000000..2bef7ffe44813 --- /dev/null +++ b/dom/security/test/https-only/browser_hsts_host.js @@ -0,0 +1,111 @@ +// Bug 1722489 - HTTPS-Only Mode - Tests evaluation order +// https://bugzilla.mozilla.org/show_bug.cgi?id=1722489 +// This test ensures that an http request to an hsts host +// gets upgraded by hsts and not by https-only. +"use strict"; + +// Set bools to track that tests ended. +let readMessage = false; +let testFinished = false; +// Visit a secure site that sends an HSTS header to set up the rest of the +// test. +add_task(async function see_hsts_header() { + let setHstsUrl = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.com" + ) + "hsts_headers.sjs"; + Services.obs.addObserver(observer, "http-on-examine-response"); + await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, setHstsUrl); + + await BrowserTestUtils.waitForCondition(() => readMessage); + // Clean up + Services.obs.removeObserver(observer, "http-on-examine-response"); +}); + +// Test that HTTPS_Only is not performed if HSTS host is visited. +add_task(async function() { + // A longer timeout is necessary for this test than the plain mochitests + // due to opening a new tab with the web console. + requestLongerTimeout(4); + + // Enable HTTPS-Only Mode and register console-listener + await SpecialPowers.pushPrefEnv({ + set: [["dom.security.https_only_mode", true]], + }); + Services.console.registerListener(onNewMessage); + const RESOURCE_LINK = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "http://example.com" + ) + "hsts_headers.sjs"; + + // 1. Upgrade page to https:// + await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, RESOURCE_LINK); + + await BrowserTestUtils.waitForCondition(() => testFinished); + + // Clean up + Services.console.unregisterListener(onNewMessage); +}); + +add_task(async function() { + // Reset HSTS header + readMessage = false; + let clearHstsUrl = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.com" + ) + "hsts_headers.sjs?reset"; + + Services.obs.addObserver(observer, "http-on-examine-response"); + // reset hsts header + await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, clearHstsUrl); + await BrowserTestUtils.waitForCondition(() => readMessage); + // Clean up + Services.obs.removeObserver(observer, "http-on-examine-response"); +}); + +function observer(subject, topic, state) { + info("observer called with " + topic); + if (topic == "http-on-examine-response") { + onExamineResponse(subject); + } +} + +function onExamineResponse(subject) { + let channel = subject.QueryInterface(Ci.nsIHttpChannel); + info("onExamineResponse with " + channel.URI.spec); + if (channel.URI.spec.includes("reset")) { + try { + let hsts = channel.getResponseHeader("Strict-Transport-Security"); + is(hsts, "max-age=0", "HSTS header is not set"); + } catch (e) { + ok(false, "HSTS header still set"); + } + readMessage = true; + return; + } + try { + let hsts = channel.getResponseHeader("Strict-Transport-Security"); + let csp = channel.getResponseHeader("Content-Security-Policy"); + // Check that HSTS and CSP upgrade headers are set + is(hsts, "max-age=60", "HSTS header is set"); + is(csp, "upgrade-insecure-requests", "CSP header is set"); + } catch (e) { + ok(false, "No header set"); + } + readMessage = true; +} + +function onNewMessage(msgObj) { + const message = msgObj.message; + // ensure that request is not upgraded HTTPS-Only. + if (message.includes("Upgrading insecure request")) { + ok(false, "Top-Level upgrade shouldn't get logged"); + testFinished = true; + } else if (gBrowser.selectedBrowser.currentURI.scheme === "https") { + ok(true, "Top-Level upgrade shouldn't get logged"); + testFinished = true; + } +} diff --git a/dom/security/test/https-only/hsts_headers.sjs b/dom/security/test/https-only/hsts_headers.sjs new file mode 100644 index 0000000000000..72e82caaf3466 --- /dev/null +++ b/dom/security/test/https-only/hsts_headers.sjs @@ -0,0 +1,24 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +function handleRequest(request, response) { + if (request.queryString === "reset") { + // Reset the HSTS policy, prevent influencing other tests + response.setStatusLine(request.httpVersion, 200, "OK"); + response.setHeader("Strict-Transport-Security", "max-age=0"); + response.write("Resetting HSTS"); + return; + } + let hstsHeader = "max-age=60"; + response.setHeader("Strict-Transport-Security", hstsHeader); + response.setHeader("Cache-Control", "no-cache", false); + response.setHeader("Content-Type", "text/html", false); + // Set header for csp upgrade + response.setHeader( + "Content-Security-Policy", + "upgrade-insecure-requests", + false + ); + response.setStatusLine(request.httpVersion, 200); + response.write("<!DOCTYPE html><html><body><h1>Ok!</h1></body></html>"); +} diff --git a/netwerk/base/nsNetUtil.cpp b/netwerk/base/nsNetUtil.cpp index e7602ce75e3b4..824f0979e506e 100644 --- a/netwerk/base/nsNetUtil.cpp +++ b/netwerk/base/nsNetUtil.cpp @@ -2827,6 +2827,112 @@ bool NS_IsSrcdocChannel(nsIChannel* aChannel) { return false; }
+// helper function for NS_ShouldSecureUpgrade for checking HSTS +bool handleResultFunc(bool aAllowSTS, bool aIsStsHost, uint32_t aHstsSource) { + if (aIsStsHost) { + LOG(("nsHttpChannel::Connect() STS permissions found\n")); + if (aAllowSTS) { + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::STS); + switch (aHstsSource) { + case nsISiteSecurityService::SOURCE_PRELOAD_LIST: + Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 0); + break; + case nsISiteSecurityService::SOURCE_ORGANIC_REQUEST: + Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1); + break; + case nsISiteSecurityService::SOURCE_UNKNOWN: + default: + // record this as an organic request + Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1); + break; + } + return true; + } + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::PrefBlockedSTS); + } else { + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::NoReasonToUpgrade); + } + return false; +}; +// That function is a helper function of NS_ShouldSecureUpgrade to check if +// CSP upgrade-insecure-requests, Mixed content auto upgrading or HTTPs-Only/- +// First should upgrade the given request. +static bool ShouldSecureUpgradeNoHSTS(nsIURI* aURI, nsILoadInfo* aLoadInfo) { + // 2. CSP upgrade-insecure-requests + if (aLoadInfo->GetUpgradeInsecureRequests()) { + // let's log a message to the console that we are upgrading a request + nsAutoCString scheme; + aURI->GetScheme(scheme); + // append the additional 's' for security to the scheme :-) + scheme.AppendLiteral("s"); + NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); + NS_ConvertUTF8toUTF16 reportScheme(scheme); + AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; + uint32_t innerWindowId = aLoadInfo->GetInnerWindowID(); + CSP_LogLocalizedStr("upgradeInsecureRequest", params, + u""_ns, // aSourceFile + u""_ns, // aScriptSample + 0, // aLineNumber + 0, // aColumnNumber + nsIScriptError::warningFlag, + "upgradeInsecureRequest"_ns, innerWindowId, + !!aLoadInfo->GetOriginAttributes().mPrivateBrowsingId); + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::CSP); + return true; + } + // 3. Mixed content auto upgrading + if (aLoadInfo->GetBrowserUpgradeInsecureRequests()) { + // let's log a message to the console that we are upgrading a request + nsAutoCString scheme; + aURI->GetScheme(scheme); + // append the additional 's' for security to the scheme :-) + scheme.AppendLiteral("s"); + NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); + NS_ConvertUTF8toUTF16 reportScheme(scheme); + AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; + + nsAutoString localizedMsg; + nsContentUtils::FormatLocalizedString(nsContentUtils::eSECURITY_PROPERTIES, + "MixedContentAutoUpgrade", params, + localizedMsg); + + // Prepending ixed Content to the outgoing console message + nsString message; + message.AppendLiteral(u"Mixed Content: "); + message.Append(localizedMsg); + + uint32_t innerWindowId = aLoadInfo->GetInnerWindowID(); + nsContentUtils::ReportToConsoleByWindowID( + message, nsIScriptError::warningFlag, "Mixed Content Message"_ns, + innerWindowId, aURI); + + // Set this flag so we know we'll upgrade because of + // 'security.mixed_content.upgrade_display_content'. + aLoadInfo->SetBrowserDidUpgradeInsecureRequests(true); + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::BrowserDisplay); + + return true; + } + + // 4. Https-Only / -First + if (nsHTTPSOnlyUtils::ShouldUpgradeRequest(aURI, aLoadInfo) || + nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(aURI, aLoadInfo)) { + return true; + } + return false; +} + +// Check if channel should be upgraded. check in the following order: +// 1. HSTS +// 2. CSP upgrade-insecure-requests +// 3. Mixed content auto upgrading +// 4. Https-Only / first +// (5. Https RR - will be checked in nsHttpChannel) nsresult NS_ShouldSecureUpgrade( nsIURI* aURI, nsILoadInfo* aLoadInfo, nsIPrincipal* aChannelResultPrincipal, bool aPrivateBrowsing, bool aAllowSTS, @@ -2839,6 +2945,7 @@ nsresult NS_ShouldSecureUpgrade( }
aWillCallback = false; + aShouldUpgrade = false;
// Even if we're in private browsing mode, we still enforce existing STS // data (it is read-only). @@ -2846,166 +2953,105 @@ nsresult NS_ShouldSecureUpgrade( // a superdomain wants to force HTTPS, do it. bool isHttps = aURI->SchemeIs("https");
- if (!isHttps && - !nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackURL(aURI)) { - if (aLoadInfo) { - // Check if the request can get upgraded with the HTTPS-Only mode - if (nsHTTPSOnlyUtils::ShouldUpgradeRequest(aURI, aLoadInfo) || - nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(aURI, aLoadInfo)) { - aShouldUpgrade = true; - return NS_OK; - } - - // If any of the documents up the chain to the root document makes use of - // the CSP directive 'upgrade-insecure-requests', then it's time to - // fulfill the promise to CSP and mixed content blocking to upgrade the - // channel from http to https. - if (aLoadInfo->GetUpgradeInsecureRequests() || - aLoadInfo->GetBrowserUpgradeInsecureRequests()) { - // let's log a message to the console that we are upgrading a request - nsAutoCString scheme; - aURI->GetScheme(scheme); - // append the additional 's' for security to the scheme :-) - scheme.AppendLiteral("s"); - NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); - NS_ConvertUTF8toUTF16 reportScheme(scheme); - - if (aLoadInfo->GetUpgradeInsecureRequests()) { - AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; - uint32_t innerWindowId = aLoadInfo->GetInnerWindowID(); - CSP_LogLocalizedStr( - "upgradeInsecureRequest", params, - u""_ns, // aSourceFile - u""_ns, // aScriptSample - 0, // aLineNumber - 0, // aColumnNumber - nsIScriptError::warningFlag, "upgradeInsecureRequest"_ns, - innerWindowId, - !!aLoadInfo->GetOriginAttributes().mPrivateBrowsingId); - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::CSP); - } else { - AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; - - nsAutoString localizedMsg; - nsContentUtils::FormatLocalizedString( - nsContentUtils::eSECURITY_PROPERTIES, "MixedContentAutoUpgrade", - params, localizedMsg); - - // Prepending ixed Content to the outgoing console message - nsString message; - message.AppendLiteral(u"Mixed Content: "); - message.Append(localizedMsg); - - uint32_t innerWindowId = aLoadInfo->GetInnerWindowID(); - nsContentUtils::ReportToConsoleByWindowID( - message, nsIScriptError::warningFlag, "Mixed Content Message"_ns, - innerWindowId, aURI); - - // Set this flag so we know we'll upgrade because of - // 'security.mixed_content.upgrade_display_content'. - aLoadInfo->SetBrowserDidUpgradeInsecureRequests(true); - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::BrowserDisplay); - } - - aShouldUpgrade = true; - return NS_OK; - } - } - - // enforce Strict-Transport-Security - nsISiteSecurityService* sss = gHttpHandler->GetSSService(); - NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY); - - bool isStsHost = false; - uint32_t hstsSource = 0; - uint32_t flags = - aPrivateBrowsing ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; - - auto handleResultFunc = [aAllowSTS](bool aIsStsHost, uint32_t aHstsSource) { - if (aIsStsHost) { - LOG(("nsHttpChannel::Connect() STS permissions found\n")); - if (aAllowSTS) { - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::STS); - switch (aHstsSource) { - case nsISiteSecurityService::SOURCE_PRELOAD_LIST: - Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 0); - break; - case nsISiteSecurityService::SOURCE_ORGANIC_REQUEST: - Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1); - break; - case nsISiteSecurityService::SOURCE_UNKNOWN: - default: - // record this as an organic request - Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1); - break; - } - return true; - } - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::PrefBlockedSTS); - } else { - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::NoReasonToUpgrade); + // If request is https, then there is nothing to do here. + if (isHttps) { + Telemetry::AccumulateCategorical( + Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::AlreadyHTTPS); + aShouldUpgrade = false; + return NS_OK; + } + // If it is a mixed content trustworthy loopback, then we shouldn't upgrade + // it. + if (nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackURL(aURI)) { + aShouldUpgrade = false; + return NS_OK; + } + // If no loadInfo exist there is nothing to upgrade here. + if (!aLoadInfo) { + aShouldUpgrade = false; + return NS_OK; + } + MOZ_ASSERT(!aURI->SchemeIs("https")); + + // enforce Strict-Transport-Security + nsISiteSecurityService* sss = gHttpHandler->GetSSService(); + NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY); + + bool isStsHost = false; + uint32_t hstsSource = 0; + uint32_t flags = + aPrivateBrowsing ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; + // Calling |IsSecureURI| before the storage is ready to read will + // block the main thread. Once the storage is ready, we can call it + // from main thread. + static Atomic<bool, Relaxed> storageReady(false); + if (!storageReady && gSocketTransportService && aResultCallback) { + nsCOMPtr<nsILoadInfo> loadInfo = aLoadInfo; + nsCOMPtr<nsIURI> uri = aURI; + auto callbackWrapper = [resultCallback{std::move(aResultCallback)}, uri, + loadInfo](bool aShouldUpgrade, nsresult aStatus) { + MOZ_ASSERT(NS_IsMainThread()); + + // 1. HSTS upgrade + if (aShouldUpgrade || NS_FAILED(aStatus)) { + resultCallback(aShouldUpgrade, aStatus); + return; } - return false; + // Check if we need to upgrade because of other reasons. + // 2. CSP upgrade-insecure-requests + // 3. Mixed content auto upgrading + // 4. Https-Only / first + bool shouldUpgrade = ShouldSecureUpgradeNoHSTS(uri, loadInfo); + resultCallback(shouldUpgrade, aStatus); }; + nsCOMPtr<nsISiteSecurityService> service = sss; + nsresult rv = gSocketTransportService->Dispatch( + NS_NewRunnableFunction( + "net::NS_ShouldSecureUpgrade", + [service{std::move(service)}, uri{std::move(uri)}, flags(flags), + originAttributes(aOriginAttributes), + handleResultFunc{std::move(handleResultFunc)}, + callbackWrapper{std::move(callbackWrapper)}, + allowSTS{std::move(aAllowSTS)}]() mutable { + bool isStsHost = false; + uint32_t hstsSource = 0; + nsresult rv = + service->IsSecureURI(uri, flags, originAttributes, nullptr, + &hstsSource, &isStsHost); + + // Successfully get the result from |IsSecureURI| implies that + // the storage is ready to read. + storageReady = NS_SUCCEEDED(rv); + bool shouldUpgrade = + handleResultFunc(allowSTS, isStsHost, hstsSource); + // Check if request should be upgraded. + NS_DispatchToMainThread(NS_NewRunnableFunction( + "net::NS_ShouldSecureUpgrade::ResultCallback", + [rv, shouldUpgrade, + callbackWrapper{std::move(callbackWrapper)}]() { + callbackWrapper(shouldUpgrade, rv); + })); + }), + NS_DISPATCH_NORMAL); + aWillCallback = NS_SUCCEEDED(rv); + return rv; + }
- // Calling |IsSecureURI| before the storage is ready to read will - // block the main thread. Once the storage is ready, we can call it - // from main thread. - static Atomic<bool, Relaxed> storageReady(false); - if (!storageReady && gSocketTransportService && aResultCallback) { - nsCOMPtr<nsIURI> uri = aURI; - nsCOMPtr<nsISiteSecurityService> service = sss; - nsresult rv = gSocketTransportService->Dispatch( - NS_NewRunnableFunction( - "net::NS_ShouldSecureUpgrade", - [service{std::move(service)}, uri{std::move(uri)}, flags(flags), - originAttributes(aOriginAttributes), - handleResultFunc{std::move(handleResultFunc)}, - resultCallback{std::move(aResultCallback)}]() mutable { - uint32_t hstsSource = 0; - bool isStsHost = false; - nsresult rv = - service->IsSecureURI(uri, flags, originAttributes, nullptr, - &hstsSource, &isStsHost); - - // Successfully get the result from |IsSecureURI| implies that - // the storage is ready to read. - storageReady = NS_SUCCEEDED(rv); - bool shouldUpgrade = handleResultFunc(isStsHost, hstsSource); - - NS_DispatchToMainThread(NS_NewRunnableFunction( - "net::NS_ShouldSecureUpgrade::ResultCallback", - [rv, shouldUpgrade, - resultCallback{std::move(resultCallback)}]() { - resultCallback(shouldUpgrade, rv); - })); - }), - NS_DISPATCH_NORMAL); - aWillCallback = NS_SUCCEEDED(rv); - return rv; - } - - nsresult rv = sss->IsSecureURI(aURI, flags, aOriginAttributes, nullptr, - &hstsSource, &isStsHost); + nsresult rv = sss->IsSecureURI(aURI, flags, aOriginAttributes, nullptr, + &hstsSource, &isStsHost);
- // if the SSS check fails, it's likely because this load is on a - // malformed URI or something else in the setup is wrong, so any error - // should be reported. - NS_ENSURE_SUCCESS(rv, rv); + // if the SSS check fails, it's likely because this load is on a + // malformed URI or something else in the setup is wrong, so any error + // should be reported. + NS_ENSURE_SUCCESS(rv, rv);
- aShouldUpgrade = handleResultFunc(isStsHost, hstsSource); - return NS_OK; + aShouldUpgrade = handleResultFunc(aAllowSTS, isStsHost, hstsSource); + if (!aShouldUpgrade) { + // Check for CSP upgrade-insecure-requests, Mixed content auto upgrading + // and Https-Only / -First. + aShouldUpgrade = ShouldSecureUpgradeNoHSTS(aURI, aLoadInfo); } - - Telemetry::AccumulateCategorical( - Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::AlreadyHTTPS); - aShouldUpgrade = false; - return NS_OK; + return rv; }
nsresult NS_GetSecureUpgradedURI(nsIURI* aURI, nsIURI** aUpgradedURI) {