From 96ee45f78e609434a3eeb043597f6c85b4189197 Mon Sep 17 00:00:00 2001 From: Mihai Alexandru Michis Date: Mon, 27 Apr 2020 18:33:36 +0300 Subject: [PATCH] Backed out changeset a37a427fac07 (bug 1631384) for causing wpt crashes in websocket.https.html CLOSED TREE --- dom/security/nsHTTPSOnlyUtils.cpp | 117 ++---------------- dom/security/nsHTTPSOnlyUtils.h | 38 +----- dom/security/test/https-only/browser.ini | 1 - .../https-only/browser_upgrade_exceptions.js | 78 ------------ dom/websocket/WebSocket.cpp | 30 ++--- modules/libpref/init/StaticPrefList.yaml | 14 --- 6 files changed, 26 insertions(+), 252 deletions(-) delete mode 100644 dom/security/test/https-only/browser_upgrade_exceptions.js diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp index fc50c205e448..ca5518c1f8b4 100644 --- a/dom/security/nsHTTPSOnlyUtils.cpp +++ b/dom/security/nsHTTPSOnlyUtils.cpp @@ -4,30 +4,20 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "nsHTTPSOnlyUtils.h" #include "mozilla/StaticPrefs_dom.h" -#include "mozilla/net/DNS.h" #include "nsContentUtils.h" +#include "nsHTTPSOnlyUtils.h" #include "nsIConsoleService.h" #include "nsIScriptError.h" -#include "prnetdb.h" - -/* ------ Upgrade ------ */ /* static */ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI, nsILoadInfo* aLoadInfo) { - // 1. Check if the HTTPS-Only Mode is even enabled, before we do anything else + // 1. Check if HTTPS-Only mode is enabled if (!mozilla::StaticPrefs::dom_security_https_only_mode()) { return false; } - - // 2. Check for general exceptions - if (OnionException(aURI) || LoopbackOrLocalException(aURI)) { - return false; - } - - // 3. Check if NoUpgrade-flag is set in LoadInfo + // 2. Check if NoUpgrade-flag is set in LoadInfo uint32_t httpsOnlyStatus = aLoadInfo->GetHttpsOnlyStatus(); if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) { // Let's log to the console, that we didn't upgrade this request @@ -41,8 +31,10 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI, return false; } - // We can upgrade the request - let's log it to the console - // Appending an 's' to the scheme for the logging. (http -> https) + // 3. Upgrade the request + + // Let's log it to the console + // Append the additional 's' just for the logging nsAutoCString scheme; aURI->GetScheme(scheme); scheme.AppendLiteral("s"); @@ -63,52 +55,11 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI, httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_NOT_REGISTERED; aLoadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); } - return true; -} - -/* static */ -bool nsHTTPSOnlyUtils::ShouldUpgradeWebSocket(nsIURI* aURI, - int32_t aInnerWindowId, - bool aFromPrivateWindow, - uint32_t aHttpsOnlyStatus) { - // 1. Check if the HTTPS-Only Mode is even enabled, before we do anything else - if (!mozilla::StaticPrefs::dom_security_https_only_mode()) { - return false; - } - - // 2. Check for general exceptions - if (OnionException(aURI) || LoopbackOrLocalException(aURI)) { - return false; - } - - // 3. Check if NoUpgrade-flag is set in LoadInfo - if (aHttpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) { - // Let's log to the console, that we didn't upgrade this request - AutoTArray params = { - NS_ConvertUTF8toUTF16(aURI->GetSpecOrDefault())}; - nsHTTPSOnlyUtils::LogLocalizedString( - "HTTPSOnlyNoUpgradeException", params, nsIScriptError::infoFlag, - aInnerWindowId, aFromPrivateWindow, aURI); - return false; - } - - // We can upgrade the request - let's log it to the console - // Appending an 's' to the scheme for the logging. (ws -> wss) - nsAutoCString scheme; - aURI->GetScheme(scheme); - scheme.AppendLiteral("s"); - NS_ConvertUTF8toUTF16 reportSpec(aURI->GetSpecOrDefault()); - NS_ConvertUTF8toUTF16 reportScheme(scheme); - - AutoTArray params = {reportSpec, reportScheme}; - nsHTTPSOnlyUtils::LogLocalizedString( - "HTTPSOnlyUpgradeRequest", params, nsIScriptError::warningFlag, - aInnerWindowId, aFromPrivateWindow, aURI); return true; } -/* ------ Logging ------ */ +/** Logging **/ /* static */ void nsHTTPSOnlyUtils::LogLocalizedString( @@ -143,55 +94,3 @@ void nsHTTPSOnlyUtils::LogMessage(const nsAString& aMessage, uint32_t aFlags, true /* from chrome context */, aFlags); } } - -/* ------ Exceptions ------ */ - -/* static */ -bool nsHTTPSOnlyUtils::OnionException(nsIURI* aURI) { - // Onion-host exception can get disabled with a pref - if (mozilla::StaticPrefs::dom_security_https_only_mode_upgrade_onion()) { - return false; - } - nsAutoCString host; - aURI->GetHost(host); - return StringEndsWith(host, NS_LITERAL_CSTRING(".onion")); -} - -/* static */ -bool nsHTTPSOnlyUtils::LoopbackOrLocalException(nsIURI* aURI) { - nsAutoCString asciiHost; - nsresult rv = aURI->GetAsciiHost(asciiHost); - NS_ENSURE_SUCCESS(rv, false); - - // Let's make a quick check if the host matches these loopback strings before - // we do anything else - if (asciiHost.EqualsLiteral("localhost") || asciiHost.EqualsLiteral("::1")) { - return true; - } - - // The local-ip and loopback checks expect a NetAddr struct. We only have a - // host-string but can convert it to a NetAddr by first converting it to - // PRNetAddr. - PRNetAddr tempAddr; - memset(&tempAddr, 0, sizeof(PRNetAddr)); - // PR_StringToNetAddr does not properly initialize the output buffer in the - // case of IPv6 input. See bug 223145. - if (PR_StringToNetAddr(asciiHost.get(), &tempAddr) != PR_SUCCESS) { - return false; - } - - // The linter wants this struct to get initialized, - // but PRNetAddrToNetAddr will do that. - mozilla::net::NetAddr addr; // NOLINT(cppcoreguidelines-pro-type-member-init) - PRNetAddrToNetAddr(&tempAddr, &addr); - - // Loopback IPs are always excempt - if (IsLoopBackAddress(&addr)) { - return true; - } - - // Local IP exception can get disabled with a pref - bool upgradeLocal = - mozilla::StaticPrefs::dom_security_https_only_mode_upgrade_local(); - return (!upgradeLocal && IsIPAddrLocal(&addr)); -} diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h index 0dee4cf11fc5..699521b483f6 100644 --- a/dom/security/nsHTTPSOnlyUtils.h +++ b/dom/security/nsHTTPSOnlyUtils.h @@ -12,28 +12,13 @@ class nsHTTPSOnlyUtils { public: /** - * Determines if a request should get upgraded because of the HTTPS-Only mode. - * If true, the httpsOnlyStatus flag in LoadInfo gets updated and a message is - * logged in the console. - * @param aURI nsIURI of request - * @param aLoadInfo nsILoadInfo of request - * @return true if request should get upgraded + * Determines if a request should get because of the HTTPS-Only mode + * @param aURI nsIURI of request + * @param aLoadInfo nsILoadInfo of request + * @param aShouldUpgrade true if request should get upgraded */ static bool ShouldUpgradeRequest(nsIURI* aURI, nsILoadInfo* aLoadInfo); - /** - * Determines if a request should get upgraded because of the HTTPS-Only mode. - * If true, a message is logged in the console. - * @param aURI nsIURI of request - * @param innerWindowId Inner Window ID - * @param aFromPrivateWindow Whether this request comes from a private window - * @param httpsOnlyStatus httpsOnlyStatus from nsILoadInfo - * @return true if request should get upgraded - */ - static bool ShouldUpgradeWebSocket(nsIURI* aURI, int32_t aInnerWindowId, - bool aFromPrivateWindow, - uint32_t aHttpsOnlyStatus); - /** * Logs localized message to either content console or browser console * @param aName Localization key @@ -61,19 +46,6 @@ class nsHTTPSOnlyUtils { static void LogMessage(const nsAString& aMessage, uint32_t aFlags, uint64_t aInnerWindowID, bool aFromPrivateWindow, nsIURI* aURI = nullptr); - - /** - * Checks whether the URI ends with .onion - * @param aURI URI object - * @return true if the URI is an Onion URI - */ - static bool OnionException(nsIURI* aURI); - - /** - * Checks whether the URI is a loopback- or local-IP - * @param aURI URI object - * @return true if the URI is either loopback or local - */ - static bool LoopbackOrLocalException(nsIURI* aURI); }; + #endif /* nsHTTPSOnlyUtils_h___ */ diff --git a/dom/security/test/https-only/browser.ini b/dom/security/test/https-only/browser.ini index d47d5a72f3da..dbad2b823f7f 100644 --- a/dom/security/test/https-only/browser.ini +++ b/dom/security/test/https-only/browser.ini @@ -3,4 +3,3 @@ support-files = file_console_logging.html [browser_console_logging.js] -[browser_upgrade_exceptions.js] diff --git a/dom/security/test/https-only/browser_upgrade_exceptions.js b/dom/security/test/https-only/browser_upgrade_exceptions.js deleted file mode 100644 index bd6848dc60c0..000000000000 --- a/dom/security/test/https-only/browser_upgrade_exceptions.js +++ /dev/null @@ -1,78 +0,0 @@ -// Bug 1625448 - HTTPS Only Mode - Exceptions for loopback and local IP addresses -// https://bugzilla.mozilla.org/show_bug.cgi?id=1631384 -// This test ensures that various configurable upgrade exceptions work -"use strict"; - -add_task(async function() { - await SpecialPowers.pushPrefEnv({ - set: [["dom.security.https_only_mode", true]], - }); - - await Promise.all([ - // Loopback test - runTest( - "Loopback IP addresses should always be excempt from upgrades (localhost)", - "http://localhost", - "http://" - ), - runTest( - "Loopback IP addresses should always be excempt from upgrades (127.0.0.1)", - "http://127.0.0.1", - "http://" - ), - // Default local-IP and onion tests - runTest( - "Local IP addresses should be excempt from upgrades by default", - "http://10.0.0.1", - "http://" - ), - runTest( - "Hosts ending with .onion should be be excempt from HTTPS-Only upgrades by default", - "http://grocery.shopping.for.one.onion", - "http://" - ), - ]); - - await SpecialPowers.pushPrefEnv({ - set: [ - ["dom.security.https_only_mode.upgrade_local", true], - ["dom.security.https_only_mode.upgrade_onion", true], - ], - }); - - await Promise.all([ - // Local-IP and onion tests with upgrade enabled - runTest( - "Local IP addresses should get upgraded when 'dom.security.https_only_mode.upgrade_local' is set to true", - "http://10.0.0.1", - "https://" - ), - runTest( - "Hosts ending with .onion should get upgraded when 'dom.security.https_only_mode.upgrade_onion' is set to true", - "http://grocery.shopping.for.one.onion", - "https://" - ), - // Local-IP request with HTTPS_ONLY_EXEMPT flag - runTest( - "The HTTPS_ONLY_EXEMPT flag should overrule upgrade-prefs", - "http://10.0.0.1", - "http://", - true - ), - ]); -}); - -async function runTest(desc, url, startsWith, exempt = false) { - const responseURL = await new Promise(resolve => { - let xhr = new XMLHttpRequest(); - xhr.open("GET", url); - if (exempt) { - xhr.channel.loadInfo.httpsOnlyStatus |= Ci.nsILoadInfo.HTTPS_ONLY_EXEMPT; - } - xhr.onerror = () => { - resolve(xhr.responseURL); - }; - xhr.send(); - }); - ok(responseURL.startsWith(startsWith), desc); -} diff --git a/dom/websocket/WebSocket.cpp b/dom/websocket/WebSocket.cpp index fc1aec5c219f..0d30f6eeb352 100644 --- a/dom/websocket/WebSocket.cpp +++ b/dom/websocket/WebSocket.cpp @@ -1579,26 +1579,22 @@ nsresult WebSocketImpl::Init(JSContext* aCx, nsIPrincipal* aLoadingPrincipal, // If the HTTPS-Only mode is enabled, we need to upgrade the websocket // connection from ws:// to wss:// and mark it as secure. - if (!mIsServerSide && !mSecure) { - nsCOMPtr uri; - nsresult rv = NS_NewURI(getter_AddRefs(uri), mURI); - NS_ENSURE_SUCCESS(rv, rv); + if (!mIsServerSide && !mSecure && + StaticPrefs::dom_security_https_only_mode()) { + // let's use the old specification before the upgrade for logging + AutoTArray params; + CopyUTF8toUTF16(mURI, *params.AppendElement()); - nsCOMPtr channel = originDoc->GetChannel(); - uint32_t httpsOnlyStatus = nsILoadInfo::HTTPS_ONLY_UNINITIALIZED; - if (channel) { - nsCOMPtr loadInfo = channel->LoadInfo(); - httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus(); + mURI.ReplaceSubstring("ws://", "wss://"); + if (NS_WARN_IF(mURI.Find("wss://") != 0)) { + return NS_OK; } + mSecure = true; - if (nsHTTPSOnlyUtils::ShouldUpgradeWebSocket( - uri, mInnerWindowID, mPrivateBrowsing, httpsOnlyStatus)) { - mURI.ReplaceSubstring("ws://", "wss://"); - if (NS_WARN_IF(mURI.Find("wss://") != 0)) { - return NS_OK; - } - mSecure = true; - } + params.AppendElement(NS_LITERAL_STRING("wss")); + nsHTTPSOnlyUtils::LogLocalizedString("HTTPSOnlyUpgradeRequest", params, + nsIScriptError::warningFlag, + mInnerWindowID, mPrivateBrowsing); } // Potentially the page uses the CSP directive 'upgrade-insecure-requests'. diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index e12c3523510e..bf2af49fbd67 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -2482,20 +2482,6 @@ value: false mirror: always -# If true and HTTPS-only mode is enabled, requests -# to local IP addresses are also upgraded -- name: dom.security.https_only_mode.upgrade_local - type: RelaxedAtomicBool - value: false - mirror: always - -# If true and HTTPS-only mode is enabled, requests -# to .onion hosts are also upgraded -- name: dom.security.https_only_mode.upgrade_onion - type: RelaxedAtomicBool - value: false - mirror: always - # WARNING: Don't ever update that pref manually! It is only used # for telemetry purposes and allows to reason about retention of # the pref dom.security.https_only_mode from above.