CGI Change Password (LDAP)

sifen has asked for the wisdom of the Perl Monks concerning the following question:

Hello,

I'm using CGI.pm to write a short dynamic HTML page for users to change their password. Scripts for this have been mentioned elsewhere on the monks, but I am using LDAP for a password backed, and I have not been able to find anything on that.

Anyways, I hacked up a bit of code to accomplish this, and I've looked it over, but I would appreciate any feedback as I'd rather not have to find security holes the hard way.

Sorry about the length of the code, I know that it goes somewhat against what Ovid says in Before You Post. . .

Code Below:
--------------------

#!/usr/bin/perl -T

use warnings;
use strict;

use CGI qw/:standard/;
use CGI::Carp;

# Debian Packages
# libnet-ldap-perl
# libcrypt-passwdmd5-perl
# libmail-sendmail-perl

use Net::LDAP;
use Crypt::PasswdMD5;
use Mail::Sendmail;

my $LDAP_SERVER  = 'csl-ldap-01.inside';
my $MAX_USER_LEN = '16';
my $MAX_PASS_LEN = '40';
my $MIN_PASS_LEN = '6';

my $g_err_msg = '';

{    # MAIN
    print header();

    my ($top_html, $main_html) = get_page();

    print "$top_html";

    # Application Logic
    # if param's are passed to the script...
    if (param) {
        if (validate_cgi_params( param('user')       || '',
                                 param('magic_word') || '',
                                 param('new_pwd_1')  || '',
                                 param('new_pwd_2')  || ''
                                 )) {

            change_password(param('user') || '',
                            param('magic_word') || '',
                            param('new_pwd_1')  || '');
        }
    }

    print "$g_err_msg" if ($g_err_msg);
    print "$main_html";

}
######

sub get_page {
    # Define HTML sections of page
    my $top_html = << "HERE";
    <html>
        <head>
            <title>Community Software Lab Password Changer</title>
        </head>
        <body>
        <h1>Change your password (0.3)</h1>
HERE

    my $main_html = << "HERE";
    <hr />
    <form method="post" action="/cgi-bin/pw.pl" enctype="application/x
+-www-form-urlencoded">
    Username: <input type="text" name="user"  /><p />
    Password: <input type="password" name="magic_word"  /><p /><p />
    New Password: <input type="password" name="new_pwd_1"  /><p />
    Repeat New Password: <input type="password" name="new_pwd_2"  /><p
+ />
    <input type="submit" name="Change Password" value="Change Password
+" />
    </form>
    </body>
    </html>
HERE

    return ($top_html, $main_html);
}

sub validate_cgi_params {
    my ( $user, $passwd, $pwd1, $pwd2 ) = @_;

    unless ( $pwd1 eq $pwd2 ) {
        print 'Error, New passwords do not match';
        return undef;
    }

    unless ( length($user) < $MAX_USER_LEN
        && length($passwd) < $MAX_PASS_LEN
        && length($pwd1) < $MAX_PASS_LEN )
    {
        print 'Error, Input too long';
        return undef;
    }

    unless ( length($pwd1) > $MIN_PASS_LEN )
    {
        print "Error, New password must be more than $MIN_PASS_LEN cha
+racters long";
        return undef;
    }

    return 1;
}
sub change_password {
    my ( $user, $pwd, $new_pwd,) = @_;

    my $ldapbinddn = "uid=$user,ou=People,dc=local";

    my $connection;
    unless ($connection = Net::LDAP->new("$LDAP_SERVER")) {
        $g_err_msg = 'Error, Cannot connect to server';
        return undef;
    }

    my $msg;
    unless ($msg = $connection->bind(
        dn       => $ldapbinddn,
        password => $pwd
      )){
       $g_err_msg = 'We are experiencing technical difficulties, pleas
+e try back later';
       return undef;
    }
    unless ($msg) {
        my $err = $msg->error;
        email_and_log_error("Bind error: $err");
    }

    my $md5pwd = unix_md5_crypt($new_pwd);

    my $result = $connection->modify( "uid=$user,ou=People,dc=local",
        changes => [ replace => [ userPassword => "{crypt}$md5pwd" ] ]
+ );
    if ( $result->code ) {
        my $err = $result->error;
        email_and_log_error ("Password change error: $err");
        $g_err_msg = 'Error, You have either entered a bad username or
+ password, your password has not
been changed';
        return undef;
    }
    else {
         return 1;
    }
}

sub email_and_log_error {
    my $error_str = shift;

    warn $error_str;

    my %mail = ( To      => 'csacca@thecsl.org',
                 From    => "$0 <>",
                 Subject => "Error Message",
                 Message => $error_str
               );

    sendmail(%mail) or warn $Mail::Sendmail::error;
}
[download]

Comment on CGI Change Password (LDAP) Download Code

Replies are listed 'Best First'.

Re: CGI Change Password (LDAP)
by tachyon (Chancellor) on Dec 11, 2004 at 01:23 UTC

Unless you run this over https it is insecure as the password goes over the wire in plaintext. You have a CGI object. It contains all the params. Why not just pass that to your validate and change functions? Typically I use the return null string if function succeeds or error string if it fails. This lets you avoid globals like your g_err_msg. Then the app logic goes:

if ( $q->param ) {
    my $err_msg = validate( $q );
    if ( $err_msg ) {
        show_form($err_msg);
    }
    else {
        my $msg = change_pass( $q );
        show_form( $msg );   # msg may be error or success message
    }
}
else {
    show_form();
}
exit 0;
[download]

I can't see how it might be exploited but is is usually wise to limit CGI user input to a selected range of characters. The null byte hack is one issue this attends to.

cheers

tachyon

[reply]
[d/l]


P is for Practical
	PerlMonks