diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php index cc755d84e3abe8048bb7c62ea31990a61d10c0fc..defcf78f5ce6cd811175ccfa679e3e2a4c19537f 100644 --- a/lib/SimpleSAML/Session.php +++ b/lib/SimpleSAML/Session.php @@ -353,7 +353,7 @@ class SimpleSAML_Session implements Serializable SimpleSAML\Logger::warning('Missing AuthToken cookie.'); return null; } - if ($_COOKIE[$authTokenCookieName] !== $session->authToken) { + if (!SimpleSAML\Utils\Crypto::secureCompare($session->authToken, $_COOKIE[$authTokenCookieName])) { SimpleSAML\Logger::warning('Invalid AuthToken cookie.'); return null; } diff --git a/lib/SimpleSAML/Utils/Crypto.php b/lib/SimpleSAML/Utils/Crypto.php index 100e7da436067f1fc6edb2e1c86866422d2589d2..d8a3356d1ec3657ea5fc008072dd58a54a4be83b 100644 --- a/lib/SimpleSAML/Utils/Crypto.php +++ b/lib/SimpleSAML/Utils/Crypto.php @@ -349,6 +349,38 @@ class Crypto } + /** + * Compare two strings securely. + * + * This method checks if two strings are equal in constant time, avoiding timing attacks. Use it every time we need + * to compare a string with a secret that shouldn't be leaked, i.e. when verifying passwords, one-time codes, etc. + * + * @param string $known A known string. + * @param string $user A user-provided string to compare with the known string. + * + * @return bool True if both strings are equal, false otherwise. + */ + public static function secureCompare($known, $user) + { + if (function_exists('hash_equals')) { + // use hash_equals() if available (PHP >= 5.6) + return hash_equals($known, $user); + } + + // compare manually in constant time + $len = mb_strlen($known, '8bit'); // see mbstring.func_overload + if ($len !== mb_strlen($user, '8bit')) { + return false; // length differs + } + $diff = 0; + for ($i = 0; $i < $len; ++$i) { + $diff |= $known[$i] ^ $user[$i]; + } + // if all the bytes in $a and $b are identical, $diff should be equal to 0 + return $diff === 0; + } + + /** * This function checks if a password is valid * @@ -374,7 +406,7 @@ class Crypto // hash w/o salt if (in_array(strtolower($alg), hash_algos())) { - return $hash === self::pwHash($password, $alg); + return self::secureCompare($hash, self::pwHash($password, $alg)); } // hash w/ salt @@ -384,7 +416,7 @@ class Crypto // get hash length of this algorithm to learn how long the salt is $hash_length = strlen(hash($php_alg, '', true)); $salt = substr(base64_decode($matches[2]), $hash_length); - return ($hash === self::pwHash($password, $alg, $salt)); + return self::secureCompare($hash, self::pwHash($password, $alg, $salt)); } } else { return $hash === $password; diff --git a/modules/authcrypt/lib/Auth/Source/Htpasswd.php b/modules/authcrypt/lib/Auth/Source/Htpasswd.php index 99923e18f96179efc931f76023f256e3e0edbde1..5731f4af191080c0dca1ca7bb3076c950cc5a053 100644 --- a/modules/authcrypt/lib/Auth/Source/Htpasswd.php +++ b/modules/authcrypt/lib/Auth/Source/Htpasswd.php @@ -74,8 +74,9 @@ class sspmod_authcrypt_Auth_Source_Htpasswd extends sspmod_core_Auth_UserPassBas $attributes = array_merge(array('uid' => array($username)), $this->attributes); // Traditional crypt(3) - if(crypt($password, $crypted) == $crypted) { + if (SimpleSAML\Utils\Crypto::secureCompare($crypted, crypt($password, $crypted))) { SimpleSAML\Logger::debug('User '. $username . ' authenticated successfully'); + SimpleSAML\Logger::warning('CRYPT authentication is insecure. Please consider using something else.'); return $attributes; } @@ -88,6 +89,7 @@ class sspmod_authcrypt_Auth_Source_Htpasswd extends sspmod_core_Auth_UserPassBas // SHA1 or plain-text if(SimpleSAML\Utils\Crypto::pwValid($crypted, $password)) { SimpleSAML\Logger::debug('User '. $username . ' authenticated successfully'); + SimpleSAML\Logger::warning('SHA1 and PLAIN TEXT authentication are insecure. Please consider using something else.'); return $attributes; } throw new SimpleSAML_Error_Error('WRONGUSERPASS');