factored date validity tests into their own sub
added tests
---
plugins/check_basicheaders | 90 ++++++++++++++++++++++++++-----------
t/plugin_tests/check_basicheaders | 64 ++++++++++++++++++++++----
2 files changed, 118 insertions(+), 36 deletions(-)
diff --git a/plugins/check_basicheaders b/plugins/check_basicheaders
index ef0e42d..6ca2056 100644
--- a/plugins/check_basicheaders
+++ b/plugins/check_basicheaders
@@ -2,33 +2,46 @@
=head1 NAME
-check_basicheaders - Make sure both From and Date headers are present, and
-do optional range checking on the Date header.
+check_basicheaders
=head1 DESCRIPTION
-Rejects messages that do not have a From or Date header or are completely
-empty.
+Checks for missing or empty values in the From or Date headers.
-Can also reject messages where the date in the Date header is more than
-some number of the days in the past or future.
+Optionally test if the Date header is too many days in the past or future. If
+I<future> or I<past> are not defined, they are not tested.
=head1 CONFIGURATION
-The following optional parameters exist:
+The following optional settings exist:
+
+=head2 future
+
+The number of days in the future beyond which messages are invalid.
+
+ check_basicheaders [ future 1 ]
+
+=head2 past
+
+The number of days in the past beyond which a message is invalid. The Date
header is added by the MUA, so there are many valid reasons a message may have
an older date in the header. It could have been delayed by the client, the
sending server, connectivity problems, recipient server problem, recipient
server configuration, etc. The I<past> setting should take those factors into
consideration.
+
+I would be surprised if a valid message ever had a date header older than a
week.
+
+ check_basicheaders [ past 5 ]
=head2 days
-The number of days in the future or past beyond which to reject messages. When
-unset, messages are not rejected based on the date.
+Deprecated. Use I<past> and I<future> instead.
+
+The number of days in the future or past beyond which messages are invalid.
check_basicheaders [ days 3 ]
=head2 reject
-A boolean. Determines if the connection is denied or not. Use this option
-when first enabling the plugin, and then watch your logs to see what would
-have been rejected.
+Determine if the connection is denied. Use the I<reject 0> option when first
enabling the plugin, and then watch your logs to see what would have been
rejected. When you are no longer concerned that valid messages will be
rejected, enable with I<reject 1>.
+
+ check_basicheaders [ reject 0 | 1 ]
Default policy is to reject.
@@ -38,9 +51,9 @@ Whether to issue a permanent or temporary rejection. The
default is permanent.
check_basicheaders reject_type [ temp | perm ]
-Switching to a temporary rejection is most useful when testing the plugin. It
-allows an administrator to watch for a test period and make sure no valid mail
-is getting rejected.
+Using a temporary rejection is a cautious way to enable rejections. It allows
an administrator to watch for a trial period and assure no valid messages are
rejected. If a deferral of valid mail is noticed, I<reject 0> can be set to
permit the deffered message to be delivered.
+
+Default policy is a permanent rejection.
=head2 loglevel
@@ -51,6 +64,7 @@ Adjust the quantity of logging for this plugin. See
docs/logging.pod
2004 - Written by Jim Winstead Jr.
2012 - added logging, named arguments, reject_type, tests - Matt Simerson
+ - deprecate days for I<past> & I<future>. Improved POD
=head1 LICENSE
@@ -58,13 +72,18 @@ Released to the public domain, 26 March 2004.
=cut
+use strict;
+use warnings;
+
+use Qpsmtpd::Constants;
+
use Date::Parse qw(str2time);
sub register {
my ($self, $qp, @args) = @_;
if ( @args == 1 ) {
- $self->log(LOGWARN, "deprecated arguments. Update your arguments to
this plugin");
+ $self->log(LOGWARN, "deprecated arguments. Update your config.");
$self->{_args}{days} = $args[0];
}
elsif ( @args % 2 ) {
@@ -73,6 +92,16 @@ sub register {
else {
$self->{_args} = { @args };
};
+# provide backwards comptibility with the old 'days' argument
+ if ( $self->{_args}{days} ) {
+ $self->log(LOGWARN, "deprecated arguments. Update your config.");
+ if ( ! defined $self->{_args}{future} ) {
+ $self->{_args}{future} = $self->{_args}{days};
+ };
+ if ( ! defined $self->{_args}{past} ) {
+ $self->{_args}{past} = $self->{_args}{days};
+ };
+ };
}
sub hook_data_post {
@@ -83,7 +112,7 @@ sub hook_data_post {
if ( $transaction->data_size == 0 ) {
$self->log(LOGINFO, "fail: no data");
- return ($deny, "You have to send some data first");
+ return ($deny, "You must send some data first");
};
my $header = $transaction->header or do {
@@ -101,27 +130,34 @@ sub hook_data_post {
return ($deny, "We require a valid Date header");
};
- my $days = $self->{_args}{days};
- if ( ! defined $days ) {
- $self->log(LOGINFO, "pass: no days arg");
- return (DECLINED);
+ my $err_msg = $self->invalid_date_range($date);
+ if ( $err_msg ) {
+ return ($deny, $err_msg );
};
+ return (DECLINED);
+};
+
+sub invalid_date_range {
+ my ($self, $date) = @_;
+
my $ts = str2time($date) or do {
$self->log(LOGINFO, "skip: date not parseable ($date)");
- return (DECLINED);
+ return;
};
- if ( $ts < time - ($days*24*3600) ) {
+ my $past = $self->{_args}{past};
+ if ( $past && $ts < time - ($past*24*3600) ) {
$self->log(LOGINFO, "fail: date too old ($date)");
- return ($deny, "The Date in the header is too far in the past")
+ return "The Date header is too far in the past";
};
- if ( $ts > time + ($days*24*3600) ) {
+ my $future = $self->{_args}{future};
+ if ( $future && $ts > time + ($future*24*3600) ) {
$self->log(LOGINFO, "fail: date in future ($date)");
- return ($deny, "The Date in the header is too far in the future")
+ return "The Date header is too far in the future";
};
$self->log(LOGINFO, "pass");
- return (DECLINED);
+ return;
}
diff --git a/t/plugin_tests/check_basicheaders
b/t/plugin_tests/check_basicheaders
index 921030e..2ac5748 100644
--- a/t/plugin_tests/check_basicheaders
+++ b/t/plugin_tests/check_basicheaders
@@ -7,51 +7,97 @@ use POSIX qw(strftime);
use Qpsmtpd::Address;
use Qpsmtpd::Constants;
+my $test_email = 'm...@example.com';
sub register_tests {
my $self = shift;
$self->register_test("test_hook_data_post", 7);
+ $self->register_test('test_invalid_date_range', 7);
}
-sub test_hook_data_post {
+sub setup_test_headers {
my $self = shift;
- my $reject = $self->{_args}{reject_type};
- my $deny = $reject =~ /^temp|soft$/i ? DENYSOFT : DENY;
-
my $transaction = $self->qp->transaction;
- my $test_email = 'm...@example.com';
my $address = Qpsmtpd::Address->new( "<$test_email>" );
my $header = Mail::Header->new(Modify => 0, MailFrom => "COERCE");
my $now = strftime "%a %b %e %H:%M:%S %Y", localtime time;
- my $future = strftime "%a %b %e %H:%M:%S %Y", localtime time + 518400; #6d
- my $past = strftime "%a %b %e %H:%M:%S %Y", localtime time - 518400; #6d
- $self->{_args}{days} = 5;
$transaction->sender($address);
$transaction->header($header);
$transaction->header->add('From', "<$test_email>");
$transaction->header->add('Date', $now );
$transaction->body_write( "test message body " );
+};
+
+sub test_invalid_date_range {
+ my $self = shift;
+
+ my $now = strftime "%a %b %e %H:%M:%S %Y", localtime time;
+ ok( ! $self->invalid_date_range($now), "valid +");
+
+ $self->{_args}{future} = 2;
+
+ my $future_6 = strftime "%a %b %e %H:%M:%S %Y", localtime time + 518400;
#6d
+ my $r = $self->invalid_date_range( $future_6 );
+ ok( $r, "too new -" );
+
+ my $future_3 = strftime "%a %b %e %H:%M:%S %Y", localtime time + 259200;
#3d
+ $r = $self->invalid_date_range( $future_3 );
+ ok( $r, "too new -" );
+
+ my $future_1 = strftime "%a %b %e %H:%M:%S %Y", localtime time + 86400;
#1d
+ $r = $self->invalid_date_range( $future_1 );
+ ok( ! $r, "a little new, +" );
+
+
+ $self->{_args}{past} = 2;
+
+ my $past_6 = strftime "%a %b %e %H:%M:%S %Y", localtime time - 518400;
#6d
+ $r = $self->invalid_date_range( $past_6 );
+ ok( $r, "too old -" );
+
+ my $past_3 = strftime "%a %b %e %H:%M:%S %Y", localtime time - 259200;
#3d
+ $r = $self->invalid_date_range( $past_3 );
+ ok( $r, "too old -" );
+
+ my $past_1 = strftime "%a %b %e %H:%M:%S %Y", localtime time - 86400;
#1d
+ $r = $self->invalid_date_range( $past_1 );
+ ok( ! $r, "a little old +" );
+};
+
+sub test_hook_data_post {
+ my $self = shift;
+
+ my $reject = $self->{_args}{reject_type};
+ my $deny = $reject =~ /^temp|soft$/i ? DENYSOFT : DENY;
+
+ $self->setup_test_headers();
+ my $transaction = $self->qp->transaction;
my ($code, $mess) = $self->hook_data_post( $transaction );
- cmp_ok( DECLINED, '==', $code, "okay" );
+ cmp_ok( DECLINED, '==', $code, "okay +" );
$transaction->header->delete('Date');
($code, $mess) = $self->hook_data_post( $transaction );
cmp_ok( $deny, '==', $code, "missing date ( $mess )" );
+ my $now = strftime "%a %b %e %H:%M:%S %Y", localtime time;
$transaction->header->add('Date', $now );
$transaction->header->delete('From');
($code, $mess) = $self->hook_data_post( $transaction );
cmp_ok( $deny, '==', $code, "missing from ( $mess )" );
$transaction->header->add('From', "<$test_email>");
+ $self->{_args}{future} = 5;
+ my $future = strftime "%a %b %e %H:%M:%S %Y", localtime time + 518400; #6d
$transaction->header->replace('Date', $future );
($code, $mess) = $self->hook_data_post( $transaction );
cmp_ok( $deny, '==', $code, "too new ( $mess )" );
+ $self->{_args}{past} = 5;
+ my $past = strftime "%a %b %e %H:%M:%S %Y", localtime time - 518400; #6d
$transaction->header->replace('Date', $past );
($code, $mess) = $self->hook_data_post( $transaction );
cmp_ok( $deny, '==', $code, "too old ( $mess )" );
--
1.7.9.6
