Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix EDNS behavior for queries #1147

Merged
merged 4 commits into from
May 2, 2023
Merged

Fix EDNS behavior for queries #1147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae6511d
Initial commit
tgreenx Nov 16, 2022
ecd21c4
Update unitary test data
tgreenx Dec 5, 2022
cbaaad2
Update after review - refactoring, documentation
tgreenx Dec 28, 2022
041271b
Update after review - refactoring, documentation
tgreenx Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions lib/Zonemaster/Engine/Constants.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ our @EXPORT_OK = qw[
$MINIMUM_NUMBER_OF_NAMESERVERS
$RESOLVER_SOURCE_OS_DEFAULT
$UDP_PAYLOAD_LIMIT
$UDP_EDNS_QUERY_DEFAULT
$UDP_COMMON_EDNS_LIMIT
@IPV4_SPECIAL_ADDRESSES
@IPV6_SPECIAL_ADDRESSES
Expand All @@ -56,7 +57,7 @@ our %EXPORT_TAGS = (
soa => [
qw($DURATION_5_MINUTES_IN_SECONDS $DURATION_1_HOUR_IN_SECONDS $DURATION_4_HOURS_IN_SECONDS $DURATION_12_HOURS_IN_SECONDS $DURATION_1_DAY_IN_SECONDS $DURATION_1_WEEK_IN_SECONDS $DURATION_180_DAYS_IN_SECONDS $SERIAL_BITS $SERIAL_MAX_VARIATION)
],
misc => [qw($UDP_PAYLOAD_LIMIT $UDP_COMMON_EDNS_LIMIT $MINIMUM_NUMBER_OF_NAMESERVERS $RESOLVER_SOURCE_OS_DEFAULT $BLACKLISTING_ENABLED)]
misc => [qw($UDP_PAYLOAD_LIMIT $UDP_EDNS_QUERY_DEFAULT $UDP_COMMON_EDNS_LIMIT $MINIMUM_NUMBER_OF_NAMESERVERS $RESOLVER_SOURCE_OS_DEFAULT $BLACKLISTING_ENABLED)]
, # everyting in %EXPORT_OK that isn't included in any of the other tags
addresses => [qw(@IPV4_SPECIAL_ADDRESSES @IPV6_SPECIAL_ADDRESSES)],
);
Expand Down Expand Up @@ -93,8 +94,9 @@ Readonly our $RESOLVER_SOURCE_OS_DEFAULT => 'os_default';
Readonly our $SERIAL_BITS => 32;
Readonly our $SERIAL_MAX_VARIATION => 0;

Readonly our $UDP_PAYLOAD_LIMIT => 512;
Readonly our $UDP_COMMON_EDNS_LIMIT => 4_096;
Readonly our $UDP_PAYLOAD_LIMIT => 512;
Readonly our $UDP_EDNS_QUERY_DEFAULT => 512;
Readonly our $UDP_COMMON_EDNS_LIMIT => 4_096;

Readonly::Array our @IPV4_SPECIAL_ADDRESSES => _extract_iana_ip_blocks($IP_VERSION_4);

Expand Down Expand Up @@ -282,6 +284,12 @@ C<$UDP_PAYLOAD_LIMIT>
=item *
C<$UDP_EDNS_QUERY_DEFAULT>
An integer, used to define the EDNS0 UDP packet size in EDNS queries.
=item *
C<UDP_COMMON_EDNS_LIMIT>
=item *
Expand Down
126 changes: 71 additions & 55 deletions lib/Zonemaster/Engine/Nameserver.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,16 +212,17 @@ sub _build_dns {
my ( $self ) = @_;

my $res = Zonemaster::LDNS->new( $self->address->ip );

$res->recurse( 0 );
$res->dnssec( 0 );
$res->edns_size( $UDP_EDNS_QUERY_DEFAULT );

$res->retry( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} ) );
$res->retrans( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retrans} ) );
$res->dnssec( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.dnssec} ) );
$res->usevc( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.usevc} ) );
$res->igntc( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.igntc} ) );
$res->recurse( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.recurse} ) );
$res->debug( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.debug} ) );
$res->edns_size( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.edns_size} ) );
$res->timeout( Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.timeout} ) );

my $ip_version = Net::IP::XS::ip_get_version( $self->address->ip );
Expand Down Expand Up @@ -268,10 +269,10 @@ sub query {
);

my $class = $href->{class} // 'IN';
my $dnssec = $href->{dnssec} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.dnssec} );
my $dnssec = $href->{dnssec} // 0;
my $usevc = $href->{usevc} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.usevc} );
my $recurse = $href->{recurse} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.recurse} );
my $edns_size = $href->{edns_size} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.edns_size} );
my $edns_size = $href->{edns_size} // $UDP_EDNS_QUERY_DEFAULT;

# Fake a DS answer
if ( $type eq 'DS' and $class eq 'IN' and $self->fake_ds->{ lc( $name ) } ) {
Expand Down Expand Up @@ -335,44 +336,35 @@ sub query {

my $p;
my $md5 = Digest::MD5->new;
my $edns_special_case = 0;
if ( defined $href->{edns_details} ) {
if ( defined $href->{edns_details}{version} and $href->{edns_details}{version} != 0 ) {
$edns_special_case = 1;
}
elsif ( defined $href->{edns_details}{z} ) {
$edns_special_case = 1;
}
elsif ( defined $href->{edns_details}{extended_rcode} ) {
$edns_special_case = 1;
}
elsif ( defined $href->{edns_details}{data} ) {
$edns_special_case = 1;
}
elsif ( defined $href->{edns_details}{udp_size} ) {
$edns_size = $href->{edns_details}{udp_size};
}
}

$md5->add( q{NAME} , $name );
$md5->add( q{TYPE} , "\U$type" );
$md5->add( q{CLASS} , "\U$class" );
$md5->add( q{DNSSEC} , $dnssec );

if ( exists $href->{edns_details} and exists $href->{edns_details}{do} ) {
$md5->add( q{DNSSEC} , $href->{edns_details}{do} );
}
else {
$md5->add( q{DNSSEC} , $dnssec );
}

$md5->add( q{USEVC} , $usevc );
$md5->add( q{RECURSE} , $recurse );
if ( $edns_special_case ) {
$md5->add( q{EDNS_VERSION} , $href->{edns_details}{version} ? $href->{edns_details}{version} : 0 );
$md5->add( q{EDNS_Z} , $href->{edns_details}{z} ? $href->{edns_details}{z} : 0 );
$md5->add( q{EDNS_EXTENDED_RCODE} , $href->{edns_details}{extended_rcode} ? $href->{edns_details}{extended_rcode} : 0 );
$md5->add( q{EDNS_DATA} , $href->{edns_details}{data} ? $href->{edns_details}{data} : q{} );
$md5->add( q{EDNS_UDP_SIZE} , $href->{edns_details}{udp_size} ? $href->{edns_details}{udp_size} : 0 );

if ( exists $href->{edns_details} ) {
$md5->add( q{EDNS_VERSION} , $href->{edns_details}{version} // 0 );
$md5->add( q{EDNS_Z} , $href->{edns_details}{z} // 0 );
$md5->add( q{EDNS_EXTENDED_RCODE} , $href->{edns_details}{rcode} // 0 );
$md5->add( q{EDNS_DATA} , $href->{edns_details}{data} // q{} );
$md5->add( q{EDNS_UDP_SIZE} , $href->{edns_details}{size} // $edns_size );
}
else {
$md5->add( q{EDNS_UDP_SIZE} , $edns_size);
$md5->add( q{EDNS_UDP_SIZE} , 0);
}

my $idx = $md5->b64digest();
if ( not exists( $self->cache->data->{$idx} ) ) {
$self->cache->data->{$idx} = $self->_query( $name, $type, $href, $edns_special_case );
$self->cache->data->{$idx} = $self->_query( $name, $type, $href );
}
$p = $self->cache->data->{$idx};

Expand Down Expand Up @@ -437,7 +429,7 @@ sub add_fake_ds {
} ## end sub add_fake_ds

sub _query {
my ( $self, $name, $type, $href, $edns_special_case ) = @_;
my ( $self, $name, $type, $href ) = @_;
my %flags;

$type //= 'A';
Expand All @@ -462,16 +454,19 @@ sub _query {
# Make sure we have a value for each flag
$flags{q{retry}} = $href->{q{retry}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retry} );
$flags{q{retrans}} = $href->{q{retrans}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.retrans} );
$flags{q{dnssec}} = $href->{q{dnssec}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.dnssec} );
$flags{q{dnssec}} = $href->{q{dnssec}} // 0;
$flags{q{usevc}} = $href->{q{usevc}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.usevc} );
$flags{q{igntc}} = $href->{q{igntc}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.igntc} );
$flags{q{fallback}} = $href->{q{fallback}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.fallback} );
$flags{q{recurse}} = $href->{q{recurse}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.recurse} );
$flags{q{edns_size}} = $href->{q{edns_size}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.edns_size} );
$flags{q{timeout}} = $href->{q{timeout}} // Zonemaster::Engine::Profile->effective->get( q{resolver.defaults.timeout} );
if ( defined $href->{edns_details} and $href->{edns_details}{udp_size} ) {
$flags{q{edns_size}} = $href->{edns_details}{udp_size};
$flags{q{edns_size}} = $href->{q{edns_size}} // $UDP_EDNS_QUERY_DEFAULT;

if ( exists $href->{edns_details} ) {
$flags{q{dnssec}} = $href->{edns_details}{do} // $flags{q{dnssec}};
$flags{q{edns_size}} = $href->{edns_details}{size} // $flags{q{edns_size}};
}

# Set flags for this query
foreach my $flag ( keys %flags ) {
$self->dns->$flag( $flags{$flag} );
Expand All @@ -493,37 +488,35 @@ sub _query {
);
}
else {
if ( $edns_special_case ) {
if ( exists $href->{edns_details} ) {
my $pkt = Zonemaster::LDNS::Packet->new("$name", $type, $href->{class} );
if ( defined $href->{edns_details} and defined $href->{edns_details}{version} and $href->{edns_details}{version} != 0 ) {
$pkt->set_edns_present();
$pkt->set_edns_present();

if ( exists $href->{edns_details}{version} ) {
$pkt->edns_version($href->{edns_details}{version});
}
if ( defined $href->{edns_details} and defined $href->{edns_details}{z} ) {
$pkt->set_edns_present();
if ( exists $href->{edns_details}{z} ) {
$pkt->edns_z($href->{edns_details}{z});
}
if ( defined $href->{edns_details} and defined $href->{edns_details}{do} ) {
$pkt->set_edns_present();
if ( exists $href->{edns_details}{do} ) {
$pkt->do($href->{edns_details}{do});
}
if ( defined $href->{edns_details} and defined $href->{edns_details}{udp_size} ) {
$pkt->set_edns_present();
$pkt->edns_size($href->{edns_details}{udp_size});
if ( exists $href->{edns_details}{size} ) {
$pkt->edns_size($href->{edns_details}{size});
}
if ( defined $href->{edns_details} and defined $href->{edns_details}{extended_rcode} ) {
$pkt->set_edns_present();
$pkt->edns_rcode($href->{edns_details}{extended_rcode});
if ( exists $href->{edns_details}{rcode} ) {
$pkt->edns_rcode($href->{edns_details}{rcode});
}
if ( defined $href->{edns_details} and defined $href->{edns_details}{data} ) {
$pkt->set_edns_present();
if ( exists $href->{edns_details}{data} ) {
$pkt->edns_data($href->{edns_details}{data});
}
$res = eval { $self->dns->query_with_pkt( $pkt ) };

$res = eval { $self->dns->query_with_pkt( $pkt ) };
}
else {
$res = eval { $self->dns->query( "$name", $type, $href->{class} ) };
}

if ( $@ ) {
my $msg = "$@";
my $trailing_info = " at ".__FILE__;
Expand Down Expand Up @@ -834,7 +827,7 @@ Remove all cached nameserver objects and queries.

Send a DNS query to the nameserver the object represents. C<$name> and C<$type> are the name and type that will be queried for (C<$type> defaults
to 'A' if it's left undefined). C<$flagref> is a reference to a hash, the keys of which are flags and the values are their corresponding values.
The available flags are as follows. All but the first directly correspond to methods in the L<Zonemaster::LDNS::Resolver> object.
The available flags are as follows. All but 'class' and 'edns_details' directly correspond to methods in the L<Zonemaster::LDNS::Resolver> object.

=over

Expand All @@ -848,11 +841,14 @@ Send the query via TCP (only).

=item retrans

The retransmission interval
The retransmission interval.

=item dnssec

Set the DO flag in the query.
Set the DO flag in the query. Defaults to false.

If set to true, it becomes an EDNS query.
Value overridden by 'edns_details->do' (if also given). More details in 'edns_details' below.

=item debug

Expand Down Expand Up @@ -882,6 +878,26 @@ If set to true, incoming response packets with the TC flag set fall back to EDNS

If set to true, prevents a server to be black-listed on a query in case there is no answer OR rcode is REFUSED.

=item edns_size
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to have both $flagref->{edns_size} and $flagref->{edns_details}{udp_size}? Since this wasn't documented before, maybe take the opportunity to remove it and replace all its uses with $flagref->{edns_details}{udp_size}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above. See packet_edns_size and edns_size.


Set the EDNS0 UDP maximum size. Defaults to 512.

Used only when the query is an EDNS query. Does not enable on its own the query to be an EDNS query.
Value overridden by 'edns_details->size' (if also given). More details in 'edns_details' below.

=item edns_details

A hash. An empty hash or a hash with any keys below will enable the query to be an EDNS query.

The currently supported keys are 'version', 'z', 'do', 'rcode', 'size' and 'data'.
See L<Zonemaster::LDNS::Packet> for more details (key names prefixed with 'edns_').

Note that flag 'edns_size' also exists (see above) and has the same effect as 'edns_details->size', although the value of the
latter will take precedence if both are given.

Similarly, note that flag 'dnssec' also exists (see above) and has the same effect as 'edns_details->do', although the value of the
latter will take precedence if both are given.

=back

=item string()
Expand Down
6 changes: 6 additions & 0 deletions lib/Zonemaster/Engine/Profile.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,14 @@ Default 3.

=head2 resolver.defaults.dnssec

*DEPRECATED as of 2023.1. Planned for removal in 2023.2*
A boolean. If true, sets the DO flag in queries. Default false.

=head2 resolver.defaults.edns_size

*DEPRECATED as of 2023.1. Planned for removal in 2023.2*
An integer. The EDNS0 UDP size used in EDNS queries. Default 512.

=head2 resolver.defaults.recurse

A boolean. If true, sets the RD flag in queries. Default false.
Expand Down
17 changes: 8 additions & 9 deletions lib/Zonemaster/Engine/Test/Nameserver.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ sub nameserver02 {

next if $nsnames_and_ip{ $local_ns->name->string . q{/} . $local_ns->address->short };

my $p = $local_ns->query( $zone->name, q{SOA}, { edns_size => 512 } );
my $p = $local_ns->query( $zone->name, q{SOA}, { edns_details => { version => 0 } } );
if ( $p ) {
if ( $p->rcode eq q{FORMERR} and not $p->has_edns) {
push @results, info( NO_EDNS_SUPPORT => { ns => $local_ns->string } );
Expand Down Expand Up @@ -1097,8 +1097,7 @@ sub nameserver10 {

next if ( _ip_disabled_message( \@results, $ns, q{SOA} ) );

#To be changed to '$ns->query( $zone->name, q{SOA}, { edns_details => { version => 0 } } );' when PR#1147 is merged.
my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { udp_size => 512 } } );
my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { version => 0 } } );

if ( $p and $p->rcode eq q{NOERROR} ){
my $p2 = $ns->query( $zone->name, q{SOA}, { edns_details => { version => 1 } } );
Expand Down Expand Up @@ -1174,15 +1173,13 @@ sub nameserver11 {

next if ( _ip_disabled_message( \@results, $ns, q{SOA} ) );

#To be changed to '$ns->query( $zone->name, q{SOA}, { edns_details => { version => 0 } } );' when PR#1147 is merged.
my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { udp_size => 512 } } );
my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { version => 0 } } );

if ( not $p or not $p->has_edns or $p->rcode ne q{NOERROR} or not $p->aa or not $p->get_records_for_name(q{SOA}, $zone->name, q{answer}) ) {
next;
}

#To be changed to '$ns->query( $zone->name, q{SOA}, { edns_details => { data => $rdata } } );' when PR#1147 is merged.
$p = $ns->query( $zone->name, q{SOA}, { edns_details => { data => $rdata, udp_size => 512 } } );
$p = $ns->query( $zone->name, q{SOA}, { edns_details => { version => 0, data => $rdata } } );

if ( $p ) {
if ( $p->rcode ne q{NOERROR} ) {
Expand Down Expand Up @@ -1271,7 +1268,8 @@ sub nameserver12 {

next if ( _ip_disabled_message( \@results, $ns, q{SOA} ) );

my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { z => 3 } } );
my $p = $ns->query( $zone->name, q{SOA}, { edns_details => { version => 0, z => 3 } } );

if ( $p ) {
if ( $p->rcode eq q{FORMERR} and not $p->edns_rcode ) {
push @results, info( NO_EDNS_SUPPORT => { ns => $ns->string } );
Expand Down Expand Up @@ -1310,7 +1308,8 @@ sub nameserver13 {

next if ( _ip_disabled_message( \@results, $ns, q{SOA} ) );

my $p = $ns->query( $zone->name, q{SOA}, { usevc => 0, fallback => 0, edns_details => { do => 1, udp_size => 512 } } );
my $p = $ns->query( $zone->name, q{SOA}, { usevc => 0, fallback => 0, edns_details => { version => 0, do => 1, size => 512 } } );

if ( $p ) {
if ( $p->rcode eq q{FORMERR} and not $p->edns_rcode ) {
push @results, info( NO_EDNS_SUPPORT => { ns => $ns->string, } );
Expand Down
Loading