From 9ee8d6f3630130676033ee49d77f886d9d3c6d6b Mon Sep 17 00:00:00 2001 From: Robin Sheat Date: Tue, 8 Oct 2013 18:04:03 +1300 Subject: [PATCH] Bug 11013 - move hashing functions into their own module As the hashing functions were centralised into C4::Auth, this required more modules (C4::Members in particular) to use it. However, C4::Auth uses the database, and so anything that used C4::Members failed to load in the 00-load.t test. This patch moves the hashing into its own small module so this isn't an issue. Test plan: * Ensure that password related functions work: ** Creating a user account ** Logging in ** Changing a password (OPAC and staff client) ** LDAP logins still work like they should * Ensure that tests pass without a database server present Note: there are testing notes for LDAP on bug 9611 that'll bypass the need to set up an LDAP server. --- C4/Auth.pm | 89 +------------------------ C4/Auth_with_ldap.pm | 3 +- C4/Members.pm | 2 +- Koha/Passwords.pm | 162 +++++++++++++++++++++++++++++++++++++++++++++ members/member-password.pl | 4 +- opac/opac-passwd.pl | 2 +- t/Koha_Passwords.t | 34 ++++++++++ 7 files changed, 204 insertions(+), 92 deletions(-) create mode 100644 Koha/Passwords.pm create mode 100755 t/Koha_Passwords.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 156be9d..51ba6f4 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -23,7 +23,6 @@ use Digest::MD5 qw(md5_base64); use JSON qw/encode_json decode_json/; use URI::Escape; use CGI::Session; -use Crypt::Eksblowfish::Bcrypt qw(bcrypt en_base64); use Fcntl qw/O_RDONLY/; # O_RDONLY is used in generate_salt require Exporter; @@ -33,6 +32,7 @@ use C4::Branch; # GetBranches use C4::VirtualShelves; use POSIX qw/strftime/; use List::MoreUtils qw/ any /; +use Koha::Passwords qw/ hash_password /; # use utf8; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $ldap $cas $caslogout); @@ -50,7 +50,7 @@ BEGIN { @EXPORT = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions); @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &checkpw_internal &checkpw_hash &get_all_subpermissions &get_user_subpermissions - ParseSearchHistoryCookie hash_password + ParseSearchHistoryCookie ); %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] ); $ldap = C4::Context->config('useldapserver') || 0; @@ -1488,91 +1488,6 @@ sub get_session { return $session; } -# Using Bcrypt method for hashing. This can be changed to something else in future, if needed. -sub hash_password { - my $password = shift; - - # Generate a salt if one is not passed - my $settings = shift; - unless( defined $settings ){ # if there are no settings, we need to create a salt and append settings - # Set the cost to 8 and append a NULL - $settings = '$2a$08$'.en_base64(generate_salt('weak', 16)); - } - # Encrypt it - return bcrypt($password, $settings); -} - -=head2 generate_salt - - use C4::Auth; - my $salt = C4::Auth::generate_salt($strength, $length); - -=over - -=item strength - -For general password salting a C<$strength> of C is recommend, -For generating a server-salt a C<$strength> of C is recommended - -'strong' uses /dev/random which may block until sufficient entropy is acheived. -'weak' uses /dev/urandom and is non-blocking. - -=item length - -C<$length> is a positive integer which specifies the desired length of the returned string - -=back - -=cut - - -# the implementation of generate_salt is loosely based on Crypt::Random::Provider::File -sub generate_salt { - # strength is 'strong' or 'weak' - # length is number of bytes to read, positive integer - my ($strength, $length) = @_; - - my $source; - - if( $length < 1 ){ - die "non-positive strength of '$strength' passed to C4::Auth::generate_salt\n"; - } - - if( $strength eq "strong" ){ - $source = '/dev/random'; # blocking - } else { - unless( $strength eq 'weak' ){ - warn "unsuppored strength of '$strength' passed to C4::Auth::generate_salt, defaulting to 'weak'\n"; - } - $source = '/dev/urandom'; # non-blocking - } - - sysopen SOURCE, $source, O_RDONLY - or die "failed to open source '$source' in C4::Auth::generate_salt\n"; - - # $bytes is the bytes just read - # $string is the concatenation of all the bytes read so far - my( $bytes, $string ) = ("", ""); - - # keep reading until we have $length bytes in $strength - while( length($string) < $length ){ - # return the number of bytes read, 0 (EOF), or -1 (ERROR) - my $return = sysread SOURCE, $bytes, $length - length($string); - - # if no bytes were read, keep reading (if using /dev/random it is possible there was insufficient entropy so this may block) - next unless $return; - if( $return == -1 ){ - die "error while reading from $source in C4::Auth::generate_salt\n"; - } - - $string .= $bytes; - } - - close SOURCE; - return $string; -} - - sub checkpw { my ( $dbh, $userid, $password, $query ) = @_; diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index c26990e..9fbc208 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -26,7 +26,8 @@ use C4::Context; use C4::Members qw(AddMember changepassword); use C4::Members::Attributes; use C4::Members::AttributeTypes; -use C4::Auth qw(hash_password checkpw_internal); +use C4::Auth qw(checkpw_internal); +use Koha::Passwords qw( hash_password ); use List::MoreUtils qw( any ); use Net::LDAP; use Net::LDAP::Filter; diff --git a/C4/Members.pm b/C4/Members.pm index f58bb76..f968c3e 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -38,8 +38,8 @@ use C4::NewsChannels; #get slip news use DateTime; use DateTime::Format::DateParse; use Koha::DateUtils; +use Koha::Passwords qw( hash_password ); use Text::Unaccent qw( unac_string ); -use C4::Auth qw(hash_password); our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug); diff --git a/Koha/Passwords.pm b/Koha/Passwords.pm new file mode 100644 index 0000000..4efdb56 --- /dev/null +++ b/Koha/Passwords.pm @@ -0,0 +1,162 @@ +package Koha::Passwords; + +# Copyright 2013 Catalyst IT Ltd. +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +=head1 NAME + +Koha::Passwords - A central point to handle user passwords + +=head1 SYNOPSIS + + use Koha::Passwords; + my $hashed1 = Koha::Passwords::hash_password($password1); + my $hashed2 = Koha::Passwords::hash_password($password2, $hashed1); + print '$password1 is the same as $password2' if $hashed1 eq $hashed2; + +=head1 DESCRIPTION + +This provides the hashing function that should be used whenever you want to +do something with a password. + +=head1 FUNCTIONS + +=cut + +use Fcntl; +use Crypt::Eksblowfish::Bcrypt qw(bcrypt en_base64); +use Modern::Perl; + +require Exporter; + +use vars qw(@ISA @EXPORT @EXPORT_OK); + +BEGIN { + @ISA = qw(Exporter); + @EXPORT = qw(); + @EXPORT_OK = qw(hash_password); +} + +=head2 hash_password + + $hashed = hash_password($password, [$settings]); + +This hashes a password using whatever the secure algorithm of the day is. +The optional C<$settings> parameter describes what an existing password is +hashed with, and is attached to the start of the returned string. This means +that you can test a password by passing in the password you were given, +the hash you already have, and compare the hash you get out to the one you +have. + +This function is exportable. + +=cut + +sub hash_password { + my $password = shift; + # In case it gets called with -> instead of :: + $password = shift if $password eq 'Koha::Passwords'; + + # Generate a salt if one is not passed + my $settings = shift; + unless ( defined $settings ) + { # if there are no settings, we need to create a salt and append settings + # Set the cost to 8 and append a NULL + $settings = '$2a$08$' . en_base64( generate_salt( 'weak', 16 ) ); + } + + # Encrypt it + return bcrypt( $password, $settings ); +} + +=head2 generate_salt + + my $salt = generate_salt($strength, $length); + +=over + +=item strength + +For general password salting a C<$strength> of C is recommend, +For generating a server-salt a C<$strength> of C is recommended + +'strong' uses /dev/random which may block until sufficient entropy is acheived. +'weak' uses /dev/urandom and is non-blocking. + +=item length + +C<$length> is a positive integer which specifies the desired length of the returned string + +=back + +This function is not exportable. + +=cut + +# the implementation of generate_salt is loosely based on Crypt::Random::Provider::File +sub generate_salt { + + # strength is 'strong' or 'weak' + # length is number of bytes to read, positive integer + my ( $strength, $length ) = @_; + + my $source; + + if ( $length < 1 ) { + die +"non-positive strength of '$strength' passed to Koha::Passwords::generate_salt\n"; + } + + if ( $strength eq "strong" ) { + $source = '/dev/random'; # blocking + } + else { + unless ( $strength eq 'weak' ) { + warn +"unsuppored strength of '$strength' passed to Koha::Passwords::generate_salt, defaulting to 'weak'\n"; + } + $source = '/dev/urandom'; # non-blocking + } + + sysopen SOURCE, $source, O_RDONLY + or die + "failed to open source '$source' in Koha:Passwords::generate_salt\n"; + + # $bytes is the bytes just read + # $string is the concatenation of all the bytes read so far + my ( $bytes, $string ) = ( "", "" ); + + # keep reading until we have $length bytes in $strength + while ( length($string) < $length ) { + + # return the number of bytes read, 0 (EOF), or -1 (ERROR) + my $return = sysread SOURCE, $bytes, $length - length($string); + +# if no bytes were read, keep reading (if using /dev/random it is possible there was insufficient entropy so this may block) + next unless $return; + if ( $return == -1 ) { + die +"error while reading from $source in Koha::Passwords::generate_salt\n"; + } + + $string .= $bytes; + } + + close SOURCE; + return $string; +} + diff --git a/members/member-password.pl b/members/member-password.pl index d2a9a31..8b62b5a 100755 --- a/members/member-password.pl +++ b/members/member-password.pl @@ -7,7 +7,6 @@ use strict; use warnings; -use C4::Auth; use C4::Output; use C4::Context; use C4::Members; @@ -15,6 +14,7 @@ use C4::Branch; use C4::Circulation; use CGI; use C4::Members::Attributes qw(GetBorrowerAttributes); +use Koha::Passwords; use Digest::MD5 qw(md5_base64); @@ -55,7 +55,7 @@ my $minpw = C4::Context->preference('minPasswordLength'); push(@errors,'SHORTPASSWORD') if( $newpassword && $minpw && (length($newpassword) < $minpw ) ); if ( $newpassword && !scalar(@errors) ) { - my $digest=C4::Auth::hash_password($input->param('newpassword')); + my $digest=Koha::Passwords::hash_password($input->param('newpassword')); my $uid = $input->param('newuserid'); my $dbh=C4::Context->dbh; if (changepassword($uid,$member,$digest)) { diff --git a/opac/opac-passwd.pl b/opac/opac-passwd.pl index 923a5cf..e37d2c3 100755 --- a/opac/opac-passwd.pl +++ b/opac/opac-passwd.pl @@ -29,7 +29,7 @@ use Digest::MD5 qw(md5_base64); use C4::Circulation; use C4::Members; use C4::Output; -use C4::Auth qw(hash_password); +use Koha::Passwords qw( hash_password ); my $query = new CGI; my $dbh = C4::Context->dbh; diff --git a/t/Koha_Passwords.t b/t/Koha_Passwords.t new file mode 100755 index 0000000..bc93caf --- /dev/null +++ b/t/Koha_Passwords.t @@ -0,0 +1,34 @@ +#!/usr/bin/perl + +# Copyright 2013 Catalyst IT Ltd. +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use strict; +use warnings; + +use Test::More tests => 4; + +BEGIN { + use_ok('Koha::Passwords'); +} + +my $hashed1 = Koha::Passwords::hash_password('sup3rs3kr1t'); +ok(defined($hashed1), 'Hash 1 created'); +my $hashed2 = Koha::Passwords::hash_password('sup3rs3kr1t', $hashed1); +ok(defined($hashed1), 'Hash 2 created'); + +cmp_ok($hashed1, 'eq', $hashed2, 'Hashes match'); -- 1.8.1.2