diff --git a/lib/Qpsmtpd/Plugin.pm b/lib/Qpsmtpd/Plugin.pm index 026ffc3..177d237 100644 --- a/lib/Qpsmtpd/Plugin.pm +++ b/lib/Qpsmtpd/Plugin.pm @@ -271,6 +271,34 @@ sub store_deferred_reject { return (DECLINED); } +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; + }; + 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', $_); + }; +}; + sub init_resolver { my $self = shift; my $timeout = $self->{_args}{dns_timeout} || shift || 5; diff --git a/lib/Qpsmtpd/SMTP.pm b/lib/Qpsmtpd/SMTP.pm index b5bb500..fd7c44d 100644 --- a/lib/Qpsmtpd/SMTP.pm +++ b/lib/Qpsmtpd/SMTP.pm @@ -770,6 +770,7 @@ sub data_respond { 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')) @@ -780,15 +781,28 @@ sub data_respond { . " encrypted) "; } - if (defined $self->{_auth} and $self->{_auth} == OK) { - $smtp .= "A" if $esmtp; # RFC3848 - $authheader = -"(smtp-auth username $self->{_auth_user}, mechanism $self->{_auth_mechanism})\n"; + 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", + $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) { diff --git a/plugins/dkim b/plugins/dkim index 13815a1..1866b25 100644 --- a/plugins/dkim +++ b/plugins/dkim @@ -222,6 +222,9 @@ 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); + #$self->add_header($mess); + foreach my $t (qw/ pass fail invalid temperror none /) { next if $t ne $result; my $handler = 'handle_sig_' . $t; @@ -286,8 +289,7 @@ sub handle_sig_fail { my ($self, $dkim, $mess) = @_; $self->adjust_karma(-1); - return - $self->get_reject("DKIM signature invalid: " . $dkim->result_detail, + return $self->get_reject("signature invalid: " . $dkim->result_detail, $mess); } @@ -316,12 +318,10 @@ sub handle_sig_invalid { $self->log(LOGINFO, $mess); if ($prs->{accept}) { - $self->add_header($mess); $self->log(LOGERROR, "error, invalid signature but accept policy!?"); return DECLINED; } elsif ($prs->{neutral}) { - $self->add_header($mess); $self->log(LOGERROR, "error, invalid signature but neutral policy?!"); return DECLINED; } @@ -333,7 +333,6 @@ sub handle_sig_invalid { # this should never happen $self->log(LOGINFO, "error, invalid signature, unhandled"); - $self->add_header($mess); return DECLINED; } @@ -527,8 +526,6 @@ sub get_selector { sub add_header { my $self = shift; my $header = shift or return; - - # consider adding Authentication-Results header, (RFC 5451) $self->qp->transaction->header->add('X-DKIM-Authentication', $header, 0); } diff --git a/plugins/dmarc b/plugins/dmarc index 3f5eab8..cd40ec0 100644 --- a/plugins/dmarc +++ b/plugins/dmarc @@ -122,24 +122,31 @@ sub data_post_handler { # evaluation returned a "pass" result. my $spf_dom = $transaction->notes('spf_pass_host'); + my $effective_policy = ( $self->{_args}{is_subdomain} && defined $policy->{sp} ) + ? $policy->{sp} : $policy->{p}; + # 5. Conduct identifier alignment checks. - return DECLINED - if $self->is_aligned($from_dom, $org_dom, $policy, $spf_dom ); + if ( $self->is_aligned($from_dom, $org_dom, $policy, $spf_dom ) ) { + $self->store_auth_results("dmarc=pass (p=$effective_policy) d=$from_dom"); + return DECLINED; + }; # 6. Apply policy. Emails that fail the DMARC mechanism check are # disposed of in accordance with the discovered DMARC policy of the # Domain Owner. See Section 6.2 for details. - if ( $self->{_args}{is_subdomain} && defined $policy->{sp} ) { - return DECLINED if lc $policy->{sp} eq 'none'; + if ( lc $effective_policy eq 'none' ) { + $self->store_auth_results("dmarc=fail (p=none) d=$from_dom"); + return DECLINED; }; - return DECLINED if lc $policy->{p} eq 'none'; my $pct = $policy->{pct} || 100; if ( $pct != 100 && int(rand(100)) >= $pct ) { $self->log("fail, tolerated, policy, sampled out"); + $self->store_auth_results("dmarc=sampled_out (p=$effective_policy) d=$from_dom"); return DECLINED; }; + $self->store_auth_results("dmarc=fail (p=$effective_policy) d=$from_dom"); return $self->get_reject("failed DMARC policy"); } diff --git a/plugins/fcrdns b/plugins/fcrdns index b8190e4..2cc2009 100644 --- a/plugins/fcrdns +++ b/plugins/fcrdns @@ -6,7 +6,7 @@ Forward Confirmed RDNS - http://en.wikipedia.org/wiki/FCrDNS =head1 DESCRIPTION -Determine if the SMTP sender has matching forward and reverse DNS. +Determine if the SMTP sender has matching forward and reverse DNS. Sets the connection note fcrdns. @@ -88,6 +88,38 @@ From Wikipedia summary: 3. Any A or AAAA record returned by the second query is then compared against the original IP address (check_ip_match), and if there is a match, then the FCrDNS check passes. +=head1 iprev + +# https://www.ietf.org/rfc/rfc5451.txt + +2.4.3. "iprev" Results + +The result values are used by the "iprev" method, defined in +Section 3, are as follows: + +pass: The DNS evaluation succeeded, i.e., the "reverse" and +"forward" lookup results were returned and were in agreement. + +fail: The DNS evaluation failed. In particular, the "reverse" and +"forward" lookups each produced results but they were not in +agreement, or the "forward" query completed but produced no +result, e.g., a DNS RCODE of 3, commonly known as NXDOMAIN, or an +RCODE of 0 (NOERROR) in a reply containing no answers, was +returned. + +temperror: The DNS evaluation could not be completed due to some +error that is likely transient in nature, such as a temporary DNS +error, e.g., a DNS RCODE of 2, commonly known as SERVFAIL, or +other error condition resulted. A later attempt may produce a +final result. + +permerror: The DNS evaluation could not be completed because no PTR +data are published for the connecting IP address, e.g., a DNS +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 @@ -136,9 +168,8 @@ sub connect_handler { sub data_post_handler { my ($self, $transaction) = @_; - - my $match = $self->connection->notes('fcrdns_match') || 0; - $transaction->header->add('X-Fcrdns', $match ? 'Yes' : 'No', 0); + my $match = $self->connection->notes('fcrdns_match') || 'error'; + $self->store_auth_results("iprev=$match"); return (DECLINED); } @@ -182,13 +213,25 @@ sub has_reverse_dns { my $res = $self->init_resolver(); my $ip = $self->qp->connection->remote_ip; - my $query = $res->query($ip) or do { + my $query = $res->query($ip, 'PTR') or do { if ($res->errorstring eq 'NXDOMAIN') { $self->adjust_karma(-1); + $self->connection->notes('fcrdns_match', 'permerror'); $self->log(LOGINFO, "fail, no rDNS: " . $res->errorstring); return; } - $self->log(LOGINFO, "fail, error getting rDNS: " . $res->errorstring); + if ( $res->errorstring eq 'SERVFAIL' ) { + $self->log(LOGINFO, "fail, error getting rDNS: " . $res->errorstring); + $self->connection->notes('fcrdns_match', 'temperror'); + } + elsif ( $res->errorstring eq 'NOERROR' ) { + $self->log(LOGINFO, "fail, no PTR (NOERROR)" ); + $self->connection->notes('fcrdns_match', 'permerror'); + } + else { + $self->connection->notes('fcrdns_match', 'fail'); + $self->log(LOGINFO, "fail, error getting rDNS: " . $res->errorstring); + }; return; }; @@ -203,6 +246,7 @@ sub has_reverse_dns { if (!$hits) { $self->adjust_karma(-1); $self->log(LOGINFO, "fail, no PTR records"); + $self->connection->notes('fcrdns_match', 'permerror'); return; } @@ -218,11 +262,13 @@ sub has_forward_dns { foreach my $host (keys %{$self->{_args}{ptr_hosts}}) { $host .= '.' if '.' ne substr($host, -1, 1); # fully qualify name - my $query = $res->search($host) or do { + my $query = $res->query($host) or do { if ($res->errorstring eq 'NXDOMAIN') { + $self->connection->notes('fcrdns_match', 'permerror'); $self->log(LOGDEBUG, "host $host does not exist"); next; } + $self->connection->notes('fcrdns_match', 'fail'); $self->log(LOGDEBUG, "query for $host failed (", $res->errorstring, ")"); next; @@ -235,11 +281,13 @@ sub has_forward_dns { $self->check_ip_match($rr->address) and return 1; } if ($hits) { + $self->connection->notes('fcrdns_match', 'fail'); $self->log(LOGDEBUG, "PTR host has forward DNS") if $hits; return 1; } } $self->adjust_karma(-1); + $self->connection->notes('fcrdns_match', 'fail'); $self->log(LOGINFO, "fail, no PTR hosts have forward DNS"); return; } @@ -250,7 +298,7 @@ sub check_ip_match { if ($ip eq $self->qp->connection->remote_ip) { $self->log(LOGDEBUG, "forward ip match"); - $self->connection->notes('fcrdns_match', 1); + $self->connection->notes('fcrdns_match', 'pass'); $self->adjust_karma(1); return 1; } @@ -262,7 +310,7 @@ sub check_ip_match { if ($dns_net eq $rem_net) { $self->log(LOGNOTICE, "forward network match"); - $self->connection->notes('fcrdns_match', 1); + $self->connection->notes('fcrdns_match', 'pass'); return 1; } return; diff --git a/plugins/sender_permitted_from b/plugins/sender_permitted_from index e80b4e4..1f16a8d 100644 --- a/plugins/sender_permitted_from +++ b/plugins/sender_permitted_from @@ -133,10 +133,12 @@ sub mail_handler { return (DECLINED, "SPF - no response"); } + $self->store_auth_results("spf=$code smtp.mailfrom=".$sender->host); + if ($code eq 'pass') { $self->adjust_karma(1); $transaction->notes('spf_pass_host', lc $sender->host); - $self->log(LOGINFO, "pass, $code: $why"); + $self->log(LOGINFO, "pass, $why"); return (DECLINED); } @@ -235,8 +237,6 @@ sub data_post_handler { $transaction->header->add('Received-SPF', $result->received_spf_header, 0); - # consider also adding SPF status to Authentication-Results header - return DECLINED; }