[tor-browser] branch tor-browser-91.12.0esr-12.0-1 updated (0858fd74d6f06 -> deafecfad5c47)
This is an automated email from the git hooks/post-receive script. richard pushed a change to branch tor-browser-91.12.0esr-12.0-1 in repository tor-browser. from 0858fd74d6f06 fixup! Bug 10760: Integrate TorButton to TorBrowser core new 3f7a8bfb12271 Bug 1722489 - Evaluate HSTS before https-only in NS_ShouldSecureUpgrade. r=ckerschb,necko-reviewers,kershaw new b4072a5e6bfc4 Bug 1724080: Have https-first and https-only rules apply to speculative connections r=kershaw new deafecfad5c47 fixup! Firefox preference overrides. The 3 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: browser/app/profile/001-base-profile.js | 12 + .../en-US/chrome/security/security.properties | 6 + dom/security/nsHTTPSOnlyUtils.cpp | 24 +- dom/security/test/https-first/browser.ini | 2 + .../browser_httpsfirst_speculative_connect.js | 69 ++++ .../https-first/browser_mixed_content_console.js | 2 +- .../file_httpsfirst_speculative_connect.html | 1 + dom/security/test/https-only/browser.ini | 5 + dom/security/test/https-only/browser_hsts_host.js | 111 +++++++ .../browser_httpsonly_speculative_connect.js | 69 ++++ .../file_httpsonly_speculative_connect.html | 1 + dom/security/test/https-only/hsts_headers.sjs | 24 ++ netwerk/base/nsIOService.cpp | 22 ++ netwerk/base/nsNetUtil.cpp | 354 ++++++++++++--------- 14 files changed, 538 insertions(+), 164 deletions(-) create mode 100644 dom/security/test/https-first/browser_httpsfirst_speculative_connect.js create mode 100644 dom/security/test/https-first/file_httpsfirst_speculative_connect.html create mode 100644 dom/security/test/https-only/browser_hsts_host.js create mode 100644 dom/security/test/https-only/browser_httpsonly_speculative_connect.js create mode 100644 dom/security/test/https-only/file_httpsonly_speculative_connect.html create mode 100644 dom/security/test/https-only/hsts_headers.sjs -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. richard pushed a commit to branch tor-browser-91.12.0esr-12.0-1 in repository tor-browser. commit 3f7a8bfb122717c3b733b023507884e0fda0ffe2 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) { -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. richard pushed a commit to branch tor-browser-91.12.0esr-12.0-1 in repository tor-browser. commit b4072a5e6bfc41ad913a9219d04df51b8579711a Author: Christoph Kerschbaumer <ckerschb@christophkerschbaumer.com> AuthorDate: Mon Nov 29 14:29:01 2021 +0000 Bug 1724080: Have https-first and https-only rules apply to speculative connections r=kershaw Differential Revision: https://phabricator.services.mozilla.com/D132239 --- .../en-US/chrome/security/security.properties | 6 ++ dom/security/nsHTTPSOnlyUtils.cpp | 24 +++++--- dom/security/test/https-first/browser.ini | 2 + .../browser_httpsfirst_speculative_connect.js | 69 ++++++++++++++++++++++ .../https-first/browser_mixed_content_console.js | 2 +- .../file_httpsfirst_speculative_connect.html | 1 + dom/security/test/https-only/browser.ini | 2 + .../browser_httpsonly_speculative_connect.js | 69 ++++++++++++++++++++++ .../file_httpsonly_speculative_connect.html | 1 + netwerk/base/nsIOService.cpp | 22 +++++++ 10 files changed, 188 insertions(+), 10 deletions(-) diff --git a/dom/locales/en-US/chrome/security/security.properties b/dom/locales/en-US/chrome/security/security.properties index d0044ce16d3a0..b1d0d005d1008 100644 --- a/dom/locales/en-US/chrome/security/security.properties +++ b/dom/locales/en-US/chrome/security/security.properties @@ -140,6 +140,12 @@ HTTPSOnlyNoUpgradeException = Not upgrading insecure request “%1$S” because HTTPSOnlyFailedRequest = Upgrading insecure request “%1$S” failed. (%2$S) # LOCALIZATION NOTE: %S is the URL of the failed request; HTTPSOnlyFailedDowngradeAgain = Upgrading insecure request “%S” failed. Downgrading to “http” again. +# LOCALIZATION NOTE: Hints or indicates a new transaction for a URL is likely coming soon. We use +# a speculative connection to start a TCP connection so that the resource is immediately ready +# when the transaction is actually submitted. HTTPS-Only and HTTPS-First will upgrade such +# speculative TCP connections from http to https. +# %1$S is the URL of the upgraded speculative TCP connection; %2$S is the upgraded scheme. +HTTPSOnlyUpgradeSpeculativeConnection = Upgrading insecure speculative TCP connection “%1$S” to use “%2$S”. # LOCALIZATION NOTE: %S is the URL of the blocked request; IframeSandboxBlockedDownload = Download of “%S” was blocked because the triggering iframe has the sandbox flag set. diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp index 1494c3894ab77..bac0fa1a7068f 100644 --- a/dom/security/nsHTTPSOnlyUtils.cpp +++ b/dom/security/nsHTTPSOnlyUtils.cpp @@ -179,10 +179,13 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI, NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); NS_ConvertUTF8toUTF16 reportScheme(scheme); + bool isSpeculative = aLoadInfo->GetExternalContentPolicyType() == + ExtContentPolicy::TYPE_SPECULATIVE; AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; - nsHTTPSOnlyUtils::LogLocalizedString("HTTPSOnlyUpgradeRequest", params, - nsIScriptError::warningFlag, aLoadInfo, - aURI); + nsHTTPSOnlyUtils::LogLocalizedString( + isSpeculative ? "HTTPSOnlyUpgradeSpeculativeConnection" + : "HTTPSOnlyUpgradeRequest", + params, nsIScriptError::warningFlag, aLoadInfo, aURI); // If the status was not determined before, we now indicate that the request // will get upgraded, but no event-listener has been registered yet. @@ -339,9 +342,10 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(nsIURI* aURI, return false; } - // 2. HTTPS-First only upgrades top-level loads - if (aLoadInfo->GetExternalContentPolicyType() != - ExtContentPolicy::TYPE_DOCUMENT) { + // 2. HTTPS-First only upgrades top-level loads (and speculative connections) + ExtContentPolicyType contentType = aLoadInfo->GetExternalContentPolicyType(); + if (contentType != ExtContentPolicy::TYPE_DOCUMENT && + contentType != ExtContentPolicy::TYPE_SPECULATIVE) { return false; } @@ -399,10 +403,12 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(nsIURI* aURI, NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); NS_ConvertUTF8toUTF16 reportScheme(scheme); + bool isSpeculative = contentType == ExtContentPolicy::TYPE_SPECULATIVE; AutoTArray<nsString, 2> params = {reportSpec, reportScheme}; - nsHTTPSOnlyUtils::LogLocalizedString("HTTPSOnlyUpgradeRequest", params, - nsIScriptError::warningFlag, aLoadInfo, - aURI, true); + nsHTTPSOnlyUtils::LogLocalizedString( + isSpeculative ? "HTTPSOnlyUpgradeSpeculativeConnection" + : "HTTPSOnlyUpgradeRequest", + params, nsIScriptError::warningFlag, aLoadInfo, aURI, true); // Set flag so we know that we upgraded the request httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_UPGRADED_HTTPS_FIRST; diff --git a/dom/security/test/https-first/browser.ini b/dom/security/test/https-first/browser.ini index 3a6483d6f3952..436ee2d0c5df8 100644 --- a/dom/security/test/https-first/browser.ini +++ b/dom/security/test/https-first/browser.ini @@ -8,3 +8,5 @@ support-files = file_mixed_content_console.html [browser_downgrade_view_source.js] support-files = file_downgrade_view_source.sjs +[browser_httpsfirst_speculative_connect.js] +support-files = file_httpsfirst_speculative_connect.html diff --git a/dom/security/test/https-first/browser_httpsfirst_speculative_connect.js b/dom/security/test/https-first/browser_httpsfirst_speculative_connect.js new file mode 100644 index 0000000000000..0d5f6c7e33260 --- /dev/null +++ b/dom/security/test/https-first/browser_httpsfirst_speculative_connect.js @@ -0,0 +1,69 @@ +"use strict"; + +const TEST_PATH_HTTP = getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "http://example.com" +); + +let console_messages = [ + { + description: "Speculative Connection should get logged", + expectLogLevel: Ci.nsIConsoleMessage.warn, + expectIncludes: [ + "Upgrading insecure speculative TCP connection", + "to use", + "example.com", + "file_httpsfirst_speculative_connect.html", + ], + }, + { + description: "Upgrade should get logged", + expectLogLevel: Ci.nsIConsoleMessage.warn, + expectIncludes: [ + "Upgrading insecure request", + "to use", + "example.com", + "file_httpsfirst_speculative_connect.html", + ], + }, +]; + +function on_new_console_messages(msgObj) { + const message = msgObj.message; + const logLevel = msgObj.logLevel; + + if (message.includes("HTTPS-First Mode:")) { + for (let i = 0; i < console_messages.length; i++) { + const testCase = console_messages[i]; + // Check if log-level matches + if (logLevel !== testCase.expectLogLevel) { + continue; + } + // Check if all substrings are included + if (testCase.expectIncludes.some(str => !message.includes(str))) { + continue; + } + ok(true, testCase.description); + console_messages.splice(i, 1); + break; + } + } +} + +add_task(async function() { + requestLongerTimeout(4); + + await SpecialPowers.pushPrefEnv({ + set: [["dom.security.https_first", true]], + }); + Services.console.registerListener(on_new_console_messages); + + await BrowserTestUtils.loadURI( + gBrowser.selectedBrowser, + `${TEST_PATH_HTTP}file_httpsfirst_speculative_connect.html` + ); + + await BrowserTestUtils.waitForCondition(() => console_messages.length === 0); + + Services.console.unregisterListener(on_new_console_messages); +}); diff --git a/dom/security/test/https-first/browser_mixed_content_console.js b/dom/security/test/https-first/browser_mixed_content_console.js index 057614ca208b8..d4e0067f8c49a 100644 --- a/dom/security/test/https-first/browser_mixed_content_console.js +++ b/dom/security/test/https-first/browser_mixed_content_console.js @@ -34,7 +34,7 @@ function on_console_message(msgObj) { // The first console message is: // "HTTPS-First Mode: Upgrading insecure request // ‘http://example.com/browser/dom/security/test/https-first/file_mixed_content_console.html’ to use ‘https’" - if (message.includes("HTTPS-First Mode:")) { + if (message.includes("HTTPS-First Mode: Upgrading insecure request")) { ok(message.includes("Upgrading insecure request"), "request got upgraded"); ok( message.includes( diff --git a/dom/security/test/https-first/file_httpsfirst_speculative_connect.html b/dom/security/test/https-first/file_httpsfirst_speculative_connect.html new file mode 100644 index 0000000000000..6542884191231 --- /dev/null +++ b/dom/security/test/https-first/file_httpsfirst_speculative_connect.html @@ -0,0 +1 @@ +<html><body>dummy file for speculative https-first upgrade test</body></html> diff --git a/dom/security/test/https-only/browser.ini b/dom/security/test/https-only/browser.ini index 5797ace1adb1d..78d1279e76228 100644 --- a/dom/security/test/https-only/browser.ini +++ b/dom/security/test/https-only/browser.ini @@ -19,3 +19,5 @@ support-files = [browser_hsts_host.js] support-files = hsts_headers.sjs +[browser_httpsonly_speculative_connect.js] +support-files = file_httpsonly_speculative_connect.html diff --git a/dom/security/test/https-only/browser_httpsonly_speculative_connect.js b/dom/security/test/https-only/browser_httpsonly_speculative_connect.js new file mode 100644 index 0000000000000..d07f335b99550 --- /dev/null +++ b/dom/security/test/https-only/browser_httpsonly_speculative_connect.js @@ -0,0 +1,69 @@ +"use strict"; + +const TEST_PATH_HTTP = getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "http://example.org" +); + +let console_messages = [ + { + description: "Speculative Connection should get logged", + expectLogLevel: Ci.nsIConsoleMessage.warn, + expectIncludes: [ + "Upgrading insecure speculative TCP connection", + "to use", + "example.org", + "file_httpsonly_speculative_connect.html", + ], + }, + { + description: "Upgrade should get logged", + expectLogLevel: Ci.nsIConsoleMessage.warn, + expectIncludes: [ + "Upgrading insecure request", + "to use", + "example.org", + "file_httpsonly_speculative_connect.html", + ], + }, +]; + +function on_new_console_messages(msgObj) { + const message = msgObj.message; + const logLevel = msgObj.logLevel; + + if (message.includes("HTTPS-Only Mode:")) { + for (let i = 0; i < console_messages.length; i++) { + const testCase = console_messages[i]; + // Check if log-level matches + if (logLevel !== testCase.expectLogLevel) { + continue; + } + // Check if all substrings are included + if (testCase.expectIncludes.some(str => !message.includes(str))) { + continue; + } + ok(true, testCase.description); + console_messages.splice(i, 1); + break; + } + } +} + +add_task(async function() { + requestLongerTimeout(4); + + await SpecialPowers.pushPrefEnv({ + set: [["dom.security.https_only_mode", true]], + }); + Services.console.registerListener(on_new_console_messages); + + await BrowserTestUtils.loadURI( + gBrowser.selectedBrowser, + `${TEST_PATH_HTTP}file_httpsonly_speculative_connect.html` + ); + + await BrowserTestUtils.waitForCondition(() => console_messages.length === 0); + + Services.console.unregisterListener(on_new_console_messages); +}); diff --git a/dom/security/test/https-only/file_httpsonly_speculative_connect.html b/dom/security/test/https-only/file_httpsonly_speculative_connect.html new file mode 100644 index 0000000000000..46a10401f9530 --- /dev/null +++ b/dom/security/test/https-only/file_httpsonly_speculative_connect.html @@ -0,0 +1 @@ +<html><body>dummy file for speculative https-only upgrade test</body></html> diff --git a/netwerk/base/nsIOService.cpp b/netwerk/base/nsIOService.cpp index 1d99b354c3642..459d51485f672 100644 --- a/netwerk/base/nsIOService.cpp +++ b/netwerk/base/nsIOService.cpp @@ -47,6 +47,7 @@ #include "mozilla/net/NeckoParent.h" #include "mozilla/dom/ClientInfo.h" #include "mozilla/dom/ContentParent.h" +#include "mozilla/dom/nsHTTPSOnlyUtils.h" #include "mozilla/dom/ServiceWorkerDescriptor.h" #include "mozilla/net/CaptivePortalService.h" #include "mozilla/net/NetworkConnectivityService.h" @@ -1950,6 +1951,27 @@ nsresult nsIOService::SpeculativeConnectInternal( return NS_ERROR_INVALID_ARG; } + // XXX Bug 1724080: Avoid TCP connections on port 80 when https-only + // or https-first is enabled. Let's create a dummy loadinfo which we + // only use to determine whether we need ot upgrade the speculative + // connection from http to https. + nsCOMPtr<nsIURI> httpsURI; + if (aURI->SchemeIs("http")) { + nsCOMPtr<nsILoadInfo> httpsOnlyCheckLoadInfo = + new LoadInfo(loadingPrincipal, loadingPrincipal, nullptr, + nsILoadInfo::SEC_ONLY_FOR_EXPLICIT_CONTENTSEC_CHECK, + nsIContentPolicy::TYPE_SPECULATIVE); + + // Check if https-only, or https-first would upgrade the request + if (nsHTTPSOnlyUtils::ShouldUpgradeRequest(aURI, httpsOnlyCheckLoadInfo) || + nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest( + aURI, httpsOnlyCheckLoadInfo)) { + rv = NS_GetSecureUpgradedURI(aURI, getter_AddRefs(httpsURI)); + NS_ENSURE_SUCCESS(rv, rv); + aURI = httpsURI.get(); + } + } + // dummy channel used to create a TCP connection. // we perform security checks on the *real* channel, responsible // for any network loads. this real channel just checks the TCP -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. richard pushed a commit to branch tor-browser-91.12.0esr-12.0-1 in repository tor-browser. commit deafecfad5c47cce61b725b68eb0ac2d7cef6822 Author: Richard Pospesel <richard@torproject.org> AuthorDate: Thu Aug 4 09:00:02 2022 +0000 fixup! Firefox preference overrides. Bug 27719: Treat unsafe renegotiation as broken --- browser/app/profile/001-base-profile.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/browser/app/profile/001-base-profile.js b/browser/app/profile/001-base-profile.js index d3cb37e4d383e..1ec066c0a15c2 100644 --- a/browser/app/profile/001-base-profile.js +++ b/browser/app/profile/001-base-profile.js @@ -64,6 +64,18 @@ pref("media.memory_cache_max_size", 16384); pref("dom.security.https_only_mode", true); pref("dom.security.https_only_mode.upgrade_onion", false); +// Require Safe Negotiation ( https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/27719 ) +// Blocks connections to servers that don't support RFC 5746 [2] as they're potentially vulnerable to a +// MiTM attack [3]. A server without RFC 5746 can be safe from the attack if it disables renegotiations +// but the problem is that the browser can't know that. Setting this pref to true is the only way for the +// browser to ensure there will be no unsafe renegotiations on the channel between the browser and the server +// [STATS] SSL Labs (July 2021) reports over 99% of top sites have secure renegotiation [4] +// [1] https://wiki.mozilla.org/Security:Renegotiation +// [2] https://datatracker.ietf.org/doc/html/rfc5746 +// [3] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-3555 +// [4] https://www.ssllabs.com/ssl-pulse/ +pref("security.ssl.require_safe_negotiation", true); + // Misc privacy: Remote pref("browser.send_pings", false); pref("geo.enabled", false); -- To stop receiving notification emails like this one, please contact the administrator of this repository.
participants (1)
-
gitolite role