From f1923f8e85501143d0be63d872c726159440f6c1 Mon Sep 17 00:00:00 2001 From: "mkanat%kerio.com" <> Date: Wed, 13 Jul 2005 10:56:58 +0000 Subject: Bug 300336: Bugzilla::Auth should not contain any exported subroutines Patch By Max Kanat-Alexander r=LpSolit, a=justdave --- Bugzilla/Auth.pm | 49 ------------------------------------------ Bugzilla/Auth/Verify/DB.pm | 5 +---- Bugzilla/User.pm | 1 - Bugzilla/Util.pm | 53 +++++++++++++++++++++++++++++++++++++++++++++- checksetup.pl | 17 ++++++--------- editusers.cgi | 1 - globals.pl | 1 - token.cgi | 2 +- userprefs.cgi | 2 +- 9 files changed, 62 insertions(+), 69 deletions(-) diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 91a0abf83..887caf049 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -23,8 +23,6 @@ package Bugzilla::Auth; use strict; -use base qw(Exporter); -@Bugzilla::Auth::EXPORT = qw(bz_crypt); use Bugzilla::Config; use Bugzilla::Constants; @@ -44,31 +42,6 @@ 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 @@ -160,11 +133,6 @@ __END__ Bugzilla::Auth - Authentication handling for Bugzilla users -=head1 SYNOPSIS - - # Class Functions - $crypted = bz_crypt($password); - =head1 DESCRIPTION Handles authentication for Bugzilla users. @@ -184,23 +152,6 @@ 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 4a45e81e7..405a737b8 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -34,10 +34,7 @@ use strict; use Bugzilla::Config; use Bugzilla::Constants; use Bugzilla::Util; -# Because of the screwy way that Auth works, it thinks -# that we're redefining subroutines if we "use" anything -# that "uses" Bugzilla::Auth. -require Bugzilla::User; +use Bugzilla::User; my $edit_options = { 'new' => 1, diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index e88135b07..847319c18 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -41,7 +41,6 @@ use Bugzilla::Error; use Bugzilla::Util; use Bugzilla::Constants; use Bugzilla::User::Setting; -use Bugzilla::Auth; use base qw(Exporter); @Bugzilla::User::EXPORT = qw(insert_new_user is_available_username diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 91e66f9f8..83c9bf7d3 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -37,7 +37,8 @@ use base qw(Exporter); diff_arrays diff_strings trim wrap_comment find_wrap_point format_time format_time_decimal - file_mod_time); + file_mod_time + bz_crypt); use Bugzilla::Config; use Bugzilla::Error; @@ -309,6 +310,31 @@ sub file_mod_time ($) { return $mtime; } +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; +} + sub ValidateDate { my ($date, $format) = @_; my $date2; @@ -369,6 +395,9 @@ Bugzilla::Util - Generic utility functions for bugzilla # Functions for dealing with files $time = file_mod_time($filename); + # Cryptographic Functions + $crypted_password = bz_crypt($password); + =head1 DESCRIPTION This package contains various utility functions which do not belong anywhere @@ -563,3 +592,25 @@ of the "mtime" parameter of the perl "stat" function. =back +=head2 Cryptography + +=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 + +=back diff --git a/checksetup.pl b/checksetup.pl index c86a2e427..3ac8eb5e3 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -1414,22 +1414,19 @@ if ($^O !~ /MSWin32/i) { # This is done here, because some modules require params to be set up, which # won't have happened earlier. -# The only use for loading globals.pl is for Crypt(), which should at some -# point probably be factored out into Bugzilla::Auth::* +# It's never safe to directly "use" a module in checksetup. If a module +# prerequisite is missing, and you "use" a module that requires it, +# then instead of our nice normal checksetup message the user would +# get a cryptic perl error about the missing module. -# 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. -require Bugzilla::Auth; -import Bugzilla::Auth 'bz_crypt'; +# So, we always wrap our "use" statements in checksetup in a string eval. # This is done so we can add new settings as developers need them. require Bugzilla::User::Setting; import Bugzilla::User::Setting qw(add_setting); +eval("use Bugzilla:Util"); + # 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 diff --git a/editusers.cgi b/editusers.cgi index ba5cc827e..18005fd94 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -27,7 +27,6 @@ use Bugzilla; use Bugzilla::User; use Bugzilla::Config; use Bugzilla::Constants; -use Bugzilla::Auth; use Bugzilla::Util; Bugzilla->login(LOGIN_REQUIRED); diff --git a/globals.pl b/globals.pl index bc9aa99ef..87d496c7b 100644 --- a/globals.pl +++ b/globals.pl @@ -36,7 +36,6 @@ 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; use Bugzilla::User; # Shut up misguided -w warnings about "used only once". For some reason, diff --git a/token.cgi b/token.cgi index 64bf8e364..0e0753807 100755 --- a/token.cgi +++ b/token.cgi @@ -33,7 +33,7 @@ use vars qw($template $vars); use Bugzilla; use Bugzilla::Constants; -use Bugzilla::Auth; +use Bugzilla::Util; my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; diff --git a/userprefs.cgi b/userprefs.cgi index 1cf15868b..07042beac 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -29,7 +29,7 @@ use lib qw(.); use Bugzilla; use Bugzilla::Constants; use Bugzilla::Search; -use Bugzilla::Auth; +use Bugzilla::Util; use Bugzilla::User; require "CGI.pl"; -- cgit v1.2.3-24-g4f1b