From a7bfb9b3e6dd9fd3f6b74469d17d4a02f125cd7e Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 5 Jun 2024 11:12:35 +0200 Subject: [PATCH 1/9] Move the code displaying the header before adding fake delegation and/or ds --- lib/Zonemaster/CLI.pm | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 989f008..ae3e425 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -639,14 +639,6 @@ sub run { Zonemaster::Engine::Recursor->add_fake_addresses( '.', $hints_data ); } - if ( $self->ns and @{ $self->ns } > 0 ) { - $self->add_fake_delegation( $domain ); - } - - if ( $self->ds and @{ $self->ds } ) { - $self->add_fake_ds( $domain ); - } - if ( $self->profile or $self->test ) { # Separate initialization from main output in human readable output mode print "\n" if $fh_diag eq *STDOUT; @@ -686,6 +678,15 @@ sub run { print $header; } + + if ( $self->ns and @{ $self->ns } > 0 ) { + $self->add_fake_delegation( $domain ); + } + + if ( $self->ds and @{ $self->ds } ) { + $self->add_fake_ds( $domain ); + } + # Actually run tests! eval { if ( $self->test and @{ $self->test } > 0 ) { From f7cf49170d5e07c7bca22c2e91133decc464c67a Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Mon, 12 Aug 2024 13:41:43 +0200 Subject: [PATCH 2/9] Split CLI argument check and Engine setting. I have extracted CLI argument verification from 'add_fake_delegation' and 'add_fake_ds' to 'check_fake_ds' and 'check_fake_delegation'. check_* functions are called before displaying the header and only displays CLI argument errors. add_* functions are called after the header and throw errors from the Engine. --- lib/Zonemaster/CLI.pm | 60 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index ae3e425..62fc862 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -40,6 +40,7 @@ our $JSON = JSON::XS->new->allow_blessed->convert_blessed->canonical; Readonly our $IPV4_RE => qr/^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$/; Readonly our $IPV6_RE => qr/^[0-9a-f:]*:[0-9a-f:]+(:[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})?$/i; +Readonly our $DS_RE => qr/^(?:[[:digit:]]+,){3}[[:xdigit:]]+$/; STDOUT->autoflush( 1 ); @@ -639,6 +640,15 @@ sub run { Zonemaster::Engine::Recursor->add_fake_addresses( '.', $hints_data ); } + + if ( $self->ns and @{ $self->ns } > 0 ) { + $self->check_fake_delegation( $domain ); + } + + if ( $self->ds and @{ $self->ds } ) { + $self->check_fake_ds( $domain ); + } + if ( $self->profile or $self->test ) { # Separate initialization from main output in human readable output mode print "\n" if $fh_diag eq *STDOUT; @@ -815,7 +825,8 @@ sub run { return; } -sub add_fake_delegation { + +sub check_fake_delegation { my ( $self, $domain ) = @_; my @ns_with_no_ip; my %data; @@ -840,16 +851,49 @@ sub add_fake_delegation { if ( $ip ) { my $net_ip = Net::IP::XS->new( $ip ); - if ( ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) - or - ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) - ) { - push @{ $data{ $name } }, $ip; - } - else { + if ( not( + ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) + or + ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) + ) + ) + { die Net::IP::XS::Error() ? "Invalid IP address in --ns argument:\n\t". Net::IP::XS::Error() ."\n" : "Invalid IP address in --ns argument.\n"; } } + } + + return; + +} + +sub check_fake_ds { + my ( $self, $domain ) = @_; + my @data; + + foreach my $str ( @{ $self->ds } ) { + + if ( not $str =~ /$DS_RE/g ) { + say STDERR __( "--ds ds data must be in the form \"keytag,algorithm,type,digest\""); + exit( 1 ); + } + } + + return; +} +sub add_fake_delegation { + my ( $self, $domain ) = @_; + my @ns_with_no_ip; + my %data; + + foreach my $pair ( @{ $self->ns } ) { + my ( $name, $ip ) = split( '/', $pair, 2 ); + + ( my $errors, $name ) = normalize_name( decode( 'utf8', $name ) ); + + if ( $ip ) { + push @{ $data{ $name } }, $ip; + } else { push @ns_with_no_ip, $name; } From b9fb33548f5889aba89ecdb46bbf3271d869f325 Mon Sep 17 00:00:00 2001 From: MichaelTimbert <110017095+MichaelTimbert@users.noreply.github.com> Date: Wed, 28 Aug 2024 09:11:11 +0200 Subject: [PATCH 3/9] Apply suggestions from code review Code formatting Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com> Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com> --- lib/Zonemaster/CLI.pm | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 62fc862..eefca30 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -640,7 +640,6 @@ sub run { Zonemaster::Engine::Recursor->add_fake_addresses( '.', $hints_data ); } - if ( $self->ns and @{ $self->ns } > 0 ) { $self->check_fake_delegation( $domain ); } @@ -688,7 +687,6 @@ sub run { print $header; } - if ( $self->ns and @{ $self->ns } > 0 ) { $self->add_fake_delegation( $domain ); } @@ -864,7 +862,6 @@ sub check_fake_delegation { } return; - } sub check_fake_ds { @@ -872,7 +869,6 @@ sub check_fake_ds { my @data; foreach my $str ( @{ $self->ds } ) { - if ( not $str =~ /$DS_RE/g ) { say STDERR __( "--ds ds data must be in the form \"keytag,algorithm,type,digest\""); exit( 1 ); @@ -881,6 +877,7 @@ sub check_fake_ds { return; } + sub add_fake_delegation { my ( $self, $domain ) = @_; my @ns_with_no_ip; From 33e722559939069c84f74f1ad8d8fc21778d6584 Mon Sep 17 00:00:00 2001 From: MichaelTimbert <110017095+MichaelTimbert@users.noreply.github.com> Date: Wed, 28 Aug 2024 13:41:15 +0200 Subject: [PATCH 4/9] Apply suggestions from code review Remove unused variables Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com> --- lib/Zonemaster/CLI.pm | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index eefca30..37b9a1a 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -825,9 +825,7 @@ sub run { sub check_fake_delegation { - my ( $self, $domain ) = @_; - my @ns_with_no_ip; - my %data; + my ( $self ) = @_; foreach my $pair ( @{ $self->ns } ) { my ( $name, $ip ) = split( '/', $pair, 2 ); @@ -865,8 +863,7 @@ sub check_fake_delegation { } sub check_fake_ds { - my ( $self, $domain ) = @_; - my @data; + my ( $self ) = @_; foreach my $str ( @{ $self->ds } ) { if ( not $str =~ /$DS_RE/g ) { From 8f7f5d3004b39bb480071624cf1b4975f7dcafa0 Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 28 Aug 2024 13:51:20 +0200 Subject: [PATCH 5/9] Apply suggestions from code review Improve code Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com> --- lib/Zonemaster/CLI.pm | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 37b9a1a..e7fbe49 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -847,11 +847,10 @@ sub check_fake_delegation { if ( $ip ) { my $net_ip = Net::IP::XS->new( $ip ); - if ( not( - ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) - or - ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) - ) + unless ( + ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) + or + ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) ) { die Net::IP::XS::Error() ? "Invalid IP address in --ns argument:\n\t". Net::IP::XS::Error() ."\n" : "Invalid IP address in --ns argument.\n"; From 1a58847c866b5a61d918db9a3ddad21d304658d7 Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 28 Aug 2024 14:03:50 +0200 Subject: [PATCH 6/9] Apply suggestions from code review Move --ns & --ds validation earlier in the code Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com> --- lib/Zonemaster/CLI.pm | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index e7fbe49..3858a61 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -619,6 +619,14 @@ sub run { ( my $errors, $domain ) = normalize_name( decode( 'utf8', $domain ) ); + if ( $self->ns and @{ $self->ns } > 0 ) { + $self->check_fake_delegation( $domain ); + } + + if ( $self->ds and @{ $self->ds } ) { + $self->check_fake_ds( $domain ); + } + if ( scalar @$errors > 0 ) { my $error_message; foreach my $err ( @$errors ) { @@ -640,14 +648,6 @@ sub run { Zonemaster::Engine::Recursor->add_fake_addresses( '.', $hints_data ); } - if ( $self->ns and @{ $self->ns } > 0 ) { - $self->check_fake_delegation( $domain ); - } - - if ( $self->ds and @{ $self->ds } ) { - $self->check_fake_ds( $domain ); - } - if ( $self->profile or $self->test ) { # Separate initialization from main output in human readable output mode print "\n" if $fh_diag eq *STDOUT; From 31c56c47a9c7c8ffc2723381cfd974a0d76afbaa Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 28 Aug 2024 14:47:35 +0200 Subject: [PATCH 7/9] Apply suggestions from code review Improve code and regex validation Co-authored-by: Marc van der Wal --- lib/Zonemaster/CLI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 3858a61..88b0160 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -865,7 +865,7 @@ sub check_fake_ds { my ( $self ) = @_; foreach my $str ( @{ $self->ds } ) { - if ( not $str =~ /$DS_RE/g ) { + unless ( $str =~ /$DS_RE/ ) { say STDERR __( "--ds ds data must be in the form \"keytag,algorithm,type,digest\""); exit( 1 ); } From 5c10b9dde1f96b3ef67daface2655b5fa37acda3 Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 28 Aug 2024 15:25:40 +0200 Subject: [PATCH 8/9] Apply suggestions from code review Reuse code from Zonemaster::Engine for IP validation Co-authored-by: Mats Dufberg --- lib/Zonemaster/CLI.pm | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 88b0160..7389efa 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -34,12 +34,11 @@ use Zonemaster::Engine::Normalization qw[normalize_name]; use Zonemaster::Engine::Logger::Entry; use Zonemaster::Engine::Translator; use Zonemaster::Engine::Util qw[parse_hints]; +use Zonemaster::Engine::Validation qw[validate_ipv4 validate_ipv6]; our %numeric = Zonemaster::Engine::Logger::Entry->levels; our $JSON = JSON::XS->new->allow_blessed->convert_blessed->canonical; -Readonly our $IPV4_RE => qr/^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$/; -Readonly our $IPV6_RE => qr/^[0-9a-f:]*:[0-9a-f:]+(:[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})?$/i; Readonly our $DS_RE => qr/^(?:[[:digit:]]+,){3}[[:xdigit:]]+$/; STDOUT->autoflush( 1 ); @@ -847,11 +846,7 @@ sub check_fake_delegation { if ( $ip ) { my $net_ip = Net::IP::XS->new( $ip ); - unless ( - ( $ip =~ /($IPV4_RE)/ && Net::IP::XS::ip_is_ipv4( $ip ) ) - or - ( $ip =~ /($IPV6_RE)/ && Net::IP::XS::ip_is_ipv6( $ip ) ) - ) + unless( validate_ipv4( $ip ) or validate_ipv6( $ip ) ) { die Net::IP::XS::Error() ? "Invalid IP address in --ns argument:\n\t". Net::IP::XS::Error() ."\n" : "Invalid IP address in --ns argument.\n"; } From 89557a3c89be2b32ea64f6c1bbd6e8aa5b9c138e Mon Sep 17 00:00:00 2001 From: Michael Timbert Date: Wed, 28 Aug 2024 15:44:07 +0200 Subject: [PATCH 9/9] Apply suggestions from code review Better hint for --ds option Co-authored-by: Mats Dufberg --- lib/Zonemaster/CLI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/CLI.pm b/lib/Zonemaster/CLI.pm index 7389efa..02d4a34 100644 --- a/lib/Zonemaster/CLI.pm +++ b/lib/Zonemaster/CLI.pm @@ -861,7 +861,7 @@ sub check_fake_ds { foreach my $str ( @{ $self->ds } ) { unless ( $str =~ /$DS_RE/ ) { - say STDERR __( "--ds ds data must be in the form \"keytag,algorithm,type,digest\""); + say STDERR __( "--ds ds data must be in the form \"keytag,algorithm,type,digest\". E.g. space is not permitted anywhere in the string."); exit( 1 ); } }