From b72c79e3070f930d758f5c269333d63ed7509e2e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez=20Crespo?= <jaime.perez@uninett.no>
Date: Fri, 17 Mar 2017 10:34:24 +0100
Subject: [PATCH] Add a SimpleSAML\Utils\Crypto::secureCompare() method.
Use it when constant-time comparisons are needed to avoid side-channel attacks.
---
lib/SimpleSAML/Session.php | 2 +-
lib/SimpleSAML/Utils/Crypto.php | 36 +++++++++++++++++--
.../authcrypt/lib/Auth/Source/Htpasswd.php | 4 ++-
3 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php
index cc755d84e..defcf78f5 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 100e7da43..d8a3356d1 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 99923e18f..5731f4af1 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');
--
GitLab