diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 090b0be8a441..0dbf1c30f810 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -8533,9 +8533,8 @@ bool nsDocShell::IsSameDocumentNavigation(nsDocShellLoadState* aLoadState, if (nsCOMPtr docChannel = GetCurrentDocChannel()) { nsCOMPtr docLoadInfo = docChannel->LoadInfo(); if (!docLoadInfo->GetLoadErrorPage() && - nsHTTPSOnlyUtils::ShouldUpgradeConnection(docLoadInfo) && - nsHTTPSOnlyUtils::IsHttpDowngrade(currentExposableURI, - aLoadState->URI())) { + nsHTTPSOnlyUtils::IsEqualURIExceptSchemeAndRef( + currentExposableURI, aLoadState->URI(), docLoadInfo)) { uint32_t status = docLoadInfo->GetHttpsOnlyStatus(); if (status & (nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_REGISTERED | nsILoadInfo::HTTPS_ONLY_UPGRADED_HTTPS_FIRST)) { diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp index fce30d4521b4..8c6fea1a931a 100644 --- a/dom/security/nsHTTPSOnlyUtils.cpp +++ b/dom/security/nsHTTPSOnlyUtils.cpp @@ -266,7 +266,7 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeWebSocket(nsIURI* aURI, /* static */ bool nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( - nsIURI* aOldURI, nsIURI* aNewURI, nsILoadInfo* aLoadInfo, + nsIURI* aURI, nsILoadInfo* aLoadInfo, const mozilla::EnumSet& aOptions) { // 1. Check if the HTTPS-Only/HTTPS-First is even enabled, before doing // anything else @@ -307,47 +307,84 @@ bool nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( return false; } - // 5. Check HTTP 3xx redirects. If the Principal that kicked off the + // 5. If the URI to be loaded is not http, then it's defnitely no endless + // loop caused by https-only. + if (!aURI->SchemeIs("http")) { + return false; + } + + nsAutoCString uriHost; + aURI->GetAsciiHost(uriHost); + + auto uriAndPrincipalComparator = [&](nsIPrincipal* aPrincipal) { + nsAutoCString principalHost; + aPrincipal->GetAsciiHost(principalHost); + bool checkPath = mozilla::StaticPrefs:: + dom_security_https_only_check_path_upgrade_downgrade_endless_loop(); + if (!checkPath) { + return uriHost.Equals(principalHost); + } + + nsAutoCString uriPath; + nsresult rv = aURI->GetFilePath(uriPath); + if (NS_FAILED(rv)) { + return false; + } + nsAutoCString principalPath; + aPrincipal->GetFilePath(principalPath); + return uriHost.Equals(principalHost) && uriPath.Equals(principalPath); + }; + + // We do dot return early when we find a loop, as we still need to set an + // exception at the end. + bool isLoop = false; + + // 6. Check actual redirects. If the Principal that kicked off the // load/redirect is not https, then it's definitely not a redirect cause by // https-only. If the scheme of the principal however is https and the // asciiHost of the URI to be loaded and the asciiHost of the Principal are // identical, then we are dealing with an upgrade downgrade scenario and we // have to break the cycle. - if (IsHttpDowngrade(aOldURI, aNewURI)) { - return true; - } - - // TODO(Bug 1896691): Don't depend on triggeringPrincipal for JS/Meta - // redirects. Call this function at the correct places instead - - // 6. Bug 1725026: Disable JS/Meta loop detection when the load was triggered - // by a user gesture. This information is only when the redirect chain is - // empty. When the redirect chain is not empty, this load is definitely - // triggered by redirection, not a user gesture. - // TODO(1896685): Verify whether check is still necessary. - if (aLoadInfo->RedirectChain().IsEmpty()) { + if (!aLoadInfo->RedirectChain().IsEmpty()) { + nsCOMPtr redirectPrincipal; + for (nsIRedirectHistoryEntry* entry : aLoadInfo->RedirectChain()) { + entry->GetPrincipal(getter_AddRefs(redirectPrincipal)); + if (redirectPrincipal && redirectPrincipal->SchemeIs("https") && + uriAndPrincipalComparator(redirectPrincipal)) { + isLoop = true; + } + } + } else { + // 6.1 We should only check if this load is triggered by a user gesture + // when the redirect chain is empty, since this information is only useful + // in our case here. When the redirect chain is not empty, this load is + // defnitely triggered by redirection, not a user gesture. if (aLoadInfo->GetHasValidUserGestureActivation()) { return false; } } - // 7. Meta redirects and JS based redirects (win.location). We detect them - // during the https upgrade internal redirect. - nsCOMPtr triggeringPrincipal = aLoadInfo->TriggeringPrincipal(); - if (!triggeringPrincipal->SchemeIs("https")) { - return false; + if (!isLoop) { + // 7. Meta redirects and JS based redirects (win.location). If the security + // context that triggered the load is not https, then it's defnitely no + // endless loop caused by https-only. If the scheme is http however and the + // asciiHost of the URI to be loaded matches the asciiHost of the Principal, + // then we are dealing with an upgrade downgrade scenario and we have to + // break the cycle. + nsCOMPtr triggeringPrincipal = + aLoadInfo->TriggeringPrincipal(); + if (!triggeringPrincipal->SchemeIs("https")) { + return false; + } + isLoop = uriAndPrincipalComparator(triggeringPrincipal); } - // We detect Meta and JS based redirects during the upgrade. Check whether - // we are currently in an upgrade situation here. - if (!IsHttpDowngrade(aNewURI, aOldURI)) { - return false; + if (isLoop && enforceForHTTPSFirstMode && + mozilla::StaticPrefs:: + dom_security_https_first_add_exception_on_failiure()) { + AddHTTPSFirstExceptionForSession(aURI, aLoadInfo); } - // If we upgrade to the same URI that the load is origining from we are - // creating a redirect loop. - bool isLoop = false; - nsresult rv = triggeringPrincipal->EqualsURI(aNewURI, &isLoop); - NS_ENSURE_SUCCESS(rv, false); + return isLoop; } @@ -399,8 +436,7 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(nsIURI* aURI, } // https-first needs to account for breaking upgrade-downgrade endless - // loops at this point for meta and js redirects because this function - // is called before we + // loops at this point because this function is called before we // check the redirect limit in HttpBaseChannel. If we encounter // a same-origin server side downgrade from e.g https://example.com // to http://example.com then we simply not annotating the loadinfo @@ -408,18 +444,12 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeHttpsFirstRequest(nsIURI* aURI, // the handling for https-only mode is different from https-first mode, // because https-only mode results in an exception page in case // we encounter and endless upgrade downgrade loop. - /* bool isUpgradeDowngradeEndlessLoop = IsUpgradeDowngradeEndlessLoop( - aURI, aURI, aLoadInfo, + aURI, aLoadInfo, {UpgradeDowngradeEndlessLoopOptions::EnforceForHTTPSFirstMode}); if (isUpgradeDowngradeEndlessLoop) { - if (mozilla::StaticPrefs:: - dom_security_https_first_add_exception_on_failiure()) { - nsHTTPSOnlyUtils::AddHTTPSFirstExceptionForSession(aURI, aLoadInfo); - } return false; } - */ // We can upgrade the request - let's log to the console and set the status // so we know that we upgraded the request. @@ -870,51 +900,38 @@ bool nsHTTPSOnlyUtils::LoopbackOrLocalException(nsIURI* aURI) { } /* static */ -bool nsHTTPSOnlyUtils::ShouldUpgradeConnection(nsILoadInfo* aLoadInfo) { - // Check if one of parameters is null then webpage can't be loaded yet +bool nsHTTPSOnlyUtils::IsEqualURIExceptSchemeAndRef(nsIURI* aHTTPSSchemeURI, + nsIURI* aOtherURI, + nsILoadInfo* aLoadInfo) { + // 1. Check if one of parameters is null then webpage can't be loaded yet // and no further inspections are needed - if (!aLoadInfo) { + if (!aHTTPSSchemeURI || !aOtherURI || !aLoadInfo) { return false; } - // Check if the HTTPS-Only Mode is even enabled, before we do anything else + // 2. If the URI to be loaded is not http, then same origin will be detected + // already + if (!mozilla::net::SchemeIsHTTP(aOtherURI)) { + return false; + } + + // 3. Check if the HTTPS-Only Mode is even enabled, before we do anything else bool isPrivateWin = aLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; if (!IsHttpsOnlyModeEnabled(isPrivateWin) && !IsHttpsFirstModeEnabled(isPrivateWin)) { return false; } - // If the load is exempt, then don't upgrade + // 4. If the load is exempt, then it's defintely not related to https-only uint32_t httpsOnlyStatus = aLoadInfo->GetHttpsOnlyStatus(); if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) { return false; } - return true; -} -/* static */ -bool nsHTTPSOnlyUtils::IsHttpDowngrade(nsIURI* aFromURI, nsIURI* aToURI) { - MOZ_ASSERT(aFromURI); - MOZ_ASSERT(aToURI); - - if (!aFromURI || !aToURI) { - return false; - } - - // 2. If the target URI is not http, then it's not a http downgrade - if (!mozilla::net::SchemeIsHTTP(aToURI)) { - return false; - } - - // 3. If the origin URI isn't https, then it's not a http downgrade either. - if (!mozilla::net::SchemeIsHTTPS(aFromURI)) { - return false; - } - - // 4. Create a new target URI with 'https' instead of 'http' and compare it - // to the origin URI + // 5. Create a new target URI with 'https' instead of 'http' and compare it + // to the current URI int32_t port = 0; - nsresult rv = aToURI->GetPort(&port); + nsresult rv = aOtherURI->GetPort(&port); NS_ENSURE_SUCCESS(rv, false); // a port of -1 indicates the default port, hence we upgrade from port 80 to // port 443 @@ -923,14 +940,15 @@ bool nsHTTPSOnlyUtils::IsHttpDowngrade(nsIURI* aFromURI, nsIURI* aToURI) { port = NS_GetDefaultPort("https"); } nsCOMPtr newHTTPSchemeURI; - rv = NS_MutateURI(aToURI) + rv = NS_MutateURI(aOtherURI) .SetScheme("https"_ns) .SetPort(port) .Finalize(newHTTPSchemeURI); NS_ENSURE_SUCCESS(rv, false); bool uriEquals = false; - if (NS_FAILED(aFromURI->EqualsExceptRef(newHTTPSchemeURI, &uriEquals))) { + if (NS_FAILED( + aHTTPSSchemeURI->EqualsExceptRef(newHTTPSchemeURI, &uriEquals))) { return false; } diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h index 70be570662cf..960b39bc7c68 100644 --- a/dom/security/nsHTTPSOnlyUtils.h +++ b/dom/security/nsHTTPSOnlyUtils.h @@ -77,7 +77,7 @@ class nsHTTPSOnlyUtils { EnforceForHTTPSRR, }; static bool IsUpgradeDowngradeEndlessLoop( - nsIURI* aOldURI, nsIURI* aNewURI, nsILoadInfo* aLoadInfo, + nsIURI* aURI, nsILoadInfo* aLoadInfo, const mozilla::EnumSet& aOptions = {}); @@ -153,20 +153,17 @@ class nsHTTPSOnlyUtils { */ static bool IsSafeToAcceptCORSOrMixedContent(nsILoadInfo* aLoadInfo); - /** - * Checks if https only or https first mode is enabled for this load - * @param aLoadInfo nsILoadInfo of the request - */ - static bool ShouldUpgradeConnection(nsILoadInfo* aLoadInfo); - /** * Checks if two URIs are same origin modulo the difference that - * aToURI scheme is downgraded to http from https aFromURI. - * @param aFromURI nsIURI using scheme of https - * @param aToURI nsIURI using scheme of http + * aHTTPSchemeURI uses an http scheme. + * @param aHTTPSSchemeURI nsIURI using scheme of https + * @param aOtherURI nsIURI using scheme of http + * @param aLoadInfo nsILoadInfo of the request * @return true, if URIs are equal except scheme and ref */ - static bool IsHttpDowngrade(nsIURI* aFromURI, nsIURI* aToURI); + static bool IsEqualURIExceptSchemeAndRef(nsIURI* aHTTPSSchemeURI, + nsIURI* aOtherURI, + nsILoadInfo* aLoadInfo); /** * Will add a special temporary HTTPS-Only exception that only applies to diff --git a/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs b/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs index 0222941245fb..eb64c59e97cb 100644 --- a/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs +++ b/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs @@ -1,96 +1,61 @@ "use strict"; -// DOWNGRADE_REDIRECT_*: http instead of https, otherwise same path -const DOWNGRADE_REDIRECT_META = ` - - - - - - META REDIRECT - - `; - -const DOWNGRADE_REDIRECT_JS = ` - - - JS REDIRECT - - - `; - -// REDIRECT_*: different path and http instead of https -const REDIRECT_META = ` - - - - - - META REDIRECT - - `; - -const REDIRECT_JS = ` - - - JS REDIRECT - - - `; +const REDIRECT_URI = + "http://example.com/tests/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs?verify"; +const DOWNGRADE_URI = + "http://example.com/tests/dom/security/test/https-first/file_downgrade_with_different_path.sjs"; +const RESPONSE_ERROR = "unexpected-query"; // An onload postmessage to window opener +const RESPONSE_HTTPS_SCHEME = ` + + + + + `; + const RESPONSE_HTTP_SCHEME = ` `; function handleRequest(request, response) { response.setHeader("Cache-Control", "no-cache", false); + const query = request.queryString; - if (request.scheme == "https") { - // allow http status code as parameter - const query = request.queryString.split("="); - if (query[0] == "downgrade_redirect_http") { - let location = `http://${request.host}${request.path}?${request.queryString}`; - response.setStatusLine(request.httpVersion, query[1], "Found"); - response.setHeader("Location", location, false); - } else if (query[0] == "redirect_http") { - response.setStatusLine(request.httpVersion, query[1], "Found"); - let location = - "http://example.com/tests/dom/security/test/https-first/file_downgrade_with_different_path.sjs?" + - request.queryString; - response.setHeader("Location", location, false); - } else if (query[0] == "downgrade_redirect_js") { - response.setStatusLine(request.httpVersion, 200, "OK"); - response.write(DOWNGRADE_REDIRECT_JS); - } else if (query[0] == "redirect_js") { - response.setStatusLine(request.httpVersion, 200, "OK"); - response.write(REDIRECT_JS); - } else if (query[0] == "downgrade_redirect_meta") { - response.setStatusLine(request.httpVersion, 200, "OK"); - response.write(DOWNGRADE_REDIRECT_META); - } else if (query[0] == "redirect_meta") { - response.setStatusLine(request.httpVersion, 200, "OK"); - response.write(REDIRECT_META); - } else { - // We should never get here, but just in case ... - response.setStatusLine(request.httpVersion, 500, "OK"); - response.write("unexepcted query"); - } + if (query == "downgrade") { + // send same-origin downgrade from https: to http: with a different path. + // we don't consider it's an endless upgrade downgrade loop in this case. + response.setStatusLine(request.httpVersion, 302, "Found"); + response.setHeader("Location", DOWNGRADE_URI, false); return; } - // return http response - response.setStatusLine(request.httpVersion, 200, "OK"); - response.write(RESPONSE_HTTP_SCHEME); + // handle the redirect case + if ((query >= 301 && query <= 303) || query == 307) { + // send same-origin downgrade from https: to http: again simluating + // and endless upgrade downgrade loop. + response.setStatusLine(request.httpVersion, query, "Found"); + response.setHeader("Location", REDIRECT_URI, false); + return; + } + + // Check if scheme is http:// or https:// + if (query == "verify") { + let response_content = + request.scheme === "https" ? RESPONSE_HTTPS_SCHEME : RESPONSE_HTTP_SCHEME; + response.setStatusLine(request.httpVersion, 200, "OK"); + response.write(response_content); + return; + } + + // We should never get here, but just in case ... + response.setStatusLine(request.httpVersion, 500, "OK"); + response.write("unexepcted query"); } diff --git a/dom/security/test/https-first/file_downgrade_with_different_path.sjs b/dom/security/test/https-first/file_downgrade_with_different_path.sjs index 5d50dc5a01a5..7450313d9826 100644 --- a/dom/security/test/https-first/file_downgrade_with_different_path.sjs +++ b/dom/security/test/https-first/file_downgrade_with_different_path.sjs @@ -5,7 +5,7 @@ const RESPONSE_HTTPS_SCHEME = ` `; @@ -14,7 +14,7 @@ const RESPONSE_HTTP_SCHEME = ` `; diff --git a/dom/security/test/https-first/file_multiple_redirection.sjs b/dom/security/test/https-first/file_multiple_redirection.sjs index 565ebda78be6..e34a360fa6cf 100644 --- a/dom/security/test/https-first/file_multiple_redirection.sjs +++ b/dom/security/test/https-first/file_multiple_redirection.sjs @@ -9,10 +9,6 @@ const OTHERHOST_REDIRECT_URI_HTTP = "http://example.org/tests/dom/security/test/https-first/file_multiple_redirection.sjs?verify"; const REDIRECT_URI_HTTPS = "https://example.com/tests/dom/security/test/https-first/file_multiple_redirection.sjs?verify"; -const REDIRECT_DOWNGRADE_URI = - "https://example.com/tests/dom/security/test/https-first/file_multiple_redirection.sjs?downgrade"; -const REDIRECT_DOWNGRADE_URI_HTTP = - "http://example.com/tests/dom/security/test/https-first/file_multiple_redirection.sjs?downgrade"; const RESPONSE_ERROR = "unexpected-query"; @@ -56,10 +52,6 @@ function sendRedirection(query, response) { if (query.includes("test4")) { response.setHeader("Location", OTHERHOST_REDIRECT_URI_HTTP, false); } - // send a redirection http downgrade uri - if (query.includes("test5")) { - response.setHeader("Location", REDIRECT_DOWNGRADE_URI, false); - } } function handleRequest(request, response) { @@ -72,7 +64,6 @@ function handleRequest(request, response) { if (request.scheme !== "https") { response.setStatusLine(request.httpVersion, 500, "OK"); response.write("Request should have been HTTPS."); - return; } // send a 302 redirection response.setStatusLine(request.httpVersion, 302, "Found"); @@ -84,20 +75,11 @@ function handleRequest(request, response) { if (request.scheme !== "https") { response.setStatusLine(request.httpVersion, 500, "OK"); response.write("Request should have been HTTPS."); - return; } response.setStatusLine(request.httpVersion, 302, "Found"); sendRedirection(query, response); return; } - // Send a http redirect downgrade - if (query.includes("downgrade")) { - response.setStatusLine(request.httpVersion, 302, "Found"); - let redirect_uri = - request.scheme === "https" ? REDIRECT_DOWNGRADE_URI : REDIRECT_URI_HTTP; - response.setHeader("Location", redirect_uri, false); - return; - } // Reset the HSTS policy, prevent influencing other tests if (request.queryString === "reset") { response.setHeader("Strict-Transport-Security", "max-age=0"); diff --git a/dom/security/test/https-first/test_break_endless_upgrade_downgrade_loop.html b/dom/security/test/https-first/test_break_endless_upgrade_downgrade_loop.html index 231bf9c51b9b..86719bf89d00 100644 --- a/dom/security/test/https-first/test_break_endless_upgrade_downgrade_loop.html +++ b/dom/security/test/https-first/test_break_endless_upgrade_downgrade_loop.html @@ -20,64 +20,55 @@ Test that same origin redirect does not cause endless loop with https-first enab SimpleTest.waitForExplicitFinish(); - let testQueries = [ - // Those are clear downgrades. Need to load http site - { query: "downgrade_redirect_meta", result: "http" }, - { query: "downgrade_redirect_js", result: "http" }, - { query: "downgrade_redirect_http=301", result: "http" }, - { query: "downgrade_redirect_http=302", result: "http" }, - { query: "downgrade_redirect_http=303", result: "http" }, - { query: "downgrade_redirect_http=307", result: "http" }, - // from here it isn't required to downgrade. Could be upgraded again - { query: "redirect_meta", result: "https" }, - { query: "redirect_js", result: "https" }, - { query: "redirect_http=301", result: "https" }, - { query: "redirect_http=302", result: "https" }, - { query: "redirect_http=303", result: "https" }, - { query: "redirect_http=307", result: "https" }, - ]; + const redirectCodes = ["301", "302","303","307"]; let currentTest = 0; - // do each test two time. One time starting with https:// one time with http:// - let currentTestStartWithHttps = false; let testWin; window.addEventListener("message", receiveMessage); // receive message from loaded site verifying the scheme of // the loaded document. async function receiveMessage(event) { - let currentTestParams = testQueries[Math.floor(currentTest / 2)]; - let expectedURI; - if(currentTestParams.result == "https") { - expectedURI = "https://example.com/tests/dom/security/test/https-first/file_downgrade_with_different_path.sjs?" + currentTestParams.query; - } else { - expectedURI = "http://example.com/tests/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs?" + currentTestParams.query; - } - is(`scheme-${currentTestParams.result}-${expectedURI}`, - event.data.result, - `${currentTest}: redirect results in '${currentTestParams.result}' for ${expectedURI}` + let currentRedirectCode = redirectCodes[currentTest]; + is(event.data.result, + "scheme-http", + "same-origin redirect results in 'http' for " + currentRedirectCode ); testWin.close(); await SpecialPowers.removePermission( "https-only-load-insecure", "http://example.com" ); - // each test gets run starting with http:// and https://. Therefore *2 - if (++currentTest < 2 * testQueries.length) { - // start next case + if (++currentTest < redirectCodes.length) { startTest(); return; } - // cleanup window.removeEventListener("message", receiveMessage); + window.addEventListener("message", receiveMessageForDifferentPathTest); + testDifferentPath(); + } + + async function receiveMessageForDifferentPathTest(event) { + is(event.data.result, + "scheme-https", + "scheme should be https when the path is different" + ); + testWin.close(); + window.removeEventListener("message", receiveMessageForDifferentPathTest); SimpleTest.finish(); } async function startTest() { - const currentTestParams = testQueries[Math.floor(currentTest / 2)]; - const scheme = currentTest % 2 == 0 ? "https" : "http"; + const currentCode = redirectCodes[currentTest]; // Load an http:// window which gets upgraded to https:// let uri = - `${scheme}://example.com/tests/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs?${currentTestParams.query}`; + `http://example.com/tests/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs?${currentCode}`; + testWin = window.open(uri); + } + + async function testDifferentPath() { + // Load an https:// window which gets downgraded to http:// + let uri = + `https://example.com/tests/dom/security/test/https-first/file_break_endless_upgrade_downgrade_loop.sjs?downgrade`; testWin = window.open(uri); } @@ -86,6 +77,7 @@ Test that same origin redirect does not cause endless loop with https-first enab ["dom.security.https_first", true], ["security.mixed_content.block_active_content", false], ["security.mixed_content.block_display_content", false], + ["dom.security.https_only_check_path_upgrade_downgrade_endless_loop", true], ]}, startTest); diff --git a/dom/security/test/https-first/test_multiple_redirection.html b/dom/security/test/https-first/test_multiple_redirection.html index a1b714543c0b..350b0ed30058 100644 --- a/dom/security/test/https-first/test_multiple_redirection.html +++ b/dom/security/test/https-first/test_multiple_redirection.html @@ -21,8 +21,10 @@ Test multiple redirects using https-first and ensure the entire redirect chain i const testCase = [ // test 1: https-first upgrades http://example.com/test1 -> https://example.com/test1 // that's redirect to https://example.com/.../redirect which then redirects - // to http://example.com/../verify. - {name: "test last redirect HTTP", result: "scheme-https", query: "test1" }, + // to http://example.com/../verify. Since the last redirect is http, and the + // the redirection chain contains already example.com we expect https-first + // to downgrade the request. + {name: "test last redirect HTTP", result: "scheme-http", query: "test1" }, // test 2: https-first upgrades http://example.com/test2 -> https://example.com/test2 // that's redirect to https://example.com/.../redirect which then redirects // to https://example.com/../verify. Since the last redirect is https, we @@ -41,13 +43,6 @@ Test multiple redirects using https-first and ensure the entire redirect chain i // http://example.org/.../verify -upgrade-> httpS://example.ORG/.../verify // Everything should be upgraded and accessed only via HTTPS! {name: "test last redirect other HTTP origin gets upgraded", result: "scheme-https", query: "test4" }, - // test 5: https-first upgrades http://example.com/test5 -> https://example.com/test5 - // that's redirect to https://example.com/.../downgrade which then redirects - // https-first upgrades http://example.com/.../downgrade -> https://example.com/.../downgrade - // that's redirect to http://example.com/.../downgrade which which is detected as http downgrade - // to http://example.com/../verify. Since the last redirect is http, and we - // had a downgrade in the redirect chain. We load the http version - {name: "test downgrade HTTP", result: "scheme-http", query: "test5" }, ] let currentTest = 0; let testWin; diff --git a/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs b/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs index 1c77ba6401ff..a8a9083ef3fb 100644 --- a/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs +++ b/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs @@ -4,7 +4,7 @@ const REDIRECT_META = ` - + META REDIRECT @@ -16,17 +16,17 @@ const REDIRECT_JS = ` JS REDIRECT `; const REDIRECT_302 = - "http://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs?test3"; + "http://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs?test3b"; const REDIRECT_302_DIFFERENT_PATH = - "http://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs?verify"; + "http://example.com/tests/dom/security/test/https-only/file_user_gesture.html"; function handleRequest(request, response) { // avoid confusing cache behaviour @@ -35,35 +35,30 @@ function handleRequest(request, response) { // if the scheme is not https, meaning that the initial request did not // get upgraded, then we rather fall through and display unexpected content. - if (request.scheme == "https") { + if (request.scheme === "https") { let query = request.queryString; - if (query == "test1") { + if (query === "test1a") { response.write(REDIRECT_META); return; } - if (query == "test2") { + if (query === "test2a") { response.write(REDIRECT_JS); return; } - if (query == "test3") { + if (query === "test3a") { response.setStatusLine("1.1", 302, "Found"); response.setHeader("Location", REDIRECT_302, false); return; } - if (query == "test4") { + if (query === "test4a") { response.setStatusLine("1.1", 302, "Found"); response.setHeader("Location", REDIRECT_302_DIFFERENT_PATH, false); return; } - - if (query == "verify") { - response.write("OK :)"); - return; - } } // we should never get here, just in case, diff --git a/dom/security/test/https-only/test_break_endless_upgrade_downgrade_loop.html b/dom/security/test/https-only/test_break_endless_upgrade_downgrade_loop.html index 68c99d58a8d6..847a71f3785f 100644 --- a/dom/security/test/https-only/test_break_endless_upgrade_downgrade_loop.html +++ b/dom/security/test/https-only/test_break_endless_upgrade_downgrade_loop.html @@ -16,85 +16,73 @@ * Test 1: Meta Refresh * Test 2: JS Redirect * Test 3: 302 redirect - * Test 4: Redirect to different origin. No redirect loop should be detected */ +SimpleTest.requestFlakyTimeout("We need to wait for the HTTPS-Only error page to appear"); +SimpleTest.requestLongerTimeout(10); SimpleTest.waitForExplicitFinish(); -const HTTP_REQUEST_URL = +const REQUEST_URL = "http://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs"; -const HTTPS_REQUEST_URL = - "https://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs"; -const testQueries = [ - // Test 1: Meta Refresh Redirect - { scheme: "http", query: "test1", error: true }, - { scheme: "https", query: "test1", error: true }, - // Test 2: JS win.location Redirect - { scheme: "http", query: "test2", error: true }, - { scheme: "https", query: "test2", error: true }, - // Test 3: 302 Redirect - { scheme: "http", query: "test3", error: true }, - { scheme: "https", query: "test3", error: true }, - // Test 4: 302 Redirect with a different path - { scheme: "http", query: "test4", error: false }, - { scheme: "https", query: "test4", error: false }, -]; +function resolveAfter6Seconds() { + return new Promise(resolve => { + setTimeout(() => { + resolve(); + }, 6000); + }); +} -let currentTest = 0; -// do each test two time. One time starting with https:// one time with http:// -let testWin; -window.addEventListener("message", receiveMessageWhenLoaded); +async function verifyResult(aTestName) { + let errorPageL10nId = "about-httpsonly-title-alert"; + let innerHTML = content.document.body.innerHTML; + ok(innerHTML.includes(errorPageL10nId), "the error page should be shown for " + aTestName); +} -function postMessageWhenLoaded() { - SimpleTest.waitForCondition(async () => { - return await SpecialPowers.spawn(testWin, [], () => { - let innerHTML = content.document.body.innerHTML; - return innerHTML == "OK :)" - || innerHTML == "DO NOT DISPLAY THIS" - || innerHTML.includes("about-httpsonly-title-alert"); - }).catch(() => false); - }, - () => window.postMessage("https-only-page-loaded", "*"), - "waiting for page load to complete" +async function verifyTest4Result() { + let pathname = content.document.location.pathname; + ok( + pathname === "/tests/dom/security/test/https-only/file_user_gesture.html", + "the http:// page should be loaded" ); } -async function receiveMessageWhenLoaded() { - const currentTestParams = testQueries[currentTest]; - let testName = currentTestParams.scheme + ":" + currentTestParams.query +async function runTests() { + await SpecialPowers.pushPrefEnv({ set: [ + ["dom.security.https_only_mode", true], + ["dom.security.https_only_mode_break_upgrade_downgrade_endless_loop", true], + ["dom.security.https_only_check_path_upgrade_downgrade_endless_loop", true], + ]}); - let innerHTML = await SpecialPowers.spawn(testWin, [], () => { - return content.document.body.innerHTML; - }); - if(currentTestParams.error) { - ok(innerHTML.includes("about-httpsonly-title-alert"), testName + ": the error page should be shown"); - } else { - is(innerHTML, "OK :)", testName + ": different path with https loaded "); - } - testWin.close(); + // Test 1: Meta Refresh Redirect + let winTest1 = window.open(REQUEST_URL + "?test1a", "_blank"); + // Test 2: JS win.location Redirect + let winTest2 = window.open(REQUEST_URL + "?test2a", "_blank"); + // Test 3: 302 Redirect + let winTest3 = window.open(REQUEST_URL + "?test3a", "_blank"); + // Test 4: 302 Redirect with a different path + let winTest4 = window.open(REQUEST_URL + "?test4a", "_blank"); + + // provide enough time for: + // the redirects to occur, and the error page to be displayed + await resolveAfter6Seconds(); + + await SpecialPowers.spawn(winTest1, ["test1"], verifyResult); + winTest1.close(); + + await SpecialPowers.spawn(winTest2, ["test2"], verifyResult); + winTest2.close(); + + await SpecialPowers.spawn(winTest3, ["test3"], verifyResult); + winTest3.close(); + + await SpecialPowers.spawn(winTest4, ["test4"], verifyTest4Result); + winTest4.close(); - if (++currentTest < testQueries.length) { - runNextTest(); - return; - } - // no more tests to run -> cleanup - window.removeEventListener("https-only-page-load", receiveMessageWhenLoaded); SimpleTest.finish(); } -async function runNextTest() { - const currentTestParams = testQueries[currentTest]; - let uri = `${currentTestParams.scheme}://example.com/tests/dom/security/test/https-only/file_break_endless_upgrade_downgrade_loop.sjs?${currentTestParams.query}`; - testWin = window.open(uri, "_blank"); - postMessageWhenLoaded(); -} - -SpecialPowers.pushPrefEnv({ set: [ - ["dom.security.https_only_mode", true], - ["dom.security.https_only_mode_break_upgrade_downgrade_endless_loop", true], - ["dom.security.https_only_mode_ever_enabled", true], // clear this pref at the end - ]}, runNextTest); +runTests(); diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index acbd803675b5..6ff399259c2e 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -3735,6 +3735,13 @@ value: true mirror: always +# If true, when checking if it's upgrade downgrade cycles, the URI path will be +# also checked. +- name: dom.security.https_only_check_path_upgrade_downgrade_endless_loop + type: RelaxedAtomicBool + value: true + 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 diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 986605325f27..4607ad4319db 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -5969,8 +5969,7 @@ HttpBaseChannel::SetNavigationStartTimeStamp(TimeStamp aTimeStamp) { return NS_ERROR_NOT_IMPLEMENTED; } -nsresult HttpBaseChannel::CheckRedirectLimit(nsIURI* aNewURI, - uint32_t aRedirectFlags) const { +nsresult HttpBaseChannel::CheckRedirectLimit(uint32_t aRedirectFlags) const { if (aRedirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) { // for internal redirect due to auth retry we do not have any limit // as we might restrict the number of times a user might retry @@ -6004,39 +6003,15 @@ nsresult HttpBaseChannel::CheckRedirectLimit(nsIURI* aNewURI, // https and the page answers with a redirect (meta, 302, win.location, ...) // then this method can break the cycle which causes the https-only exception // page to appear. Note that https-first mode breaks upgrade downgrade endless - // loops within ShouldUpgradeHttpsFirstRequest because https-first does not + // loops within ShouldUpgradeHTTPSFirstRequest because https-first does not // display an exception page but needs a soft fallback/downgrade. if (nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( - mURI, aNewURI, mLoadInfo, + mURI, mLoadInfo, {nsHTTPSOnlyUtils::UpgradeDowngradeEndlessLoopOptions:: EnforceForHTTPSOnlyMode})) { - // Mark that we didn't upgrade to https due to loop detection in https-only - // mode to show https-only error page. We know that we are in https-only - // mode, because we passed `EnforceForHTTPSOnlyMode` to - // `IsUpgradeDowngradeEndlessLoop`. In other words we upgrade the request - // with https-only mode, but then immediately cancel the request. - uint32_t httpsOnlyStatus = mLoadInfo->GetHttpsOnlyStatus(); - if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_UNINITIALIZED) { - httpsOnlyStatus ^= nsILoadInfo::HTTPS_ONLY_UNINITIALIZED; - httpsOnlyStatus |= - nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_NOT_REGISTERED; - mLoadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); - } - LOG(("upgrade downgrade redirect loop!\n")); return NS_ERROR_REDIRECT_LOOP; } - // in case of http-first mode we want to add an exception to disable the - // upgrade behavior if we have upgrade-downgrade loop to break the loop and - // load the http request next - if (mozilla::StaticPrefs:: - dom_security_https_first_add_exception_on_failiure() && - nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( - mURI, aNewURI, mLoadInfo, - {nsHTTPSOnlyUtils::UpgradeDowngradeEndlessLoopOptions:: - EnforceForHTTPSFirstMode})) { - nsHTTPSOnlyUtils::AddHTTPSFirstExceptionForSession(mURI, mLoadInfo); - } return NS_OK; } diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index f3bce00deac5..b47a3d6c35d0 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -651,7 +651,7 @@ class HttpBaseChannel : public nsHashPropertyBag, static void CallTypeSniffers(void* aClosure, const uint8_t* aData, uint32_t aCount); - nsresult CheckRedirectLimit(nsIURI* aNewURI, uint32_t aRedirectFlags) const; + nsresult CheckRedirectLimit(uint32_t aRedirectFlags) const; bool MaybeWaitForUploadStreamNormalization(nsIStreamListener* aListener, nsISupports* aContext); diff --git a/netwerk/protocol/http/InterceptedHttpChannel.cpp b/netwerk/protocol/http/InterceptedHttpChannel.cpp index 5695d46f924a..c58fbc96391f 100644 --- a/netwerk/protocol/http/InterceptedHttpChannel.cpp +++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp @@ -80,7 +80,7 @@ nsresult InterceptedHttpChannel::SetupReplacementChannel( return rv; } - rv = CheckRedirectLimit(aURI, aRedirectFlags); + rv = CheckRedirectLimit(aRedirectFlags); NS_ENSURE_SUCCESS(rv, rv); // While we can't resume an synthetic response, we can still propagate diff --git a/netwerk/protocol/http/TRRServiceChannel.cpp b/netwerk/protocol/http/TRRServiceChannel.cpp index a2a0c4127fd9..08e72a85c7f7 100644 --- a/netwerk/protocol/http/TRRServiceChannel.cpp +++ b/netwerk/protocol/http/TRRServiceChannel.cpp @@ -1165,7 +1165,7 @@ nsresult TRRServiceChannel::SetupReplacementChannel(nsIURI* aNewURI, return rv; } - rv = CheckRedirectLimit(aNewURI, aRedirectFlags); + rv = CheckRedirectLimit(aRedirectFlags); NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr httpChannel = do_QueryInterface(aNewChannel); diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 17c203540352..d24e0dde7d40 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -703,7 +703,23 @@ nsresult nsHttpChannel::MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade, nsAutoCString uriHost; mURI->GetAsciiHost(uriHost); - return gHttpHandler->IsHostExcludedForHTTPSRR(uriHost); + if (gHttpHandler->IsHostExcludedForHTTPSRR(uriHost)) { + return true; + } + + if (nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( + mURI, mLoadInfo, + {nsHTTPSOnlyUtils::UpgradeDowngradeEndlessLoopOptions:: + EnforceForHTTPSRR})) { + // Add the host to a excluded list because: + // 1. We don't need to do the same check again. + // 2. Other subresources in the same host will be also excluded. + gHttpHandler->ExcludeHTTPSRRHost(uriHost); + LOG(("[%p] skip HTTPS upgrade for host [%s]", this, uriHost.get())); + return true; + } + + return false; }; if (shouldSkipUpgradeWithHTTPSRR()) { @@ -5397,27 +5413,7 @@ nsresult nsHttpChannel::SetupReplacementChannel(nsIURI* newURI, newURI, newChannel, preserveMethod, redirectFlags); if (NS_FAILED(rv)) return rv; - nsAutoCString uriHost; - mURI->GetAsciiHost(uriHost); - // disable https-rr when encountering a downgrade from https to http. - // If the host would have https-rr dns-entries, it would be misconfigured - // due to giving us mixed signals: - // 1. the signal to upgrade all http requests to https, - // 2. but also downgrading to http on https via redirects. - // Add to exclude list for that reason - if (!gHttpHandler->IsHostExcludedForHTTPSRR(uriHost) && - nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop( - mURI, newURI, mLoadInfo, - {nsHTTPSOnlyUtils::UpgradeDowngradeEndlessLoopOptions:: - EnforceForHTTPSRR})) { - // Add the host to a excluded list because: - // 1. We don't need to do the same check again. - // 2. Other subresources in the same host will be also excluded. - gHttpHandler->ExcludeHTTPSRRHost(uriHost); - LOG(("[%p] skip HTTPS upgrade for host [%s]", this, uriHost.get())); - } - - rv = CheckRedirectLimit(newURI, redirectFlags); + rv = CheckRedirectLimit(redirectFlags); NS_ENSURE_SUCCESS(rv, rv); // pass on the early hint observer to be able to process `103 Early Hints` diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js index 0183355fbb10..30bf09cdac8e 100644 --- a/testing/xpcshell/moz-http2/moz-http2.js +++ b/testing/xpcshell/moz-http2/moz-http2.js @@ -1692,7 +1692,7 @@ function handleRequest(req, res) { } else if (u.pathname === "/redirect_to_http") { res.setHeader( "Location", - `http://test.httpsrr.redirect.com:${u.query.port}/redirect_to_http?port=${u.query.port}` + `http://test.httpsrr.redirect.com:${u.query.port}/redirect_to_http` ); res.writeHead(307); res.end("");