These 3 auth plugins all have a data store they fetch the reference password or hash from. They then match the attemped password or hash against the reference. This consolidates the latter portion (validating the password/hash) into Plugin.pm.
Pros: less duplicated code in the plugins. Pass validation consistently handled for these 3 plugins. less work to create new auth plugins Cons: adds a 'use MD5' in Plugin.pm ??? --- lib/Qpsmtpd/Plugin.pm | 66 ++++++++++++-- plugins/auth/auth_flat_file | 34 ++++--- plugins/auth/auth_vpopmail | 43 +++------ plugins/auth/auth_vpopmail_sql | 42 +++------ t/Test/Qpsmtpd/Plugin.pm | 54 ++++++++++- t/plugin_tests/resolvable_fromhost | 172 ++++++++++++++++++++++++++++++++++++ 6 files changed, 329 insertions(+), 82 deletions(-) create mode 100644 t/plugin_tests/resolvable_fromhost diff --git a/lib/Qpsmtpd/Plugin.pm b/lib/Qpsmtpd/Plugin.pm index 7758788..b2d657f 100644 --- a/lib/Qpsmtpd/Plugin.pm +++ b/lib/Qpsmtpd/Plugin.pm @@ -1,12 +1,16 @@ package Qpsmtpd::Plugin; -use Qpsmtpd::Constants; + use strict; +use warnings; + +use Qpsmtpd::Constants; +use Digest::HMAC_MD5 qw(hmac_md5_hex); # more or less in the order they will fire our @hooks = qw( logging config post-fork pre-connection connect ehlo_parse ehlo helo_parse helo auth_parse auth auth-plain auth-login auth-cram-md5 - rcpt_parse rcpt_pre rcpt mail_parse mail mail_pre + rcpt_parse rcpt_pre rcpt mail_parse mail mail_pre data data_headers_end data_post queue_pre queue queue_post vrfy noop quit reset_transaction disconnect post-connection unrecognized_command deny ok received_line help @@ -19,7 +23,7 @@ sub new { bless ({}, $class); } -sub hook_name { +sub hook_name { return shift->{_hook}; } @@ -138,10 +142,10 @@ sub isa_plugin { # why isn't compile private? it's only called from Plugin and Qpsmtpd. sub compile { my ($class, $plugin, $package, $file, $test_mode, $orig_name) = @_; - + my $sub; open F, $file or die "could not open $file: $!"; - { + { local $/ = undef; $sub = <F>; } @@ -182,6 +186,58 @@ sub compile { die "eval $@" if $@; } +sub validate_password { + my ( $self, %a ) = @_; + + my ($pkg, $file, $line) = caller(); + $file = (split '/', $file)[-1]; # strip off the path + + my $src_clear = $a{src_clear}; + my $src_crypt = $a{src_crypt}; + my $attempt_clear = $a{attempt_clear}; + my $attempt_hash = $a{attempt_hash}; + my $method = $a{method} or die "missing method"; + my $ticket = $a{ticket}; + my $deny = $a{deny} || DENY; + + if ( ! $src_crypt && ! $src_clear ) { + $self->log(LOGINFO, "fail: missing password"); + return ( $deny, "$file - no such user" ); + }; + + if ( ! $src_clear && $method =~ /CRAM-MD5/i ) { + $self->log(LOGINFO, "skip: cram-md5 not supported w/o clear pass"); + return ( DECLINED, $file ); + } + + if ( defined $attempt_clear ) { + if ( $src_clear && $src_clear eq $attempt_clear ) { + $self->log(LOGINFO, "pass: clear match"); + return ( OK, $file ); + }; + + if ( $src_crypt && $src_crypt eq crypt( $attempt_clear, $src_crypt ) ) { + $self->log(LOGINFO, "pass: crypt match"); + return ( OK, $file ); + } + }; + + if ( defined $attempt_hash && $src_clear ) { + if ( ! $ticket ) { + $self->log(LOGERROR, "skip: missing ticket"); + return ( DECLINED, $file ); + }; + + if ( $attempt_hash eq hmac_md5_hex( $ticket, $src_clear ) ) { + $self->log(LOGINFO, "pass: hash match"); + return ( OK, $file ); + }; + }; + + $self->log(LOGINFO, "fail: wrong password"); + return ( $deny, "$file - wrong password" ); +}; + sub _register_standard_hooks { my ($plugin, $qp) = @_; diff --git a/plugins/auth/auth_flat_file b/plugins/auth/auth_flat_file index 4d0abbc..2b4b334 100644 --- a/plugins/auth/auth_flat_file +++ b/plugins/auth/auth_flat_file @@ -30,6 +30,12 @@ algorithm so no password is transfered in the clear. =cut +use strict; +use warnings; + +use base 'Qpsmtpd::Plugin'; +use Qpsmtpd::Constants; + use Digest::HMAC_MD5 qw(hmac_md5_hex); sub register { @@ -45,35 +51,35 @@ sub auth_flat_file { @_; if ( ! defined $passClear && ! defined $passHash ) { + $self->log(LOGINFO, "fail: missing password"); return ( DENY, "authflat - missing password" ); } my ( $pw_name, $pw_domain ) = split '@', lc($user); unless ( defined $pw_domain ) { + $self->log(LOGINFO, "fail: missing domain"); return DECLINED; } my ($auth_line) = grep {/^$pw_name\@$pw_domain:/} $self->qp->config('flat_auth_pw'); - + if ( ! defined $auth_line) { - $self->log(LOGINFO, "User not found: $pw_name\@$pw_domain"); + $self->log(LOGINFO, "fail: no such user: $user"); return DECLINED; } - - $self->log(LOGINFO, "Authentication for: $pw_name\@$pw_domain"); my ($auth_user, $auth_pass) = split(/:/, $auth_line, 2); - - # at this point we can assume the user name matched - if ( defined $passClear && $auth_pass eq $passClear ) { - return ( OK, "authflat" ); - }; - - if ( defined $passHash && $passHash eq hmac_md5_hex($ticket, $auth_pass) ) { - return ( OK, "authflat" ); - }; - return ( DENY, "authflat - wrong password" ); + # at this point we can assume the user name matched + return $self->validate_password( + src_clear => $auth_pass, + src_crypt => undef, + attempt_clear => $passClear, + attempt_hash => $passHash, + method => $method, + ticket => $ticket, + deny => DENY, + ); } diff --git a/plugins/auth/auth_vpopmail b/plugins/auth/auth_vpopmail index ab05698..6d7fe2b 100644 --- a/plugins/auth/auth_vpopmail +++ b/plugins/auth/auth_vpopmail @@ -60,47 +60,26 @@ sub register { sub auth_vpopmail { my ($self, $transaction, $method, $user, $passClear, $passHash, $ticket) = @_; - my ($pw_name, $pw_domain) = split "@", lc($user); - $self->log(LOGINFO, "Authenticating against vpopmail: $user"); - - my $pw = vauth_getpw($pw_name, $pw_domain); + my $pw = vauth_getpw( split '@', lc($user) ); my $pw_clear_passwd = $pw->{pw_clear_passwd}; my $pw_passwd = $pw->{pw_passwd}; - # make sure the user exists if (!$pw || (!$pw_clear_passwd && !$pw_passwd)) { + $self->log(LOGINFO, "fail: invalid user $user"); return (DENY, "auth_vpopmail - invalid user"); - # change DENY to DECLINED to support multiple auth plugins } - return (OK, "auth_vpopmail") - if $pw_passwd eq crypt($passClear, $pw_passwd); - - # simplest case: clear text passwords - if (defined $passClear && defined $pw_clear_passwd) { - return (DENY, "auth_vpopmail - incorrect password") - if $passClear ne $pw_clear_passwd; - return (OK, "auth_vpopmail"); - } - - if ($method =~ /CRAM-MD5/i) { - - # clear_passwd isn't defined so we cannot support CRAM-MD5 - return (DECLINED, "auth_vpopmail") if !defined $pw_clear_passwd; - - if (defined $passHash - and $passHash eq hmac_md5_hex($ticket, $pw_clear_passwd)) - { - } - } - - return (OK, "auth_vpopmail") - if (defined $passHash - && $passHash eq hmac_md5_hex($ticket, $pw_clear_passwd)); - - return (DENY, "auth_vpopmail - unknown error"); + return $self->validate_password( + src_clear => $pw->{pw_clear_passwd}, + src_crypt => $pw->{pw_passwd}, + attempt_clear => $passClear, + attempt_hash => $passHash, + method => $method, + ticket => $ticket, + deny => DENY, + ); } sub test_vpopmail_module { diff --git a/plugins/auth/auth_vpopmail_sql b/plugins/auth/auth_vpopmail_sql index b68cec2..c88687e 100644 --- a/plugins/auth/auth_vpopmail_sql +++ b/plugins/auth/auth_vpopmail_sql @@ -122,45 +122,27 @@ sub auth_vmysql { my ( $self, $transaction, $method, $user, $passClear, $passHash, $ticket ) = @_; my $dbh = $self->get_db_handle() or return DECLINED; - my $db_user = $self->get_vpopmail_user($dbh, $user) or return DECLINED; + my $u = $self->get_vpopmail_user($dbh, $user) or return DECLINED; # if vpopmail was not built with '--enable-clear-passwd=y' # then pw_clear_passwd may not even exist - my $pw_clear_passwd = $db_user->{'pw_clear_passwd'}; - my $pw_passwd = $db_user->{'pw_passwd'}; # always present + # my $pw_clear_passwd = $db_user->{'pw_clear_passwd'}; - if ( ! $pw_passwd && ! $pw_clear_passwd ) { + if ( ! $u->{pw_passwd} && ! $u->{pw_clear_passwd} ) { $self->log(LOGINFO, "fail: no such user"); return ( DENY, "auth_vmysql - no such user" ); }; # at this point, the user name has matched - if ( ! $pw_clear_passwd && $method =~ /CRAM-MD5/i ) { - $self->log(LOGINFO, "skip: cram-md5 not supported w/o clear pass"); - return ( DECLINED, "auth_vmysql" ); - } - - if ( defined $passClear ) { - if ( $pw_clear_passwd && $pw_clear_passwd eq $passClear ) { - $self->log(LOGINFO, "pass: clear match"); - return ( OK, "auth_vmysql" ); - }; - - if ( $pw_passwd eq crypt( $passClear, $pw_passwd ) ) { - $self->log(LOGINFO, "pass: crypt match"); - return ( OK, "auth_vmysql" ); - } - }; - - if ( defined $passHash && $pw_clear_passwd && - $passHash eq hmac_md5_hex( $ticket, $pw_clear_passwd ) - ) { - $self->log(LOGINFO, "pass: hash match"); - return ( OK, "auth_vmysql" ); - }; - - $self->log(LOGINFO, "fail: wrong password"); - return ( DENY, "auth_vmysql - wrong password" ); + return $self->validate_password( + src_clear => $u->{pw_clear_passwd}, + src_crypt => $u->{pw_passwd}, + attempt_clear => $passClear, + attempt_hash => $passHash, + method => $method, + ticket => $ticket, + deny => DENY, + ); } diff --git a/t/Test/Qpsmtpd/Plugin.pm b/t/Test/Qpsmtpd/Plugin.pm index 12edc9f..6e7773d 100644 --- a/t/Test/Qpsmtpd/Plugin.pm +++ b/t/Test/Qpsmtpd/Plugin.pm @@ -4,8 +4,9 @@ package Test::Qpsmtpd::Plugin; # Additional plugin methods used during testing package Qpsmtpd::Plugin; -use Test::More; use strict; +use Test::More; +use Qpsmtpd::Constants; sub register_tests { # Virtual base method - implement in plugin @@ -37,4 +38,55 @@ sub run_tests { } } +sub validate_password { + my ( $self, %a ) = @_; + + my ($pkg, $file, $line) = caller(); + + my $src_clear = $a{src_clear}; + my $src_crypt = $a{src_crypt}; + my $attempt_clear = $a{attempt_clear}; + my $attempt_hash = $a{attempt_hash}; + my $method = $a{method} or die "missing method"; + my $ticket = $a{ticket}; + my $deny = $a{deny} || DENY; + + if ( ! $src_crypt && ! $src_clear ) { + $self->log(LOGINFO, "fail: missing password"); + return ( $deny, "$file - no such user" ); + }; + + if ( ! $src_clear && $method =~ /CRAM-MD5/i ) { + $self->log(LOGINFO, "skip: cram-md5 not supported w/o clear pass"); + return ( DECLINED, $file ); + } + + if ( defined $attempt_clear ) { + if ( $src_clear && $src_clear eq $attempt_clear ) { + $self->log(LOGINFO, "pass: clear match"); + return ( OK, $file ); + }; + + if ( $src_crypt && $src_crypt eq crypt( $attempt_clear, $src_crypt ) ) { + $self->log(LOGINFO, "pass: crypt match"); + return ( OK, $file ); + } + }; + + if ( defined $attempt_hash && $src_clear ) { + if ( ! $ticket ) { + $self->log(LOGERROR, "skip: missing ticket"); + return ( DECLINED, $file ); + }; + + if ( $attempt_hash eq hmac_md5_hex( $ticket, $src_clear ) ) { + $self->log(LOGINFO, "pass: hash match"); + return ( OK, $file ); + }; + }; + + $self->log(LOGINFO, "fail: wrong password"); + return ( $deny, "$file - wrong password" ); +}; + 1; diff --git a/t/plugin_tests/resolvable_fromhost b/t/plugin_tests/resolvable_fromhost new file mode 100644 index 0000000..7553e81 --- /dev/null +++ b/t/plugin_tests/resolvable_fromhost @@ -0,0 +1,172 @@ +#!perl -w + +use strict; + +use Data::Dumper; +use Net::DNS; +use Qpsmtpd::Address; +use Qpsmtpd::Constants; + +my $res = new Net::DNS::Resolver(dnsrch => 0); +my $test_email = 'u...@example.com'; + +sub register_tests { + my $self = shift; + + my %args = ( ); + $self->register( $self->qp, reject => 'none' ); + + $self->register_test('test_is_immune', 3); + $self->register_test('test_populate_invalid_networks', 2); + $self->register_test('test_mx_address_resolves', 2); + $self->register_test('test_get_host_records', 2); + $self->register_test('test_get_and_validate_mx', 2); + $self->register_test('test_check_dns', 2); + $self->register_test('test_hook_rcpt', 10); + $self->register_test('test_hook_mail', 4); +} + +sub test_hook_mail { + my $self = shift; + + my $transaction = $self->qp->transaction; + my $address = Qpsmtpd::Address->new('rem...@example.com'); + $transaction->sender($address); + + my $sender = $transaction->sender; + $sender->host('perl.com'); + + ok( $self->hook_mail( $transaction, $sender ), "hook_mail +"); + ok( $self->hook_mail( $transaction, $sender ), "hook_mail +"); + + $sender->host(''); + $self->{_args}{reject} = 'soft'; + my ($r) = $self->hook_mail( $transaction, $sender ); + ok( $r == DENYSOFT, "hook_mail - ($r)"); + + $self->{_args}{reject} = 'hard'; + ($r) = $self->hook_mail( $transaction, $sender ); + ok( $r == DENY, "hook_mail - ($r)"); + +}; + +sub test_hook_rcpt { + my $self = shift; + + my $transaction = $self->qp->transaction; + my $recipient = 'f...@example.com'; + + $transaction->notes('resolvable_fromhost', 'a'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'mx'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'ip'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'whitelist'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'null'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'config'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'oops!'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'oops!'); + ok( DECLINED == $self->hook_rcpt( $transaction, $recipient ), "hook_rcpt +"); + + $transaction->notes('resolvable_fromhost', 'oops!'); + $self->{_args}{reject} = 'soft'; + my ($r) = $self->hook_rcpt( $transaction, $recipient ); + ok( DENYSOFT == $r, "hook_rcpt - ($r)"); + + $transaction->notes('resolvable_fromhost', 'failed again'); + $self->{_args}{reject} = 'hard'; + ($r) = $self->hook_rcpt( $transaction, $recipient ); + ok( DENY == $r, "hook_rcpt - ($r)"); +}; + +sub test_check_dns { + my $self = shift; + + my $transaction = $self->qp->transaction; + ok( ! $self->check_dns( '', $transaction ), 'check_dns -'); + ok( $self->check_dns( 'perl.com', $transaction ), 'check_dns +'); +} + +sub test_get_and_validate_mx { + my $self = shift; + my $transaction = $self->qp->transaction; + + ok( scalar $self->get_and_validate_mx( $res, 'perl.com', $transaction ), + "get_and_validate_mx +"); + + ok( ! scalar $self->get_host_records( $res, 'fake-domain-name-for-test.com', $transaction ), + "get_and_validate_mx -"); + +}; + +sub test_get_host_records { + my $self = shift; + my $transaction = $self->qp->transaction; + + ok( scalar $self->get_host_records( $res, 'perl.com', $transaction ), + "get_host_records +"); + + ok( ! scalar $self->get_host_records( $res, 'fake-domain-name-for-test.com', $transaction ), + "get_host_records -"); + +}; + +sub test_mx_address_resolves { + my $self = shift; + + my $fromhost = 'perl.com'; + + ok( $self->mx_address_resolves('mail.perl.com', $fromhost), + "mx_address_resolves +"); + ok( ! $self->mx_address_resolves('no-such-mx.perl.com', $fromhost), + "mx_address_resolves -"); +}; + +sub test_populate_invalid_networks { + my $self = shift; + + my $ip = '10.9.8.7'; + ok( $self->ip_is_valid($ip), "ip_is_valid +" ); + + $self->qp->config('invalid_resolvable_fromhost', $ip); + $self->populate_invalid_networks(); + ok( ! $self->ip_is_valid($ip), "populate_invalid_networks" ); + + # clean up afterwards + $self->qp->config('invalid_resolvable_fromhost', undef ); + $self->{invalid} = (); +}; + +sub test_is_immune { + my $self = shift; + + my $transaction = $self->qp->transaction; + + # null sender should be immune + $transaction->sender('<>'); + ok( $self->is_immune( $transaction->sender, $transaction ) ); + + # whitelisted host should be immune + my $connection = $self->qp->connection->notes('whitelisthost', 1); + ok( $self->is_immune( $transaction->sender, $transaction ) ); + $self->qp->connection->notes('whitelisthost', undef); + + # reject is not defined, so email should not be immune + my $address = Qpsmtpd::Address->new( "<$test_email>" ); + $transaction->sender($address); + ok( ! $self->is_immune( $transaction->sender, $transaction ) ); +}; + + -- 1.7.9.6