Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize password hash function usage #9480

Merged
merged 7 commits into from Jan 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 11 additions & 11 deletions plugins/Login/Auth.php
Expand Up @@ -11,14 +11,16 @@
use Exception;
use Piwik\AuthResult;
use Piwik\Db;
use Piwik\Piwik;
use Piwik\Plugins\UsersManager\Model;
use Piwik\Plugins\UsersManager\UsersManager;
use Piwik\Session;

class Auth implements \Piwik\Auth
{
protected $login;
protected $token_auth;
protected $md5Password;
protected $hashedPassword;

/**
* @var Model
Expand Down Expand Up @@ -47,7 +49,7 @@ public function getName()
*/
public function authenticate()
{
if (!empty($this->md5Password)) { // favor authenticating by password
if (!empty($this->hashedPassword)) { // favor authenticating by password
return $this->authenticateWithPassword($this->login, $this->getTokenAuthSecret());
} elseif (is_null($this->login)) {
return $this->authenticateWithToken($this->token_auth);
Expand Down Expand Up @@ -132,7 +134,7 @@ public function setLogin($login)
*/
public function getTokenAuthSecret()
{
return $this->md5Password;
return $this->hashedPassword;
}

/**
Expand All @@ -153,29 +155,27 @@ public function setTokenAuth($token_auth)
public function setPassword($password)
{
if (empty($password)) {
$this->md5Password = null;
$this->hashedPassword = null;
} else {
$this->md5Password = md5($password);
$this->hashedPassword = UsersManager::getPasswordHash($password);
}
}

/**
* Sets the password hash to use when authentication.
*
* @param string $passwordHash The password hash.
* @throws Exception if $passwordHash does not have 32 characters in it.
*/
public function setPasswordHash($passwordHash)
{
if ($passwordHash === null) {
$this->md5Password = null;
$this->hashedPassword = null;
return;
}

if (strlen($passwordHash) != 32) {
throw new Exception("Invalid hash: incorrect length " . strlen($passwordHash));
}
// check that the password hash is valid (sanity check)
UsersManager::checkPasswordHash($passwordHash, Piwik::translate('Login_ExceptionPasswordMD5HashExpected'));

$this->md5Password = $passwordHash;
$this->hashedPassword = $passwordHash;
}
}
3 changes: 2 additions & 1 deletion plugins/Login/Controller.php
Expand Up @@ -184,9 +184,10 @@ public function ajaxNoAccess($errorMessage)
* Authenticate user and password. Redirect if successful.
*
* @param string $login user name
* @param string $password md5 password
* @param string $password plain-text or hashed password
* @param bool $rememberMe Remember me?
* @param string $urlToRedirect URL to redirect to, if successfully authenticated
* @param bool $passwordHashed indicates if $password is hashed
* @return string failure message if unable to authenticate
*/
protected function authenticateAndRedirect($login, $password, $rememberMe, $urlToRedirect = false, $passwordHashed = false)
Expand Down
14 changes: 6 additions & 8 deletions plugins/Login/PasswordResetter.php
Expand Up @@ -310,7 +310,7 @@ protected function getSalt()
}

/**
* Hashes a string. By default generates an MD5 hash.
* Hashes a string.
*
* Derived classes can override this to provide a different hashing implementation.
*
Expand Down Expand Up @@ -378,14 +378,12 @@ protected function getUserInformation($loginOrMail)
*
* Derived classes can override this method to provide fewer or more checks.
*
* @param string $password The password to check.
* @throws Exception if the password is not 32 bytes long.
* @param string $passwordHash The password hash to check.
* @throws Exception if the password hash length is incorrect.
*/
protected function checkPasswordHash($password)
protected function checkPasswordHash($passwordHash)
{
if (strlen($password) != 32) {
throw new Exception(Piwik::translate('Login_ExceptionPasswordMD5HashExpected'));
}
UsersManager::checkPasswordHash($passwordHash, Piwik::translate('Login_ExceptionPasswordMD5HashExpected'));
}

/**
Expand Down Expand Up @@ -477,4 +475,4 @@ public static function getPasswordResetInfoOptionName($login)
{
return $login . '_reset_password_info';
}
}
}
11 changes: 4 additions & 7 deletions plugins/UsersManager/API.php
Expand Up @@ -342,7 +342,7 @@ public function getSitesAccessFromUser($userLogin)
}

/**
* Returns the user information (login, password md5, alias, email, date_registered, etc.)
* Returns the user information (login, password hash, alias, email, date_registered, etc.)
*
* @param string $userLogin the user login
*
Expand All @@ -359,7 +359,7 @@ public function getUser($userLogin)
}

/**
* Returns the user information (login, password md5, alias, email, date_registered, etc.)
* Returns the user information (login, password hash, alias, email, date_registered, etc.)
*
* @param string $userEmail the user email
*
Expand Down Expand Up @@ -783,15 +783,12 @@ private function isUserTheOnlyUserHavingSuperUserAccess($userLogin)
* Generates a unique MD5 for the given login & password
*
* @param string $userLogin Login
* @param string $md5Password MD5ied string of the password
* @throws Exception
* @param string $md5Password hashed string of the password (using current hash function; MD5-named for historical reasons)
* @return string
*/
public function getTokenAuth($userLogin, $md5Password)
{
if (strlen($md5Password) != 32) {
throw new Exception(Piwik::translate('UsersManager_ExceptionPasswordMD5HashExpected'));
}
UsersManager::checkPasswordHash($md5Password, Piwik::translate('UsersManager_ExceptionPasswordMD5HashExpected'));

return md5($userLogin . $md5Password);
}
Expand Down
14 changes: 14 additions & 0 deletions plugins/UsersManager/UsersManager.php
Expand Up @@ -157,6 +157,20 @@ public static function getPasswordHash($password)
return md5($password);
}

/**
* Checks the password hash length. Used as a sanity check.
*
* @param string $passwordHash The password hash to check.
* @param string $exceptionMessage Message of the exception thrown.
* @throws Exception if the password hash length is incorrect.
*/
public static function checkPasswordHash($passwordHash, $exceptionMessage)
{
if (strlen($passwordHash) != 32) { // MD5 hash length
throw new Exception($exceptionMessage);
}
}

public function getClientSideTranslationKeys(&$translationKeys)
{
$translationKeys[] = "General_OrCancel";
Expand Down