From f76ecfe71ef2949799cf9fb20f8d72b468364fa3 Mon Sep 17 00:00:00 2001 From: Demian Katz <demian.katz@villanova.edu> Date: Fri, 11 Jan 2013 12:18:29 -0500 Subject: [PATCH] Resolving VUFIND-340 by adding optional password security in the database. Thanks to Mark Triggs for initial research and Kyle McGrogan for implementation work. --- config/vufind/config.ini | 11 +++ module/VuFind/sql/mysql.sql | 2 + module/VuFind/sql/postgres.sql | 2 + module/VuFind/src/VuFind/Auth/Database.php | 37 +++++++++- module/VuFind/src/VuFind/Auth/Manager.php | 3 + module/VuFind/src/VuFind/Db/Row/User.php | 68 ++++++++++++++++++- .../src/VuFind/Exception/PasswordSecurity.php | 41 +++++++++++ 7 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 module/VuFind/src/VuFind/Exception/PasswordSecurity.php diff --git a/config/vufind/config.ini b/config/vufind/config.ini index 5d22b0ce3e0..cabd47b28e0 100644 --- a/config/vufind/config.ini +++ b/config/vufind/config.ini @@ -169,6 +169,17 @@ method = Database ; Whether or not to hide the Login Options hideLogin = false +; Set this to false if you would like to store local passwords in plain text +; (only applies when method = Database above). +hash_passwords = false + +; Set this to false if you would like to store catalog passwords in plain text +encrypt_ils_password = false + +; This is the key used to encrypt and decrypt catalog passwords. This must be +; filled in with a random string value when encrypt_ils_passwords is set to true. +ils_encryption_key = false + ; See the comments in library/VF/Auth/MultiAuth.php for full details ; on using multiple authentication methods. Note that MultiAuth assumes login ; with username and password, so some methods (i.e. Shibboleth) may not be diff --git a/module/VuFind/sql/mysql.sql b/module/VuFind/sql/mysql.sql index 91fe565edd8..ceb68591982 100644 --- a/module/VuFind/sql/mysql.sql +++ b/module/VuFind/sql/mysql.sql @@ -93,11 +93,13 @@ CREATE TABLE `user` ( `id` int(11) NOT NULL AUTO_INCREMENT, `username` varchar(30) NOT NULL DEFAULT '', `password` varchar(32) NOT NULL DEFAULT '', + `pass_hash` varchar(60) DEFAULT NULL, `firstname` varchar(50) NOT NULL DEFAULT '', `lastname` varchar(50) NOT NULL DEFAULT '', `email` varchar(250) NOT NULL DEFAULT '', `cat_username` varchar(50) DEFAULT NULL, `cat_password` varchar(50) DEFAULT NULL, + `cat_pass_enc` varchar(110) DEFAULT NULL, `college` varchar(100) NOT NULL DEFAULT '', `major` varchar(100) NOT NULL DEFAULT '', `home_library` varchar(100) NOT NULL DEFAULT '', diff --git a/module/VuFind/sql/postgres.sql b/module/VuFind/sql/postgres.sql index fdc5e4a6b3b..eb4c419725a 100644 --- a/module/VuFind/sql/postgres.sql +++ b/module/VuFind/sql/postgres.sql @@ -99,11 +99,13 @@ CREATE TABLE "user"( id SERIAL, username varchar(30) NOT NULL DEFAULT '', password varchar(32) NOT NULL DEFAULT '', +pass_hash varchar(60) DEFAULT NULL, firstname varchar(50) NOT NULL DEFAULT '', lastname varchar(50) NOT NULL DEFAULT '', email varchar(250) NOT NULL DEFAULT '', cat_username varchar(50) DEFAULT NULL, cat_password varchar(50) DEFAULT NULL, +cat_pass_enc varchar(110) DEFAULT NULL, college varchar(100) NOT NULL DEFAULT '', major varchar(100) NOT NULL DEFAULT '', home_library varchar(100) NOT NULL DEFAULT '', diff --git a/module/VuFind/src/VuFind/Auth/Database.php b/module/VuFind/src/VuFind/Auth/Database.php index 4d16163ef53..ee57e90cb2b 100644 --- a/module/VuFind/src/VuFind/Auth/Database.php +++ b/module/VuFind/src/VuFind/Auth/Database.php @@ -28,7 +28,7 @@ * @link http://vufind.org/wiki/building_an_authentication_handler Wiki */ namespace VuFind\Auth; -use VuFind\Exception\Auth as AuthException; +use VuFind\Exception\Auth as AuthException, Zend\Crypt\Password\Bcrypt; /** * Database authentication class @@ -74,6 +74,17 @@ class Database extends AbstractBase return $user; } + /** + * Is password hashing enabled? + * + * @return bool + */ + protected function passwordHashingEnabled() + { + return isset($this->config->Authentication->hash_passwords) + ? $this->config->Authentication->hash_passwords : false; + } + /** * Create a new user account from the request. * @@ -127,13 +138,18 @@ class Database extends AbstractBase // If we got this far, we're ready to create the account: $data = array( 'username' => $params['username'], - 'password' => $params['password'], 'firstname' => $params['firstname'], 'lastname' => $params['lastname'], 'email' => $params['email'], 'created' => date('Y-m-d h:i:s') ); + if ($this->passwordHashingEnabled()) { + $bcrypt = new Bcrypt(); + $data['pass_hash'] = $bcrypt->create($params['password']); + } else { + $data['password'] = $params['password']; + } // Create the row and send it back to the caller: $table->insert($data); return $table->getByUsername($params['username'], false); @@ -143,12 +159,27 @@ class Database extends AbstractBase * Check that the user's password matches the provided value. * * @param string $password Password to check. - * @param object $userRow The user row. + * @param object $userRow The user row. We pass this instead of the password + * because we may need to check different values depending on the password + * hashing configuration. * * @return bool */ protected function checkPassword($password, $userRow) { + // Special case: hashing enabled: + if ($this->passwordHashingEnabled()) { + if ($userRow->password) { + throw new \VuFind\Exception\PasswordSecurity( + 'Unexpected unencrypted password found in database' + ); + } + + $bcrypt = new Bcrypt(); + return $bcrypt->verify($password, $userRow->pass_hash); + } + + // Default case: unencrypted passwords: return $password == $userRow->password; } diff --git a/module/VuFind/src/VuFind/Auth/Manager.php b/module/VuFind/src/VuFind/Auth/Manager.php index 00aee5b9737..4afc1d8aeea 100644 --- a/module/VuFind/src/VuFind/Auth/Manager.php +++ b/module/VuFind/src/VuFind/Auth/Manager.php @@ -263,6 +263,9 @@ class Manager implements ServiceLocatorAwareInterface } catch (AuthException $e) { // Pass authentication exceptions through unmodified throw $e; + } catch (\VuFind\Exception\PasswordSecurity $e) { + // Pass password security exceptions through unmodified + throw $e; } catch (\Exception $e) { // Catch other exceptions and treat them as technical difficulties throw new AuthException('authentication_error_technical'); diff --git a/module/VuFind/src/VuFind/Db/Row/User.php b/module/VuFind/src/VuFind/Db/Row/User.php index 639e6e5750f..9e1b094039d 100644 --- a/module/VuFind/src/VuFind/Db/Row/User.php +++ b/module/VuFind/src/VuFind/Db/Row/User.php @@ -26,7 +26,13 @@ * @link http://vufind.org Main Site */ namespace VuFind\Db\Row; -use Zend\Db\Sql\Expression, Zend\Db\Sql\Predicate\Predicate, Zend\Db\Sql\Sql; +use Zend\Db\Sql\Expression, + Zend\Db\Sql\Predicate\Predicate, + Zend\Db\Sql\Sql, + Zend\Crypt\Symmetric\Mcrypt, + Zend\Crypt\Password\Bcrypt, + Zend\Crypt\BlockCipher as BlockCipher, + VuFind\Config\Reader as ConfigReader; /** * Row Definition for user @@ -39,6 +45,13 @@ use Zend\Db\Sql\Expression, Zend\Db\Sql\Predicate\Predicate, Zend\Db\Sql\Sql; */ class User extends ServiceLocatorAwareGateway { + /** + * Encryption key used for catalog passwords (null if encryption disabled): + * + * @var string + */ + protected $encryptionKey = null; + /** * Constructor * @@ -47,6 +60,19 @@ class User extends ServiceLocatorAwareGateway public function __construct($adapter) { parent::__construct('id', 'user', $adapter); + $config = ConfigReader::getConfig(); + $encryption = isset($config->Authentication->encrypt_ils_password) + ? $config->Authentication->encrypt_ils_password : false; + if ($encryption) { + if (!isset($config->Authentication->ils_encryption_key) + || empty($config->Authentication->ils_encryption_key) + ) { + throw new \VuFind\Exception\PasswordSecurity( + 'ILS password encryption on, but no key set.' + ); + } + $this->encryptionKey = $config->Authentication->ils_encryption_key; + } } /** @@ -95,6 +121,7 @@ class User extends ServiceLocatorAwareGateway { $this->cat_username = null; $this->cat_password = null; + $this->cat_pass_enc = null; } /** @@ -108,7 +135,13 @@ class User extends ServiceLocatorAwareGateway public function saveCredentials($username, $password) { $this->cat_username = $username; - $this->cat_password = $password; + if ($this->passwordEncryptionEnabled()) { + $this->cat_password = null; + $this->cat_pass_enc = $this->encryptOrDecrypt($password, true); + } else { + $this->cat_password = $password; + $this->cat_pass_enc = null; + } return $this->save(); } @@ -120,7 +153,36 @@ class User extends ServiceLocatorAwareGateway */ public function getCatPassword() { - return isset($this->cat_password) ? $this->cat_password : null; + return $this->passwordEncryptionEnabled() + ? $this->encryptOrDecrypt($this->cat_pass_enc, false) + : (isset($this->cat_password) ? $this->cat_password : null); + } + + /** + * Is ILS password encryption enabled? + * + * @var bool + */ + protected function passwordEncryptionEnabled() + { + return null !== $this->encryptionKey; + } + + /** + * This is a central function for encrypting and decrypting so that + * logic is all in one location + * + * @param string $text The text to be encrypted or decrypted + * @param bool $encrypt True if we wish to encrypt text, False if we wish to + * decrypt text. + * + * @return string|bool The encrypted/decrypted string + */ + protected function encryptOrDecrypt($text, $encrypt = true) + { + $cipher = new BlockCipher(new Mcrypt(array('algorithm' => 'blowfish'))); + $cipher->setKey($this->encryptionKey); + return $encrypt ? $cipher->encrypt($text) : $cipher->decrypt($text); } /** diff --git a/module/VuFind/src/VuFind/Exception/PasswordSecurity.php b/module/VuFind/src/VuFind/Exception/PasswordSecurity.php new file mode 100644 index 00000000000..8223d5c50b2 --- /dev/null +++ b/module/VuFind/src/VuFind/Exception/PasswordSecurity.php @@ -0,0 +1,41 @@ +<?php +/** + * Password Security Exception + * + * PHP version 5 + * + * Copyright (C) Villanova University 2011. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program 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 this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * @category VuFind2 + * @package Exceptions + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link http://vufind.org/wiki/system_classes#index_interface Wiki + */ +namespace VuFind\Exception; + +/** + * Password Security Exception + * + * @category VuFind2 + * @package Exceptions + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link http://vufind.org/wiki/system_classes#index_interface Wiki + */ +class PasswordSecurity extends \Exception +{ +} -- GitLab