zonemaster-engine icon indicating copy to clipboard operation
zonemaster-engine copied to clipboard
verified publisher icon zonemaster

Update DNSSEC01 implementation

Open tgreenx opened this issue 2 months ago • 11 comments

Purpose

This PR proposes an update of test case DNSSEC01 implementation.

Context

Test case specification: https://github.com/zonemaster/zonemaster/pull/1412 Test scenarios specification: https://github.com/zonemaster/zonemaster/pull/1413

Changes

  • Update implementation (test case, message tags, profile, DS digest algorithms table)
  • Update unit tests
  • Update unit test data

How to test this PR

Unit tests are created and should pass.

tgreenx avatar Oct 28 '25 16:10 tgreenx

I get strange output from zonemaster-cli after installing this update:

$ zonemaster-cli --hints=hintfile.zone --test=dnssec01 --level=info --show-testcase --raw ALGO-DEPRECATED-1.dnssec01.xa
   0.00 INFO     Unspecified    GLOBAL_VERSION  version=v8.0.0
127.15.1.27
fda1:b2:c3:0:127:15:1:27
127.15.1.28
fda1:b2:c3:0:127:15:1:28
127.15.1.21
fda1:b2:c3:0:127:15:1:21
fda1:b2:c3:0:127:15:1:22
127.15.1.22
   0.05 ERROR    DNSSEC01       DS01_DS_ALGO_DEPRECATED  ds_algo_descr=SHA-1; ds_algo_num=1; keytag=42581; ns_list=ns1.dnssec01.xa/127.15.1.21;ns1.dnssec01.xa/fda1:b2:c3:0:127:15:1:21;ns1.dnssec01.xa/fda1:b2:c3:0:127:15:1:22;ns2.dnssec01.xa/127.15.1.22
   0.05 NOTICE   DNSSEC01       DS01_DS_ALGO_2_MISSING  keytag=42581; ns_list=ns1.dnssec01.xa/127.15.1.21;ns1.dnssec01.xa/fda1:b2:c3:0:127:15:1:21;ns1.dnssec01.xa/fda1:b2:c3:0:127:15:1:22;ns2.dnssec01.xa/127.15.1.22

Some debug output?

matsduf avatar Oct 29 '25 15:10 matsduf

@matsduf All unit tests now pass and your comments have been addressed, please re-review.

tgreenx avatar Oct 30 '25 10:10 tgreenx

Scenario name Mandatory tags Forbidden tags
SHARED-IP-1 DS01_DS_ALGO_OK 2) 

$ zonemaster-cli --hints=hintfile.zone --test=dnssec01 --level=info --show-testcase --raw child.shared-ip-1.dnssec01.xa                                                   
   0.00 INFO     Unspecified    GLOBAL_VERSION  version=v8.0.0                                                                                                            
   0.06 INFO     DNSSEC01       DS01_DS_ALGO_OK  ds_algo_descr=SHA-256; ds_algo_num=2; keytag=42581; ns_list=ns1a.shared-ip-1.dnssec01.xa/127.15.1.31;ns1a.shared-ip-1.dn\
ssec01.xa/fda1:b2:c3:0:127:15:1:31

--> Not OK, expects ns_list=ns1a.shared-ip-1.dnssec01.xa/127.15.1.31;ns1a.shared-ip-1.dnssec01.xa/fda1:b2:c3:0:127:15:1:31;ns_list=ns1b.shared-ip-1.dnssec01.xa/127.15.1.31;ns1b.shared-ip-1.dnssec01.xa/fda1:b2:c3:0:127:15:1:31

matsduf avatar Oct 30 '25 14:10 matsduf

Scenario name Mandatory tags Forbidden tags
SHARED-IP-2 DS01_DS_ALGO_OK 2) 

$ zonemaster-cli --hints=hintfile.zone --test=dnssec01 --level=info --show-testcase --raw child.shared-ip-2.dnssec01.xa                                                   
   0.00 INFO     Unspecified    GLOBAL_VERSION  version=v8.0.0                                                                                                            
   0.07 INFO     DNSSEC01       DS01_DS_ALGO_OK  ds_algo_descr=SHA-256; ds_algo_num=2; keytag=42581; ns_list=ns1.shared-ip-2.dnssec01.xa/127.15.1.31;ns1.shared-ip-2.dnss\
ec01.xa/fda1:b2:c3:0:127:15:1:31;ns2.shared-ip-2.dnssec01.xa/127.15.1.32;ns2.shared-ip-2.dnssec01.xa/fda1:b2:c3:0:127:15:1:32

--> Not OK, expects ns_list=ns1.shared-ip-2.dnssec01.xa/127.15.1.31;ns1.shared-ip-2.dnssec01.xa/fda1:b2:c3:0:127:15:1:31;ns2.shared-ip-2.dnssec01.xa/127.15.1.32;ns2.shared-ip-2.dnssec01.xa/fda1:b2:c3:0:127:15:1:32;ns_list=dns1.shared-ip-2.dnssec01.xa/127.15.1.31;dns1.shared-ip-2.dnssec01.xa/fda1:b2:c3:0:127:15:1:31;dns2.shared-ip-2.dnssec01.xa/127.15.1.32;dns2.shared-ip-2.dnssec01.xa/fda1:b2:c3:0:127:15:1:32

matsduf avatar Oct 30 '25 14:10 matsduf

All scenarios give correct tags in output, but two scenarios give incomplete argument.

This will be fixed once #1475 is merged and this PR is rebased/updated on top.

$ git log -3 --oneline
af57c7fe (HEAD -> update-dnssec01-methodsv2) Update DNSSEC01 implementation
d6e9c1a7 (test-PR1475) Rerecord test data after updating MethodV2
810cd8d3 New MethodV2: Get-Parent-NS-Names-and-IPs

$ git diff
diff --git a/lib/Zonemaster/Engine/Test/DNSSEC.pm b/lib/Zonemaster/Engine/Test/DNSSEC.pm
index 77f34e2e..35bad8d0 100644
--- a/lib/Zonemaster/Engine/Test/DNSSEC.pm
+++ b/lib/Zonemaster/Engine/Test/DNSSEC.pm
@@ -1784,7 +1784,7 @@ sub dnssec01 {
         'DS01_DS_ALGO_OK' => {}
     );

-    my $parent_nss = Zonemaster::Engine::TestMethodsV2->get_parent_ns_ips( $zone );
+    my $parent_nss = Zonemaster::Engine::TestMethodsV2->get_parent_ns_names_and_ips( $zone );

     my @undelegated_ds;
     if ( my $parent = $zone->parent ) {

$ zonemaster-cli --hints=../zonemaster/test-zone-data/DNSSEC-TP/dnssec01/hintfile.zone --test=dnssec01 --level=info --show-testcase --raw child.shared-ip-1.dnssec01.xa
   0.00 INFO     Unspecified    GLOBAL_VERSION  version=v8.0.0
   0.05 INFO     DNSSEC01       DS01_DS_ALGO_OK  ds_algo_descr=SHA-256; ds_algo_num=2; keytag=42581; ns_list=ns1a.shared-ip-1.dnssec01.xa/127.15.1.31;ns1a.shared-ip-1.dnssec01.xa/fda1:b2:c3:0:127:15:1:31;ns1b.shared-ip-1.dnssec01.xa/127.15.1.31;ns1b.shared-ip-1.dnssec01.xa/fda1:b2:c3:0:127:15:1:31

tgreenx avatar Oct 30 '25 16:10 tgreenx

@matsduf I've merged #1475 and this PR has been rebased on top, please re-review.

tgreenx avatar Nov 04 '25 09:11 tgreenx

Unit test data for t/Test-dnssec.t needs to be re-recorded in order for all unit tests to pass, but that can't be done right now (zut-root.rd.nic.fr is temporarily offline). It will be done at a later time.

tgreenx avatar Nov 04 '25 10:11 tgreenx

Unit test data for t/Test-dnssec.t needs to be re-recorded in order for all unit tests to pass, but that can't be done right now (zut-root.rd.nic.fr is temporarily offline). It will be done at a later time.

I suggest that the failing tests are marked as TODO. What test cases are affected? I could possibly create scenarios for them.

matsduf avatar Nov 04 '25 15:11 matsduf

Scenarios SHARED-IP-1 and SHARED-IP-2 now output correct argument values. See https://github.com/zonemaster/zonemaster-engine/pull/1474#issuecomment-3468392217 and https://github.com/zonemaster/zonemaster-engine/pull/1474#issuecomment-3468440032

matsduf avatar Nov 04 '25 16:11 matsduf

Unit test data for t/Test-dnssec.t needs to be re-recorded in order for all unit tests to pass, but that can't be done right now (zut-root.rd.nic.fr is temporarily offline). It will be done at a later time.

I suggest that the failing tests are marked as TODO. What test cases are affected? I could possibly create scenarios for them.

Many. But it should be fixed before end of development.

tgreenx avatar Nov 05 '25 13:11 tgreenx

Unit test data for t/Test-dnssec.t needs to be re-recorded in order for all unit tests to pass, but that can't be done right now (zut-root.rd.nic.fr is temporarily offline). It will be done at a later time.

I ran into the same problem when preparing #1475. I was able to fix t/Test-dnssec.t by re-recording the .data file while keeping the old one around, then merging some of the old data into the new data. I don’t remember exactly how I managed to fix the .t file, unfortunately.

Have you tried rebasing your branch on the latest state of develop and running the tests again? With a little bit of luck, they might pass.

marc-vanderwal avatar Nov 05 '25 15:11 marc-vanderwal

@matsduf @marc-vanderwal please re-review, unit tests have been re-recorded (the test zones are back online).

tgreenx avatar Nov 13 '25 13:11 tgreenx

@matsduf I've fixed some conflicts after merging other PRs, please re-review

tgreenx avatar Nov 14 '25 09:11 tgreenx