From 182878bbe09d236b032f0f9a4f811dee39cd6038 Mon Sep 17 00:00:00 2001 From: Malte Juergens Date: Wed, 8 Nov 2023 13:44:26 +0000 Subject: [PATCH] Bug 1855734 - Use innermost nested URI in `PopulateTopLevelInfoFromURI` r=freddyb,timhuang Differential Revision: https://phabricator.services.mozilla.com/D190468 --- build/pgo/server-locations.txt | 5 ++ caps/OriginAttributes.cpp | 27 +++++++---- .../httpsonlyerror/tests/browser/browser.toml | 2 + .../tests/browser/browser_exception.js | 22 +-------- .../tests/browser/browser_fpi_nested_uri.js | 46 +++++++++++++++++++ .../httpsonlyerror/tests/browser/head.js | 29 ++++++++++++ 6 files changed, 102 insertions(+), 29 deletions(-) create mode 100644 toolkit/components/httpsonlyerror/tests/browser/browser_fpi_nested_uri.js diff --git a/build/pgo/server-locations.txt b/build/pgo/server-locations.txt index 6bd8bfb4feda..9cbd13785336 100644 --- a/build/pgo/server-locations.txt +++ b/build/pgo/server-locations.txt @@ -384,3 +384,8 @@ http://profile.stage.mozaws.net:80 privileged https://profile.stage.mozaws.net:443 privileged http://ocsp.pki.goog:80 privileged https://ocsp.pki.goog:443 privileged + + +# External IP address only available via http (Bug 1855734) +http://123.123.123.123:80 privileged +https://123.123.123.123:443 privileged,nocert diff --git a/caps/OriginAttributes.cpp b/caps/OriginAttributes.cpp index b6746a54c430..734a23049582 100644 --- a/caps/OriginAttributes.cpp +++ b/caps/OriginAttributes.cpp @@ -70,8 +70,19 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, nsAString& topLevelInfo = aOriginAttributes.*aTarget; nsAutoCString scheme; - rv = aURI->GetScheme(scheme); - NS_ENSURE_SUCCESS_VOID(rv); + nsCOMPtr uri = aURI; + // The URI could be nested (for example view-source:http://example.com), in + // that case we want to get the innermost URI (http://example.com). + nsCOMPtr nestedURI; + do { + NS_ENSURE_SUCCESS_VOID(uri->GetScheme(scheme)); + nestedURI = do_QueryInterface(uri); + // We can't just use GetInnermostURI on the nested URI, since that would + // also unwrap some about: URIs to hidden moz-safe-about: URIs, which we do + // not want. Thus we loop through with GetInnerURI until the URI isn't + // nested anymore or we encounter a about: scheme. + } while (nestedURI && !scheme.EqualsLiteral("about") && + NS_SUCCEEDED(nestedURI->GetInnerURI(getter_AddRefs(uri)))); if (scheme.EqualsLiteral("about")) { MakeTopLevelInfo(scheme, nsLiteralCString(ABOUT_URI_FIRST_PARTY_DOMAIN), @@ -84,7 +95,7 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, if (scheme.EqualsLiteral("moz-nullprincipal")) { // Get the UUID portion of the URI, ignoring the precursor principal. nsAutoCString filePath; - rv = aURI->GetFilePath(filePath); + rv = uri->GetFilePath(filePath); MOZ_ASSERT(NS_SUCCEEDED(rv)); // Remove the `{}` characters from both ends. filePath.Mid(filePath, 1, filePath.Length() - 2); @@ -103,7 +114,7 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, nsCOMPtr blobPrincipal; if (dom::BlobURLProtocolHandler::GetBlobURLPrincipal( - aURI, getter_AddRefs(blobPrincipal))) { + uri, getter_AddRefs(blobPrincipal))) { MOZ_ASSERT(blobPrincipal); topLevelInfo = blobPrincipal->OriginAttributesRef().*aTarget; return; @@ -115,7 +126,7 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, NS_ENSURE_TRUE_VOID(tldService); nsAutoCString baseDomain; - rv = tldService->GetBaseDomain(aURI, 0, baseDomain); + rv = tldService->GetBaseDomain(uri, 0, baseDomain); if (NS_SUCCEEDED(rv)) { MakeTopLevelInfo(scheme, baseDomain, aUseSite, topLevelInfo); return; @@ -126,11 +137,11 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, bool isInsufficientDomainLevels = (rv == NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS); int32_t port; - rv = aURI->GetPort(&port); + rv = uri->GetPort(&port); NS_ENSURE_SUCCESS_VOID(rv); nsAutoCString host; - rv = aURI->GetHost(host); + rv = uri->GetHost(host); NS_ENSURE_SUCCESS_VOID(rv); if (isIpAddress) { @@ -160,7 +171,7 @@ static void PopulateTopLevelInfoFromURI(const bool aIsTopLevelDocument, if (isInsufficientDomainLevels) { nsAutoCString publicSuffix; - rv = tldService->GetPublicSuffix(aURI, publicSuffix); + rv = tldService->GetPublicSuffix(uri, publicSuffix); if (NS_SUCCEEDED(rv)) { MakeTopLevelInfo(scheme, publicSuffix, port, aUseSite, topLevelInfo); return; diff --git a/toolkit/components/httpsonlyerror/tests/browser/browser.toml b/toolkit/components/httpsonlyerror/tests/browser/browser.toml index 1c2b39b27a82..5bfaddbe1194 100644 --- a/toolkit/components/httpsonlyerror/tests/browser/browser.toml +++ b/toolkit/components/httpsonlyerror/tests/browser/browser.toml @@ -13,3 +13,5 @@ skip-if = ["toolkit == 'android'"] # no https-only errorpage support in android ["browser_exception.js"] support-files = ["file_upgrade_insecure_server.sjs"] + +["browser_fpi_nested_uri.js"] diff --git a/toolkit/components/httpsonlyerror/tests/browser/browser_exception.js b/toolkit/components/httpsonlyerror/tests/browser/browser_exception.js index 3226e1c83437..7cf98b467fb3 100644 --- a/toolkit/components/httpsonlyerror/tests/browser/browser_exception.js +++ b/toolkit/components/httpsonlyerror/tests/browser/browser_exception.js @@ -85,27 +85,7 @@ add_task(async function () { true ); - // click on exception-button and wait for page to load - await SpecialPowers.spawn(browser, [], async function () { - let openInsecureButton = content.document.getElementById("openInsecure"); - ok(openInsecureButton != null, "openInsecureButton should exist."); - info("Waiting for openInsecureButton to be enabled."); - function callback() { - if (!openInsecureButton.inert) { - info("openInsecureButton was enabled, waiting two frames."); - observer.disconnect(); - content.requestAnimationFrame(() => { - content.requestAnimationFrame(() => { - info("clicking openInsecureButton."); - openInsecureButton.click(); - }); - }); - } - } - const observer = new content.MutationObserver(callback); - observer.observe(openInsecureButton, { attributeFilter: ["inert"] }); - callback(); - }); + await waitForAndClickOpenInsecureButton(browser); await pageShownPromise; diff --git a/toolkit/components/httpsonlyerror/tests/browser/browser_fpi_nested_uri.js b/toolkit/components/httpsonlyerror/tests/browser/browser_fpi_nested_uri.js new file mode 100644 index 000000000000..45bf718f3f7d --- /dev/null +++ b/toolkit/components/httpsonlyerror/tests/browser/browser_fpi_nested_uri.js @@ -0,0 +1,46 @@ +/* Any copyright is dedicated to the Public Domain. + * https://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that a nested URI (in this case `view-source:`) does not result +// in a redirect loop when HTTPS-Only and First Party Isolation are +// enabled (Bug 1855734). + +const INSECURE_VIEW_SOURCE_URL = "view-source:http://123.123.123.123/"; + +function promiseIsErrorPage() { + return new Promise(resolve => { + BrowserTestUtils.waitForErrorPage(gBrowser.selectedBrowser).then(() => + resolve(true) + ); + BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser).then(() => + resolve(false) + ); + }); +} + +add_task(async function () { + await SpecialPowers.pushPrefEnv({ + set: [ + ["dom.security.https_only_mode", true], + ["dom.security.https_only_mode.upgrade_local", true], + ["privacy.firstparty.isolate", true], + ], + }); + + let loaded = BrowserTestUtils.waitForErrorPage(gBrowser.selectedBrowser); + info(`Starting to load ${INSECURE_VIEW_SOURCE_URL}`); + BrowserTestUtils.startLoadingURIString(gBrowser, INSECURE_VIEW_SOURCE_URL); + await loaded; + info(`${INSECURE_VIEW_SOURCE_URL} finished loading`); + + loaded = promiseIsErrorPage(); + await waitForAndClickOpenInsecureButton(gBrowser.selectedBrowser); + info(`Waiting for normal or error page to load`); + const isErrorPage = await loaded; + + ok(!isErrorPage, "We should not land on an error page"); + + await Services.perms.removeAll(); +}); diff --git a/toolkit/components/httpsonlyerror/tests/browser/head.js b/toolkit/components/httpsonlyerror/tests/browser/head.js index 05efdf18cbf5..79fa6b84ac1b 100644 --- a/toolkit/components/httpsonlyerror/tests/browser/head.js +++ b/toolkit/components/httpsonlyerror/tests/browser/head.js @@ -64,3 +64,32 @@ async function openErrorPage(src, useFrame, privateWindow, sandboxed) { return tab; } + +/** + * On a loaded HTTPS-Only error page, waits until the "Open Insecure" + * button gets enabled and then presses it. + * + * @returns {Promise} + */ +function waitForAndClickOpenInsecureButton(browser) { + return SpecialPowers.spawn(browser, [], async function () { + let openInsecureButton = content.document.getElementById("openInsecure"); + ok(openInsecureButton != null, "openInsecureButton should exist."); + info("Waiting for openInsecureButton to be enabled."); + function callback() { + if (!openInsecureButton.inert) { + info("openInsecureButton was enabled, waiting two frames."); + observer.disconnect(); + content.requestAnimationFrame(() => { + content.requestAnimationFrame(() => { + info("clicking openInsecureButton."); + openInsecureButton.click(); + }); + }); + } + } + const observer = new content.MutationObserver(callback); + observer.observe(openInsecureButton, { attributeFilter: ["inert"] }); + callback(); + }); +}