EDNS0 parsing, multi RR test

- Issue #291: - `dns_protocol`: fix EDNS0 parsing, check that it's root domain and haven't found EDNS before - Add test with multiple resource records, EDNS0 and nsid - Fix test 285
DNS-OARC · Jun 29, 2023 · cee2bf7 · cee2bf7
1 parent b0e0df7
commit cee2bf7
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 20 deletions.
diff --git a/src/dns_protocol.c b/src/dns_protocol.c
@@ -210,23 +210,27 @@ static off_t skip_question(const u_char* buf, int len, off_t offset)
 
 static off_t grok_additional_for_opt_rr(const u_char* buf, int len, off_t offset, dns_message* m)
 {
-    int            x;
-    unsigned short sometype;
-    unsigned short someclass;
     unsigned short us;
-    x = rfc1035NameSkip(buf, len, &offset);
-    if (0 != x)
-        return 0;
-    if (offset + 10 > len)
-        return 0;
-    sometype  = nptohs(buf + offset);
-    someclass = nptohs(buf + offset + 2);
-    if (sometype == T_OPT) {
-        m->edns.found  = 1;
-        m->edns.bufsiz = someclass;
-        memcpy(&m->edns.version, buf + offset + 5, 1);
-        us         = nptohs(buf + offset + 6);
-        m->edns.DO = (us >> 15) & 0x01; /* RFC 3225 */
+    /*
+     * OPT RR for EDNS0 MUST be 0 (root domain), so if the first byte of
+     * the name is anything it can't be a valid EDNS0 record.
+     */
+    if (*(buf + offset)) {
+        if (rfc1035NameSkip(buf, len, &offset))
+            return 0;
+        if (offset + 10 > len)
+            return 0;
+    } else {
+        offset++;
+        if (offset + 10 > len)
+            return 0;
+        if (nptohs(buf + offset) == T_OPT && !m->edns.found) {
+            m->edns.found  = 1;
+            m->edns.bufsiz = nptohs(buf + offset + 2);
+            memcpy(&m->edns.version, buf + offset + 5, 1);
+            us         = nptohs(buf + offset + 6);
+            m->edns.DO = (us >> 15) & 0x01; /* RFC 3225 */
+        }
     }
     /* get rdlength */
     us = nptohs(buf + offset + 8);

diff --git a/src/test/Makefile.am b/src/test/Makefile.am
@@ -16,14 +16,15 @@ CLEANFILES = test*.log test*.trs \
   tld_list.dat \
   dotdoh.dnstap.dist 1643283234.dscdata.xml \
   test13.conf \
-  test_285.pcap.dist test_285.tldlist.dist 1683879752.xml
+  test_285.pcap.dist test_285.tldlist.dist 1683879752.dscdata.xml \
+  h-root-aaa.pcap-dist 1688028728.dscdata.xml
 
 EXTRA_DIST =
 
 TESTS = test1.sh test2.sh test3.sh test4.sh test6.sh test7.sh test8.sh \
   test9.sh test10.sh test11.sh test12.sh test_dnstap_unixsock.sh \
   test_dnstap_tcp.sh test_pslconv.sh test_encrypted.sh test13.sh \
-  test_285.sh
+  test_285.sh test_291.sh
 
 if USE_DNSTAP
 TESTS += test5.sh
@@ -89,6 +90,11 @@ test_285.tldlist.dist: test_285.tldlist
 
 test_285.sh: test_285.pcap.dist test_285.tldlist.dist
 
+h-root-aaa.pcap-dist: h-root-aaa.pcap
+	ln -s "$(srcdir)/h-root-aaa.pcap" h-root-aaa.pcap-dist
+
+test_291.sh: h-root-aaa.pcap-dist
+
 EXTRA_DIST += $(TESTS) \
   1458044657.conf 1458044657.pcap 1458044657.json_gold 1458044657.xml_gold \
   pid.conf pid.pcap \
@@ -109,4 +115,5 @@ EXTRA_DIST += $(TESTS) \
   1458044657.tld_list \
   public_suffix_list.dat tld_list.dat.gold \
   dnstap_encrypted.conf dnstap_encrypted.gold dotdoh.dnstap \
-  test_285.pcap test_285.conf test_285.tldlist test_285.xml_gold
+  test_285.pcap test_285.conf test_285.tldlist test_285.xml_gold \
+  h-root-aaa.pcap test_291.conf test_291.xml_gold
diff --git a/src/test/h-root-aaa.pcap b/src/test/h-root-aaa.pcap
diff --git a/src/test/test_285.sh b/src/test/test_285.sh
@@ -1,6 +1,6 @@
 #!/bin/sh -xe
 
-rm -f 1683879752.xml
+rm -f 1683879752.dscdata.xml
 
 ../dsc "$srcdir/test_285.conf"
 

diff --git a/src/test/test_291.conf b/src/test/test_291.conf
@@ -0,0 +1,6 @@
+run_dir ".";
+interface h-root-aaa.pcap-dist;
+dataset qtype dns All:null Qtype:qtype queries-only;
+dataset rcode dns All:null Rcode:rcode replies-only;
+dataset edns_version dns All:null EDNSVersion:edns_version queries-only;
+dataset edns_bufsiz dns All:null EDNSBufSiz:edns_bufsiz queries-only;
diff --git a/src/test/test_291.sh b/src/test/test_291.sh
@@ -0,0 +1,11 @@
+#!/bin/sh -xe
+
+rm -f 1688028728.dscdata.xml
+
+../dsc "$srcdir/test_291.conf"
+
+test -f 1688028728.dscdata.xml || sleep 1
+test -f 1688028728.dscdata.xml || sleep 2
+test -f 1688028728.dscdata.xml || sleep 3
+test -f 1688028728.dscdata.xml
+diff -u 1688028728.dscdata.xml "$srcdir/test_291.xml_gold"
diff --git a/src/test/test_291.xml_gold b/src/test/test_291.xml_gold
@@ -0,0 +1,51 @@
+<dscdata>
+<array name="pcap_stats" dimensions="2" start_time="1688028728" stop_time="1688028728">
+  <dimension number="1" type="ifname"/>
+  <dimension number="2" type="pcap_stat"/>
+  <data>
+    <ifname val="h-root-aaa.pcap-dist">
+      <pcap_stat val="pkts_captured" count="6"/>
+    </ifname>
+  </data>
+</array>
+<array name="edns_bufsiz" dimensions="2" start_time="1688028728" stop_time="1688028728">
+  <dimension number="1" type="All"/>
+  <dimension number="2" type="EDNSBufSiz"/>
+  <data>
+    <All val="ALL">
+      <EDNSBufSiz val="None" count="2"/>
+      <EDNSBufSiz val="4096-4607" count="1"/>
+    </All>
+  </data>
+</array>
+<array name="edns_version" dimensions="2" start_time="1688028728" stop_time="1688028728">
+  <dimension number="1" type="All"/>
+  <dimension number="2" type="EDNSVersion"/>
+  <data>
+    <All val="ALL">
+      <EDNSVersion val="none" count="2"/>
+      <EDNSVersion val="0" count="1"/>
+    </All>
+  </data>
+</array>
+<array name="rcode" dimensions="2" start_time="1688028728" stop_time="1688028728">
+  <dimension number="1" type="All"/>
+  <dimension number="2" type="Rcode"/>
+  <data>
+    <All val="ALL">
+      <Rcode val="0" count="3"/>
+    </All>
+  </data>
+</array>
+<array name="qtype" dimensions="2" start_time="1688028728" stop_time="1688028728">
+  <dimension number="1" type="All"/>
+  <dimension number="2" type="Qtype"/>
+  <data>
+    <All val="ALL">
+      <Qtype val="1" count="1"/>
+      <Qtype val="28" count="1"/>
+      <Qtype val="2" count="1"/>
+    </All>
+  </data>
+</array>
+</dscdata>