From 28154321a356050d479c04a3fc48b02b97a83dda Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 10 Sep 2021 11:27:33 +0200 Subject: [PATCH] Bug 28998: Store encrypted secret --- C4/Auth.pm | 5 ++- Koha/Auth/TwoFactorAuth.pm | 11 ++--- Koha/Encryption.pm | 47 ++++++++++++++++++++ members/two_factor_auth.pl | 7 +-- t/db_dependent/Koha/Encryption.t | 17 +++++++ t/db_dependent/selenium/authentication_2fa.t | 10 +++-- 6 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 Koha/Encryption.pm create mode 100755 t/db_dependent/Koha/Encryption.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 3c37966cb5a..4bd0ba4ea97 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -33,6 +33,7 @@ use Koha::Logger; use Koha::Caches; use Koha::AuthUtils qw( get_script_name hash_password ); use Koha::Auth::TwoFactorAuth; +use Koha::Encryption; use Koha::Checkouts; use Koha::DateUtils qw( dt_from_string ); use Koha::Library::Groups; @@ -890,7 +891,9 @@ sub checkauth { && ( my $otp_token = $query->param('otp_token') ) ) { my $patron = Koha::Patrons->find( { userid => $userid } ); - my $auth = Koha::Auth::TwoFactorAuth::get_auth( { patron => $patron } ); + my $secret32 = Koha::Encryption->new->decrypt_hex($patron->secret); + my $auth = Koha::Auth::TwoFactorAuth->new( + { patron => $patron, secret32 => $secret32 } ); my $verified = $auth->verify($otp_token); $auth->clear; if ( $verified ) { diff --git a/Koha/Auth/TwoFactorAuth.pm b/Koha/Auth/TwoFactorAuth.pm index 4cf9b165f70..8ad7ec5b7f1 100644 --- a/Koha/Auth/TwoFactorAuth.pm +++ b/Koha/Auth/TwoFactorAuth.pm @@ -38,21 +38,18 @@ It's based on Auth::GoogleAuth =cut -sub get_auth { - my ($params) = @_; +sub new { + my ($class, $params) = @_; + my $patron = $params->{patron}; my $secret = $params->{secret}; my $secret32 = $params->{secret32}; - if (!$secret && !$secret32){ - $secret32 = $patron->secret; - } - my $issuer = $patron->library->branchname; my $key_id = sprintf "%s_%s", $issuer, ( $patron->email || $patron->userid ); - return Auth::GoogleAuth->new( + return $class->SUPER::new( { ( $secret ? ( secret => $secret ) : () ), ( $secret32 ? ( secret32 => $secret32 ) : () ), diff --git a/Koha/Encryption.pm b/Koha/Encryption.pm new file mode 100644 index 00000000000..50abc147cc3 --- /dev/null +++ b/Koha/Encryption.pm @@ -0,0 +1,47 @@ +package Koha::Encryption; + +# 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 base qw( Crypt::CBC ); + +=head1 NAME + +Koha::Encryption - Koha class to encrypt or decrypt strings + +=head1 SYNOPSIS + +use Koha::Encryption; +my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); +my $crypt = Koha::Encryption->new; +my $encrypted = $crypt->encrypt_hex($secret); +my $decrypted = crypt->decrypt_hex($encrypted); + +It's based on Crypt::CBC + +=cut + +sub new { + my ( $class ) = @_; + my $key = C4::Context->config('encryption_key'); + return $class->SUPER::new( + -key => $key, + -cipher => 'Cipher::AES' + ); +} + +1; diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index ce5143ea7da..0bbe89839e6 100644 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -48,7 +48,7 @@ my $op = $cgi->param('op') // ''; if ( $op eq 'register-2FA' ) { my $pin_code = $cgi->param('pin_code'); my $secret32 = $cgi->param('secret32'); - my $auth = Koha::Auth::TwoFactorAuth::get_auth( + my $auth = Koha::Auth::TwoFactorAuth->new( { patron => $logged_in_user, secret32 => $secret32 } ); my $verified = $auth->verify( @@ -60,7 +60,8 @@ if ( $op eq 'register-2FA' ) { ); if ($verified) { - $logged_in_user->secret($secret32); + my $encrypted_secret = Koha::Encryption->new->encrypt_hex($secret32); + $logged_in_user->secret($encrypted_secret); $op = 'registered'; # FIXME Generate a (new?) secret @@ -75,7 +76,7 @@ if ( $op eq 'register-2FA' ) { if ( $op eq 'enable-2FA' ) { my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); - my $auth = Koha::Auth::TwoFactorAuth::get_auth( + my $auth = Koha::Auth::TwoFactorAuth->new( { patron => $logged_in_user, secret => $secret } ); my $secret32 = $auth->generate_secret32; diff --git a/t/db_dependent/Koha/Encryption.t b/t/db_dependent/Koha/Encryption.t new file mode 100755 index 00000000000..5e234b41a9b --- /dev/null +++ b/t/db_dependent/Koha/Encryption.t @@ -0,0 +1,17 @@ +use Modern::Perl; + +use Test::More tests => 1; +use Koha::Encryption; +use t::lib::Mocks; + +t::lib::Mocks::mock_config('encryption_key', 'my secret passphrase'); + +my $string = 'a string to encrypt'; + +my $crypt = Koha::Encryption->new(); +my $encrypted_string = $crypt->encrypt($string); +is( $crypt->decrypt_hex($encrypted_string), $string ); + +use Koha::AuthUtils; +my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); +my $encrypted_secret = Koha::Encryption->new->encrypt_hex($secret); diff --git a/t/db_dependent/selenium/authentication_2fa.t b/t/db_dependent/selenium/authentication_2fa.t index ebc0c7f991a..6fc42228e78 100755 --- a/t/db_dependent/selenium/authentication_2fa.t +++ b/t/db_dependent/selenium/authentication_2fa.t @@ -21,6 +21,7 @@ use Test::More tests => 2; use C4::Context; use Koha::AuthUtils; use Koha::Auth::TwoFactorAuth; +use Koha::Encryption; use t::lib::Mocks; use t::lib::Selenium; use t::lib::TestBuilder; @@ -80,7 +81,7 @@ SKIP: { $s->submit_form; is( $driver->find_element('//div[@class="two-factor-status"]')->get_text(), 'Status: Enabled', '2FA is enabled' ); $patron = $patron->get_from_storage; - is( $patron->secret, $secret32, 'secret is set in DB' ); + is( Koha::Encryption->new->decrypt_hex($patron->secret), $secret32, 'encrypted secret is set in DB' ); }; @@ -89,6 +90,7 @@ SKIP: { my $mainpage = $s->base_url . q|mainpage.pl|; + my $secret32 = Koha::Encryption->new->decrypt_hex($patron->secret); { # ok first try $driver->get($mainpage . q|?logout.x=1|); $driver->get($s->base_url . q|circ/circulation.pl?borrowernumber=|.$patron->borrowernumber); @@ -98,7 +100,8 @@ SKIP: { like( $driver->get_title, qr(Two-factor authentication), 'Must be on the second auth screen' ); is( login_error($s), undef ); - my $auth = Koha::Auth::TwoFactorAuth->new({patron => $patron}); + my $auth = Koha::Auth::TwoFactorAuth->new( + { patron => $patron, secret32 => $secret32 } ); my $code = $auth->code(); $auth->clear; $driver->find_element('//form[@id="loginform"]//input[@id="otp_token"]')->send_keys($code); @@ -139,7 +142,8 @@ SKIP: { $driver->find_element('//input[@type="submit"]')->click; ok($driver->find_element('//div[@class="dialog error"][contains(text(), "Invalid two-factor code")]')); - my $auth = Koha::Auth::TwoFactorAuth->new({patron => $patron}); + my $auth = Koha::Auth::TwoFactorAuth->new( + { patron => $patron, secret32 => $secret32 } ); my $code = $auth->code(); $auth->clear; $driver->find_element('//form[@id="loginform"]//input[@id="otp_token"]')->send_keys($code); -- 2.25.1