From a6b563a40c0319eb2d929c4e780600d91faeeaa1 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Thu, 2 May 2013 03:30:48 -0400 Subject: [PATCH] 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 | 2 +- lib/Qpsmtpd/Plugin.pm | 31 +++--------- lib/Qpsmtpd/SMTP.pm | 111 +++++++++++++++++++++++++++--------------- plugins/dkim | 6 ++- plugins/domainkeys | 7 ++- plugins/fcrdns | 32 +++++------- 6 files changed, 101 insertions(+), 88 deletions(-) diff --git a/lib/Qpsmtpd.pm b/lib/Qpsmtpd.pm index da36d68..fc41789 100644 --- a/lib/Qpsmtpd.pm +++ b/lib/Qpsmtpd.pm @@ -7,7 +7,7 @@ use Qpsmtpd::Constants; #use DashProfiler; -$VERSION = "0.92"; +$VERSION = "0.93"; my $git; diff --git a/lib/Qpsmtpd/Plugin.pm b/lib/Qpsmtpd/Plugin.pm index 177d237..9693524 100644 --- a/lib/Qpsmtpd/Plugin.pm +++ b/lib/Qpsmtpd/Plugin.pm @@ -272,31 +272,14 @@ sub store_deferred_reject { } sub store_auth_results { - my ($self, $value) = @_; - - my @headers = $self->transaction->header->get('Authentication-Results'); - chomp @headers; - 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; + my ($self, $result) = @_; + my $auths = $self->qp->connection->notes('authentication_results') or do { + $self->qp->connection->notes('authentication_results', $result); + return; }; - push @values, $value; - $self->log(LOGINFO, "appended to auth-results: $value"); - $self->transaction->header->replace('Authentication->Results', join('; ', @values ), $i); - } - foreach ( @deleteme ) { - $self->transaction->header->delete('Authentication-Results', $_); - }; + my $ar = join('; ', $auths, $result); + $self->log(LOGDEBUG, "auth-results: $ar"); + $self->qp->connection->notes('authentication_results', $ar ); }; sub init_resolver { diff --git a/lib/Qpsmtpd/SMTP.pm b/lib/Qpsmtpd/SMTP.pm index fd7c44d..e9f857c 100644 --- a/lib/Qpsmtpd/SMTP.pm +++ b/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); } } diff --git a/plugins/dkim b/plugins/dkim index 1866b25..39049dc 100644 --- a/plugins/dkim +++ b/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 /) { diff --git a/plugins/domainkeys b/plugins/domainkeys index eac7abb..5b9a33b 100644 --- a/plugins/domainkeys +++ b/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; } diff --git a/plugins/fcrdns b/plugins/fcrdns index 2cc2009..53b62c2 100644 --- a/plugins/fcrdns +++ b/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;