Bug 1641222 - Follow CNAME/AliasForm chains r=dragana,necko-reviewers
☠☠ backed out by a2bb355e129f ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 16 Jun 2020 10:12:00 +0000
changeset 535843 4e775c2385b2df56b00eddb2f32474818abc54e6
parent 535842 48b5a8b518d94b14abfd7851f6c23c9caec6f463
child 535844 22a19cf66ef3608bab8f807f64a9a7312a6c70dc
push id119103
push uservalentin.gosu@gmail.com
push dateTue, 16 Jun 2020 10:44:04 +0000
treeherderautoland@4e775c2385b2 [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/test_trr_httpssvc.js
--- a/netwerk/dns/TRR.cpp
+++ b/netwerk/dns/TRR.cpp
@@ -830,17 +830,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
@@ -1022,16 +1023,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;
@@ -1278,17 +1297,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/test_trr_httpssvc.js
+++ b/netwerk/test/unit/test_trr_httpssvc.js
@@ -200,8 +200,228 @@ 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`);
+  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`
+  );
+});