diff --git a/config/example-config.php b/config/example-config.php index 5fa72d57..088e47a2 100644 --- a/config/example-config.php +++ b/config/example-config.php @@ -623,12 +623,34 @@ $c->admin_email = 'calendar-admin@example.com'; ***************************************************************************/ /** -* Cache credentials, requires memcached to be enabled. +* Cache credentials, requires memcache_servers to be defined. +* +* If you enable this, please ensure that your memcached server(s) are +* suitably protected. While we store the credentials hashed in memcached, +* you may still be exposing youself, please ensure you understand the risk +* you may be taking by enabling this feature. +* +* We use HMAC-SHA256 and bcrypt using the password set in +* $c->auth_cache_secret as a pepper and a per user salt that is generated +* as required and cached for 10 days. +* +* Both the hash and the per user salt are stored in memcached. The hash has +* an expiry set as either $c->auth_cache_pass or $c->auth_cache_fail as +* appropriate. * * Default: false */ // $c->auth_cache = false; +/** +* Secret to use for caching passwords (aka pepper). For example, the output +* of: +* pwgen 32 +* +* Default: unset, which disables caching +*/ +// $c->auth_cache_secret = NULL; + /** * How long to cache credentials which username & password match. * diff --git a/inc/HTTPAuthSession.php b/inc/HTTPAuthSession.php index 66cad498..c260dda7 100644 --- a/inc/HTTPAuthSession.php +++ b/inc/HTTPAuthSession.php @@ -314,10 +314,10 @@ class HTTPAuthSession { if(isset($c->login_append_domain_if_missing) && $c->login_append_domain_if_missing && !preg_match('/@/',$username)) $username.='@'.$c->domain_name; - $cache_result = $this->CheckCache($username, $password); + $cache_result = $this->_CheckCache($username, $password); if ($cache_result == 0) { - # noop as CheckCache has nothing for this username/password combo + # noop as _CheckCache has nothing for this username/password combo # and we need to perform full authentication check. } else if ($cache_result == 1) { @@ -349,7 +349,7 @@ class HTTPAuthSession { if ( $principal = new Principal('username', $username) ) { if ( isset($c->dbg['password']) ) dbg_error_log( "password", ":CheckPassword: Name:%s, Pass:%s, File:%s, Active:%s", $username, $password, $principal->password, ($principal->user_active?'Yes':'No') ); if ( $principal->user_active && session_validate_password( $password, $principal->password ) ) { - $this->SetCache($username, $password, 'pass'); + $this->_SetCache($username, $password, 'pass'); return $principal; } } @@ -373,15 +373,15 @@ class HTTPAuthSession { } if ( $principal === false ) { - $this->SetCache($username, $password, 'fail'); + $this->_SetCache($username, $password, 'fail'); } else { - $this->SetCache($username, $password, 'pass'); + $this->_SetCache($username, $password, 'pass'); } return $principal; } - $this->SetCache($username, $password, 'fail'); + $this->_SetCache($username, $password, 'fail'); return false; } @@ -446,10 +446,14 @@ class HTTPAuthSession { * Internal function used to check a cache for username/password details. This is * intended to reduce the load on external authentication sources. */ - function CheckCache( $username, $password ) { + function _CheckCache( $username, $password ) { global $c; if (! $c->auth_cache) return 0; + if (! isset($c->auth_cache_secret) || $c->auth_cache_secret == '') { + dbg_error_log('LOG', "HTTPAuthSession:CheckCache: caching of credentials is enabled, but \$c->auth_cache_secret isn't set, using auth source"); + return 0; + } $cache = getCacheInstance(); if ($cache->isActive() === false) return 0; @@ -458,26 +462,31 @@ class HTTPAuthSession { $salt = $cache->get($cache_ns, 'salt'); - if (isset($salt) && $salt != '') { - $sha1_sent = session_salted_sha1($password, $salt); - $cached_credentials = $cache->get($cache_ns, $sha1_sent); + if (!isset($salt) || (is_bool($salt) && $salt == false) || $salt == '') { + dbg_error_log('HTTPAuthLogin', 'CheckCache: No stored salt for %s, need to use auth source', $username); + return 0; + } - if (isset($cached_credentials)) { - if ($cached_credentials == 'pass') { - dbg_error_log('HTTPAuthLogin', 'CheckCache: Cached credentials for %s are good and valid', $username); - return 1; + # The hashed password to fetch + $hash = $this->_MakeHash($password, $salt); + if (! isset($hash)) { + dbg_error_log('LOG', 'HTTPAuthSession:CheckCache: failed to generate hash of credential, using auth source'); + return 0; + } - } else if ($cached_credentials == 'fail') { - dbg_error_log('HTTPAuthLogin', 'CheckCache: Cached credentials for %s are good and invalid', $username); - return 2; - } + $cached_credentials = $cache->get($cache_ns, $hash); - } else { - dbg_error_log('HTTPAuthLogin', 'CheckCache: No stored salted password for %s, need to use auth source', $username); + if (isset($cached_credentials)) { + if ($cached_credentials === 'pass') { + dbg_error_log('HTTPAuthLogin', 'CheckCache: Cached credentials for %s are good and valid', $username); + return 1; + + } else if ($cached_credentials === 'fail') { + dbg_error_log('HTTPAuthLogin', 'CheckCache: Cached credentials for %s are good and invalid', $username); + return 2; } - } else { - dbg_error_log('HTTPAuthLogin', 'CheckCache: No salt stored for %s, assuming no cached credentials, need to use auth source', $username); + dbg_error_log('HTTPAuthLogin', 'CheckCache: No stored salted password for %s, need to use auth source', $username); } # All hope is lost, we must have failed to find anything decent. @@ -488,10 +497,14 @@ class HTTPAuthSession { * Internal function used to set a cache for username/password details. This is * intended to reduce the load on external authentication sources. */ - function SetCache( $username, $password, $state ) { + function _SetCache( $username, $password, $state ) { global $c; if (! $c->auth_cache) return 0; + if (! isset($c->auth_cache_secret) || $c->auth_cache_secret === '') { + dbg_error_log('LOG', "HTTPAuthSession:CheckCache: caching of credentials is enabled, \$c->auth_cache_secret isn't set, not caching credential"); + return 0; + } $cache = getCacheInstance(); if ($cache->isActive() === false) return 0; @@ -500,12 +513,13 @@ class HTTPAuthSession { $salt = $cache->get($cache_ns, 'salt'); - if (!isset($salt) || $salt == '') { - $salt = substr( str_replace('*','',base64_encode(sha1(rand(100000,9999999),true))), 2, 9); + if (!isset($salt) || (is_bool($salt) && $salt == false) || $salt == '') { + # bcrypt requires the salt is 22 bytes. + $salt = $this->_GenerateSalt(22); - # We use the default expiry setting for the salt, because the worse + # We use the default expiry setting for the salt, because the worst # case scenario is that we won't access the cached credentials and - # need to fail to the full authentication source. + # need to fail back to the full authentication source. if (! $cache->set($cache_ns, 'salt', $salt) ) { dbg_error_log('ERROR', 'HTTPCheckCache: SetCache: Failed to store salt, bailing out from caching credential.'); return 0; @@ -513,25 +527,68 @@ class HTTPAuthSession { } # The hashed password to store - $sha1_sent = session_salted_sha1($password, $salt); + $hash = $this->_MakeHash($password, $salt); + if (! isset($hash)) { + dbg_error_log('ERROR', 'HTTPCheckCache: SetCache: Failed to generate hash, bailing out from caching credential.'); + return 0; + } # Work out the expiry to use, some sites might prefer different TTLs for # pass/fail results. - if ($state == 'pass') { + if ($state === 'pass') { $expiry = $c->auth_cache_pass; - } else if ($state == 'fail') { + } else if ($state === 'fail') { $expiry = $c->auth_cache_fail; } else { dbg_error_log('ERROR', 'HTTPCheckCache: SetCache: Unexpected state %s, bailing out from caching credential.', $state); return 0; } - if (! $cache->set($cache_ns, $sha1_sent, $state, $expiry) ) { + if (! $cache->set($cache_ns, $hash, $state, $expiry) ) { dbg_error_log('ERROR', 'HTTPCheckCache: SetCache: Failed to store credential.'); return 0; } return 1; } -} + /** + * Internal function to generate hash of the password. + */ + function _MakeHash( $password, $salt ) { + global $c; + + $password_salt = '$2y$07$' . $salt . '$'; + $password_peppered = hash_hmac('sha256', $password, $c->auth_cache_secret); + $password_hashed = crypt($password_peppered, $password_salt); + + if ($password_hashed == $password_salt || $password_hashed == '*0') { + return NULL; + } else { + return $password_hashed; + } + } + + /* + * Internal function to generate a salt. + * + * Uses random_bytes if available, falls back to mcrypt_create_iv if it + * isn't. + */ + function _GenerateSalt( $length ) { + # We fetch a bit more incase we need to strip out unacceptable characters. + if (function_exists('random_bytes')) { + $salt = random_bytes($length + 20); + } else { + $salt = mcrypt_create_iv($length + 20, MCRYPT_DEV_URANDOM); + } + + # We use this salt with CRYPT_BLOWFISH which only accepts the characters + #./0-9a-zA-Z as per https://www.php.net/manual/en/function.crypt.php + $salt = base64_encode($salt); + $salt = preg_replace('/[^0-9\.\/a-zA-Z]/', '', $salt); + + $salt = substr($salt, 1, $length); + return $salt; + } +} diff --git a/testing/regression-conf.php.example b/testing/regression-conf.php.example index 435eed69..4176cc10 100644 --- a/testing/regression-conf.php.example +++ b/testing/regression-conf.php.example @@ -27,6 +27,7 @@ // if testing cached of auth with memcache //memcache_auth $c->auth_cache = true; + //memcache_auth $c->auth_cache_secret = 'not safe, regression testing only'; // if testing LDAP //ldap $c->authenticate_hook['call'] = 'LDAP_check'; diff --git a/testing/tests/ldap_memcache/0001-test-ldap.result b/testing/tests/ldap_memcache/0001-test-ldap.result index b6bfbf33..a3777de5 100644 --- a/testing/tests/ldap_memcache/0001-test-ldap.result +++ b/testing/tests/ldap_memcache/0001-test-ldap.result @@ -66,5 +66,6 @@ SQL Query 1 Result: user_no: >1001< username: >ldap1< +No stored salt for ldap1, need to use auth source No salt for ldap1 found, passed No cached credentials found, passed diff --git a/testing/tests/ldap_memcache/0001-test-ldap.test b/testing/tests/ldap_memcache/0001-test-ldap.test index dd96acc8..29acf5e7 100644 --- a/testing/tests/ldap_memcache/0001-test-ldap.test +++ b/testing/tests/ldap_memcache/0001-test-ldap.test @@ -76,7 +76,8 @@ if (defined $request_id) { while (<$log>) { if (/davical: $request_id: ALL: HTTPAuthLogin:CheckCache: (.*)/) { my $msg = $1; - if ($msg =~ /^No salt, assuming no cached credentials/) { +print "$msg\n"; + if ($msg =~ /^No stored salt for ldap1,/) { $no_salt = 1; } elsif ($msg =~ /^Cached credentials for ldap1 are good/) { $cached_creds = 1; diff --git a/testing/tests/ldap_memcache/0003-test-ldap.result b/testing/tests/ldap_memcache/0003-test-ldap.result index 4c8bfdf5..d5a89e66 100644 --- a/testing/tests/ldap_memcache/0003-test-ldap.result +++ b/testing/tests/ldap_memcache/0003-test-ldap.result @@ -66,3 +66,5 @@ SQL Query 1 Result: user_no: >1001< username: >ldap1< +Salt found for ldap1, passed +Cached credentials found, passed