auth_checkpassword refactored and better tested added support for configuration arguments in plugins/config
test ability to run during register, instead of failing later. added logging for additional errors. tests are automatically skipped if they aren't likely to succeed (no vpopmail installed, missing domain) --- plugins/auth/auth_checkpassword | 69 +++++++++++++++++++++++++------- t/plugin_tests/auth/auth_checkpassword | 38 ++++++++++++------ 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/plugins/auth/auth_checkpassword b/plugins/auth/auth_checkpassword index e6baa3b..520d608 100644 --- a/plugins/auth/auth_checkpassword +++ b/plugins/auth/auth_checkpassword @@ -14,7 +14,12 @@ or with sudo. =head1 CONFIGURATION -Configure the path to your checkpassword binary: +Configure the path to your checkpassword binary. You can configure this in +config/plugins by defining the checkpw and true arguments as follows: + + auth/auth_checkpassword checkpw /usr/local/vpopmail/bin/vchkpw true /usr/bin/true + +or by editing the config file config/smtpauth-checkpassword: echo "/usr/local/vpopmail/bin/vchkpw /usr/bin/true" > ~qpsmtpd/config/smtpauth-checkpassword @@ -101,50 +106,86 @@ Please see the LICENSE file included with qpsmtpd for details. =cut sub register { - my ($self, $qp) = @_; + my ($self, $qp, %args ) = @_; + + my ($checkpw, $true) = $self->get_checkpw( \%args ); + return DECLINED if ! $checkpw || ! $true; - $self->register_hook("auth-plain", "auth_checkpassword"); - $self->register_hook("auth-login", "auth_checkpassword"); + $self->connection->notes('auth_checkpassword_bin', $checkpw); + $self->connection->notes('auth_checkpassword_true', $true); + + $self->register_hook('auth-plain', 'auth_checkpassword'); + $self->register_hook('auth-login', 'auth_checkpassword'); } sub auth_checkpassword { my ($self, $transaction, $method, $user, $passClear, $passHash, $ticket) = @_; - my $command = $self->qp->config("smtpauth-checkpassword") - or return (DECLINED); - my ($binary, $params) = $command =~ /^(\S+)(.*)$/; + my $binary = $self->connection->notes('auth_checkpassword_bin'); + my $true = $self->connection->notes('auth_checkpassword_true'); - return (DECLINED) if (!-x $binary); my $sudo = get_sudo($binary); - open(CPW, "|$sudo $binary $params 3<&0"); + $self->log(LOGDEBUG, "auth_checkpassword: $sudo $binary $true 3<&0"); + open(CPW, "|$sudo $binary $true 3<&0"); printf(CPW "%s\0%s\0Y123456\0", $user, $passClear); close(CPW); my $status = $?; - return (DECLINED) if ($status != 0); + if ($status != 0) { + $self->log(LOGNOTICE, "authentication failed ($status)"); + return (DECLINED); + }; $self->connection->notes('authuser', $user); return (OK, "auth_checkpassword"); } +sub get_checkpw { + my ($self, $args) = @_; + + my ($checkpw) = $args->{checkpw} =~ /^(.*)$/ if $args->{checkpw}; # untaint + my ($true) = $args->{true} =~ /^(.*)$/ if $args->{true}; # untaint + + return ( $checkpw, $true ) + if ( $checkpw && $true && -x $checkpw && -x $true ); + + my $missing_config = "disabled due to invalid configuration. See 'perldoc plugins/auth/auth_checkpassword' for how to configure."; + + if ( ! $self->qp->config('smtpauth-checkpassword') ) { + $self->log(LOGERROR, $missing_config ); + return; + }; + + $self->log(LOGNOTICE, "reading config from smtpauth-checkpassword"); + my $config = $self->qp->config("smtpauth-checkpassword"); + ($checkpw, $true) = $config =~ /^(\S+)\s+(\S+)\s*$/; + + if ( ! $checkpw || ! $true || ! -x $checkpw || ! -x $true ) { + $self->log(LOGERROR, $missing_config ); + return; + }; + return ($checkpw, $true); +}; + sub get_sudo { my $binary = shift; return '' if $> == 0; # running as root - return '' if $> == 89 && $binary =~ /vchkpw/; # running as vpopmail + return '' if $> == 89 && $binary =~ /vchkpw$/; # running as vpopmail my $mode = (stat($binary))[2]; $mode = sprintf "%lo", $mode & 07777; return '' if $mode eq '4711'; # $binary is setuid my $sudo = `which sudo` || '/usr/local/bin/sudo'; - return '' if !-x $sudo; + return '' if ! -x $sudo; + $sudo .= ' -C4'; # prevent sudo from clobbering file descriptor 3 - return "$sudo -u vpopmail" if $binary =~ /vchkpw/; - return $sudo; + return $sudo if $binary !~ /vchkpw$/; + return "$sudo -u vpopmail"; } diff --git a/t/plugin_tests/auth/auth_checkpassword b/t/plugin_tests/auth/auth_checkpassword index 61801af..c51fa2d 100644 --- a/t/plugin_tests/auth/auth_checkpassword +++ b/t/plugin_tests/auth/auth_checkpassword @@ -1,9 +1,22 @@ #!perl -w -warn "loaded test auth_checkpassword\n"; +warn "loaded auth_checkpassword\n"; sub register_tests { my $self = shift; + + my ($vpopdir) = (getpwnam('vpopmail'))[7]; + + if ( ! $vpopdir ) { + warn "skipping tests, vpopmail not installed\n"; + return; + }; + + if ( ! -d "$vpopdir/domains/example.com" ) { + warn "skipping tests, no example users set up.\n"; + return; + }; + $self->register_test("test_auth_checkpassword", 3); } @@ -16,15 +29,16 @@ my %u_data = ( sub test_auth_checkpassword { my $self = shift; - ok( 1 == 1, "test 1"); - ok( 1 == 1, "test 2"); - ok( 1 == 1, "test 3"); -# my ($tran, $ret, $note, $u, $r, $p, $a ); -# $tran = $self->qp->transaction; -# for $u ( @u_list ) { -# ( $a,$r,$p ) = @{$u_data{$u}}; -# ($ret, $note) = $self->auth_vpopmaild($tran,'LOGIN',$a,$p); -# defined $note or $note='No-Message'; -# is ($ret, $r, $note); -# } + my ($tran, $ret, $note, $u, $r, $p, $a ); + $tran = $self->qp->transaction; + for $u ( @u_list ) { + ( $a,$r,$p ) = @{$u_data{$u}}; + ($ret, $note) = $self->auth_checkpassword($tran,'LOGIN',$a,$p); + defined $note or $note='No-Message'; + is ($ret, $r, $note); + + ($ret, $note) = $self->auth_checkpassword($tran,'PLAIN',$a,$p); + defined $note or $note='No-Message'; + is ($ret, $r, $note); + } } -- 1.7.9.6