Skip to content

Conversation

@tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Oct 28, 2025

Purpose

This PR proposes an update of test case DNSSEC01 implementation.

Context

Test case specification: zonemaster/zonemaster#1412
Test scenarios specification: zonemaster/zonemaster#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 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-Features Release category: Features. labels Oct 28, 2025
@tgreenx tgreenx added this to the v2025.2 milestone Oct 28, 2025
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

"/usr/local/bin/perl" "-Iinc" "-MExtUtils::Manifest=fullcheck" -e fullcheck
Not in MANIFEST: t/Test-dnssec01.data
Not in MANIFEST: t/Test-dnssec01.t

@matsduf
Copy link
Contributor

matsduf commented Oct 29, 2025

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?

@tgreenx tgreenx marked this pull request as ready for review October 30, 2025 10:18
@tgreenx
Copy link
Contributor Author

tgreenx commented Oct 30, 2025

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

@tgreenx tgreenx requested a review from matsduf October 30, 2025 10:18
@matsduf
Copy link
Contributor

matsduf commented Oct 30, 2025

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
Copy link
Contributor

matsduf commented Oct 30, 2025

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

Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

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

@tgreenx
Copy link
Contributor Author

tgreenx commented Oct 30, 2025

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 tgreenx requested a review from matsduf November 4, 2025 09:30
@tgreenx
Copy link
Contributor Author

tgreenx commented Nov 4, 2025

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

@tgreenx
Copy link
Contributor Author

tgreenx commented Nov 4, 2025

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.

@matsduf
Copy link
Contributor

matsduf commented Nov 4, 2025

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
Copy link
Contributor

matsduf commented Nov 4, 2025

Scenarios SHARED-IP-1 and SHARED-IP-2 now output correct argument values. See #1474 (comment) and #1474 (comment)

@tgreenx
Copy link
Contributor Author

tgreenx commented Nov 5, 2025

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.

@marc-vanderwal
Copy link
Contributor

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.

@tgreenx
Copy link
Contributor Author

tgreenx commented Nov 13, 2025

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

matsduf
matsduf previously approved these changes Nov 13, 2025
@tgreenx
Copy link
Contributor Author

tgreenx commented Nov 14, 2025

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

@tgreenx tgreenx merged commit 138c878 into zonemaster:develop Nov 14, 2025
3 checks passed
@tgreenx tgreenx deleted the update-dnssec01 branch November 14, 2025 10:04
@MichaelTimbert MichaelTimbert added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Dec 8, 2025
@MichaelTimbert
Copy link
Contributor

Tested and all unit tests pass.

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-Features Release category: Features. S-ReleaseTested Status: The PR has been successfully tested in release testing 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.

4 participants