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 6886d4968b8914fae9ab5e4d1ade3b55492dbd66 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_... 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