Bug 1716069 - P2: Use nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop to check if there is an endless loop when HTTPS RR is presented, r=ckerschb

Depends on D118598

Differential Revision: https://phabricator.services.mozilla.com/D118599

dom/security/nsHTTPSOnlyUtils.cpp

@@ -236,14 +236,18 @@ bool nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop(
       IsHttpsFirstModeEnabled(isPrivateWin) &&
       aOptions.contains(
           UpgradeDowngradeEndlessLoopOptions::EnforceForHTTPSFirstMode);
-  if (!enforceForHTTPSOnlyMode && !enforceForHTTPSFirstMode) {
+  bool enforceForHTTPSRR =
+      aOptions.contains(UpgradeDowngradeEndlessLoopOptions::EnforceForHTTPSRR);
+  if (!enforceForHTTPSOnlyMode && !enforceForHTTPSFirstMode &&
+      !enforceForHTTPSRR) {
     return false;
   }
 
   // 2. Check if the upgrade downgrade pref even wants us to try to break the
-  // cycle.
+  // cycle. In the case that HTTPS RR is presented, we ignore this pref.
   if (!mozilla::StaticPrefs::
-          dom_security_https_only_mode_break_upgrade_downgrade_endless_loop()) {
+          dom_security_https_only_mode_break_upgrade_downgrade_endless_loop() &&
+      !enforceForHTTPSRR) {
     return false;
   }
 
@@ -255,7 +259,8 @@ bool nsHTTPSOnlyUtils::IsUpgradeDowngradeEndlessLoop(
 
   // 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) {
+  if ((httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) &&
+      !enforceForHTTPSRR) {
     return false;
   }
 

dom/security/nsHTTPSOnlyUtils.h

@@ -63,15 +63,18 @@ class nsHTTPSOnlyUtils {
    * https-only upgrades to https and the website answers with a meta-refresh
    * to downgrade to same-origin http version. Similarly this method breaks
    * the endless cycle for JS based redirects and 302 based redirects.
+   * Note this function is also used when we got an HTTPS RR for the website.
    * @param  aURI      nsIURI of request
    * @param  aLoadInfo nsILoadInfo of request
    * @param  aOptions an options object indicating if the function
-   *                  should be consulted for https-only or https-first mode.
+   *                  should be consulted for https-only or https-first mode or
+   *                  the case that an HTTPS RR is presented.
    * @return           true if an endless loop is detected
    */
   enum class UpgradeDowngradeEndlessLoopOptions {
     EnforceForHTTPSOnlyMode,
     EnforceForHTTPSFirstMode,
+    EnforceForHTTPSRR,
   };
   static bool IsUpgradeDowngradeEndlessLoop(
       nsIURI* aURI, nsILoadInfo* aLoadInfo,

netwerk/protocol/http/nsHttpChannel.cpp

@@ -123,6 +123,7 @@
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/ServiceWorkerUtils.h"
 #include "mozilla/dom/nsHTTPSOnlyStreamListener.h"
+#include "mozilla/dom/nsHTTPSOnlyUtils.h"
 #include "mozilla/net/AsyncUrlChannelClassifier.h"
 #include "mozilla/net/CookieJarSettings.h"
 #include "mozilla/net/NeckoChannelParams.h"
@@ -556,32 +557,27 @@ nsresult nsHttpChannel::MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade,
       return true;
     }
 
-    nsCOMPtr<nsIPrincipal> triggeringPrincipal =
-        mLoadInfo->TriggeringPrincipal();
-    // If the security context that triggered the load is not https, then it's
-    // not a downgrade scenario.
-    if (!triggeringPrincipal->SchemeIs("https")) {
-      return false;
+    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;
     }
 
-    nsAutoCString triggeringHost;
-    triggeringPrincipal->GetAsciiHost(triggeringHost);
-
-    // If the initial request's host is not the same, we should upgrade this
-    // request.
-    if (!triggeringHost.Equals(uriHost)) {
-      return false;
-    }
-
-    // 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);
-    return true;
+    return false;
   };
 
   if (shouldSkipUpgradeWithHTTPSRR()) {
     StoreUseHTTPSSVC(false);
+    // If the website does not want to use HTTPS RR, we should set
+    // NS_HTTP_DISALLOW_HTTPS_RR. This is for avoiding HTTPS RR being used by
+    // the transaction.
+    mCaps |= NS_HTTP_DISALLOW_HTTPS_RR;
     return ContinueOnBeforeConnect(aShouldUpgrade, aStatus);
   }
 

netwerk/test/unit/test_httpssvc_https_upgrade.js

@@ -215,3 +215,30 @@ add_task(async function testLiteralIP() {
   Assert.equal(response, content);
   await new Promise(resolve => httpserv.stop(resolve));
 });
+
+// Test the case that an HTTPS RR is available and the server returns a 307
+// for redirecting back to http.
+add_task(async function testEndlessUpgradeDowngrade() {
+  dns.clearCache(true);
+
+  let httpserv = new HttpServer();
+  let content = "okok";
+  httpserv.start(-1);
+  let port = httpserv.identity.primaryPort;
+  httpserv.registerPathHandler(`/redirect_to_http`, function handler(
+    metadata,
+    response
+  ) {
+    response.setHeader("Content-Length", `${content.length}`);
+    response.bodyOutputStream.write(content, content.length);
+  });
+  httpserv.identity.setPrimary("http", "test.httpsrr.redirect.com", port);
+
+  let chan = makeChan(
+    `http://test.httpsrr.redirect.com:${port}/redirect_to_http?port=${port}`
+  );
+
+  let [, response] = await channelOpenPromise(chan);
+  Assert.equal(response, content);
+  await new Promise(resolve => httpserv.stop(resolve));
+});

testing/xpcshell/moz-http2/moz-http2.js

@@ -1657,6 +1657,14 @@ function handleRequest(req, res) {
     });
     res.end();
     return;
+  } else if (u.pathname === "/redirect_to_http") {
+    res.setHeader(
+      "Location",
+      `http://test.httpsrr.redirect.com:${u.query.port}/redirect_to_http`
+    );
+    res.writeHead(307);
+    res.end("");
+    return;
   }
 
   res.setHeader("Content-Type", "text/html");