Skip to content

Comments

Update Nameserver13#1505

Open
tgreenx wants to merge 1 commit intozonemaster:release/v2025.2.1from
tgreenx:fix-nameserver13
Open

Update Nameserver13#1505
tgreenx wants to merge 1 commit intozonemaster:release/v2025.2.1from
tgreenx:fix-nameserver13

Conversation

@tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Feb 24, 2026

Purpose

This PR proposes to fix discrepancies in the implementation of Nameserver13 with respect to its specification.

Context

Fixes #1503

Changes

How to test this PR

This test case (among others in this test module) currently have no unit test. See
Manual testing:

$ zonemaster-cli --show-testcase --level DEBUG --test nameserver13 --no-ipv6 afnic.fr | grep DNSKEY
   1.63 DEBUG    Nameserver13   System:Nameserver13:EXTERNAL_QUERY flags={"class":"IN","edns_details":{"do":1,"size":512,"version":0},"fallback":0,"usevc":0}; ip=194.0.36.1; name=afnic.fr; type=DNSKEY
   1.64 DEBUG    Nameserver13   IPv6 is disabled, not sending "DNSKEY" query to g.ext.nic.fr/2001:678:4c::1.
   1.64 DEBUG    Nameserver13   System:Nameserver13:EXTERNAL_QUERY flags={"class":"IN","edns_details":{"do":1,"size":512,"version":0},"fallback":0,"usevc":0}; ip=192.134.4.1; name=afnic.fr; type=DNSKEY
   1.65 DEBUG    Nameserver13   IPv6 is disabled, not sending "DNSKEY" query to ns1.nic.fr/2001:67c:2218:2::4:1.
   1.65 DEBUG    Nameserver13   System:Nameserver13:EXTERNAL_QUERY flags={"class":"IN","edns_details":{"do":1,"size":512,"version":0},"fallback":0,"usevc":0}; ip=192.93.0.4; name=afnic.fr; type=DNSKEY
   1.65 DEBUG    Nameserver13   IPv6 is disabled, not sending "DNSKEY" query to ns2.nic.fr/2001:660:3005:1::1:2.
   1.66 DEBUG    Nameserver13   System:Nameserver13:EXTERNAL_QUERY flags={"class":"IN","edns_details":{"do":1,"size":512,"version":0},"fallback":0,"usevc":0}; ip=192.134.0.49; name=afnic.fr; type=DNSKEY
   1.67 DEBUG    Nameserver13   IPv6 is disabled, not sending "DNSKEY" query to ns3.nic.fr/2001:660:3006:1::1:1.

- Change query type to DNSKEY
- Remove uneeded conditionals
- Add a mention for missing unit tests
@tgreenx tgreenx added this to the v2025.2.1 milestone Feb 24, 2026
@tgreenx tgreenx added A-TestCase Area: Test case specification or implementation of test case V-Patch Versioning: The change gives an update of patch in version. RC-Fixes Release category: Fixes. labels Feb 24, 2026

if ( $p ) {
if ( $p->rcode eq q{FORMERR} and not $p->edns_rcode ) {
if ( $p->rcode eq q{FORMERR} ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ( $p->rcode eq q{FORMERR} ) {
if ( $p->rcode eq q{FORMERR} and not $p->has_edns ) {

A non-EDNS FORMERR response means the server does not support EDNS, in which case it is appropriate to generate a NO_EDNS_SUPPORT message. But an EDNS-enabled FORMERR response means something else: that the server understood that the query is an EDNS query, but it is malformed. In that case we should generate NS_ERROR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe so, but that is not what the current specification is saying and this PR was about correcting discrepancies. I'm fine with reverting this change though until a proper specification update is done.
@matsduf thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-TestCase Area: Test case specification or implementation of test case RC-Fixes Release category: Fixes. V-Patch Versioning: The change gives an update of patch in version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nameserver13 does not follow the test specification and queries for SOA instead of DNSKEY

2 participants