From 829edd4ca7c4977ab6f0fee8418bf2c5f649ee57 Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Tue, 1 Oct 2024 18:12:23 +0200 Subject: [PATCH 1/2] Fix handling of CDS/CDNSKEY RRSet comparison in DNSSEC15 --- lib/Zonemaster/Engine/Test/DNSSEC.pm | 69 ++++++++++++++-------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/lib/Zonemaster/Engine/Test/DNSSEC.pm b/lib/Zonemaster/Engine/Test/DNSSEC.pm index 5373644f5..9bd56777e 100644 --- a/lib/Zonemaster/Engine/Test/DNSSEC.pm +++ b/lib/Zonemaster/Engine/Test/DNSSEC.pm @@ -3845,6 +3845,7 @@ sub dnssec15 { local $Zonemaster::Engine::Logger::TEST_CASE_NAME = 'DNSSEC15'; push my @results, _emit_log( TEST_CASE_START => { testcase => $Zonemaster::Engine::Logger::TEST_CASE_NAME } ); + my @query_types = qw{CDS CDNSKEY}; my %cds_rrsets; my %cdnskey_rrsets; @@ -3949,27 +3950,24 @@ sub dnssec15 { ) { # - # Need a fix in Zonemaster::LDNS to prevent that trick + # Quick hack. Proper fix should be available in LDNS 1.8.5: https://github.com/NLnetLabs/ldns/commit/b39813870a5fb0f4e8ff1570b3b09416aaee716c # - my (@ds, @dnskey); - foreach my $cds ( @{ $cds_rrsets{ $ns_ip } } ) { - my $rr_string = $cds->string; - $rr_string =~ s/\s+CDS\s+/ DS /; - push @ds, Zonemaster::LDNS::RR->new( $rr_string ); - } + my @dnskey; foreach my $cdnskey ( @{ $cdnskey_rrsets{ $ns_ip } } ) { my $rr_string = $cdnskey->string; $rr_string =~ s/\s+CDNSKEY\s+/ DNSKEY /; push @dnskey, Zonemaster::LDNS::RR->new( $rr_string ); } - foreach my $ds ( @ds ) { - my @matching_keys = grep { $ds->keytag == $_->keytag or ($ds->algorithm == 0 and $_->algorithm == 0)} @dnskey; + + foreach my $cds ( @{ $cds_rrsets{ $ns_ip } } ) { + my @matching_keys = grep { $cds->keytag == $_->keytag or ($cds->algorithm == 0 and $_->algorithm == 0)} @dnskey; if ( not scalar @matching_keys ) { $mismatch_cds_cdnskey{ $ns_ip } = 1; } } + foreach my $dnskey ( @dnskey ) { - my @matching_keys = grep { $dnskey->keytag == $_->keytag or ($dnskey->algorithm == 0 and $_->algorithm == 0)} @ds; + my @matching_keys = grep { $dnskey->keytag == $_->keytag or ($dnskey->algorithm == 0 and $_->algorithm == 0)} @{ $cds_rrsets{ $ns_ip } }; if ( not scalar @matching_keys ) { $mismatch_cds_cdnskey{ $ns_ip } = 1; } @@ -4004,42 +4002,45 @@ sub dnssec15 { ); } - my $first_rrset_string = undef; + my $first = 1; + my $first_rrlist; + my $inconsistent_rrset = 0; for my $ns_ip ( keys %cds_rrsets ) { - my $rrset_string; - if ( scalar @{ $cds_rrsets{ $ns_ip } } ) { - $rrset_string = join( "\n", sort map { $_->string } @{ $cds_rrsets{ $ns_ip } } ); - } - else { - $rrset_string = q{}; - } - if ( not defined $first_rrset_string ) { - $first_rrset_string = $rrset_string; + if ( $first ) { + $first_rrlist = scalar @{ $cds_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ) : undef; + $first = 0; + next; } - elsif ( $rrset_string ne $first_rrset_string ) { - push @results, _emit_log( DS15_INCONSISTENT_CDS => {} ); + + my $rrlist = scalar @{ $cds_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ) : undef; + + if ( ( $rrlist and not $first_rrlist ) or ( not $rrlist and $first_rrlist ) or ( $rrlist and $first_rrlist and $rrlist ne $first_rrlist ) ) { + $inconsistent_rrset = 1; last; } } - $first_rrset_string = undef; + push @results, _emit_log( DS15_INCONSISTENT_CDS => {} ) if $inconsistent_rrset; + + $first = 1; + $inconsistent_rrset = 0; for my $ns_ip ( keys %cdnskey_rrsets ) { - my $rrset_string; - if ( scalar @{ $cdnskey_rrsets{ $ns_ip } } ) { - $rrset_string = join( "\n", sort map { $_->string } @{ $cdnskey_rrsets{ $ns_ip } } ); - } - else { - $rrset_string = q{}; - } - if ( not defined $first_rrset_string ) { - $first_rrset_string = $rrset_string; + if ( $first ) { + $first_rrlist = scalar @{ $cdnskey_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ) : undef; + $first = 0; + next; } - elsif ( $rrset_string ne $first_rrset_string ) { - push @results, _emit_log( DS15_INCONSISTENT_CDNSKEY => {} ); + + my $rrlist = scalar @{ $cdnskey_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ) : undef; + + if ( ( $rrlist and not $first_rrlist ) or ( not $rrlist and $first_rrlist ) or ( $rrlist and $first_rrlist and $rrlist ne $first_rrlist ) ) { + $inconsistent_rrset = 1; last; } } + push @results, _emit_log( DS15_INCONSISTENT_CDNSKEY => {} ) if $inconsistent_rrset; + if ( scalar keys %mismatch_cds_cdnskey ) { push @results, _emit_log( From 58cbaf7e3f0d56d8abb0063f289799eec3690f4f Mon Sep 17 00:00:00 2001 From: "[Thomas Green]" Date: Thu, 7 Nov 2024 11:58:34 +0100 Subject: [PATCH 2/2] Adjust implementation following an update to Zonemaster::LDNS::RRList These objects can now be created from empty lists, see https://github.com/zonemaster/zonemaster-ldns/pull/209 --- lib/Zonemaster/Engine/Test/DNSSEC.pm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Zonemaster/Engine/Test/DNSSEC.pm b/lib/Zonemaster/Engine/Test/DNSSEC.pm index 9bd56777e..42d657140 100644 --- a/lib/Zonemaster/Engine/Test/DNSSEC.pm +++ b/lib/Zonemaster/Engine/Test/DNSSEC.pm @@ -4007,14 +4007,14 @@ sub dnssec15 { my $inconsistent_rrset = 0; for my $ns_ip ( keys %cds_rrsets ) { if ( $first ) { - $first_rrlist = scalar @{ $cds_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ) : undef; + $first_rrlist = Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ); $first = 0; next; } - my $rrlist = scalar @{ $cds_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ) : undef; + my $rrlist = Zonemaster::LDNS::RRList->new( $cds_rrsets{ $ns_ip } ); - if ( ( $rrlist and not $first_rrlist ) or ( not $rrlist and $first_rrlist ) or ( $rrlist and $first_rrlist and $rrlist ne $first_rrlist ) ) { + if ( $rrlist ne $first_rrlist ) { $inconsistent_rrset = 1; last; } @@ -4026,14 +4026,14 @@ sub dnssec15 { $inconsistent_rrset = 0; for my $ns_ip ( keys %cdnskey_rrsets ) { if ( $first ) { - $first_rrlist = scalar @{ $cdnskey_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ) : undef; + $first_rrlist = Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ); $first = 0; next; } - my $rrlist = scalar @{ $cdnskey_rrsets{ $ns_ip } } ? Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ) : undef; + my $rrlist = Zonemaster::LDNS::RRList->new( $cdnskey_rrsets{ $ns_ip } ); - if ( ( $rrlist and not $first_rrlist ) or ( not $rrlist and $first_rrlist ) or ( $rrlist and $first_rrlist and $rrlist ne $first_rrlist ) ) { + if ( $rrlist ne $first_rrlist ) { $inconsistent_rrset = 1; last; }