tested and working Authentication-Results

changed the method of saving results. Instead of appending to/from a header, plugins save results to a connection note.

Qpsmtpd::SMTP.pm has a new method that inserts the Authentication-Results header
The smtp-auth information has been removed from the Received header

Authentication-Results providing plugins have been updated to store results in connection note

lib/Qpsmtpd.pm

@@ -7,7 +7,7 @@ use Qpsmtpd::Constants;
 
 #use DashProfiler;
 
-$VERSION = "0.92";
+$VERSION = "0.93";
 
 my $git;
 

lib/Qpsmtpd/Plugin.pm

@@ -272,31 +272,14 @@ sub store_deferred_reject {
 }
 
 sub store_auth_results {
-    my ($self, $value) = @_;
+    my ($self, $result) = @_;
-
+    my $auths = $self->qp->connection->notes('authentication_results') or do {
-    my @headers = $self->transaction->header->get('Authentication-Results');
+        $self->qp->connection->notes('authentication_results', $result);
-    chomp @headers;
+        return;
-    my @deleteme;
-    for ( my $i = 0; $i < scalar @headers; $i++ ) {
-        my @values = split /;/, $headers[$i];
-        if ( $self->config->('me') ne $values[0] ) {  # some other MTA
-# we generally want to remove Authentication-Results headers added by other
-# MTAs (so our downstream can trust the A-R header we insert), but we also
-# don't want to invalidate DKIM signatures.
-# TODO: parse the DKIM signature(s) to see if A-R header is signed
-            if ( ! $self->transaction->header->get('DKIM-Signature') ) {
-                $self->log(LOGINFO, "deleted auth-results from $_");
-                push @deleteme, $i;
-            };
-            next;
         };
-        push @values, $value;
+    my $ar = join('; ', $auths, $result);
-        $self->log(LOGINFO, "appended to auth-results: $value");
+    $self->log(LOGDEBUG, "auth-results: $ar");
-        $self->transaction->header->replace('Authentication->Results', join('; ', @values ), $i);
+    $self->qp->connection->notes('authentication_results', $ar );
-    }
-    foreach ( @deleteme ) {
-        $self->transaction->header->delete('Authentication-Results', $_);
-    };
 };
 
 sub init_resolver {

lib/Qpsmtpd/SMTP.pm

@@ -766,43 +766,6 @@ sub data_respond {
 
     $self->log(LOGDEBUG, "max_size: $max_size / size: $size");
 
-    my $smtp = $self->connection->hello eq "ehlo" ? "ESMTP" : "SMTP";
-    my $esmtp      = substr($smtp, 0, 1) eq "E";
-    my $authheader = '';
-    my $sslheader  = '';
-    my $auth_result = 'none';
-
-    if (defined $self->connection->notes('tls_enabled')
-        and $self->connection->notes('tls_enabled'))
-    {
-        $smtp .= "S" if $esmtp;    # RFC3848
-        $sslheader = "("
-          . $self->connection->notes('tls_socket')->get_cipher()
-          . " encrypted) ";
-    }
-
-    if (defined $self->{_auth} ) {
-        my $mech = $self->{_auth_mechanism};
-        my $user = $self->{_auth_user};
-        $auth_result = "auth=";
-        if ( $self->{_auth} == OK) {
-            $smtp .= "A" if $esmtp;    # RFC3848
-            $authheader = "(smtp-auth username $user, mechanism $mech)\n";
-            $auth_result .= 'pass';
-        }
-        else {
-            $auth_result .= 'fail';
-        };
-        $auth_result .= " ($mech) smtp.auth=$user";
-    }
-
-    $header->add('Received',
-                 $self->received_line($smtp, $authheader, $sslheader), 0);
-
-    # RFC 5451: used in AUTH, DKIM, DOMAINKEYS, SENDERID, SPF
-    $header->add('Authentication-Results',
-                join('; ', $self->config('me'), $auth_result ) );
-
     # if we get here without seeing a terminator, the connection is
     # probably dead.
     unless ($complete) {
@@ -823,8 +786,75 @@ sub data_respond {
     $self->run_hooks("data_post");
 }
 
+sub authentication_results {
+    my ($self) = @_;
+
+    my @auth_list = $self->config('me');
+#   $self->clean_authentication_results();
+
+    if ( ! defined $self->{_auth} ) {
+        push @auth_list, 'auth=none';
+    }
+    else {
+        my $mechanism = "(" . $self->{_auth_mechanism} . ")";
+        my $user = "smtp.auth=" . $self->{_auth_user};
+        if ( $self->{_auth} == OK) {
+            push @auth_list, "auth=pass $mechanism $user";
+        }
+        else {
+            push @auth_list, "auth=fail $mechanism $user";
+        };
+    };
+
+    # RFC 5451: used in AUTH, DKIM, DOMAINKEYS, SENDERID, SPF
+    if ( $self->connection->notes('authentication_results') ) {
+        push @auth_list, $self->connection->notes('authentication_results');
+    };
+
+    $self->log(LOGDEBUG, "adding auth results header" );
+    $self->transaction->header->add('Authentication-Results', join('; ', @auth_list) );
+};
+
+sub clean_authentication_results {
+    my $self = shift;
+
+# On messages received from the internet, we may want to remove
+# the Authentication-Results headers added by other MTAs, so our downstream
+# can trust the new A-R header we insert.
+# We do not want to invalidate DKIM signatures.
+# TODO: parse the DKIM signature(s) to see if A-R header is signed
+    return if $self->transaction->header->get('DKIM-Signature');
+
+    my @headers = $self->transaction->header->get('Authentication-Results');
+    for ( my $i = 0; $i < scalar @headers; $i++ ) {
+        $self->transaction->header->delete('Authentication-Results', $i);
+    }
+};
+
 sub received_line {
-    my ($self, $smtp, $authheader, $sslheader) = @_;
+    my ($self) = @_;
+
+    my $smtp = $self->connection->hello eq "ehlo" ? "ESMTP" : "SMTP";
+    my $esmtp      = substr($smtp, 0, 1) eq "E";
+    my $authheader = '';
+    my $sslheader  = '';
+
+    if (defined $self->connection->notes('tls_enabled')
+        and $self->connection->notes('tls_enabled'))
+    {
+        $smtp .= "S" if $esmtp;    # RFC3848
+        $sslheader = "("
+          . $self->connection->notes('tls_socket')->get_cipher()
+          . " encrypted) ";
+    }
+    if (defined $self->{_auth} && $self->{_auth} == OK) {
+        my $mech = $self->{_auth_mechanism};
+        my $user = $self->{_auth_user};
+        $smtp .= "A" if $esmtp;    # RFC3848
+        $authheader = "(smtp-auth username $user, mechanism $mech)\n";
+    }
+
+    my $header_str;
     my ($rc, @received) =
       $self->run_hooks("received_line", $smtp, $authheader, $sslheader);
     if ($rc == YIELD) {
@@ -834,7 +864,7 @@ sub received_line {
         return join("\n", @received);
     }
     else {    # assume $rc == DECLINED
-        return
+        $header_str = 
             "from "
           . $self->connection->remote_info
           . " (HELO "
@@ -847,6 +877,7 @@ sub received_line {
           . ") with $sslheader$smtp; "
           . (strftime('%a, %d %b %Y %H:%M:%S %z', localtime));
     }
+    $self->transaction->header->add('Received', $header_str, 0 );
 }
 
 sub data_post_respond {
@@ -881,6 +912,8 @@ sub data_post_respond {
         return 1;
     }
     else {
+        $self->authentication_results();
+        $self->received_line();
         $self->queue($self->transaction);
     }
 }

plugins/dkim

@@ -222,7 +222,11 @@ sub validate_it {
     my $result = $dkim->result;
     my $mess   = $self->get_details($dkim);
 
-    $self->store_auth_results("dkim=" .$dkim->result_detail . " header.i=@".$dkim->signature->domain);
+    my $auth_str = "dkim=" .$dkim->result_detail;
+    if ( $dkim->signature && $dkim->signature->domain ) {
+        $auth_str .= " header.i=@" . $dkim->signature->domain;
+    };
+    $self->store_auth_results( $auth_str );
     #$self->add_header($mess);
 
     foreach my $t (qw/ pass fail invalid temperror none /) {

plugins/domainkeys

@@ -43,7 +43,9 @@ the same terms as Perl itself.
 
 =head1 AUTHORS
 
- Matt Simerson    - 2012
+ Matt Simerson    - 2013 - safe results to Authentication-Results header
+                           instead of DomainKey-Status
+ Matt Simerson    - 2012 - refactored, added tests, safe loading
  John Peacock     - 2005-2006
  Anthony D. Urso. - 2004
 
@@ -113,7 +115,8 @@ sub data_post_handler {
     my $status = $self->get_message_status($message);
 
     if (defined $status) {
-        $transaction->header->add("DomainKey-Status", $status, 0);
+#$transaction->header->add("DomainKey-Status", $status, 0);
+        $self->store_auth_results('domainkey=' . $status);
         $self->log(LOGINFO, "pass, $status");
         return DECLINED;
     }

plugins/fcrdns

@@ -119,8 +119,6 @@ RCODE of 3, commonly known as NXDOMAIN, or an RCODE of 0 (NOERROR)
 in a reply containing no answers, was returned.  This prevented
 completion of the evaluation.
 
-=cut
-
 =head1 AUTHOR
 
 2013 - Matt Simerson
@@ -146,7 +144,6 @@ sub register {
     $self->init_resolver() or return;
 
     $self->register_hook('connect',   'connect_handler');
-    $self->register_hook('data_post', 'data_post_handler');
 }
 
 sub connect_handler {
@@ -166,13 +163,6 @@ sub connect_handler {
     return DECLINED;
 }
 
-sub data_post_handler {
-    my ($self, $transaction) = @_;
-    my $match = $self->connection->notes('fcrdns_match') || 'error';
-    $self->store_auth_results("iprev=$match");
-    return (DECLINED);
-}
-
 sub invalid_localhost {
     my ($self) = @_;
     return 1 if lc $self->qp->connection->remote_host ne 'localhost';
@@ -216,20 +206,20 @@ sub has_reverse_dns {
     my $query = $res->query($ip, 'PTR') or do {
         if ($res->errorstring eq 'NXDOMAIN') {
             $self->adjust_karma(-1);
-            $self->connection->notes('fcrdns_match', 'permerror');
+            $self->store_auth_results("iprev=permerror");
             $self->log(LOGINFO, "fail, no rDNS: " . $res->errorstring);
             return;
         }
         if ( $res->errorstring eq 'SERVFAIL' ) {
             $self->log(LOGINFO, "fail, error getting rDNS: " . $res->errorstring);
-            $self->connection->notes('fcrdns_match', 'temperror');
+            $self->store_auth_results("iprev=temperror");
         }
         elsif ( $res->errorstring eq 'NOERROR' ) {
             $self->log(LOGINFO, "fail, no PTR (NOERROR)" );
-            $self->connection->notes('fcrdns_match', 'permerror');
+            $self->store_auth_results("iprev=permerror");
         }
         else {
-            $self->connection->notes('fcrdns_match', 'fail');
+            $self->store_auth_results("iprev=fail");
             $self->log(LOGINFO, "fail, error getting rDNS: " . $res->errorstring);
         };
         return;
@@ -246,7 +236,7 @@ sub has_reverse_dns {
     if (!$hits) {
         $self->adjust_karma(-1);
         $self->log(LOGINFO, "fail, no PTR records");
-        $self->connection->notes('fcrdns_match', 'permerror');
+        $self->store_auth_results("iprev=permerror");
         return;
     }
 
@@ -264,11 +254,11 @@ sub has_forward_dns {
         $host .= '.' if '.' ne substr($host, -1, 1);    # fully qualify name
         my $query = $res->query($host) or do {
             if ($res->errorstring eq 'NXDOMAIN') {
-                $self->connection->notes('fcrdns_match', 'permerror');
+                $self->store_auth_results("iprev=permerror");
                 $self->log(LOGDEBUG, "host $host does not exist");
                 next;
             }
-            $self->connection->notes('fcrdns_match', 'fail');
+            $self->store_auth_results("iprev=fail");
             $self->log(LOGDEBUG, "query for $host failed (",
                        $res->errorstring, ")");
             next;
@@ -281,13 +271,13 @@ sub has_forward_dns {
             $self->check_ip_match($rr->address) and return 1;
         }
         if ($hits) {
-            $self->connection->notes('fcrdns_match', 'fail');
+            $self->store_auth_results("iprev=fail");
             $self->log(LOGDEBUG, "PTR host has forward DNS") if $hits;
             return 1;
         }
     }
     $self->adjust_karma(-1);
-    $self->connection->notes('fcrdns_match', 'fail');
+    $self->store_auth_results("iprev=fail");
     $self->log(LOGINFO, "fail, no PTR hosts have forward DNS");
     return;
 }
@@ -298,7 +288,7 @@ sub check_ip_match {
 
     if ($ip eq $self->qp->connection->remote_ip) {
         $self->log(LOGDEBUG, "forward ip match");
-        $self->connection->notes('fcrdns_match', 'pass');
+        $self->store_auth_results("iprev=pass");
         $self->adjust_karma(1);
         return 1;
     }
@@ -310,7 +300,7 @@ sub check_ip_match {
 
     if ($dns_net eq $rem_net) {
         $self->log(LOGNOTICE, "forward network match");
-        $self->connection->notes('fcrdns_match', 'pass');
+        $self->store_auth_results("iprev=pass");
         return 1;
     }
     return;