Bugzilla – Attachment 21873 Details for
Bug 11013
Tests broken by new crypto stuff
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 11013 - move hashing functions into their own module
SIGNED-OFF-Bug-11013---move-hashing-functions-into.patch (text/plain), 14.00 KB, created by
Bernardo Gonzalez Kriegel
on 2013-10-08 11:02:15 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 11013 - move hashing functions into their own module
Filename:
MIME Type:
Creator:
Bernardo Gonzalez Kriegel
Created:
2013-10-08 11:02:15 UTC
Size:
14.00 KB
patch
obsolete
>From d3f72c80625d0ca71294935e2ed343ccf2677209 Mon Sep 17 00:00:00 2001 >From: Robin Sheat <robin@catalyst.net.nz> >Date: Tue, 8 Oct 2013 18:04:03 +1300 >Subject: [PATCH] [SIGNED-OFF] 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. > >Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> >All Works. No koha-qa errors > >Test >1) patch applied >2) new user created >3) change pass from opac/staff >4) change pass using LDAP (sort of, same as on Bug 9611) > >NOTE: Robin, you just sent an email about copyright headers, >KohaPassword.pm's reads "...either version 2 of the License,...", >coding guidelines suggests other wording and version 3 of GPL. >--- > C4/Auth.pm | 90 +------------------------ > C4/Auth_with_ldap.pm | 3 +- > C4/Members.pm | 2 +- > Koha/Passwords.pm | 161 ++++++++++++++++++++++++++++++++++++++++++++ > members/member-password.pl | 4 +- > opac/opac-passwd.pl | 2 +- > t/Koha_Passwords.t | 34 ++++++++++ > 7 files changed, 203 insertions(+), 93 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..07996ee 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -23,8 +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; > use C4::Context; >@@ -33,6 +31,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 +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<weak> is recommend, >-For generating a server-salt a C<$strength> of C<strong> 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..e678074 >--- /dev/null >+++ b/Koha/Passwords.pm >@@ -0,0 +1,161 @@ >+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 qw/O_RDONLY/; # O_RDONLY is used in generate_salt >+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<weak> is recommend, >+For generating a server-salt a C<$strength> of C<strong> 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.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11013
:
21866
|
21867
| 21873 |
21887
|
21888