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

Reply via email to