Bug 1652670 - P2: Make nsSocketTransport use IP hint address to connect r=valentin,dragana

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

netwerk/base/nsISocketTransport.idl

@@ -276,6 +276,11 @@ interface nsISocketTransport : nsITransport
     }
 %}
 
+    /**
+     * If set, we will use IP hint addresses to connect to the host.
+     */
+    const unsigned long USE_IP_HINT_ADDRESS = (1 << 13);
+
     /**
      * An opaque flags for non-standard behavior of the TLS system.
      * It is unlikely this will need to be set outside of telemetry studies

netwerk/base/nsSocketTransport2.cpp

@@ -1045,6 +1045,10 @@ nsresult nsSocketTransport::ResolveHost() {
   if (mConnectionFlags & nsSocketTransport::DISABLE_TRR)
     dnsFlags |= nsIDNSService::RESOLVE_DISABLE_TRR;
 
+  if (mConnectionFlags & nsSocketTransport::USE_IP_HINT_ADDRESS) {
+    dnsFlags |= nsIDNSService::RESOLVE_IP_HINT;
+  }
+
   dnsFlags |= nsIDNSService::GetFlagsFromTRRMode(
       nsISocketTransport::GetTRRModeFromFlags(mConnectionFlags));
 

netwerk/dns/HTTPSSVC.cpp

@@ -208,6 +208,11 @@ NS_IMETHODIMP SVCBRecord::GetValues(nsTArray<RefPtr<nsISVCParam>>& aValues) {
   return NS_OK;
 }
 
+NS_IMETHODIMP SVCBRecord::GetHasIPHintAddress(bool* aHasIPHintAddress) {
+  *aHasIPHintAddress = mData.mHasIPHints;
+  return NS_OK;
+}
+
 already_AddRefed<nsISVCBRecord>
 DNSHTTPSSVCRecordBase::GetServiceModeRecordInternal(
     bool aNoHttp2, bool aNoHttp3, const nsTArray<SVCB>& aRecords,

netwerk/dns/HTTPSSVC.h

@@ -91,6 +91,7 @@ struct SVCB {
   void GetIPHints(CopyableTArray<mozilla::net::NetAddr>& aAddresses) const;
   uint16_t mSvcFieldPriority = 0;
   nsCString mSvcDomainName;
+  bool mHasIPHints = false;
   CopyableTArray<SvcFieldValue> mSvcFieldValue;
 };
 

netwerk/dns/TRR.cpp

@@ -1091,6 +1091,11 @@ nsresult TRR::DohDecode(nsCString& aHost) {
             if (key == SvcParamKeyMandatory || key > SvcParamKeyLast) {
               continue;
             }
+
+            if (value.mValue.is<SvcParamIpv4Hint>() ||
+                value.mValue.is<SvcParamIpv6Hint>()) {
+              parsed.mHasIPHints = true;
+            }
             parsed.mSvcFieldValue.AppendElement(value);
           }
 

netwerk/dns/nsIDNSByTypeRecord.idl

@@ -92,6 +92,7 @@ interface nsISVCBRecord : nsISupports {
   readonly attribute ACString name;
   [noscript, nostdcall, notxpcom] readonly attribute MaybePort port;
   [noscript, nostdcall, notxpcom] readonly attribute MaybeAlpn alpn;
+  readonly attribute bool hasIPHintAddress;
   readonly attribute Array<nsISVCParam> values;
 };
 

netwerk/ipc/NeckoChannelParams.ipdlh

@@ -369,6 +369,7 @@ struct HttpConnectionInfoCloneArgs
   bool isIPv6Disabled;
   nsCString topWindowOrigin;
   bool isHttp3;
+  bool hasIPHintAddress;
   ProxyInfoCloneArgs[] proxyInfo;
 };
 

netwerk/protocol/http/nsHttpConnectionInfo.cpp

@@ -113,6 +113,7 @@ void nsHttpConnectionInfo::Init(const nsACString& host, int32_t port,
   mTRRMode = nsIRequest::TRR_DEFAULT_MODE;
   mIPv4Disabled = false;
   mIPv6Disabled = false;
+  mHasIPHintAddress = false;
 
   mUsingHttpsProxy = (proxyInfo && proxyInfo->IsHTTPS());
   mUsingHttpProxy = mUsingHttpsProxy || (proxyInfo && proxyInfo->IsHTTP());
@@ -337,6 +338,7 @@ already_AddRefed<nsHttpConnectionInfo> nsHttpConnectionInfo::Clone() const {
   clone->SetTRRMode(GetTRRMode());
   clone->SetIPv4Disabled(GetIPv4Disabled());
   clone->SetIPv6Disabled(GetIPv6Disabled());
+  clone->SetHasIPHintAddress(HasIPHintAddress());
   MOZ_ASSERT(clone->Equals(this));
 
   return clone.forget();
@@ -386,6 +388,12 @@ nsHttpConnectionInfo::CloneAndAdoptHTTPSSVCRecord(
   clone->SetIPv4Disabled(GetIPv4Disabled());
   clone->SetIPv6Disabled(GetIPv6Disabled());
 
+  bool hasIPHint = false;
+  Unused << aRecord->GetHasIPHintAddress(&hasIPHint);
+  if (hasIPHint) {
+    clone->SetHasIPHintAddress(hasIPHint);
+  }
+
   return clone.forget();
 }
 
@@ -413,6 +421,7 @@ void nsHttpConnectionInfo::SerializeHttpConnectionInfo(
   aArgs.isIPv6Disabled() = aInfo->GetIPv6Disabled();
   aArgs.topWindowOrigin() = aInfo->GetTopWindowOrigin();
   aArgs.isHttp3() = aInfo->IsHttp3();
+  aArgs.hasIPHintAddress() = aInfo->HasIPHintAddress();
 
   if (!aInfo->ProxyInfo()) {
     return;
@@ -456,6 +465,7 @@ nsHttpConnectionInfo::DeserializeHttpConnectionInfoCloneArgs(
   cinfo->SetTRRMode(static_cast<nsIRequest::TRRMode>(aInfoArgs.trrMode()));
   cinfo->SetIPv4Disabled(aInfoArgs.isIPv4Disabled());
   cinfo->SetIPv6Disabled(aInfoArgs.isIPv6Disabled());
+  cinfo->SetHasIPHintAddress(aInfoArgs.hasIPHintAddress());
 
   return cinfo.forget();
 }
@@ -481,6 +491,7 @@ void nsHttpConnectionInfo::CloneAsDirectRoute(nsHttpConnectionInfo** outCI) {
   clone->SetTRRMode(GetTRRMode());
   clone->SetIPv4Disabled(GetIPv4Disabled());
   clone->SetIPv6Disabled(GetIPv6Disabled());
+  clone->SetHasIPHintAddress(HasIPHintAddress());
 
   clone.forget(outCI);
 }

netwerk/protocol/http/nsHttpConnectionInfo.h

@@ -209,6 +209,9 @@ class nsHttpConnectionInfo final : public ARefBase {
 
   bool IsHttp3() const { return mIsHttp3; }
 
+  void SetHasIPHintAddress(bool aHasIPHint) { mHasIPHintAddress = aHasIPHint; }
+  bool HasIPHintAddress() const { return mHasIPHintAddress; }
+
  private:
   // These constructor versions are intended to be used from Clone() and
   // DeserializeHttpConnectionInfoCloneArgs().
@@ -260,6 +263,8 @@ class nsHttpConnectionInfo final : public ARefBase {
                         // is 1.3 or greater the value will be false.
   bool mIsHttp3;
 
+  bool mHasIPHintAddress = false;
+
   // for RefPtr
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsHttpConnectionInfo, override)
 };

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -4246,6 +4246,26 @@ nsresult nsHttpConnectionMgr::nsHalfOpenSocket::SetupStreams(
     tmpFlags |= nsISocketTransport::BE_CONSERVATIVE;
   }
 
+  if (ci->HasIPHintAddress()) {
+    nsCOMPtr<nsIDNSService> dns =
+        do_GetService("@mozilla.org/network/dns-service;1", &rv);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    // The spec says: "If A and AAAA records for TargetName are locally
+    // available, the client SHOULD ignore these hints.", so we check if the DNS
+    // record is in cache before setting USE_IP_HINT_ADDRESS.
+    nsCOMPtr<nsIDNSRecord> record;
+    rv = dns->ResolveNative(ci->GetRoutedHost(), nsIDNSService::RESOLVE_OFFLINE,
+                            mEnt->mConnInfo->GetOriginAttributes(),
+                            getter_AddRefs(record));
+    if (NS_FAILED(rv) || !record) {
+      LOG(("Setting Socket to use IP hint address"));
+      tmpFlags |= nsISocketTransport::USE_IP_HINT_ADDRESS;
+    }
+  }
+
   if (mCaps & NS_HTTP_DISABLE_IPV4) {
     tmpFlags |= nsISocketTransport::DISABLE_IPV4;
   } else if (mCaps & NS_HTTP_DISABLE_IPV6) {

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -2740,6 +2740,14 @@ NS_IMETHODIMP nsHttpTransaction::OnLookupComplete(nsICancelable* aRequest,
   LOG(("nsHttpTransaction::OnLookupComplete [this=%p] mActivated=%d", this,
        mActivated));
 
+  nsCOMPtr<nsIDNSAddrRecord> addrRecord = do_QueryInterface(aRecord);
+  // nsHttpTransaction::OnLookupComplete will be called again when receving the
+  // result of speculatively loading the addr records of the target name. In
+  // this case, just return NS_OK.
+  if (addrRecord) {
+    return NS_OK;
+  }
+
   {
     MutexAutoLock lock(mLock);
     mDNSRequest = nullptr;
@@ -2790,6 +2798,27 @@ NS_IMETHODIMP nsHttpTransaction::OnLookupComplete(nsICancelable* aRequest,
     // can be added to the right connection entry.
     UpdateConnectionInfo(newInfo);
   }
+
+  // Prefetch the A/AAAA records of the target name.
+  nsCOMPtr<nsIDNSService> dns = do_GetService(NS_DNSSERVICE_CONTRACTID);
+  if (dns) {
+    uint32_t flags =
+        nsIDNSService::GetFlagsFromTRRMode(mConnInfo->GetTRRMode());
+    if (mCaps & NS_HTTP_REFRESH_DNS) {
+      flags |= nsIDNSService::RESOLVE_BYPASS_CACHE;
+    }
+
+    nsCOMPtr<nsICancelable> tmpOutstanding;
+    nsAutoCString targetName;
+    Unused << svcbRecord->GetName(targetName);
+
+    Unused << dns->AsyncResolveNative(
+        targetName, nsIDNSService::RESOLVE_TYPE_DEFAULT,
+        flags | nsIDNSService::RESOLVE_SPECULATE, nullptr, this,
+        GetCurrentEventTarget(), mConnInfo->GetOriginAttributes(),
+        getter_AddRefs(tmpOutstanding));
+  }
+
   return NS_OK;
 }
 

netwerk/test/unit/test_httpssvc_iphint.js

@@ -231,3 +231,65 @@ add_task(async function testStoreIPHint() {
     ["1.2.3.4", "5.6.7.8"]
   );
 });
+
+function makeChan(url) {
+  let chan = NetUtil.newChannel({
+    uri: url,
+    loadUsingSystemPrincipal: true,
+  }).QueryInterface(Ci.nsIHttpChannel);
+  return chan;
+}
+
+function channelOpenPromise(chan) {
+  return new Promise(resolve => {
+    function finish(req, buffer) {
+      resolve([req, buffer]);
+    }
+    let internal = chan.QueryInterface(Ci.nsIHttpChannelInternal);
+    internal.setWaitForHTTPSSVCRecord();
+    chan.asyncOpen(new ChannelListener(finish, null, CL_ALLOW_UNKNOWN_CL));
+  });
+}
+
+// Test if we can connect to the server with the IP hint address.
+add_task(async function testConnectionWithIPHint() {
+  dns.clearCache(true);
+  prefs.setIntPref("network.trr.mode", 3);
+  prefs.setCharPref(
+    "network.trr.uri",
+    "https://127.0.0.1:" + h2Port + "/httpssvc_use_iphint"
+  );
+
+  // Resolving test.iphint.com should be failed.
+  let listener = new DNSListener();
+  let request = dns.asyncResolve(
+    "test.iphint.com",
+    dns.RESOLVE_TYPE_DEFAULT,
+    0,
+    null, // resolverInfo
+    listener,
+    mainThread,
+    defaultOriginAttributes
+  );
+
+  let [inRequest, inRecord, inStatus] = await listener;
+  Assert.equal(inRequest, request, "correct request was used");
+  Assert.equal(
+    inStatus,
+    Cr.NS_ERROR_UNKNOWN_HOST,
+    "status is NS_ERROR_UNKNOWN_HOST"
+  );
+
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    true
+  );
+
+  // The connection should be succeeded since the IP hint is 127.0.0.1.
+  let chan = makeChan(`https://test.iphint.com:8080/`);
+  let [req, resp] = await channelOpenPromise(chan);
+  Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
+
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    false
+  );
+});

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

@@ -924,9 +924,6 @@ function handleRequest(req, res) {
             values: [
               { key: "alpn", value: "h2" },
               { key: "port", value: serverPort },
-              { key: "ipv4hint", value: "1.2.3.4" },
-              { key: "echconfig", value: "123..." },
-              { key: "ipv6hint", value: "::1" },
               { key: 30, value: "somelargestring" },
             ],
           },
@@ -956,6 +953,46 @@ function handleRequest(req, res) {
       res.end("");
     });
     return;
+  } else if (u.pathname === "/httpssvc_use_iphint") {
+    let payload = Buffer.from("");
+    req.on("data", function receiveData(chunk) {
+      payload = Buffer.concat([payload, chunk]);
+    });
+    req.on("end", function finishedData() {
+      let packet = dnsPacket.decode(payload);
+      let answers = [];
+      answers.push({
+        name: packet.questions[0].name,
+        type: "HTTPS",
+        ttl: 55,
+        class: "IN",
+        flush: false,
+        data: {
+          priority: 1,
+          name: ".",
+          values: [
+            { key: "alpn", value: "h2" },
+            { key: "port", value: serverPort },
+            { key: "ipv4hint", value: "127.0.0.1" },
+          ],
+        },
+      });
+
+      let buf = dnsPacket.encode({
+        type: "response",
+        id: packet.id,
+        flags: dnsPacket.RECURSION_DESIRED,
+        questions: packet.questions,
+        answers,
+      });
+
+      res.setHeader("Content-Type", "application/dns-message");
+      res.setHeader("Content-Length", buf.length);
+      res.writeHead(200);
+      res.write(buf);
+      res.end("");
+    });
+    return;
   } else if (u.pathname === "/dns-cname-a") {
     // test23 asks for cname-a.example.com
     // this responds with a CNAME to here.example.com *and* an A record