Merge pull request systemd#30815 from yuwata/resolve-ixfr

resolve: NSCOUNT of DNS query may not be zero
mrc0mmand · Jan 7, 2024 · 96f1f03 · 96f1f03
2 parents 307b6a4 + 5bd1122
commit 96f1f03
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 17 deletions.
diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
@@ -310,9 +310,23 @@ int dns_packet_validate_query(DnsPacket *p) {
 
         switch (p->protocol) {
 
-        case DNS_PROTOCOL_LLMNR:
         case DNS_PROTOCOL_DNS:
-                if (DNS_PACKET_TC(p)) /* mDNS query may have truncation flag. */
+                if (DNS_PACKET_TC(p))
+                        return -EBADMSG;
+
+                if (DNS_PACKET_QDCOUNT(p) != 1)
+                        return -EBADMSG;
+
+                if (DNS_PACKET_ANCOUNT(p) > 0)
+                        return -EBADMSG;
+
+                /* Note, in most cases, DNS query packet does not have authority section. But some query
+                 * types, e.g. IXFR, have Authority sections. Hence, unlike the check for LLMNR, we do not
+                 * check DNS_PACKET_NSCOUNT(p) here. */
+                break;
+
+        case DNS_PROTOCOL_LLMNR:
+                if (DNS_PACKET_TC(p))
                         return -EBADMSG;
 
                 /* RFC 4795, Section 2.1.1. says to discard all queries with QDCOUNT != 1 */
@@ -330,6 +344,9 @@ int dns_packet_validate_query(DnsPacket *p) {
                 break;
 
         case DNS_PROTOCOL_MDNS:
+                /* Note, mDNS query may have truncation flag. So, unlike the check for DNS and LLMNR,
+                 * we do not check DNS_PACKET_TC(p) here. */
+
                 /* RFC 6762, Section 18 specifies that messages with non-zero RCODE
                  * must be silently ignored, and that we must ignore the values of
                  * AA, RD, RA, AD, and CD bits. */

diff --git a/test/knot-data/knot.conf b/test/knot-data/knot.conf
@@ -19,6 +19,11 @@ acl:
       address: fd00:dead:beef:cafe::/64
       action: update
 
+    - id: transfer_acl
+      address: 10.0.0.0/24
+      address: fd00:dead:beef:cafe::/64
+      action: transfer
+
 remote:
     - id: parent_zone_server
       address: 10.0.0.1@53
@@ -29,9 +34,9 @@ submission:
       check-interval: 2s
       parent: [parent_zone_server]
 
-# Auto ZSK/KSK rollover for DNSSEC-enabled zones + pushing the respective DS
-# records to the parent zone
 policy:
+    # Auto ZSK/KSK rollover for DNSSEC-enabled zones + pushing the respective DS
+    # records to the parent zone
     - id: auto_rollover
       algorithm: ECDSAP256SHA256
       cds-cdnskey-publish: always
@@ -43,8 +48,7 @@ policy:
       zone-max-ttl: 1s
       zsk-lifetime: 60d
 
-# Same as auto_rollover, but with NSEC3 turned on
-policy:
+    # Same as auto_rollover, but with NSEC3 turned on
     - id: auto_rollover_nsec3
       algorithm: ECDSAP256SHA256
       cds-cdnskey-publish: always
@@ -58,17 +62,15 @@ policy:
       zone-max-ttl: 1s
       zsk-lifetime: 60d
 
-policy:
     - id: untrusted
       cds-cdnskey-publish: none
 
-# Manual ZSK/KSK management
-policy:
+    # Manual ZSK/KSK management
     - id: manual
       manual: on
 
-# Sign everything by default and propagate the respective DS records to the parent
 template:
+    # Sign everything by default and propagate the respective DS records to the parent
     - id: default
       acl: update_acl
       dnssec-policy: auto_rollover
@@ -77,8 +79,7 @@ template:
       semantic-checks: on
       storage: "/var/lib/knot/zones"
 
-# A template for unsigned zones (i.e. without DNSSEC)
-template:
+    # A template for unsigned zones (i.e. without DNSSEC)
     - id: unsigned
       dnssec-signing: off
       file: "%s.zone"
@@ -98,8 +99,9 @@ zone:
     - domain: test
       dnssec-policy: auto_rollover_nsec3
 
-    # A fully (pre-)signed zone
+    # A fully (pre-)signed zone with allowed zone transfers (AXFR/IXFR)
     - domain: signed.test
+      acl: [update_acl, transfer_acl]
 
     # A fully (online)-signed zone
     # See: https://www.knot-dns.cz/docs/3.1/singlehtml/index.html#mod-onlinesign

diff --git a/test/units/testsuite-75.sh b/test/units/testsuite-75.sh
@@ -254,10 +254,7 @@ resolvectl log-level debug
 systemd-run -u resolvectl-monitor.service -p Type=notify resolvectl monitor
 systemd-run -u resolvectl-monitor-json.service -p Type=notify resolvectl monitor --json=short
 
-# Check if all the zones are valid (zone-check always returns 0, so let's check
-# if it produces any errors/warnings)
-run knotc zone-check
-[[ ! -s "$RUN_OUT" ]]
+knotc --force zone-check
 # We need to manually propagate the DS records of onlinesign.test. to the parent
 # zone, since they're generated online
 knotc zone-begin test.
@@ -416,6 +413,18 @@ grep -qF "; fully validated" "$RUN_OUT"
 run resolvectl openpgp [email protected]
 grep -qF "5a786cdc59c161cdafd818143705026636962198c66ed4c5b3da321e._openpgpkey.signed.test" "$RUN_OUT"
 grep -qF "authenticated: yes" "$RUN_OUT"
+# Check zone transfers (AXFR/IXFR)
+# Note: since resolved doesn't support zone transfers, let's just make sure it
+#       simply refuses such requests without choking on them
+# See: https://github.com/systemd/systemd/pull/30809#issuecomment-1880102804
+run dig @ns1.unsigned.test AXFR signed.test
+grep -qE "SOA\s+ns1.unsigned.test. root.unsigned.test." "$RUN_OUT"
+run dig AXFR signed.test
+grep -qF "; Transfer failed" "$RUN_OUT"
+run dig @ns1.unsigned.test IXFR=43 signed.test
+grep -qE "SOA\s+ns1.unsigned.test. root.unsigned.test." "$RUN_OUT"
+run dig IXFR=43 signed.test
+grep -qF "; Transfer failed" "$RUN_OUT"
 
 # DNSSEC validation with multiple records of the same type for the same name
 # Issue: https://github.com/systemd/systemd/issues/22002