From 8b8a3be2c511f5363b259d80cb1e69c5ac4b3b43 Mon Sep 17 00:00:00 2001 From: Alexandros Weigl Date: Tue, 18 Oct 2016 18:35:25 +0200 Subject: [PATCH 1/4] Added SaltQuery parameter --- .../Appserver/ServletEngine/Security/Utils/ParamKeys.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php index 51677ae2a..b02e04f34 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php @@ -53,6 +53,13 @@ class ParamKeys */ const ROLES_QUERY = 'rolesQuery'; + /** + * The key for the "saltQuery" parameter. + * + * @var string + */ + const SALT_QUERY = 'saltQuery'; + /** * The key for the "passwordStacking" parameter. * From 5e6623d1ddd1b2195c92312978bd2956c5ae6704 Mon Sep 17 00:00:00 2001 From: Alexandros Weigl Date: Tue, 18 Oct 2016 18:52:22 +0200 Subject: [PATCH 2/4] Implemented functionality for getUserSalt --- .../Auth/Spi/DatabasePDOLoginModule.php | 52 +++++++++++++++++++ .../Auth/Spi/UsernamePasswordLoginModule.php | 8 +++ 2 files changed, 60 insertions(+) diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php index 87783df08..9d9323235 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php @@ -65,6 +65,13 @@ class DatabasePDOLoginModule extends UsernamePasswordLoginModule */ protected $principalsQuery; + /** + * The database query used to load the password salt for the user. + * + * @var \AppserverIo\Lang\String + */ + protected $saltQuery; + /** * Initialize the login module. This stores the subject, callbackHandler and sharedState and options * for the login session. Subclasses should override if they need to process their own options. A call @@ -75,6 +82,7 @@ class DatabasePDOLoginModule extends UsernamePasswordLoginModule * lookupName: The datasource name used to lookup in the naming directory * rolesQuery: The database query used to load the user's roles * principalsQuery: The database query used to load the user + * saltQuery: The database query used to load the password salt for the user * * @param \AppserverIo\Psr\Security\Auth\Subject $subject The Subject to update after a successful login * @param \AppserverIo\Psr\Security\Auth\Callback\CallbackHandlerInterface $callbackHandler The callback handler that will be used to obtain the user identity and credentials @@ -93,6 +101,7 @@ public function initialize(Subject $subject, CallbackHandlerInterface $callbackH $this->lookupName = new String($params->get(ParamKeys::LOOKUP_NAME)); $this->rolesQuery = new String($params->get(ParamKeys::ROLES_QUERY)); $this->principalsQuery = new String($params->get(ParamKeys::PRINCIPALS_QUERY)); + $this->saltQuery = new String($params->get(ParamKeys::SALT_QUERY)); } /** @@ -148,4 +157,47 @@ protected function getRoleSets() { return Util::getRoleSets($this->getUsername(), new String($this->lookupName), new String($this->rolesQuery), $this); } + + /** + * Returns the salt for the user from the sharedMap data. + * + * @return \AppserverIo\Lang\String The user's salt + * @throws \AppserverIo\Psr\Security\Auth\Login\LoginException Is thrown if password can't be loaded + */ + protected function getUsersSalt() + { + + // load the application context + $application = RequestHandler::getApplicationContext(); + + /** @var \AppserverIo\Appserver\Core\Api\Node\DatabaseNode $databaseNode */ + $databaseNode = $application->getNamingDirectory()->search($this->lookupName)->getDatabase(); + + // prepare the connection parameters and create the DBAL connection + $connection = DriverManager::getConnection(ConnectionUtil::get($application)->fromDatabaseNode($databaseNode)); + + // try to load the principal's credential from the database + $statement = $connection->prepare($this->saltQuery); + $statement->bindParam(1, $this->getUsername()); + $statement->execute(); + + // close the PDO connection + if ($connection != null) { + try { + $connection->close(); + } catch (\Exception $e) { + $application + ->getNamingDirectory() + ->search(NamingDirectoryKeys::SYSTEM_LOGGER) + ->error($e->__toString()); + } + } + + // query whether or not we've a password found or not + if ($row = $statement->fetch(\PDO::FETCH_NUM)) { + return new String($row[0]); + } else { + throw new LoginException('No matching username found in principals'); + } + } } diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php index 293632aa0..d5f47638c 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php @@ -145,6 +145,14 @@ public function initialize(Subject $subject, CallbackHandlerInterface $callbackH */ abstract protected function getUsersPassword(); + /** + * Returns the salt for the user from the sharedMap data. + * + * @return \AppserverIo\Lang\String The user's salt + * @throws \AppserverIo\Psr\Security\Auth\Login\LoginException Is thrown if salt can't be loaded + */ + abstract protected function getUsersSalt(); + /** * Perform the authentication of username and password. * From 1639fc97f2b7e8b57c033dc83f940aca994f6380 Mon Sep 17 00:00:00 2001 From: Alexandros Weigl Date: Fri, 14 Oct 2016 17:55:34 +0200 Subject: [PATCH 3/4] Added salt support for password hashing - Also added more hashing algorithms which can be used - Salt is retrieved by the saltQuery parameter in context.xml which should include a DQL query --- .../Auth/Spi/DatabasePDOLoginModule.php | 5 +- .../Auth/Spi/UsernamePasswordLoginModule.php | 10 +- .../ServletEngine/Security/Utils/HashKeys.php | 40 ++++ .../Security/Utils/ParamKeys.php | 7 + .../ServletEngine/Security/Utils/Util.php | 24 +- .../ServletEngine/Security/Utils/UtilTest.php | 211 ++++++++++++++++++ 6 files changed, 288 insertions(+), 9 deletions(-) create mode 100644 src/AppserverIo/Appserver/ServletEngine/Security/Utils/HashKeys.php create mode 100644 src/AppserverIo/Appserver/ServletEngine/Security/Utils/UtilTest.php diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php index 9d9323235..d8d23990b 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php @@ -162,7 +162,6 @@ protected function getRoleSets() * Returns the salt for the user from the sharedMap data. * * @return \AppserverIo\Lang\String The user's salt - * @throws \AppserverIo\Psr\Security\Auth\Login\LoginException Is thrown if password can't be loaded */ protected function getUsersSalt() { @@ -193,11 +192,11 @@ protected function getUsersSalt() } } - // query whether or not we've a password found or not + // query whether or not we've found a salt or not if ($row = $statement->fetch(\PDO::FETCH_NUM)) { return new String($row[0]); } else { - throw new LoginException('No matching username found in principals'); + return null; } } } diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php index d5f47638c..53e477605 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/UsernamePasswordLoginModule.php @@ -266,6 +266,9 @@ protected function createPasswordHash(String $name, String $password) // initialize the callback $callback = null; + //get users salt + $hashSalt = $this->getUsersSalt(); + // query whether or not we've a callback configured if ($this->params->exists(ParamKeys::DIGEST_CALLBACK)) { try { @@ -278,14 +281,13 @@ protected function createPasswordHash(String $name, String $password) $tmp->add(SharedStateKeys::LOGIN_NAME, $name); $tmp->add(SharedStateKeys::LOGIN_PASSWORD, $password); $callback->init($tmp); - } catch (\Exception $e) { throw new SecurityException("Failed to load DigestCallback"); } } // hash and return the password - return Util::createPasswordHash($this->hashAlgorithm, $this->hashEncoding, $this->hashCharset, $name, $password, $callback); + return Util::createPasswordHash($this->hashAlgorithm, $this->hashEncoding, $this->hashCharset, $name, $password, $callback, $hashSalt); } /** @@ -317,6 +319,10 @@ protected function validatePassword(String $inputPassword, String $expectedPassw $valid = $inputPassword->equals($expectedPassword); } + if ($this->hashAlgorithm == "PASSWORD_BCRYPT" || $this->hashAlgorithm == "PASSWORD_DEFAULT") { + $valid = password_verify($inputPassword, $expectedPassword); + } + // return the flag return $valid; } diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/HashKeys.php b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/HashKeys.php new file mode 100644 index 000000000..57999c03a --- /dev/null +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/HashKeys.php @@ -0,0 +1,40 @@ + + * @copyright 2016 TechDivision GmbH + * @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0) + * @link https://github.com/appserver-io/appserver + */ +class HashKeys +{ + /** + * They key for the "md5" hash algorithm + * + * @var string + */ + const MD5 = 'md5'; + + /** + * They key for the "sha1" hash algorithm + * + * @var string + */ + const SHA1 = 'sha1'; + + /** + * They key for the "sha256" hash algorithm + * + * @var string + */ + const SHA256 = 'sha256'; + + /** + * They key for the "sha512" hash algorithm + * + * @var string + */ + const SHA512 = 'sha512'; +} diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php index b02e04f34..2a5a917b6 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/ParamKeys.php @@ -88,6 +88,13 @@ class ParamKeys */ const HASH_CHARSET = 'hashCharset'; + /** + * the key for the "salt" parameter. + * + * @var string + */ + const HASH_SALT = 'hashSalt'; + /** * The key for the "ignorePasswordCase" parameter. * diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/Util.php b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/Util.php index 0426de81e..d3c113c44 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/Util.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/Util.php @@ -60,23 +60,40 @@ class Util /** * Creates and returns a hashed version of the passed password. * + * * @param string $hashAlgorithm The hash algorithm to use * @param string $hashEncoding The hash encoding to use * @param string $hashCharset The hash charset to use * @param \AppserverIo\Lang\String $name The login name * @param \AppserverIo\Lang\String $password The password credential * @param mixed $callback The callback providing some additional hashing functionality + * @param string $hashSalt The hash salt to use * * @return \AppserverIo\Lang\String The hashed password */ - public static function createPasswordHash($hashAlgorithm, $hashEncoding, $hashCharset, String $name, String $password, $callback) + public static function createPasswordHash($hashAlgorithm, $hashEncoding, $hashCharset, String $name, String $password, $callback, $hashSalt = null) { $newPassword = clone $password; - return $newPassword->md5(); + switch ($hashAlgorithm) { + case HashKeys::MD5: + return $newPassword->md5($hashSalt); + case HashKeys::SHA1: + return $newPassword->sha1($hashSalt); + case HashKeys::SHA256: + return $newPassword->sha256($hashSalt); + case HashKeys::SHA512: + return $newPassword->sha512($hashSalt); + case PASSWORD_BCRYPT: + return $newPassword; + case PASSWORD_DEFAULT: + return $newPassword; + case 'default': + return $newPassword; + } } /** - * Execute the rolesQuery against the dsJndiName to obtain the roles for the authenticated user. + * Execute the rolesQuery against the UserName to obtain the roles for the authenticated user. * * @param \AppserverIo\Lang\String $username The username to load the roles for * @param \AppserverIo\Lang\String $lookupName The lookup name for the datasource @@ -156,7 +173,6 @@ public static function getRoleSets(String $username, String $lookupName, String // load one group after another } while ($row = $statement->fetch(\PDO::FETCH_OBJ)); - } catch (NamingException $ne) { throw new LoginException($ne->__toString()); } catch (\PDOException $pdoe) { diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Utils/UtilTest.php b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/UtilTest.php new file mode 100644 index 000000000..944de9c57 --- /dev/null +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Utils/UtilTest.php @@ -0,0 +1,211 @@ + + * @copyright 2016 TechDivision GmbH + * @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0) + * @link https://github.com/appserver-io/appserver + */ +class UtilTest extends \PHPUnit_Framework_TestCase +{ + protected $password; + protected $name; + protected $salt; + protected $hashAlgorithm; + protected $hashEncoding; + protected $hashCharset; + protected $callback; + + /** + * Setup the test data + * + * @return void + */ + public function setUp() + { + $this->name = new String("test"); + $this->password = new String("test"); + } + + /** + * Test if createPasswordHash hashes md5 correctly without a given salt + * + * @return void + */ + public function testCreatePasswordHashedMd5WithoutSalt() + { + $this->hashAlgorithm = HashKeys::MD5; + $expectedPassword = md5($this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback + ); + $this->assertEquals($expectedPassword, $password); + } + + /** + * Test if createPasswordHash hashes md5 correctly with a salt + * + * @return void + */ + public function testCreatePasswordHashedMd5WithSalt() + { + $this->hashAlgorithm = HashKeys::MD5; + $this->salt = '1234'; + $expectedPassword = md5($this->salt . $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback, + $this->salt + ); + + $this->assertEquals($expectedPassword, $password->stringValue()); + } + + /** + * Test if createPasswordHash hashes SHA1 correctly without a salt + * + * @return void + */ + public function testCreatePasswordHashedSha1WithoutSalt() + { + $this->hashAlgorithm = HashKeys::SHA1; + $expectedPassword = hash(HashKeys::SHA1, $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback + ); + + $this->assertEquals($expectedPassword, $password); + } + + /** + * Test if createPasswordHash hashes SHA1 correctly with a salt + * + * @return void + */ + public function testCreatePasswordHashedSha1WithSalt() + { + $this->hashAlgorithm = HashKeys::SHA1; + $this->salt = '1234'; + $expectedPassword = hash(HashKeys::SHA1, $this->salt . $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback, + $this->salt + ); + + $this->assertEquals($expectedPassword, $password->stringValue()); + } + + /** + * Test if createPasswordHash hashes SHA256 correclty without a salt + * + * @return void + */ + public function testCreatePasswordHashedSha256WithoutSalt() + { + $this->hashAlgorithm = HashKeys::SHA256; + $expectedPassword = hash(HashKeys::SHA256, $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback + ); + + $this->assertEquals($expectedPassword, $password); + } + + /** + * Test if createPasswordHash hashes SHA256 correctly with a salt + * + * @return void + */ + public function testCreatePasswordHashedSha256WithSalt() + { + $this->hashAlgorithm = HashKeys::SHA256; + $this->salt = '1234'; + $expectedPassword = hash(HashKeys::SHA256, $this->salt . $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback, + $this->salt + ); + + $this->assertEquals($expectedPassword, $password->stringValue()); + } + + /** + * Test if createPasswordHash hashes SHA512 correclty without a salt + * + * @return void + */ + public function testCreatePasswordHashedSha512WithoutSalt() + { + $this->hashAlgorithm = HashKeys::SHA512; + $expectedPassword = hash(HashKeys::SHA512, $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback + ); + + $this->assertEquals($expectedPassword, $password); + } + + /** + * Test if createPasswordHash hashes SHA512 correctly with a salt + * + * @return void + */ + public function testCreatePasswordHashedSha512WithSalt() + { + $this->hashAlgorithm = HashKeys::SHA512; + $this->salt = '1234'; + $expectedPassword = hash(HashKeys::SHA512, $this->salt . $this->password); + $password = Util::createPasswordHash( + $this->hashAlgorithm, + $this->hashEncoding, + $this->hashCharset, + $this->name, + $this->password, + $this->callback, + $this->salt + ); + + $this->assertEquals($expectedPassword, $password->stringValue()); + } +} From 81730dad2adf7e6c94df9e1afcb3ac5ca4433c5a Mon Sep 17 00:00:00 2001 From: Alexandros Weigl Date: Wed, 19 Oct 2016 17:47:29 +0200 Subject: [PATCH 4/4] Check if saltQuery string is set before executing saltQuery --- .../Security/Auth/Spi/DatabasePDOLoginModule.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php index d8d23990b..8476dd44a 100644 --- a/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php +++ b/src/AppserverIo/Appserver/ServletEngine/Security/Auth/Spi/DatabasePDOLoginModule.php @@ -166,6 +166,10 @@ protected function getRoleSets() protected function getUsersSalt() { + if ($this->saltQuery->stringValue() == 0) { + return null; + } + // load the application context $application = RequestHandler::getApplicationContext(); @@ -196,7 +200,7 @@ protected function getUsersSalt() if ($row = $statement->fetch(\PDO::FETCH_NUM)) { return new String($row[0]); } else { - return null; + throw new LoginException('No matching salt found in principals'); } } }