You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by mm...@apache.org on 2015/01/27 20:05:45 UTC
svn commit: r1655106 - in /spamassassin/trunk:
lib/Mail/SpamAssassin/Plugin/DKIM.pm t/dkim.t
Author: mmartinec
Date: Tue Jan 27 19:05:45 2015
New Revision: 1655106
URL: http://svn.apache.org/r1655106
Log:
Bug 7124 - DKIM: RFC 6376 - Signers MUST use RSA keys of at least 1024 bits
Modified:
spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm
spamassassin/trunk/t/dkim.t
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm?rev=1655106&r1=1655105&r2=1655106&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm Tue Jan 27 19:05:45 2015
@@ -360,6 +360,24 @@ Example:
score DKIM_ADSP_CUSTOM_MED 3.5
score DKIM_ADSP_CUSTOM_HIGH 8
+
+=item dkim_minimum_key_bits n (default: 1024)
+
+The smallest size of a signing key (in bits) for a valid signature to be
+considered for whitelisting. Additionally, the eval function check_dkim_valid()
+will return false on short keys when called with explicitly listed domains,
+and the eval function check_dkim_valid_author_sig() will return false on short
+keys (regardless of its arguments). Setting the option to 0 disables a key
+size check.
+
+Note that the option has no effect when the eval function check_dkim_valid()
+is called with no arguments (like in a rule DKIM_VALID). A mere presence of
+some valid signature on a message has no reputational value (without being
+associated with a particular domain), regardless of its key size - anyone can
+prepend its own signature on a copy of some third party mail and re-send it,
+which makes it no more trustworthy than without such signature. This is also
+a reason for a rule DKIM_VALID to have a near-zero score.
+
=cut
push (@cmds, {
@@ -450,6 +468,13 @@ Example:
}
});
+ # minimal signing key size in bits that is acceptable for whitelisting
+ push (@cmds, {
+ setting => 'dkim_minimum_key_bits',
+ default => 1024,
+ type => $Mail::SpamAssassin::Conf::CONF_TYPE_NUMERIC,
+ });
+
=back
=head1 ADMINISTRATOR SETTINGS
@@ -500,7 +525,8 @@ sub check_dkim_valid {
if (!$pms->{dkim_valid}) {
# don't bother
} elsif (!@acceptable_domains) {
- $result = 1; # no additional constraints, any signing domain will do
+ $result = 1; # no additional constraints, any signing domain will do,
+ # also any signing key size will do
} else {
$result = $self->_check_dkim_signed_by($pms,1,0,\@acceptable_domains);
}
@@ -513,8 +539,6 @@ sub check_dkim_valid_author_sig {
my $result = 0;
if (!%{$pms->{dkim_has_valid_author_sig}}) {
# don't bother
- } elsif (!@acceptable_domains) {
- $result = 1; # no additional constraints, any signing domain will do
} else {
$result = $self->_check_dkim_signed_by($pms,1,1,\@acceptable_domains);
}
@@ -657,6 +681,7 @@ sub _check_dkim_signed_by {
$acceptable_domains_ref) = @_;
my $result = 0;
my $verifier = $pms->{dkim_verifier};
+ my $minimum_key_bits = $pms->{conf}->{dkim_minimum_key_bits};
foreach my $sig (@{$pms->{dkim_signatures}}) {
next if !defined $sig;
if ($must_be_valid) {
@@ -664,6 +689,8 @@ sub _check_dkim_signed_by {
->result ne 'pass';
next if $sig->UNIVERSAL::can("check_expiration") &&
!$sig->check_expiration;
+ next if $minimum_key_bits && $sig->{_spamassassin_key_size} &&
+ $sig->{_spamassassin_key_size} < $minimum_key_bits;
}
my $sdid = $sig->domain;
next if !defined $sdid; # a signature with a missing required tag 'd' ?
@@ -671,12 +698,16 @@ sub _check_dkim_signed_by {
if ($must_be_author_domain_signature) {
next if !$pms->{dkim_author_domains}->{$sdid};
}
- foreach my $ad (@$acceptable_domains_ref) {
- if ($ad =~ /^\*?\.(.*)\z/s) { # domain itself or its subdomain
- my $d = lc $1;
- if ($sdid eq $d || $sdid =~ /\.\Q$d\E\z/s) { $result = 1; last }
- } else { # match on domain (not a subdomain)
- if ($sdid eq lc $ad) { $result = 1; last }
+ if (!@$acceptable_domains_ref) {
+ $result = 1;
+ } else {
+ foreach my $ad (@$acceptable_domains_ref) {
+ if ($ad =~ /^\*?\.(.*)\z/s) { # domain itself or its subdomain
+ my $d = lc $1;
+ if ($sdid eq $d || $sdid =~ /\.\Q$d\E\z/s) { $result = 1; last }
+ } else { # match on domain (not a subdomain)
+ if ($sdid eq lc $ad) { $result = 1; last }
+ }
}
}
last if $result;
@@ -705,7 +736,9 @@ sub _get_authors {
sub _check_dkim_signature {
my ($self, $pms) = @_;
+ my $conf = $pms->{conf};
my($verifier, @signatures, @valid_signatures);
+
$pms->{dkim_checked_signature} = 1; # has this sub already been invoked?
$pms->{dkim_signatures_ready} = 0; # have we obtained & verified signatures?
$pms->{dkim_signatures_dependable} = 0;
@@ -747,7 +780,7 @@ sub _check_dkim_signature {
my $timemethod = $self->{main}->UNIVERSAL::can("time_method") &&
$self->{main}->time_method("check_dkim_signature");
if (Mail::DKIM::Verifier->VERSION >= 0.40) {
- my $edns = $pms->{conf}->{dns_options}->{edns};
+ my $edns = $conf->{dns_options}->{edns};
if ($edns && $edns >= 1024) {
# Let Mail::DKIM use our interface to Net::DNS::Resolver.
# Only do so if EDNS0 provides a reasonably-sized UDP payload size,
@@ -780,7 +813,7 @@ sub _check_dkim_signature {
return 0; # cannot verify message
};
- my $timeout = $pms->{conf}->{dkim_timeout};
+ my $timeout = $conf->{dkim_timeout};
my $timer = Mail::SpamAssassin::Timeout->new(
{ secs => $timeout, deadline => $pms->{master_deadline} });
@@ -815,26 +848,41 @@ sub _check_dkim_signature {
if ($pms->{dkim_signatures_ready}) {
my $sig_result_supported;
+ my $minimum_key_bits = $conf->{dkim_minimum_key_bits};
foreach my $signature (@signatures) {
# old versions of Mail::DKIM would give undef for an invalid signature
next if !defined $signature;
+
$sig_result_supported = $signature->UNIVERSAL::can("result_detail");
- my $valid =
+ my($info, $valid, $expired);
+ $valid =
($sig_result_supported ? $signature : $verifier)->result eq 'pass';
- my $expired = 0;
+ $info = $valid ? 'VALID' : 'FAILED';
if ($valid && $signature->UNIVERSAL::can("check_expiration")) {
$expired = !$signature->check_expiration;
+ $info .= ' EXPIRED' if $expired;
+ }
+ my $key_size;
+ if ($valid && !$expired && $minimum_key_bits) {
+ $key_size = eval { my $pk = $signature->get_public_key;
+ $pk && $pk->cork && $pk->cork->size * 8 };
+ if ($key_size) {
+ $signature->{_spamassassin_key_size} = $key_size; # stash it for later
+ $info .= " WEAK($key_size)" if $key_size < $minimum_key_bits;
+ }
}
push(@valid_signatures, $signature) if $valid && !$expired;
+
# check if we have a potential Author Domain Signature, valid or not
my $d = $signature->domain;
if (!defined $d) {
- # can be undefined on a broken signatures with missing required tags
+ # can be undefined on a broken signature with missing required tags
} else {
$d = lc $d;
if ($pms->{dkim_author_domains}->{$d}) { # SDID matches author domain
$pms->{dkim_has_any_author_sig}->{$d} = 1;
- if ($valid && !$expired) {
+ if ($valid && !$expired &&
+ $key_size && $key_size >= $minimum_key_bits) {
$pms->{dkim_has_valid_author_sig}->{$d} = 1;
} elsif ( ($sig_result_supported ? $signature
: $verifier)->result_detail
@@ -844,16 +892,17 @@ sub _check_dkim_signature {
}
}
if (would_log("dbg","dkim")) {
- dbg("dkim: %s, i=%s, d=%s, s=%s, a=%s, c=%s, %s%s, %s",
- map { !defined $_ ? '(undef)' : $_ }
+ dbg("dkim: %s %s, i=%s, d=%s, s=%s, a=%s, c=%s, %s, %s",
+ $info,
$signature->isa('Mail::DKIM::DkSignature') ? 'DK' : 'DKIM',
- $signature->identity, $d, $signature->selector,
- $signature->algorithm, scalar($signature->canonicalization),
- ($sig_result_supported ? $signature : $verifier)->result,
- !$expired ? '' : ', expired',
+ map(!defined $_ ? '(undef)' : $_,
+ $signature->identity, $d, $signature->selector,
+ $signature->algorithm, scalar($signature->canonicalization),
+ $key_size ? "key_bits=$key_size" : (),
+ ($sig_result_supported ? $signature : $verifier)->result ),
defined $d && $pms->{dkim_author_domains}->{$d}
? 'matches author domain'
- : 'does not match author domain'
+ : 'does not match author domain',
);
}
}
@@ -1149,24 +1198,32 @@ sub _wlcheck_list {
my %any_match_by_wl;
my $any_match_at_all = 0;
my $verifier = $pms->{dkim_verifier};
+ my $minimum_key_bits = $pms->{conf}->{dkim_minimum_key_bits};
# walk through all signatures present in a message
foreach my $signature (@{$pms->{dkim_signatures}}) {
# old versions of Mail::DKIM would give undef for an invalid signature
next if !defined $signature;
+
my $sig_result_supported = $signature->UNIVERSAL::can("result_detail");
- my $valid =
+ my($info, $valid, $expired, $key_size_weak);
+ $valid =
($sig_result_supported ? $signature : $verifier)->result eq 'pass';
- my $expired = 0;
+ $info = $valid ? 'VALID' : 'FAILED';
if ($valid && $signature->UNIVERSAL::can("check_expiration")) {
$expired = !$signature->check_expiration;
+ $info .= ' EXPIRED' if $expired;
+ }
+ if ($valid && !$expired && $minimum_key_bits) {
+ my $key_size = $signature->{_spamassassin_key_size};
+ if ($key_size && $key_size < $minimum_key_bits) {
+ $info .= " WEAK($key_size)"; $key_size_weak = 1;
+ }
}
+
my $sdid = $signature->domain;
$sdid = lc $sdid if defined $sdid;
- my $info = $valid ? 'VALID' : 'FAILED';
- $info .= ' EXPIRED' if $expired;
-
my %tried_authors;
foreach my $entry (@$acceptable_sdid_tuples_ref) {
my($author, $acceptable_sdid, $wl, $re) = @$entry;
@@ -1218,7 +1275,7 @@ sub _wlcheck_list {
$any_match_by_wl{$wl} = '' if !exists $any_match_by_wl{$wl};
}
# only valid signature can cause whitelisting
- $matches = 0 if !$valid || $expired;
+ $matches = 0 if !$valid || $expired || $key_size_weak;
if ($matches) {
$any_match_at_all = 1;
Modified: spamassassin/trunk/t/dkim.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/dkim.t?rev=1655106&r1=1655105&r2=1655106&view=diff
==============================================================================
--- spamassassin/trunk/t/dkim.t (original)
+++ spamassassin/trunk/t/dkim.t Tue Jan 27 19:05:45 2015
@@ -89,6 +89,7 @@ sub test_samples($$) {
# ensure rules will fire, and disable some expensive ones
tstlocalrules("
+ dkim_minimum_key_bits 512
score DKIM_SIGNED -0.1
score DKIM_VALID -0.1
score DKIM_VALID_AU -0.1