[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor-browser] 02/03: Bug 1724080: Have https-first and https-only rules apply to speculative connections r=kershaw
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@xxxxxxxxxxxxxxxxxxxxxxxxx>
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.
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits