Bug 1896625 - When using HTTPS proxy, setup TLS tunnel for CONNECT only request, r=necko-reviewers,jesup

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

netwerk/protocol/http/TLSTransportLayer.cpp

@@ -360,6 +360,7 @@ NS_INTERFACE_MAP_BEGIN(TLSTransportLayer)
   NS_INTERFACE_MAP_ENTRY(nsIInputStreamCallback)
   NS_INTERFACE_MAP_ENTRY(nsIOutputStreamCallback)
   NS_INTERFACE_MAP_ENTRY_CONCRETE(TLSTransportLayer)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsITransport)
 NS_INTERFACE_MAP_END
 
 TLSTransportLayer::TLSTransportLayer(nsISocketTransport* aTransport,

netwerk/protocol/http/nsHttpConnection.cpp

@@ -1067,25 +1067,25 @@ void nsHttpConnection::HandleTunnelResponse(uint16_t responseStatus,
       *reset = true;
     }
     nsresult rv;
-    // CONNECT only flag doesn't do the tls setup. https here only
-    // ensures a proxy tunnel was used not that tls is setup.
     if (isHttps) {
-      if (!onlyConnect) {
+      bool skipSSL = false;
-        if (mConnInfo->UsingHttpsProxy()) {
+      if (mConnInfo->UsingHttpsProxy()) {
-          LOG(("%p new TLSFilterTransaction %s %d\n", this, mConnInfo->Origin(),
+        LOG(("%p SetupSecondaryTLS %s %d\n", this, mConnInfo->Origin(),
-               mConnInfo->OriginPort()));
+             mConnInfo->OriginPort()));
-          SetupSecondaryTLS();
+        SetupSecondaryTLS();
-        }
+      } else if (onlyConnect) {
+        MOZ_ASSERT(mConnInfo->UsingOnlyHttpProxy(), "Must be a HTTP proxy");
 
+        // We have CONNECT only flag and a HTTP proxy is used here, so we can
+        // just skip setting up SSL. We have to mark this as complete to finish
+        // the transaction and be upgraded.
+        mTlsHandshaker->SetNPNComplete();
+        skipSSL = true;
+      }
+
+      if (!skipSSL) {
         rv = mTlsHandshaker->InitSSLParams(false, true);
         LOG(("InitSSLParams [rv=%" PRIx32 "]\n", static_cast<uint32_t>(rv)));
-      } else {
-        // We have an https protocol but the CONNECT only flag was
-        // specified. The consumer only wants a raw socket to the
-        // proxy. We have to mark this as complete to finish the
-        // transaction and be upgraded. OnSocketReadable() uses this
-        // to detect an inactive tunnel and blocks completion.
-        mTlsHandshaker->SetNPNComplete();
       }
     }
     rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
@@ -1612,14 +1612,15 @@ nsresult nsHttpConnection::OnSocketWritable() {
       return NS_ERROR_FAILURE;
     }
 
-    if (mState == HttpConnectionState::REQUEST) {
+    if (mState == HttpConnectionState::REQUEST &&
+        mTlsHandshaker->EnsureNPNComplete()) {
       // Don't need to check this each write attempt since it is only
       // updated after OnSocketWritable completes.
       // We've already done primary tls (if needed) and sent our CONNECT.
       // If we're doing a CONNECT only request there's no need to write
-      // the http transaction or do the SSL handshake here.
+      // the http transaction.
-      LOG(("return ok because proxy connect successful\n"));
+      LOG(("return NS_BASE_STREAM_CLOSED to make transaction closed\n"));
-      return NS_OK;
+      return NS_BASE_STREAM_CLOSED;
     }
   }
 

netwerk/protocol/http/nsHttpConnectionInfo.h

@@ -236,6 +236,9 @@ class nsHttpConnectionInfo final : public ARefBase {
   // Returns true when proxying over HTTP or HTTPS
   bool UsingHttpProxy() const { return mUsingHttpProxy || mUsingHttpsProxy; }
 
+  // Returns true when only proxying over HTTP
+  bool UsingOnlyHttpProxy() const { return mUsingHttpProxy; }
+
   // Returns true when proxying over HTTPS
   bool UsingHttpsProxy() const { return mUsingHttpsProxy; }
 

netwerk/test/unit/test_proxyconnect_https.js

@@ -0,0 +1,62 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+"use strict";
+
+/* import-globals-from head_cache.js */
+/* import-globals-from head_cookies.js */
+/* import-globals-from head_channels.js */
+/* import-globals-from head_servers.js */
+
+const { TestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/TestUtils.sys.mjs"
+);
+
+function makeChan(uri) {
+  let chan = NetUtil.newChannel({
+    uri,
+    loadUsingSystemPrincipal: true,
+  }).QueryInterface(Ci.nsIHttpChannel);
+  chan.loadFlags = Ci.nsIChannel.LOAD_INITIAL_DOCUMENT_URI;
+  return chan;
+}
+
+let gotTransport = false;
+var upgradeListener = {
+  onTransportAvailable: () => {
+    gotTransport = true;
+  },
+  QueryInterface: ChromeUtils.generateQI(["nsIHttpUpgradeListener"]),
+};
+
+add_task(async function test_connect_only_https() {
+  let certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(
+    Ci.nsIX509CertDB
+  );
+  addCertFromFile(certdb, "http2-ca.pem", "CTu,u,u");
+  addCertFromFile(certdb, "proxy-ca.pem", "CTu,u,u");
+
+  let proxy = new NodeHTTPSProxyServer();
+  await proxy.start();
+  let server = new NodeHTTPSServer();
+  await server.start();
+  registerCleanupFunction(async () => {
+    await proxy.stop();
+    await server.stop();
+  });
+
+  let chan = makeChan(`https://localhost:${server.port()}/test`);
+  var internal = chan.QueryInterface(Ci.nsIHttpChannelInternal);
+  internal.HTTPUpgrade("webrtc", upgradeListener);
+  internal.setConnectOnly();
+  await new Promise(resolve => {
+    chan.asyncOpen(new ChannelListener(resolve, null, CL_ALLOW_UNKNOWN_CL));
+  });
+
+  await TestUtils.waitForCondition(() => gotTransport);
+  Assert.ok(gotTransport);
+
+  await proxy.stop();
+  await server.stop();
+});

netwerk/test/unit/xpcshell.toml

@@ -1289,6 +1289,11 @@ skip-if = [
 ["test_connection_coalescing.js"]
 
 ["test_default_uri_bypass.js"]
+
+["test_proxyconnect_https.js"]
+skip-if = [
+  "socketprocess_networking",
+]
 ["test_bug1883496.js"]
 skip-if = [
   "os != 'android'",