Skip to content

Commit

Permalink
Merge pull request #1287 from marc-vanderwal/feature/spf-test
Browse files Browse the repository at this point in the history
Provide implementation for Zone11 (SPF test)
  • Loading branch information
marc-vanderwal authored Dec 4, 2023
2 parents fbd5fd7 + 9471d82 commit 40dcce5
Show file tree
Hide file tree
Showing 9 changed files with 411 additions and 8 deletions.
25 changes: 20 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,39 @@ jobs:
- name: Install Zonemaster::LDNS (latest)
if: ${{ matrix.compatibility == 'latest' }}
run: |
cpanm --notest Module::Install Zonemaster::LDNS
cpanm --sudo --notest Module::Install Zonemaster::LDNS
- name: Install Zonemaster::LDNS (develop)
if: ${{ matrix.compatibility == 'develop' }}
run: |
cpanm --notest Devel::CheckLib Module::Install Module::Install::XSUtil
cpanm --sudo --notest Devel::CheckLib Module::Install Module::Install::XSUtil
git clone --branch=develop --depth=1 https://github.com/zonemaster/zonemaster-ldns.git
perl Makefile.PL # Generate MYMETA.yml to appease cpanm .
( cd zonemaster-ldns ; cpanm --notest . )
( cd zonemaster-ldns ; cpanm --sudo --notest . )
rm -rf zonemaster-ldns
# Installing Zonemaster::Engine requires root privileges, because of a
# bug in Mail::SPF preventing normal installation with cpanm as
# non-root user (see link below [1]).
#
# The alternative, if one still wishes to install Zonemaster::Engine
# as non-root user, is to install Mail::SPF first with a command like:
#
# % cpanm --notest \
# --install-args="--install_path sbin=$HOME/.local/sbin" \
# Mail::SPF
#
# For the sake of consistency, other Perl packages installed from CPAN
# are also installed as root.
#
# [1]: https://rt.cpan.org/Public/Bug/Display.html?id=34768
- name: Install remaining dependencies
run: |
cpanm --verbose --notest --installdeps .
cpanm --sudo --verbose --notest --installdeps .
- name: Install Zonemaster::Engine
run: |
cpanm --verbose --notest .
cpanm --sudo --verbose --notest .
- name: Show content of log files
if: ${{ failure() }}
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ RUN apk add --no-cache \
perl-list-moreutils \
perl-locale-msgfmt \
perl-lwp-protocol-https \
perl-mail-spf \
perl-module-install \
perl-moose \
perl-pod-coverage \
Expand Down
2 changes: 2 additions & 0 deletions MANIFEST
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ t/Test-zone09-1.data
t/Test-zone09-1.t
t/Test-zone09.data
t/Test-zone09.t
t/Test-zone11.data
t/Test-zone11.t
t/translator.t
t/undelegated.data
t/undelegated.t
Expand Down
3 changes: 2 additions & 1 deletion Makefile.PL
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ requires 'File::Slurp' => 0;
requires 'IO::Socket::INET6' => 2.69;
requires 'List::MoreUtils' => 0;
requires 'Locale::TextDomain' => 1.20;
requires 'Log::Any' => 0,
requires 'Log::Any' => 0;
requires 'Mail::SPF' => 0;
requires 'Module::Find' => 0.10;
requires 'Moose' => 2.0401;
requires 'MooseX::Singleton' => 0.30;
Expand Down
157 changes: 157 additions & 0 deletions lib/Zonemaster/Engine/Test/Zone.pm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use List::Util qw[max];
use Locale::TextDomain qw[Zonemaster-Engine];
use Readonly;
use JSON::PP;
use Mail::SPF::v1::Record;
use Try::Tiny;

use Zonemaster::Engine::Profile;
use Zonemaster::Engine::Constants qw[:soa :ip];
Expand Down Expand Up @@ -71,6 +73,7 @@ sub all {

if ( none { $_->tag eq q{NO_RESPONSE_SOA_QUERY} } @results ) {
push @results, $class->zone10( $zone ) if Zonemaster::Engine::Util::should_run_test( q{zone10} );
push @results, $class->zone11( $zone ) if Zonemaster::Engine::Util::should_run_test( q{zone11} );
}

return @results;
Expand Down Expand Up @@ -206,6 +209,17 @@ sub metadata {
TEST_CASE_START
)
],
zone11 => [
qw(
Z11_INCONSISTENT_SPF_POLICIES
Z11_DIFFERENT_SPF_POLICIES_FOUND
Z11_NO_SPF_FOUND
Z11_SPF1_MULTIPLE_RECORDS
Z11_SPF1_SYNTAX_ERROR
Z11_SPF1_SYNTAX_OK
Z11_UNABLE_TO_CHECK_FOR_SPF
)
],
};
} ## end sub metadata

Expand Down Expand Up @@ -250,6 +264,10 @@ Readonly my %TAG_DESCRIPTIONS => (
__x # ZONE:ZONE10
'No multiple SOA records';
},
ZONE11 => sub {
__x # ZONE:ZONE11
'SPF policy validation', @_;
},
RETRY_MINIMUM_VALUE_LOWER => sub {
__x # ZONE:RETRY_MINIMUM_VALUE_LOWER
'SOA \'retry\' value ({retry}) is less than the recommended one ({required_retry}).', @_;
Expand Down Expand Up @@ -460,6 +478,34 @@ Readonly my %TAG_DESCRIPTIONS => (
__x # ZONE:Z09_UNEXPECTED_RCODE_MX
'Unexpected RCODE value ({rcode}) on the MX query from name servers "{ns_ip_list}".', @_;
},
Z11_INCONSISTENT_SPF_POLICIES => sub {
__x # ZONE:Z11_INCONSISTENT_SPF_POLICIES
'The zone publishes different SPF policies on different name servers.', @_;
},
Z11_DIFFERENT_SPF_POLICIES_FOUND => sub {
__x # ZONE:Z11_DIFFERENT_SPF_POLICIES_FOUND
'The following name servers returned the same SPF version 1 policy, but other name servers returned a different policy. Name servers: {ns_ip_list}.', @_;
},
Z11_NO_SPF_FOUND => sub {
__x # ZONE:Z11_NO_SPF_FOUND
'The zone does not publish an SPF policy.', @_;
},
Z11_SPF1_MULTIPLE_RECORDS => sub {
__x # ZONE:Z11_SPF1_MULTIPLE_RECORDS
'The following name servers returned more than one SPF version 1 policy. Name servers: {ns_ip_list}.', @_;
},
Z11_SPF1_SYNTAX_ERROR => sub {
__x # ZONE:Z11_SPF1_SYNTAX_ERROR
'The SPF version 1 policy has a syntax error. Policy retrieved from the following nameservers: {ns_ip_list}.', @_;
},
Z11_SPF1_SYNTAX_OK => sub {
__x # ZONE:Z11_SPF1_SYNTAX_OK
'The SPF version 1 policy has correct syntax.', @_;
},
Z11_UNABLE_TO_CHECK_FOR_SPF => sub {
__x # ZONE:Z11_UNABLE_TO_CHECK_FOR_SPF
'None of the name servers responded with an authoritative response to queries for SPF policies.', @_;
},
);

=over
Expand Down Expand Up @@ -579,6 +625,28 @@ sub _retrieve_record_from_zone {
return;
}

=over
=item _spf_syntax_ok()
_spf_syntax_ok( $spf_string );
Attempts to run L<Mail::SPF::v1::Record/new_from_string($text, %options)> on the provided string.
Takes a string (SPF text).
=back
=cut

sub _spf_syntax_ok {
my $spf = shift;

try {
Mail::SPF::v1::Record->new_from_string($spf);
}
}

=head1 TESTS
=over
Expand Down Expand Up @@ -1397,4 +1465,93 @@ sub zone10 {
return ( @results, _emit_log( TEST_CASE_END => { testcase => $Zonemaster::Engine::Logger::TEST_CASE_NAME } ) )
} ## end sub zone10

=over
=item zone11()
my @logentry_array = zone11( $zone );
Runs the L<Zone11 Test Case|https://github.com/zonemaster/zonemaster/blob/master/docs/public/specifications/tests/Zone-TP/zone11.md>.
Takes a L<Zonemaster::Engine::Zone> object.
Returns a list of L<Zonemaster::Engine::Logger::Entry> objects.
=back
=cut

sub zone11 {
my ( $class, $zone ) = @_;

local $Zonemaster::Engine::Logger::TEST_CASE_NAME = 'Zone11';
push my @results, _emit_log( TEST_CASE_START => { testcase => $Zonemaster::Engine::Logger::TEST_CASE_NAME } );

# This hash maps nameserver IP addresses to arrayrefs of TXT resource
# record data matching the signature for SPF policies. These arrays
# usually contain at most one string.
my %ns_spf = ();

foreach my $ns ( @{ Zonemaster::Engine::TestMethods->method4and5( $zone ) } ) {
if ( _ip_disabled_message( \@results, $ns, q{TXT} ) ) {
next;
}

my $p = $ns->query( $zone->name, q{TXT} );

if ( $p and $p->rcode eq q{NOERROR} and $p->aa ) {
my @txt_rrs = $p->get_records_for_name( q{TXT}, $zone->name );
my @txt_rdata = map { lc $_->txtdata() } @txt_rrs;
my @spf1_policies = grep /\Av=spf1(?:\Z|\s+)/, @txt_rdata;

$ns_spf{$ns->address->short} = \@spf1_policies;
}
}

# At this point, the values of %ns_spf contain *lists* of SPF policies.
# There should be at most one item in each of those lists, but zones may
# mistakenly publish more than one policy.
#
# We can’t use a list of strings directly as a hash key; we need flat
# strings and a conversion method that can disambiguate between
# [qw(a b c)] and [qw(ab c)]. The best method is to prefix each string in
# the list with its length, then concatenate all of these strings
# together. Hence, [qw(a b c)] becomes "<1>a<1>b<1>c" and [qw(ab c)]
# becomes "<2>ab<1>c".
my %spf_ns = ();
for my $ns ( keys %ns_spf ) {
my $mangled_spfs = join '', map { sprintf '<%d>%s', length $_, $_ } sort @{$ns_spf{$ns}};
push @{$spf_ns{$mangled_spfs}}, $ns;
}

if ( not scalar %ns_spf ) {
push @results, _emit_log( Z11_UNABLE_TO_CHECK_FOR_SPF => {} );
}
elsif ( List::MoreUtils::all { $_ eq '' } keys %spf_ns ) {
push @results, _emit_log( Z11_NO_SPF_FOUND => {} );
}
elsif ( scalar keys %spf_ns > 1 ) {
push @results, _emit_log( Z11_INCONSISTENT_SPF_POLICIES => {} );

for my $ns ( values %spf_ns ) {
push @results, _emit_log( Z11_DIFFERENT_SPF_POLICIES_FOUND => { ns_ip_list => join( q{;}, sort @$ns ) } );
}
}
elsif ( my @bad_ns = grep { scalar @{$ns_spf{$_}} > 1 } keys %ns_spf ) {
push @results, _emit_log( Z11_SPF1_MULTIPLE_RECORDS => { ns_ip_list => join( q{;}, sort @bad_ns ) } );
}
else {
my $spf_text = (values %ns_spf)[0][0];

if ( _spf_syntax_ok($spf_text) ) {
push @results, _emit_log( Z11_SPF1_SYNTAX_OK => {} );
}
else {
push @results, _emit_log( Z11_SPF1_SYNTAX_ERROR => { ns_ip_list => join( q{;}, sort (keys %ns_spf) ) } );
}
}

return ( @results, info( TEST_CASE_END => { testcase => $Zonemaster::Engine::Logger::TEST_CASE_NAME } ) )
} ## end sub zone11

1;
12 changes: 10 additions & 2 deletions share/profile.json
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,14 @@
"Z09_NULL_MX_WITH_OTHER_MX" : "WARNING",
"Z09_ROOT_EMAIL_DOMAIN" : "NOTICE",
"Z09_TLD_EMAIL_DOMAIN" : "WARNING",
"Z09_UNEXPECTED_RCODE_MX" : "WARNING"
"Z09_UNEXPECTED_RCODE_MX" : "WARNING",
"Z11_INCONSISTENT_SPF_POLICIES": "WARNING",
"Z11_DIFFERENT_SPF_POLICIES_FOUND": "NOTICE",
"Z11_NO_SPF_FOUND": "NOTICE",
"Z11_SPF1_MULTIPLE_RECORDS": "ERROR",
"Z11_SPF1_SYNTAX_ERROR": "ERROR",
"Z11_SPF1_SYNTAX_OK": "INFO",
"Z11_UNABLE_TO_CHECK_FOR_SPF": "ERROR"
}
},
"test_cases": [
Expand Down Expand Up @@ -584,6 +591,7 @@
"zone07",
"zone08",
"zone09",
"zone10"
"zone10",
"zone11"
]
}
8 changes: 8 additions & 0 deletions share/profile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ test_cases:
- zone08
- zone09
- zone10
- zone11
test_levels:
ADDRESS:
NAMESERVERS_IP_WITH_REVERSE: INFO
Expand Down Expand Up @@ -552,3 +553,10 @@ test_levels:
Z09_ROOT_EMAIL_DOMAIN: NOTICE
Z09_TLD_EMAIL_DOMAIN: WARNING
Z09_UNEXPECTED_RCODE_MX: WARNING
Z11_INCONSISTENT_SPF_POLICIES: WARNING
Z11_DIFFERENT_SPF_POLICIES_FOUND: NOTICE
Z11_NO_SPF_FOUND: NOTICE
Z11_SPF1_MULTIPLE_RECORDS: ERROR
Z11_SPF1_SYNTAX_ERROR: ERROR
Z11_SPF1_SYNTAX_OK: INFO
Z11_UNABLE_TO_CHECK_FOR_SPF: ERROR
32 changes: 32 additions & 0 deletions t/Test-zone11.data

Large diffs are not rendered by default.

Loading

0 comments on commit 40dcce5

Please sign in to comment.