From ba1749f6a5a2793e558485a8c6a871daba7bf533 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 8 Jan 2024 02:02:33 +0900 Subject: [PATCH 1/4] resolve: NSCOUNT of DNS query may not be zero This also separates check for DNS and LLMNR, as the existing comments are for LLMNR, not DNS. And this moves the comment for mDNS. Fixes the issue reported at https://github.com/systemd/systemd/pull/30809#issuecomment-1880102804. --- src/resolve/resolved-dns-packet.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index b0b5bab6bd..a031ffecca 100644 --- 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. */ From 45b2bf0efc75c66fac05771ebc50649931457197 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 7 Jan 2024 13:29:50 +0100 Subject: [PATCH 2/4] test: zone-check with --force to fail on warnings --- test/units/testsuite-75.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/units/testsuite-75.sh b/test/units/testsuite-75.sh index 54234484c4..f3c27db7b4 100755 --- 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. From b4f17b07cf0f3a30e355a021f2ba0bfe572423c0 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 7 Jan 2024 13:32:14 +0100 Subject: [PATCH 3/4] test: merge config sections --- test/knot-data/knot.conf | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/test/knot-data/knot.conf b/test/knot-data/knot.conf index 6ea0cca3db..a1fc64f7a5 100644 --- a/test/knot-data/knot.conf +++ b/test/knot-data/knot.conf @@ -29,9 +29,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 +43,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 +57,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 +74,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" From 5bd11228436978be24582f073daf6bf7a5bae8d2 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 7 Jan 2024 22:22:52 +0100 Subject: [PATCH 4/4] test: check how systemd-resolved deals with zone transfers Even though systemd-resolved doesn't support zone transfers (AXFR/IXFR), it should still just refuse such requests without choking on them. See: https://github.com/systemd/systemd/pull/30809#issuecomment-1880102804 --- test/knot-data/knot.conf | 8 +++++++- test/units/testsuite-75.sh | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/test/knot-data/knot.conf b/test/knot-data/knot.conf index a1fc64f7a5..245fa75cf7 100644 --- 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 @@ -94,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 index f3c27db7b4..a4e2e0547b 100755 --- a/test/units/testsuite-75.sh +++ b/test/units/testsuite-75.sh @@ -413,6 +413,18 @@ grep -qF "; fully validated" "$RUN_OUT" run resolvectl openpgp mr.smith@signed.test 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