Bug 1641222 - Follow CNAME/AliasForm chains r=dragana,necko-reviewers
☠☠ backed out by 12a7de90bb95 ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 22 Jun 2020 13:24:19 +0000
changeset 600835 ce1a126dcf105e348ab891cf285a7ca73b295dcb
parent 600834 69e5de33bd0797d51b019982741eb927920dd471
child 600836 d07d66ecd6b9ca685444912826f25e9751ef13b5
push id13310
push userffxbld-merge
push dateMon, 29 Jun 2020 14:50:06 +0000
treeherdermozilla-beta@15a59a0afa5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, necko-reviewers
bugs1641222
milestone79.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_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`
+  );
+});
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