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.

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

Provide implementation for Zone11 (SPF test) #1287

Merged
merged 7 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -301,6 +301,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} );
marc-vanderwal marked this conversation as resolved.
Show resolved Hide resolved

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
tgreenx marked this conversation as resolved.
Show resolved Hide resolved
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
Loading