From 62a063b3662cb5c9da517ca6e8ba85dc5c53333f Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Sun, 4 Jan 2015 14:26:27 -0800 Subject: [PATCH] completed DKIM signing detection to A-R header cleaning function, making it 'safe' * added test coverage to authentication_results and clean_authentication_results --- lib/Qpsmtpd/SMTP.pm | 34 ++++++++++++--------- t/qpsmtpd-smtp.t | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/lib/Qpsmtpd/SMTP.pm b/lib/Qpsmtpd/SMTP.pm index 1bee2ac..0ce6aa2 100644 --- a/lib/Qpsmtpd/SMTP.pm +++ b/lib/Qpsmtpd/SMTP.pm @@ -197,9 +197,9 @@ sub is_deny_response { sub helo_respond { my ($self, $rc, $msg, $args) = @_; return if $rc == DONE; # do nothing - + $self->is_deny_response($rc, $msg) and return; - + my $conn = $self->connection; $conn->hello('helo'); $conn->hello_host($args->[0]); # store helo hostname @@ -222,7 +222,7 @@ sub ehlo { sub ehlo_respond { my ($self, $rc, $msg, $args) = @_; - + return if $rc == DONE; $self->is_deny_response($rc, $msg) and return; @@ -767,9 +767,10 @@ sub data_respond { return 1; } - $self->run_hooks("data_post_headers"); - $self->authentication_results(); + $self->run_hooks('data_post_headers'); + $self->clean_authentication_results(); $self->received_line(); + $self->authentication_results(); $self->run_hooks('data_post'); } @@ -778,8 +779,6 @@ sub authentication_results { my @auth_list = $self->config('me'); - #$self->clean_authentication_results(); - if (!defined $self->{_auth}) { push @auth_list, 'auth=none'; } @@ -807,16 +806,23 @@ sub authentication_results { sub clean_authentication_results { my $self = shift; - # http://tools.ietf.org/html/draft-kucherawy-original-authres-00.html - # On messages received from the internet, move Authentication-Results headers # to Original-AR, so our downstream can trust the A-R header we insert. - # TODO: Do not invalidate DKIM signatures. - # if $self->transaction->header->get('DKIM-Signature') - # Parse the DKIM signature(s) - # return if A-R header is signed; - # } + # TODO: was A-R header added by a Trusted MTA? Remove otherwise. Maybe. + # See Also: RFC 5451, Removing The Header Field: + # http://tools.ietf.org/html/rfc5451#section-5 + # http://tools.ietf.org/html/draft-kucherawy-original-authres-00.html + + # Do not rename A-R header if DKIM signed + my $dkim_sig = $self->transaction->header->get('DKIM-Signature'); + if ($dkim_sig) { + my %fields = map { split /=/ } split /;\s+/, $dkim_sig; + # print Data::Dumper::Dumper(\%fields); + if ($fields{h} && $fields{h} =~ /Authentication-Results/i) { + return; + } + } my @ar_headers = $self->transaction->header->get('Authentication-Results'); for (my $i = 0 ; $i < scalar @ar_headers ; $i++) { diff --git a/t/qpsmtpd-smtp.t b/t/qpsmtpd-smtp.t index dc29cf3..f823c6b 100644 --- a/t/qpsmtpd-smtp.t +++ b/t/qpsmtpd-smtp.t @@ -24,6 +24,8 @@ __helo_repeat_host(); __helo_respond('helo_respond'); __helo_respond('ehlo_respond'); __data_respond('data_respond'); +__clean_authentication_results(); +__authentication_results(); done_testing(); @@ -161,6 +163,76 @@ sub __data_respond { #is( $smtpd->data_respond(DECLINED), 1, 'data_respond, DECLINED' ); } +sub __clean_authentication_results { + + my $smtpd = _transaction_with_headers(); + $smtpd->transaction->header->add('Authentication-Results', _test_ar_header()); + $smtpd->clean_authentication_results(); + ok(!$smtpd->transaction->header->get('Authentication-Results'), "clean_authentication_results removes A-R header"); + ok($smtpd->transaction->header->get('Original-Authentication-Results'), "clean_authentication_results adds Original-A-R header"); + + # A-R header is _not_ DKIM signed + $smtpd = _transaction_with_headers(); + $smtpd->transaction->header->add('Authentication-Results', _test_ar_header()); + $smtpd->transaction->header->add('DKIM-Signature', _test_dkim_header()); + $smtpd->clean_authentication_results(); + ok(!$smtpd->transaction->header->get('Authentication-Results'), "clean_authentication_results removes non-DKIM-signed A-R header"); + ok($smtpd->transaction->header->get('Original-Authentication-Results'), "clean_authentication_results adds non-DKIM-signed Original-A-R header"); + + # A-R header _is_ DKIM signed + $smtpd = _transaction_with_headers(); + $smtpd->transaction->header->add('Authentication-Results', _test_ar_header()); + $smtpd->transaction->header->add('DKIM-Signature', _test_dkim_sig_ar_signed()); + $smtpd->clean_authentication_results(); + ok($smtpd->transaction->header->get('Authentication-Results'), "clean_authentication_results removes non-DKIM-signed A-R header"); + ok(!$smtpd->transaction->header->get('Original-Authentication-Results'), "clean_authentication_results adds non-DKIM-signed Original-A-R header"); +} + +sub _test_ar_header { + return 'mail.theartfarm.com; iprev=pass; spf=pass smtp.mailfrom=ietf.org; dkim=fail (body hash did not verify) header.i=@kitterman.com; dkim=pass header.i=@ietf.org'; +} + +sub _test_dkim_header { + return <new_conn(); + $smtpd->transaction->header( + Mail::Header->new(Modify => 0, MailFrom => 'COERCE') + ); + return $smtpd; +} + +sub __authentication_results { + my $smtpd = _transaction_with_headers(); + $smtpd->authentication_results(); + my $ar = $smtpd->transaction->header->get('Authentication-Results'); chomp $ar; + ok($ar, "added A-R header: $ar"); + + $smtpd->{_auth} = OK; + $smtpd->{_auth_mechanism} = 'test_mech'; + $smtpd->{_auth_user} = 'test@example'; + $smtpd->authentication_results(); + $ar = $smtpd->transaction->header->get('Authentication-Results'); chomp $ar; + ok($ar =~ /auth=pass/, "added A-R header with auth: $ar"); + + delete $smtpd->{_auth}; + $smtpd->connection->notes('authentication_results', 'spf=pass smtp.mailfrom=ietf.org' ); + $smtpd->authentication_results(); + $ar = $smtpd->transaction->header->get('Authentication-Results'); chomp $ar; + ok($ar =~ /spf/, "added A-R header with SPF: $ar"); + +} + sub response_is { my ( $expected, $descr ) = @_; my $response;