mirror of
https://github.com/Piwigo/Piwigo.git
synced 2026-03-28 17:42:57 +01:00
fixes #2440 enhance login security and user activity display
Improves login security by: - implementing constant-time authentication to reduce timing attacks - refactoring user lookup into find_user_by_username_or_email() for username or email login - introducing a fake user to keep password verification time consistent - adding a finalize_login hook so plugins can control the authentication flow (2FA, rate limiting, etc.) Also updates user activity JS to: - better display action details - properly handle user lists (login/logout with multiple users)
This commit is contained in:
@@ -1261,65 +1261,152 @@ function pwg_login($success, $username, $password, $remember_me)
|
||||
|
||||
global $conf;
|
||||
|
||||
$user_found = false;
|
||||
// retrieving the encrypted password of the login submitted
|
||||
// Find user by username or email (if it exists)
|
||||
$user_found = find_user_by_username_or_email($username);
|
||||
|
||||
// SECURITY: Constant-time authentication to prevent timing attacks
|
||||
//
|
||||
// We always perform password verification, even when the user doesn't exist,
|
||||
// to prevent attackers from distinguishing between:
|
||||
// - "user exists, wrong password" (slow: runs password_verify)
|
||||
// - "user doesn't exist" (fast: would skip verification)
|
||||
//
|
||||
// This timing difference could allow user enumeration. By using a fake user
|
||||
// with a pre-generated hash, we ensure consistent execution time regardless
|
||||
// of whether the account exists or not.
|
||||
$fake_user = generate_fake_user();
|
||||
|
||||
// Verify password with fallback to fake user
|
||||
$password_verify = $conf['password_verify'](
|
||||
$password,
|
||||
$user_found['password'] ?? $fake_user['password'],
|
||||
$user_found['id'] ?? $fake_user['id']
|
||||
);
|
||||
|
||||
// If the user was not found, is a guest, or the password is incorrect
|
||||
if (empty($user_found) || 'guest' === $user_found['status'] || !$password_verify)
|
||||
{
|
||||
if (!empty($user_found) && !$password_verify)
|
||||
{
|
||||
pwg_activity('user', $user_found['id'], 'login_failure_wrong_password');
|
||||
}
|
||||
trigger_notify('login_failure', stripslashes($username));
|
||||
return false;
|
||||
}
|
||||
|
||||
// PLUGIN HOOK: Allow plugins to intercept authentication before log_user()
|
||||
//
|
||||
// Expected $state array structure:
|
||||
// - 'can_login' (bool): Set to false to block login
|
||||
// - 'reason' (string|null): Custom activity log reason if login blocked
|
||||
// - 'authenticated' (bool): Set to true if plugin handles log_user() itself
|
||||
//
|
||||
// Example plugin implementation:
|
||||
// add_event_handler('finalize_login', 'my_2fa_check');
|
||||
// function my_2fa_check($state, $user, $remember_me) {
|
||||
// if (!verify_2fa_code()) {
|
||||
// $state['can_login'] = false;
|
||||
// $state['reason'] = '2fa_failed';
|
||||
// }
|
||||
// return $state;
|
||||
// }
|
||||
$state = array(
|
||||
'can_login' => true,
|
||||
'reason' => null,
|
||||
'authenticated' => false,
|
||||
);
|
||||
$state = trigger_change('finalize_login', $state, $user_found, $remember_me);
|
||||
|
||||
if (!$state['can_login'])
|
||||
{
|
||||
pwg_activity('user', $user_found['id'], $state['reason'] ?? 'login_failure_before_log_user');
|
||||
trigger_notify('login_failure_before_log_user', stripslashes($username));
|
||||
return false;
|
||||
}
|
||||
|
||||
// If plugin handled authentication, skip log_user()
|
||||
if (!$state['authenticated'])
|
||||
{
|
||||
log_user($user_found['id'], $remember_me);
|
||||
}
|
||||
|
||||
trigger_notify('login_success', stripslashes($username));
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Find user by username or email
|
||||
* search by username first then email
|
||||
*
|
||||
* @since 16
|
||||
* @param string $username_or_email
|
||||
* @return array|null
|
||||
*/
|
||||
function find_user_by_username_or_email($username_or_email)
|
||||
{
|
||||
global $conf;
|
||||
|
||||
$username_or_email = pwg_db_real_escape_string($username_or_email);
|
||||
|
||||
$query = '
|
||||
SELECT '.$conf['user_fields']['id'].' AS id,
|
||||
'.$conf['user_fields']['password'].' AS password
|
||||
FROM '.USERS_TABLE.'
|
||||
WHERE '.$conf['user_fields']['username'].' = \''.pwg_db_real_escape_string($username).'\'
|
||||
;';
|
||||
SELECT
|
||||
'.$conf['user_fields']['id'].' AS id,
|
||||
'.$conf['user_fields']['username'].' AS username,
|
||||
'.$conf['user_fields']['email'].' AS email,
|
||||
'.$conf['user_fields']['password'].' AS password,
|
||||
status
|
||||
FROM '.USERS_TABLE.' AS u
|
||||
LEFT JOIN '.USER_INFOS_TABLE.' AS i
|
||||
ON u.'.$conf['user_fields']['id'].' = i.user_id
|
||||
WHERE ';
|
||||
|
||||
$row = pwg_db_fetch_assoc(pwg_query($query));
|
||||
if (isset($row['id']) and $conf['password_verify']($password, $row['password'], $row['id']))
|
||||
$where_username = $conf['user_fields']['username'].' = \'' . $username_or_email . '\'';
|
||||
$where_email = $conf['user_fields']['email'].' = \'' . $username_or_email . '\'';
|
||||
|
||||
$user = pwg_db_fetch_assoc(pwg_query($query.$where_username))
|
||||
?: pwg_db_fetch_assoc(pwg_query($query.$where_email));
|
||||
|
||||
if (!empty($user))
|
||||
{
|
||||
$user_found = true;
|
||||
}
|
||||
|
||||
// If we didn't find a matching user name, we search for email address
|
||||
if (!$user_found)
|
||||
{
|
||||
$query = '
|
||||
SELECT '.$conf['user_fields']['id'].' AS id,
|
||||
'.$conf['user_fields']['password'].' AS password
|
||||
FROM '.USERS_TABLE.'
|
||||
WHERE '.$conf['user_fields']['email'].' = \''.pwg_db_real_escape_string($username).'\'
|
||||
;';
|
||||
|
||||
$row = pwg_db_fetch_assoc(pwg_query($query));
|
||||
if (isset($row['id']) and $conf['password_verify']($password, $row['password'], $row['id']))
|
||||
{
|
||||
$user_found = true;
|
||||
}
|
||||
}
|
||||
|
||||
if ($user_found)
|
||||
{
|
||||
// if user status is "guest" then she should not be granted to log in.
|
||||
// The user may not exist in the user_infos table, so we consider it's a "normal" user by default
|
||||
$status = 'normal';
|
||||
|
||||
$query = '
|
||||
SELECT
|
||||
*
|
||||
FROM '.USER_INFOS_TABLE.'
|
||||
WHERE user_id = '.$row['id'].'
|
||||
;';
|
||||
$result = pwg_query($query);
|
||||
while ($user_infos_row = pwg_db_fetch_assoc($result))
|
||||
{
|
||||
$status = $user_infos_row['status'];
|
||||
}
|
||||
|
||||
if ('guest' != $status)
|
||||
{
|
||||
log_user($row['id'], $remember_me);
|
||||
trigger_notify('login_success', stripslashes($username));
|
||||
return true;
|
||||
}
|
||||
$user['status'] = $user['status'] ?? 'normal';
|
||||
return $user;
|
||||
}
|
||||
trigger_notify('login_failure', stripslashes($username));
|
||||
return false;
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a fake user with hashed password (with the current algo)
|
||||
*
|
||||
* SECURITY: This function is used for timing attack mitigation in pwg_login().
|
||||
* The fake user hash is cached per session to avoid repeated hashing overhead
|
||||
* while maintaining constant-time authentication behavior.
|
||||
*
|
||||
* @since 16
|
||||
* @return array id and password
|
||||
*/
|
||||
function generate_fake_user()
|
||||
{
|
||||
global $conf;
|
||||
|
||||
// Check if password_hash or password_verify has been changed
|
||||
$is_verify_hash_changed = 'pwg_password_hash' !== $conf['password_hash']
|
||||
|| 'pwg_password_verify' !== $conf['password_verify'];
|
||||
|
||||
// Generate once per session to avoid repeated hashing overhead.
|
||||
// Uses current password_hash algorithm to match real user verification costs.
|
||||
if (!isset($_SESSION['fake_user_cache']) || $is_verify_hash_changed)
|
||||
{
|
||||
$fake_password = bin2hex(random_bytes(10));
|
||||
$_SESSION['fake_user_cache'] = array(
|
||||
'id' => null,
|
||||
'password' => $conf['password_hash']($fake_password)
|
||||
);
|
||||
}
|
||||
|
||||
return $_SESSION['fake_user_cache'];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user