From 547c6d29499a3e1306a27050b407560a1c4f8fb6 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Wed, 9 Oct 2013 03:29:22 +0000 Subject: [PATCH] Bug 9611: (follow-up) move new password hashing routines to separate module The move avoids a problem where many modules would gain a dependency on C4::Auth just because C4::Members needs access to hash_password(). This patch also adds a couple unit tests for the new password hashing code. To test: [1] Verify that there are no regressions on the test plan for bug 9611. [2] Verify that t/AuthUtils.t and t/db_dependent/Auth.t pass. Signed-off-by: Galen Charlton --- C4/Auth.pm | 90 +---------------------- C4/Auth_with_ldap.pm | 3 +- C4/Members.pm | 2 +- Koha/AuthUtils.pm | 141 +++++++++++++++++++++++++++++++++++++ members/member-password.pl | 3 +- opac/opac-passwd.pl | 2 +- t/AuthUtils.t | 26 +++++++ t/db_dependent/Auth.t | 9 ++- 8 files changed, 183 insertions(+), 93 deletions(-) create mode 100644 Koha/AuthUtils.pm create mode 100644 t/AuthUtils.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 156be9d7ae..d73edaf5f7 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -23,14 +23,13 @@ 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; use C4::Context; use C4::Templates; # to get the template use C4::Branch; # GetBranches use C4::VirtualShelves; +use Koha::AuthUtils qw(hash_password); use POSIX qw/strftime/; use List::MoreUtils qw/ any /; @@ -50,7 +49,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 +1487,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 c26990e966..bf5393643a 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::AuthUtils 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 f58bb76793..1d966b301d 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -39,7 +39,7 @@ use DateTime; use DateTime::Format::DateParse; use Koha::DateUtils; use Text::Unaccent qw( unac_string ); -use C4::Auth qw(hash_password); +use Koha::AuthUtils qw(hash_password); our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug); diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm new file mode 100644 index 0000000000..a8024d928f --- /dev/null +++ b/Koha/AuthUtils.pm @@ -0,0 +1,141 @@ +package Koha::AuthUtils; + +# Copyright 2013 Catalyst IT +# +# 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 3 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, see . + +use Modern::Perl; +use Crypt::Eksblowfish::Bcrypt qw(bcrypt en_base64); +use Fcntl qw/O_RDONLY/; # O_RDONLY is used in generate_salt + +use base 'Exporter'; + +our $VERSION = '1.01'; +our @EXPORT_OK = qw(hash_password); + +=head1 NAME + +Koha::AuthUtils - utility routines for authentication + +=head1 SYNOPSIS + + use Koha::AuthUtils qw/hash_password/; + my $hash = hash_password($password); + +=head1 DESCRIPTION + +This module provides utility functions related to managing +user passwords. + +=head1 FUNCTIONS + +=head2 hash_password + + my $hash = Koha::AuthUtils::hash_password($password, $settings); + +=cut + +# 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 + + my $salt = Koha::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 Koha::AuthUtils::generate_salt\n"; + } + + if( $strength eq "strong" ){ + $source = '/dev/random'; # blocking + } else { + unless( $strength eq 'weak' ){ + warn "unsuppored strength of '$strength' passed to Koha::AuthUtils::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::AuthUtils::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::AuthUtils::generate_salt\n"; + } + + $string .= $bytes; + } + + close SOURCE; + return $string; +} +1; + +__END__ + +=head1 SEE ALSO + +Crypt::Eksblowfish::Bcrypt(3) + +=cut diff --git a/members/member-password.pl b/members/member-password.pl index d2a9a31ee0..9495e0e8d6 100755 --- a/members/member-password.pl +++ b/members/member-password.pl @@ -8,6 +8,7 @@ use strict; use warnings; use C4::Auth; +use Koha::AuthUtils; use C4::Output; use C4::Context; use C4::Members; @@ -55,7 +56,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::AuthUtils::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 923a5cf194..440f9a23ef 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::AuthUtils qw(hash_password); my $query = new CGI; my $dbh = C4::Context->dbh; diff --git a/t/AuthUtils.t b/t/AuthUtils.t new file mode 100644 index 0000000000..f0fa81bd46 --- /dev/null +++ b/t/AuthUtils.t @@ -0,0 +1,26 @@ +# This file is part of Koha. +# +# Copyright (C) 2013 Equinox Software, Inc. +# +# 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 3 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, see . + +use Modern::Perl; +use Test::More tests => 1; + +use Koha::AuthUtils qw/hash_password/; + +my $hash1 = hash_password('password'); +my $hash2 = hash_password('password'); + +ok($hash1 ne $hash2, 'random salts used when generating password hash'); diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index 814bb5f1eb..8bb01bbff6 100644 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -8,8 +8,9 @@ use Modern::Perl; use CGI; use Test::MockModule; use List::MoreUtils qw/all any none/; -use Test::More tests => 4; +use Test::More tests => 6; use C4::Members; +use Koha::AuthUtils qw/hash_password/; BEGIN { use_ok('C4::Auth'); @@ -106,4 +107,10 @@ $dbh->{RaiseError} = 1; 'BZ9735: invalid language, then default to en'); } +my $hash1 = hash_password('password'); +my $hash2 = hash_password('password'); + +ok(C4::Auth::checkpw_hash('password', $hash1), 'password validates with first hash'); +ok(C4::Auth::checkpw_hash('password', $hash2), 'password validates with second hash'); + $dbh->rollback; -- 2.39.5