added logging and tests to auth_checkpassword

plugins/auth/auth_checkpassword

@@ -14,9 +14,14 @@ 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:
 
- echo "/usr/local/vpopmail/bin/vchkpw /usr/bin/true" > ~qpsmtpd/config/smtpauth-checkpassword
+ auth/auth_checkpassword checkpw /usr/local/vpopmail/bin/vchkpw true /bin/true
+
+or by editing the config file config/smtpauth-checkpassword:
+
+ echo "/usr/local/vpopmail/bin/vchkpw /bin/true" > ~qpsmtpd/config/smtpauth-checkpassword
 
 vchkpw is the checkpassword program provided by vpopmail. Substitute
 your own checkpassword app as appropriate.
@@ -65,7 +70,7 @@ that script:
    my $sudo = "/usr/local/bin/sudo";
    $sudo .= " -C4 -u vpopmail";
    my $vchkpw = "/usr/local/vpopmail/bin/vchkpw";
-   my $true   = "/usr/bin/true";
+   my $true   = "/bin/true";
 
    open(CPW,"|$sudo $vchkpw $true 3<&0");
    printf(CPW "%s\0%s\0Y123456\0",'user@example.com','pa55word');
@@ -101,50 +106,86 @@ Please see the LICENSE file included with qpsmtpd for details.
 =cut
 
 sub register {
-    my ($self, $qp) = @_;
+    my ($self, $qp, %args ) = @_;
 
-    $self->register_hook("auth-plain", "auth_checkpassword");
+    my ($checkpw, $true) = $self->get_checkpw( \%args );
-    $self->register_hook("auth-login", "auth_checkpassword");
+    return DECLINED if ! $checkpw || ! $true;
+
+    $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")
+    my $binary = $self->connection->notes('auth_checkpassword_bin');
-      or return (DECLINED);
+    my $true = $self->connection->notes('auth_checkpassword_true');
-    my ($binary, $params) = $command =~ /^(\S+)(.*)$/;
 
-    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 if $binary !~ /vchkpw$/;
-    return $sudo;
+    return "$sudo -u vpopmail";
 }
 

t/plugin_tests/auth/auth_checkpassword

@@ -0,0 +1,44 @@
+#!perl -w
+
+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);
+}
+
+my @u_list = qw ( good bad none );
+my %u_data = (
+	      good => [ 'postmaster@example.com', OK, 'Good Strong Passphrase' ],
+	      bad  => [ 'bad@example.com', DENY, 'not_bad_pass' ],
+	      none => [ 'none@example.com', DECLINED, '' ],
+	      );
+
+sub test_auth_checkpassword {
+    my $self = shift;
+    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);
+    }
+}