Comments? Matt
On May 8, 2012, at 10:03 PM, Matt Simerson wrote: > These 3 auth plugins all have a data store they fetch the reference > password or hash from. They then match the attempted password or hash > against the reference. This consolidates the latter portion (validating > the password/hash) into Auth.pm. > > * less duplicated code in the plugins. > * Pass validation consistently handled for these 3 plugins. > * less work to create similar auth plugins > > Qpsmtpd::Auth also caches the CRAM-MD5 ticket. It could also cache > user/pass info if this was desirable. > --- > lib/Qpsmtpd/Auth.pm | 59 +++++++++++++++++++++++++++++++-- > plugins/auth/auth_flat_file | 34 ++++++++++--------- > plugins/auth/auth_vpopmail | 49 ++++++++------------------- > plugins/auth/auth_vpopmail_sql | 54 ++++++++++++------------------ > t/Test/Qpsmtpd.pm | 1 + > t/plugin_tests/auth/auth_vpopmail_sql | 5 +++ > 6 files changed, 118 insertions(+), 84 deletions(-) > > diff --git a/lib/Qpsmtpd/Auth.pm b/lib/Qpsmtpd/Auth.pm > index ec885b4..52e441d 100644 > --- a/lib/Qpsmtpd/Auth.pm > +++ b/lib/Qpsmtpd/Auth.pm > @@ -4,9 +4,11 @@ package Qpsmtpd::Auth; > use strict; > use warnings; > > -use MIME::Base64; > use Qpsmtpd::Constants; > > +use Digest::HMAC_MD5 qw(hmac_md5_hex); > +use MIME::Base64; > + > sub e64 { > my ($arg) = @_; > my $res = encode_base64($arg); > @@ -144,6 +146,7 @@ sub get_auth_details_cram_md5 { > return; > } > > + $session->{auth}{ticket} = $ticket; > return ($ticket, $user, $passHash); > }; > > @@ -159,6 +162,58 @@ sub get_base64_response { > return $answer; > }; > > -# tag: qpsmtpd plugin that sets RELAYCLIENT when the user authentifies > +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} || $self->{auth}{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" ); > +}; > + > +# tag: qpsmtpd plugin that sets RELAYCLIENT when the user authenticates > > 1; > diff --git a/plugins/auth/auth_flat_file b/plugins/auth/auth_flat_file > index 4d0abbc..a17d051 100644 > --- a/plugins/auth/auth_flat_file > +++ b/plugins/auth/auth_flat_file > @@ -30,7 +30,11 @@ algorithm so no password is transfered in the clear. > > =cut > > -use Digest::HMAC_MD5 qw(hmac_md5_hex); > +use strict; > +use warnings; > + > +use Qpsmtpd::Auth; > +use Qpsmtpd::Constants; > > sub register { > my ( $self, $qp ) = @_; > @@ -45,35 +49,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 Qpsmtpd::Auth::validate_password( $self, > + 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..91a5ac6 100644 > --- a/plugins/auth/auth_vpopmail > +++ b/plugins/auth/auth_vpopmail > @@ -42,10 +42,10 @@ Please see the LICENSE file included with qpsmtpd for > details. > use strict; > use warnings; > > +use Qpsmtpd::Auth; > use Qpsmtpd::Constants; > > -use Digest::HMAC_MD5 qw(hmac_md5_hex); > -#use vpopmail; # we eval this in $test_vpopmail > +#use vpopmail; # we eval this in $test_vpopmail_module > > sub register { > my ($self, $qp) = @_; > @@ -60,54 +60,33 @@ 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 Qpsmtpd::Auth::validate_password( $self, > + 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 { > my $self = shift; > # vpopmail will not allow vauth_getpw to succeed unless the requesting user > is vpopmail or root. > # by default, qpsmtpd runs as the user 'qpsmtpd' and does not have permission. > - eval "use vpopmail"; > + eval 'use vpopmail'; > if ( $@ ) { > $self->log(LOGERROR, "skip: is vpopmail perl module installed?"); > return; > diff --git a/plugins/auth/auth_vpopmail_sql b/plugins/auth/auth_vpopmail_sql > index b68cec2..dd9b3cb 100644 > --- a/plugins/auth/auth_vpopmail_sql > +++ b/plugins/auth/auth_vpopmail_sql > @@ -66,13 +66,21 @@ Please see the LICENSE file included with qpsmtpd for > details. > use strict; > use warnings; > > -use DBI; > +use Qpsmtpd::Auth; > use Qpsmtpd::Constants; > -use Digest::HMAC_MD5 qw(hmac_md5_hex); > + > +#use DBI; # done in ->register > > sub register { > my ( $self, $qp ) = @_; > > + eval 'use DBI'; > + if ( $@ ) { > + warn "plugin disabled. is DBI installed?\n"; > + $self->log(LOGERROR, "skip: plugin disabled. is DBI installed?\n"); > + return; > + }; > + > $self->register_hook('auth-plain', 'auth_vmysql'); > $self->register_hook('auth-login', 'auth_vmysql'); > $self->register_hook('auth-cram-md5', 'auth_vmysql'); > @@ -122,45 +130,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 Qpsmtpd::Auth::validate_password( $self, > + 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.pm b/t/Test/Qpsmtpd.pm > index 0d830e0..e2c39c5 100644 > --- a/t/Test/Qpsmtpd.pm > +++ b/t/Test/Qpsmtpd.pm > @@ -1,6 +1,7 @@ > package Test::Qpsmtpd; > use strict; > use lib 't'; > +use lib 'lib'; > use Carp qw(croak); > use base qw(Qpsmtpd::SMTP); > use Test::More; > diff --git a/t/plugin_tests/auth/auth_vpopmail_sql > b/t/plugin_tests/auth/auth_vpopmail_sql > index 0e6c84e..e61cad0 100644 > --- a/t/plugin_tests/auth/auth_vpopmail_sql > +++ b/t/plugin_tests/auth/auth_vpopmail_sql > @@ -6,6 +6,11 @@ use warnings; > sub register_tests { > my $self = shift; > > + eval 'use DBI'; > + if ( $@ ) { > + warn "skipping auth_vpopmail_sql tests, is DBI installed?\n"; > + return; > + }; > $self->register_test("auth_vpopmail_sql", 3); > } > > -- > 1.7.9.6 >