[tbb-commits] [tor-browser] 02/02: Bug 1724080: Have https-first and https-only rules apply to speculative connections r=kershaw

gitolite role git at cupani.torproject.org
Wed Aug 3 17:03:45 UTC 2022


This is an automated email from the git hooks/post-receive script.

richard pushed a commit to annotated tag tor-browser-91.12.0esr-12.0-1-build2
in repository tor-browser.

commit b4072a5e6bfc41ad913a9219d04df51b8579711a
Author: Christoph Kerschbaumer <ckerschb at 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.


More information about the tbb-commits mailing list