Bug 1641222 - Follow CNAME/AliasForm chains r=dragana,necko-reviewers
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 30 Jun 2020 12:27:09 +0000
changeset 537976 083212102e9657e755b8f81c1f7299126061e7f1
parent 537975 91db228d3d12f206fc2eed243bd32bbae0cc4d79
child 537977 ccbcfd014a0a84d8a62f3d8fa4c0e15cf5cc8000
push id37556
push userabutkovits@mozilla.com
push dateTue, 30 Jun 2020 21:45:13 +0000
treeherdermozilla-central@47f18d1138df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, necko-reviewers
bugs1641222
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1641222 - Follow CNAME/AliasForm chains r=dragana,necko-reviewers Differential Revision: https://phabricator.services.mozilla.com/D79264
netwerk/dns/TRR.cpp
netwerk/test/unit/head_trr.js
netwerk/test/unit/test_trr_httpssvc.js
netwerk/test/unit/xpcshell.ini
netwerk/test/unit_ipc/head_trr_clone.js
netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js
netwerk/test/unit_ipc/xpcshell.ini
--- a/netwerk/dns/TRR.cpp
+++ b/netwerk/dns/TRR.cpp
@@ -827,17 +827,18 @@ nsresult TRR::DohDecode(nsCString& aHost
     }
     // 16 bit TYPE
     if (mBodySize < (index + 2)) {
       LOG(("TRR: Dohdecode:%d fail at index %d\n", __LINE__, index + 2));
       return NS_ERROR_ILLEGAL_VALUE;
     }
     uint16_t TYPE = get16bit(mResponse, index);
 
-    if ((TYPE != TRRTYPE_CNAME) && (TYPE != static_cast<uint16_t>(mType))) {
+    if ((TYPE != TRRTYPE_CNAME) && (TYPE != TRRTYPE_HTTPSSVC) &&
+        (TYPE != static_cast<uint16_t>(mType))) {
       // Not the same type as was asked for nor CNAME
       LOG(("TRR: Dohdecode:%d asked for type %d got %d\n", __LINE__, mType,
            TYPE));
       return NS_ERROR_UNEXPECTED;
     }
     index += 2;
 
     // 16 bit class
@@ -1019,16 +1020,34 @@ nsresult TRR::DohDecode(nsCString& aHost
 
             // If this is an unknown key, we will simply ignore it.
             if (key == SvcParamKeyNone || key > SvcParamKeyLast) {
               continue;
             }
             parsed.mSvcFieldValue.AppendElement(value);
           }
 
+          // Check for AliasForm
+          if (mCname.IsEmpty() && parsed.mSvcFieldPriority == 0) {
+            // Alias form SvcDomainName must not have the "." value (empty)
+            if (parsed.mSvcDomainName.IsEmpty()) {
+              return NS_ERROR_UNEXPECTED;
+            }
+            mCname = parsed.mSvcDomainName;
+            ToLowerCase(mCname);
+            LOG(("TRR::DohDecode HTTPSSVC AliasForm host %s => %s\n",
+                 host.get(), mCname.get()));
+            break;
+          }
+
+          if (mType != TRRTYPE_HTTPSSVC) {
+            // Ignore the entry that we just parsed if we didn't ask for it.
+            break;
+          }
+
           if (!mResult.is<TypeRecordHTTPSSVC>()) {
             mResult = mozilla::AsVariant(CopyableTArray<SVCB>());
           }
           {
             auto& results = mResult.as<TypeRecordHTTPSSVC>();
             results.AppendElement(parsed);
           }
           break;
@@ -1275,17 +1294,17 @@ nsresult TRR::FailData(nsresult error) {
 }
 
 nsresult TRR::On200Response(nsIChannel* aChannel) {
   // decode body and create an AddrInfo struct for the response
   nsresult rv = DohDecode(mHost);
 
   if (NS_SUCCEEDED(rv)) {
     if (!mDNS.mAddresses.getFirst() && !mCname.IsEmpty() &&
-        mType != TRRTYPE_TXT && mType != TRRTYPE_HTTPSSVC) {
+        mType != TRRTYPE_TXT) {
       nsCString cname = mCname;
       LOG(("TRR: check for CNAME record for %s within previous response\n",
            cname.get()));
       rv = DohDecode(cname);
       if (NS_SUCCEEDED(rv) && mDNS.mAddresses.getFirst()) {
         LOG(("TRR: Got the CNAME record without asking for it\n"));
         ReturnData(aChannel);
         return NS_OK;
--- a/netwerk/test/unit/head_trr.js
+++ b/netwerk/test/unit/head_trr.js
@@ -97,33 +97,37 @@ class TRRDNSListener {
     this.delay = delay;
     this.promise = new Promise(resolve => {
       this.resolve = resolve;
     });
 
     const dns = Cc["@mozilla.org/network/dns-service;1"].getService(
       Ci.nsIDNSService
     );
+    const threadManager = Cc["@mozilla.org/thread-manager;1"].getService(
+      Ci.nsIThreadManager
+    );
+    const currentThread = threadManager.currentThread;
 
     if (trrServer == "") {
       this.request = dns.asyncResolve(
         name,
         0,
         this,
-        Services.tm.currentThread,
+        currentThread,
         {} // defaultOriginAttributes
       );
     } else {
       try {
         this.request = dns.asyncResolveWithTrrServer(
           name,
           trrServer,
           0,
           this,
-          Services.tm.currentThread,
+          currentThread,
           {} // defaultOriginAttributes
         );
         Assert.ok(!expectEarlyFail);
       } catch (e) {
         Assert.ok(expectEarlyFail);
         this.resolve([e]);
       }
     }
--- a/netwerk/test/unit/test_trr_httpssvc.js
+++ b/netwerk/test/unit/test_trr_httpssvc.js
@@ -200,8 +200,235 @@ add_task(async function testHTTPSSVC() {
       .address,
     "fe80::794f:6d2c:3d5e:7836",
     "got correct answer"
   );
   Assert.equal(answer[2].priority, 3);
   Assert.equal(answer[2].name, "hello");
   Assert.equal(answer[2].values.length, 0);
 });
+
+add_task(async function test_aliasform() {
+  let trrServer = new TRRServer();
+  registerCleanupFunction(async () => trrServer.stop());
+  await trrServer.start();
+  dump(`port = ${trrServer.port}\n`);
+
+  if (inChildProcess()) {
+    do_send_remote_message("mode3-port", trrServer.port);
+    await do_await_remote_message("mode3-port-done");
+  } else {
+    Services.prefs.setIntPref("network.trr.mode", 3);
+    Services.prefs.setCharPref(
+      "network.trr.uri",
+      `https://foo.example.com:${trrServer.port}/dns-query`
+    );
+  }
+
+  // Test that HTTPS priority = 0 (AliasForm) behaves like a CNAME
+  await trrServer.registerDoHAnswers("test.com", "A", [
+    {
+      name: "test.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "something.com",
+        values: [],
+      },
+    },
+  ]);
+  await trrServer.registerDoHAnswers("something.com", "A", [
+    {
+      name: "something.com",
+      ttl: 55,
+      type: "A",
+      flush: false,
+      data: "1.2.3.4",
+    },
+  ]);
+
+  await new TRRDNSListener("test.com", "1.2.3.4");
+
+  // Test a chain of HTTPSSVC AliasForm and CNAMEs
+  await trrServer.registerDoHAnswers("x.com", "A", [
+    {
+      name: "x.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "y.com",
+        values: [],
+      },
+    },
+  ]);
+  await trrServer.registerDoHAnswers("y.com", "A", [
+    {
+      name: "y.com",
+      type: "CNAME",
+      ttl: 55,
+      class: "IN",
+      flush: false,
+      data: "z.com",
+    },
+  ]);
+  await trrServer.registerDoHAnswers("z.com", "A", [
+    {
+      name: "z.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "target.com",
+        values: [],
+      },
+    },
+  ]);
+  await trrServer.registerDoHAnswers("target.com", "A", [
+    {
+      name: "target.com",
+      ttl: 55,
+      type: "A",
+      flush: false,
+      data: "4.3.2.1",
+    },
+  ]);
+
+  await new TRRDNSListener("x.com", "4.3.2.1");
+
+  // We get a ServiceForm instead of a A answer, CNAME or AliasForm
+  await trrServer.registerDoHAnswers("no-ip-host.com", "A", [
+    {
+      name: "no-ip-host.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 1,
+        name: "h3pool",
+        values: [
+          { key: "alpn", value: "h2,h3" },
+          { key: "no-default-alpn" },
+          { key: "port", value: 8888 },
+          { key: "ipv4hint", value: "1.2.3.4" },
+          { key: "esniconfig", value: "123..." },
+          { key: "ipv6hint", value: "::1" },
+        ],
+      },
+    },
+  ]);
+
+  let [, , inStatus] = await new TRRDNSListener(
+    "no-ip-host.com",
+    undefined,
+    false
+  );
+  Assert.ok(
+    !Components.isSuccessCode(inStatus),
+    `${inStatus} should be an error code`
+  );
+
+  // Test CNAME/AliasForm loop
+  await trrServer.registerDoHAnswers("loop.com", "A", [
+    {
+      name: "loop.com",
+      type: "CNAME",
+      ttl: 55,
+      class: "IN",
+      flush: false,
+      data: "loop2.com",
+    },
+  ]);
+  await trrServer.registerDoHAnswers("loop2.com", "A", [
+    {
+      name: "loop2.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "loop.com",
+        values: [],
+      },
+    },
+  ]);
+
+  [, , inStatus] = await new TRRDNSListener("loop.com", undefined, false);
+  Assert.ok(
+    !Components.isSuccessCode(inStatus),
+    `${inStatus} should be an error code`
+  );
+
+  // Alias form for .
+  await trrServer.registerDoHAnswers("empty.com", "A", [
+    {
+      name: "empty.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "", // This is not allowed
+        values: [],
+      },
+    },
+  ]);
+
+  [, , inStatus] = await new TRRDNSListener("empty.com", undefined, false);
+  Assert.ok(
+    !Components.isSuccessCode(inStatus),
+    `${inStatus} should be an error code`
+  );
+
+  // We should ignore ServiceForm if an AliasForm record is also present
+  await trrServer.registerDoHAnswers("multi.com", "HTTPSSVC", [
+    {
+      name: "multi.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 1,
+        name: "h3pool",
+        values: [
+          { key: "alpn", value: "h2,h3" },
+          { key: "no-default-alpn" },
+          { key: "port", value: 8888 },
+          { key: "ipv4hint", value: "1.2.3.4" },
+          { key: "esniconfig", value: "123..." },
+          { key: "ipv6hint", value: "::1" },
+        ],
+      },
+    },
+    {
+      name: "multi.com",
+      ttl: 55,
+      type: "HTTPSSVC",
+      flush: false,
+      data: {
+        priority: 0,
+        name: "example.com",
+        values: [],
+      },
+    },
+  ]);
+
+  let listener = new DNSListener();
+  let request = dns.asyncResolveByType(
+    "multi.com",
+    dns.RESOLVE_TYPE_HTTPSSVC,
+    0,
+    listener,
+    mainThread,
+    defaultOriginAttributes
+  );
+
+  let [inRequest, inRecord, inStatus2] = await listener;
+  Assert.equal(inRequest, request, "correct request was used");
+  Assert.ok(
+    !Components.isSuccessCode(inStatus2),
+    `${inStatus2} should be an error code`
+  );
+});
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -426,11 +426,12 @@ skip-if = true || asan || tsan || os == 
 [test_node_execute.js]
 [test_loadgroup_cancel.js]
 [test_obs-fold.js]
 [test_defaultURI.js]
 [test_port_remapping.js]
 [test_dns_override.js]
 [test_no_cookies_after_last_pb_exit.js]
 [test_trr_httpssvc.js]
+skip-if = os == "android"
 [test_trr_case_sensitivity.js]
 skip-if = os == "android"
 [test_trr_proxy.js]
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit_ipc/head_trr_clone.js
@@ -0,0 +1,8 @@
+/* import-globals-from ../unit/head_trr.js */
+
+var { NetUtil } = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
+var { XPCOMUtils } = ChromeUtils.import(
+  "resource://gre/modules/XPCOMUtils.jsm"
+);
+
+load("../unit/head_trr.js");
--- a/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js
+++ b/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js
@@ -62,10 +62,20 @@ registerCleanupFunction(() => {
   prefs.clearUserPref("network.trr.clear-cache-on-pref-change");
 });
 
 function run_test() {
   prefs.setCharPref(
     "network.trr.uri",
     "https://foo.example.com:" + h2Port + "/httpssvc"
   );
+
+  do_await_remote_message("mode3-port").then(port => {
+    prefs.setIntPref("network.trr.mode", 3);
+    prefs.setCharPref(
+      "network.trr.uri",
+      `https://foo.example.com:${port}/dns-query`
+    );
+    do_send_remote_message("mode3-port-done");
+  });
+
   run_test_in_child("../unit/test_trr_httpssvc.js");
 }
--- a/netwerk/test/unit_ipc/xpcshell.ini
+++ b/netwerk/test/unit_ipc/xpcshell.ini
@@ -1,10 +1,10 @@
 [DEFAULT]
-head = head_channels_clone.js
+head = head_channels_clone.js  head_trr_clone.js
 skip-if = toolkit == 'android'
 support-files =
   child_channel_id.js
   !/netwerk/test/unit/test_XHR_redirects.js
   !/netwerk/test/unit/test_bug528292.js
   !/netwerk/test/unit/test_cache-entry-id.js
   !/netwerk/test/unit/test_cache_jar.js
   !/netwerk/test/unit/test_cacheflags.js
@@ -33,16 +33,17 @@ support-files =
   !/netwerk/test/unit/test_reentrancy.js
   !/netwerk/test/unit/test_reply_without_content_type.js
   !/netwerk/test/unit/test_resumable_channel.js
   !/netwerk/test/unit/test_simple.js
   !/netwerk/test/unit/test_synthesized_response.js
   !/netwerk/test/unit/test_trackingProtection_annotateChannels.js
   !/netwerk/test/unit/test_xmlhttprequest.js
   !/netwerk/test/unit/head_channels.js
+  !/netwerk/test/unit/head_trr.js
   !/netwerk/test/unit/head_cache2.js
   !/netwerk/test/unit/data/image.png
   !/netwerk/test/unit/data/system_root.lnk
   !/netwerk/test/unit/data/test_psl.txt
   !/netwerk/test/unit/data/test_readline1.txt
   !/netwerk/test/unit/data/test_readline2.txt
   !/netwerk/test/unit/data/test_readline3.txt
   !/netwerk/test/unit/data/test_readline4.txt
@@ -106,8 +107,9 @@ skip-if = true
 [test_trackingProtection_annotateChannels_wrap1.js]
 [test_trackingProtection_annotateChannels_wrap2.js]
 [test_channel_priority_wrap.js]
 [test_multipart_streamconv_wrap.js]
 [test_alt-data_cross_process_wrap.js]
 [test_httpcancel_wrap.js]
 [test_esni_dns_fetch_wrap.js]
 [test_trr_httpssvc_wrap.js]
+skip-if = os == "android"