Skip to content

Commit

Permalink
Add CNAME followage in Recursor
Browse files Browse the repository at this point in the history 
This commit makes Engine able to follow CNAMEs when doing recursive lookups.
Currently CNAMEs will be followed when all of the following are true:
	- the response has RCODE "NoError"
	- the answer section of the response does not contain records of the queried type, but does contain at least one CNAME record for the query name
	- the answer section of the response does not contain multiple CNAME records with the same owner name
	- the final target of the CNAME record(s) chain has not been followed before

Three system, debug level messages are created: 'CNAME_LOOP_INNER', 'CNAME_LOOP_OUTER' and 'CNAME_MULTIPLE_FOR_NAME'.

Some test cases have been modified to account for this new behavior. Unitary tests have also been updated.
  • Loading branch information
@tgreenx
tgreenx committed Oct 31, 2023
1 parent 5fd6423 commit 2cab9b6
Show file tree
Hide file tree
Showing 11 changed files with 577 additions and 335 deletions.
2 changes: 2 additions & 0 deletions MANIFEST
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ t/profiles/Test-syntax-all.json
t/profiles/Test-zone-all.json
t/recursor.data
t/recursor.t
t/recursor-A.data
t/recursor-A.t
t/Test-address.data
t/Test-address.t
t/Test-basic.data
Expand Down
65 changes: 60 additions & 5 deletions lib/Zonemaster/Engine/Recursor.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ sub recurse {

my ( $p, $state ) =
$class->_recurse( $name, $type, $dns_class,
{ ns => [ root_servers() ], count => 0, common => 0, seen => {}, glue => {} } );
{ ns => [ root_servers() ], count => 0, common => 0, seen => {}, cseen => {}, glue => {} } );
$recurse_cache{$name}{$type}{$dns_class} = $p;

return $p;
Expand All @@ -111,7 +111,7 @@ sub parent {

my ( $p, $state ) =
$class->_recurse( $name, 'SOA', 'IN',
{ ns => [ root_servers() ], count => 0, common => 0, seen => {}, glue => {} } );
{ ns => [ root_servers() ], count => 0, common => 0, seen => {}, cseen => {}, glue => {} } );

my $pname;
if ( name( $state->{trace}[0][0] ) eq name( $name ) ) {
Expand Down Expand Up @@ -193,8 +193,63 @@ sub _recurse {
return ( $p, $state );
}

if ( $class->_is_answer( $p ) ) { # Return answer
return ( $p, $state );
if ( $class->_is_answer( $p ) ) { # Return answer, or follow CNAME
if ( not $p->has_rrs_of_type_for_name( $type, $name ) and $p->has_rrs_of_type_for_name( 'CNAME', $name ) ) {
my @cname_rrs_for_name = $p->get_records_for_name( 'CNAME', $name, 'answer');

if ( scalar @cname_rrs_for_name > 1 ) {
Zonemaster::Engine->logger->add( CNAME_MULTIPLE_FOR_NAME => { name => $name } );
return ( undef, $state );
}
else {
my %cnames;
my %tseen;

for my $rr ( $p->get_records( 'CNAME', 'answer' ) ) {
my $rr_owner = name( $rr->owner );
my $rr_target = name( $rr->cname );

if ( scalar $p->has_rrs_of_type_for_name( 'CNAME', $rr_owner ) > 1 ) {
Zonemaster::Engine->logger->add( CNAME_MULTIPLE_FOR_NAME => { name => $rr_owner->string } );
return ( undef, $state );
}

if ( lc( $rr_owner ) eq lc( $rr_target ) or exists $tseen{lc( $rr_target )} ) {
Zonemaster::Engine->logger->add( CNAME_LOOP_INNER => { name => $rr_owner->string, target => $rr_target->string } );
return ( undef, $state );
}

$tseen{lc( $rr_target )} = 1;
$cnames{$rr_owner} = $rr_target;
}

my $target = $name;
$target = $cnames{$target} while $cnames{$target};

if ( $state->{cseen}{lc( $target )} ) {
Zonemaster::Engine->logger->add( CNAME_LOOP_OUTER => { name => $name, target => $target, cnames => join( ';', keys %{ $state->{cseen} } ) } );
return ( undef, $state );
}

$state->{cseen}{lc( $target )} = 1;

if ( lc( $target ) eq lc( $name ) ) {
Zonemaster::Engine->logger->add( CNAME_LOOP_INNER => { name => $name, target => $target } );
return ( undef, $state );
}

$state->{count} += 1;
return ( undef, $state ) if $state->{count} > 100; # Loop protection

( $p, $state ) = $class->_recurse( $target, $type, $dns_class,
{ ns => [ root_servers() ], count => $state->{count}, common => 0, seen => {}, cseen => $state->{cseen}, glue => {} });

return ( $p, $state );
}
}
else {
return ( $p, $state );
}
}

# So it's not an error, not an empty response and not an answer
Expand Down Expand Up @@ -297,7 +352,7 @@ sub get_addresses_for {
my ( $class, $name, $state ) = @_;
my @res;
$state //=
{ ns => [ root_servers() ], count => 0, common => 0, seen => {} };
{ ns => [ root_servers() ], count => 0, common => 0, seen => {}, cseen => {} };

my ( $pa ) = $class->_recurse(
"$name", 'A', 'IN',
Expand Down
10 changes: 4 additions & 6 deletions lib/Zonemaster/Engine/Test/Syntax.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,16 +918,15 @@ sub syntax06 {

my $domain = ( $rname =~ s/.*@//r );
my $p_mx = Zonemaster::Engine::Recursor->recurse( $domain, q{MX} );

if ( not $p_mx or $p_mx->rcode ne 'NOERROR' ) {
push @results, info( RNAME_MAIL_DOMAIN_INVALID => { domain => $domain } );
next;
}

# Follow CNAMEs in the MX response
my %cnames =
map { $_->owner => $_->cname } $p_mx->get_records( q{CNAME}, q{answer} );
$domain .= q{.}; # Add back final dot
$domain = $cnames{$domain} while $cnames{$domain};
# Retrieve followed CNAME, if any
my @p_question_sec = $p_mx->question;
$domain = $p_question_sec[0]->owner;

# Determine mail domain(s)
my @mail_domains;
Expand All @@ -939,7 +938,6 @@ sub syntax06 {
}

for my $mail_domain ( @mail_domains ) {

# Assume mail domain is invalid until we see an actual IP address
my $exchange_valid = 0;

Expand Down
10 changes: 8 additions & 2 deletions lib/Zonemaster/Engine/Test/Zone.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1056,13 +1056,18 @@ sub zone07 {
my $soa_mname = $soa->mname;
$soa_mname =~ s/[.]\z//smx;
my $addresses_nb = 0;

foreach my $address_type ( q{A}, q{AAAA} ) {
my $p_mname = Zonemaster::Engine::Recursor->recurse( $soa_mname, $address_type );
if ( $p_mname ) {
if ( $p_mname->has_rrs_of_type_for_name( $address_type, $soa_mname ) ) {
my @p_question_sec = $p_mname->question;
my $final_name = name( $p_question_sec[0]->owner );

if ( $p_mname->has_rrs_of_type_for_name( $address_type, $final_name ) ) {
$addresses_nb++;
}
if ( $p_mname->has_rrs_of_type_for_name( q{CNAME}, $soa_mname ) ) {

if ( $final_name ne $soa_mname ) {
push @results,
info(
MNAME_IS_CNAME => {
Expand All @@ -1080,6 +1085,7 @@ sub zone07 {
}
} ## end if ( $p_mname )
} ## end foreach my $address_type ( ...)

if ( not $addresses_nb ) {
push @results,
info(
Expand Down
3 changes: 3 additions & 0 deletions share/profile.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,9 @@
"CACHE_FETCHED" : "DEBUG2",
"CACHED_RETURN" : "DEBUG3",
"CANNOT_CONTINUE" : "CRITICAL",
"CNAME_LOOP_INNER" : "DEBUG",
"CNAME_LOOP_OUTER" : "DEBUG",
"CNAME_MULTIPLE_FOR_NAME" : "DEBUG",
"DEPENDENCY_VERSION" : "DEBUG",
"EMPTY_RETURN" : "DEBUG3",
"EXTERNAL_RESPONSE" : "DEBUG3",
Expand Down
3 changes: 3 additions & 0 deletions share/profile.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ test_levels:
CACHE_CREATED: DEBUG2
CACHE_FETCHED: DEBUG2
CANNOT_CONTINUE: CRITICAL
CNAME_LOOP_INNER : DEBUG
CNAME_LOOP_OUTER : DEBUG
CNAME_MULTIPLE_FOR_NAME : DEBUG
DEPENDENCY_VERSION: DEBUG
EMPTY_RETURN: DEBUG3
EXTERNAL_RESPONSE: DEBUG3
Expand Down
Loading

0 comments on commit 2cab9b6

Please sign in to comment.