1.005 - fix in handling invalid DKIM signature headers: don't wait fo…

…r more data
noxxi · Feb 28, 2019 · 56feb12 · 56feb12
1 parent ddbcdac
commit 56feb12
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 17 deletions.
diff --git a/Changes b/Changes
@@ -1,3 +1,5 @@
+1.005 - 2019/02/28
+- fix in handling invalid DKIM signature headers: don't wait for more data
 1.004 - 2019/02/20
 - rename DKIM_* constants and description according to RFC 7601 2.7.1
   DKIM_SUCCESS is now DKIM_PASS etc

diff --git a/lib/Mail/DKIM/Iterator.pm b/lib/Mail/DKIM/Iterator.pm
@@ -1,7 +1,7 @@
 package Mail::DKIM::Iterator;
 use v5.10.0;
 
-our $VERSION = '1.004';
+our $VERSION = '1.005';
 
 use strict;
 use warnings;
@@ -191,6 +191,18 @@ sub _compute_result {
 	    next;
 	}
 
+	if ($sig->{error}) {
+	    # something wrong with the DKIM-Signature header, return error
+	    push @rv, $sig->{':result'} =
+		Mail::DKIM::Iterator::VerifyRecord->new(
+		    $sig,
+		    ($sig->{s}//'UNKNOWN')."_domainkey".($sig->{d}//'UNKNOWN'),
+		    DKIM_PERMERROR,
+		    $sig->{error}
+		);
+	    next;
+	}
+
 	if (!$sig->{b}) {
 	    # sig is not for verification but for signing
 	    if (!$sig->{'bh:computed'}) {
@@ -209,18 +221,6 @@ sub _compute_result {
 	    next;
 	}
 
-	if ($sig->{error}) {
-	    # something wrong with the DKIM-Signature header, return error
-	    push @rv, $sig->{':result'} =
-		Mail::DKIM::Iterator::VerifyRecord->new(
-		    $sig,
-		    ($sig->{s}//'UNKNOWN')."_domainkey".($sig->{d}//'UNKNOWN'),
-		    DKIM_PERMERROR,
-		    $sig->{error}
-		);
-	    next;
-	}
-
 	my $dns = "$sig->{s}._domainkey.$sig->{d}";
 
 	if ($sig->{x} && $sig->{x} < time()) {
@@ -862,7 +862,7 @@ sub authentication_results {
     return if ! $self->[2];
     my $ar = "dkim=$self->[2]";
     $ar .= " ($self->[3])" if defined $self->[3] and $self->[3] ne '';
-    $ar .= " header.d=".$self->[0]{d};
+    $ar .= " header.d=".( $self->[0]{d} // 'unknown');
     return $ar;
 }
 

diff --git a/scripts/validate-emails.pl b/scripts/validate-emails.pl
@@ -56,7 +56,8 @@ sub usage {
 	my $todo = shift(@todo);
 	if (ref($todo)) {
 	    # need more data from mail
-	    $buf //= $mbox->nextdata // die "no more data from mail";
+	    $buf //= $mbox->nextdata
+		// die "need more data but no more data available in mail";
 	    (undef,@todo) = $dkim->next($buf);
 	    $buf = undef;
 	} else {
@@ -78,7 +79,7 @@ sub usage {
 
     for(@{$dkim->result || []}) {
 	my $status = $_->status;
-	my $domain = $_->domain;
+	my $domain = $_->domain // 'unknown';
 	if (!defined $status) {
 	    print STDERR " unkown $domain\n";
 	} else {

diff --git a/t/sign-and-verify.t b/t/sign-and-verify.t
@@ -3,7 +3,7 @@ use warnings;
 use Test::More;
 use Mail::DKIM::Iterator;
 
-plan tests => 18;
+plan tests => 19;
 
 # basic tests with different canonicalizations and algorithms
 for my $c (qw(
@@ -99,6 +99,17 @@ for my $c (qw(
 	"DKIM key invalid syntax");
 }
 
+# expect verification perm-fail because DKIM key has invalid syntax
+{
+    my $ok = eval {
+	my $m = sign([mail()], v => '2' );
+	verify([$m],dns());
+    };
+    my $err = $@ || ($ok ? '':'unknown error');
+    is( $err,"status status=permerror error=invalid DKIM-Signature header: bad DKIM signature version: 2 a=rsa-sha256\n",
+	"DKIM signature invalid syntax");
+}
+
 
 ############################################################################
 # functions
@@ -108,6 +119,7 @@ for my $c (qw(
 sub sign {
     my ($mail,%args) = @_;
     push @$mail,'';
+    my $v = delete $args{v};
     my $dkim = Mail::DKIM::Iterator->new( sign => {
 	d => 'example.com',
 	s => 'good',
@@ -135,6 +147,7 @@ sub sign {
     $rv->[0]->status == DKIM_PASS
 	or die "unexpected status ".( $rv->[0]->status // '<undef>' )."\n";
     my $dkim_sig = $rv->[0]->signature;
+    $dkim_sig =~s{\bv=1;}{v=$v} if defined $v;
     return $dkim_sig . $total_mail;
 }