From 5ddb84da8800728b887f2497a205fad01c44be8a Mon Sep 17 00:00:00 2001 From: "travis%sedsystems.ca" <> Date: Tue, 1 Feb 2005 03:26:00 +0000 Subject: Bug 278792 : Move Crypt() to Bugzilla::Auth Patch by Max Kanat-Alexander r=vladd a=justdave --- Bugzilla/Auth.pm | 49 ++++++++++++++++++++++++++++++++++++++++++++++ Bugzilla/Auth/Verify/DB.pm | 2 +- checksetup.pl | 14 ++++++++++--- editusers.cgi | 5 +++-- globals.pl | 36 ++-------------------------------- token.cgi | 3 ++- userprefs.cgi | 3 ++- 7 files changed, 70 insertions(+), 42 deletions(-) diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 71b125e45..6071d3abd 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -23,6 +23,8 @@ package Bugzilla::Auth; use strict; +use Exporter qw(import); +@Bugzilla::Auth::EXPORT = qw(bz_crypt); use Bugzilla::Config; use Bugzilla::Constants; @@ -42,6 +44,31 @@ BEGIN { } } +sub bz_crypt ($) { + my ($password) = @_; + + # The list of characters that can appear in a salt. Salts and hashes + # are both encoded as a sequence of characters from a set containing + # 64 characters, each one of which represents 6 bits of the salt/hash. + # The encoding is similar to BASE64, the difference being that the + # BASE64 plus sign (+) is replaced with a forward slash (/). + my @saltchars = (0..9, 'A'..'Z', 'a'..'z', '.', '/'); + + # Generate the salt. We use an 8 character (48 bit) salt for maximum + # security on systems whose crypt uses MD5. Systems with older + # versions of crypt will just use the first two characters of the salt. + my $salt = ''; + for ( my $i=0 ; $i < 8 ; ++$i ) { + $salt .= $saltchars[rand(64)]; + } + + # Crypt the password. + my $cryptedpassword = crypt($password, $salt); + + # Return the crypted password. + return $cryptedpassword; +} + # PRIVATE # A number of features, like password change requests, require the DB @@ -128,6 +155,11 @@ __END__ Bugzilla::Auth - Authentication handling for Bugzilla users +=head1 SYNOPSIS + + # Class Functions + $crypted = bz_crypt($password); + =head1 DESCRIPTION Handles authentication for Bugzilla users. @@ -147,6 +179,23 @@ authentication or login modules. =over 4 +=item C + +Takes a string and returns a Ced value for it, using a random salt. + +Please always use this function instead of the built-in perl "crypt" +when initially encrypting a password. + +=begin undocumented + +Random salts are generated because the alternative is usually +to use the first two characters of the password itself, and since +the salt appears in plaintext at the beginning of the encrypted +password string this has the effect of revealing the first two +characters of the password to anyone who views the encrypted version. + +=end undocumented + =item C Given an ip address, this returns the associated network address, using diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index ec13bacf8..1d5c6850c 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -111,7 +111,7 @@ sub check_password { sub change_password { my ($class, $userid, $password) = @_; my $dbh = Bugzilla->dbh; - my $cryptpassword = Crypt($password); + my $cryptpassword = bz_crypt($password); $dbh->do("UPDATE profiles SET cryptpassword = ? WHERE userid = ?", undef, $cryptpassword, $userid); } diff --git a/checksetup.pl b/checksetup.pl index 6bbedaaea..37f96df0a 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -1464,6 +1464,14 @@ if ($^O !~ /MSWin32/i) { # The only use for loading globals.pl is for Crypt(), which should at some # point probably be factored out into Bugzilla::Auth::* +# XXX - bug 278792: Crypt has been moved to Bugzilla::Auth::bz_crypt. +# This section is probably no longer needed, but we need to make sure +# that things still work if we remove globals.pl. So that's for later. + +# It's safe to use Bugzilla::Auth here because parameters have now been +# defined. +use Bugzilla::Auth; + # globals.pl clears the PATH, but File::Find uses Cwd::cwd() instead of # Cwd::getcwd(), which we need to do because `pwd` isn't in the path - see # http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2001-09/msg00115.html @@ -2776,7 +2784,7 @@ if (GetFieldDef('bugs', 'long_desc')) { "(login_name, cryptpassword," . " disabledtext) VALUES (" . $dbh->quote($name) . - ", " . $dbh->quote(Crypt('okthen')) . ", " . + ", " . $dbh->quote(bz_crypt('okthen')) . ", " . "'Account created only to maintain database integrity')"); $s2 = $dbh->prepare("SELECT LAST_INSERT_ID()"); $s2->execute(); @@ -3200,7 +3208,7 @@ ENDTEXT print "Fixing password #1... "; while (my ($userid, $password) = $sth->fetchrow_array()) { - my $cryptpassword = $dbh->quote(Crypt($password)); + my $cryptpassword = $dbh->quote(bz_crypt($password)); $dbh->do("UPDATE profiles SET cryptpassword = $cryptpassword WHERE userid = $userid"); ++$i; # Let the user know where we are at every 500 records. @@ -4474,7 +4482,7 @@ if ($sth->rows == 0) { } # Crypt the administrator's password - my $cryptedpassword = Crypt($pass1); + my $cryptedpassword = bz_crypt($pass1); if ($^O !~ /MSWin32/i) { system("stty","echo"); # re-enable input echoing diff --git a/editusers.cgi b/editusers.cgi index a70e3fcf2..a1eccd956 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -38,6 +38,7 @@ require "globals.pl"; use Bugzilla; use Bugzilla::User; use Bugzilla::Constants; +use Bugzilla::Auth; # Shut up misguided -w warnings about "used only once". "use vars" just # doesn't work for me. @@ -452,7 +453,7 @@ if ($action eq 'new') { "emailflags, disabledtext" . " ) VALUES ( " . SqlQuote($user) . "," . - SqlQuote(Crypt($password)) . "," . + SqlQuote(bz_crypt($password)) . "," . SqlQuote($realname) . "," . SqlQuote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS) . "," . SqlQuote($disabledtext) . ")" ); @@ -784,7 +785,7 @@ if ($action eq 'update') { if ( $editall && $password ) { my $passworderror = ValidatePassword($password); if ( !$passworderror ) { - my $cryptpassword = SqlQuote(Crypt($password)); + my $cryptpassword = SqlQuote(bz_crypt($password)); my $loginname = SqlQuote($userold); SendSQL("UPDATE profiles SET cryptpassword = $cryptpassword diff --git a/globals.pl b/globals.pl index 694d02f49..0badac43e 100644 --- a/globals.pl +++ b/globals.pl @@ -34,6 +34,7 @@ use Bugzilla::Util; # Bring ChmodDataFile in until this is all moved to the module use Bugzilla::Config qw(:DEFAULT ChmodDataFile $localconfig $datadir); use Bugzilla::BugMail; +use Bugzilla::Auth; # Shut up misguided -w warnings about "used only once". For some reason, # "use vars" chokes on me when I try it here. @@ -414,7 +415,7 @@ sub InsertNewUser { # Generate a new random password for the user. my $password = GenerateRandomPassword(); - my $cryptpassword = Crypt($password); + my $cryptpassword = bz_crypt($password); my $defaultflagstring = SqlQuote(Bugzilla::Constants::DEFAULT_EMAIL_SETTINGS); @@ -696,39 +697,6 @@ sub ValidatePassword { } } - -sub Crypt { - # Crypts a password, generating a random salt to do it. - # Random salts are generated because the alternative is usually - # to use the first two characters of the password itself, and since - # the salt appears in plaintext at the beginning of the crypted - # password string this has the effect of revealing the first two - # characters of the password to anyone who views the crypted version. - - my ($password) = @_; - - # The list of characters that can appear in a salt. Salts and hashes - # are both encoded as a sequence of characters from a set containing - # 64 characters, each one of which represents 6 bits of the salt/hash. - # The encoding is similar to BASE64, the difference being that the - # BASE64 plus sign (+) is replaced with a forward slash (/). - my @saltchars = (0..9, 'A'..'Z', 'a'..'z', '.', '/'); - - # Generate the salt. We use an 8 character (48 bit) salt for maximum - # security on systems whose crypt uses MD5. Systems with older - # versions of crypt will just use the first two characters of the salt. - my $salt = ''; - for ( my $i=0 ; $i < 8 ; ++$i ) { - $salt .= $saltchars[rand(64)]; - } - - # Crypt the password. - my $cryptedpassword = crypt($password, $salt); - - # Return the crypted password. - return $cryptedpassword; -} - sub DBID_to_real_or_loginname { my ($id) = (@_); PushGlobalSQLState(); diff --git a/token.cgi b/token.cgi index 03d0e8b03..8b4636a79 100755 --- a/token.cgi +++ b/token.cgi @@ -33,6 +33,7 @@ use vars qw($template $vars); use Bugzilla; use Bugzilla::Constants; +use Bugzilla::Auth; my $cgi = Bugzilla->cgi; @@ -192,7 +193,7 @@ sub cancelChangePassword { sub changePassword { # Quote the password and token for inclusion into SQL statements. - my $cryptedpassword = Crypt($cgi->param('password')); + my $cryptedpassword = bz_crypt($cgi->param('password')); my $quotedpassword = SqlQuote($cryptedpassword); # Get the user's ID from the tokens table. diff --git a/userprefs.cgi b/userprefs.cgi index 1c9cf2068..6950fea88 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -28,6 +28,7 @@ use lib qw(.); use Bugzilla; use Bugzilla::Constants; use Bugzilla::Search; +use Bugzilla::Auth; require "CGI.pl"; @@ -94,7 +95,7 @@ sub SaveAccount { || ThrowUserError("new_password_missing"); ValidatePassword($pwd1, $pwd2); - my $cryptedpassword = SqlQuote(Crypt($pwd1)); + my $cryptedpassword = SqlQuote(bz_crypt($pwd1)); SendSQL("UPDATE profiles SET cryptpassword = $cryptedpassword WHERE userid = $userid"); -- cgit v1.2.1