基于数据库会话的双重登录限制用户认证方案及PHP实现的漏洞排查请求

Hey there, this is absolutely a suitable place to discuss this kind of security-focused application design and code review question — security implementations like brute-force protection are exactly the sort of topics that benefit from peer feedback here.

方案背景与核心目标 #

我们决定将应用的会话管理迁移至数据库层，在此处处理用户状态。核心目标是通过会话层（基于数据库会话条目）和账户层的双重登录尝试限制抵御暴力破解攻击。

同时，考虑到恶意攻击者可能通过频繁尝试锁定合法用户，我们计划添加账户级锁定的可选开关：关闭该开关可避免合法用户被恶意锁定，但代价是账户失去账户级暴力破解防护；未来我们可能会扩展功能，允许为特定账户单独配置该开关。

登录尝试校验用户流程 #

我们构建了check_login_attempts_exceeded函数，用于在登录请求到达认证服务器前拦截超限尝试，流程如下：

• 已认证用户：直接返回FALSE（无需为已登录用户记录登录尝试），支持管理员传入其他用户ID以查询对应账户状态
• 爬虫：视为访客账户，防止爬虫绕过登录限制机制
• 访客账户：
  1. 若访客已登录匹配传入ID的账户，返回FALSE（同已认证用户场景）
  2. 检查会话中的登录锁定标记：
     • 未设置锁定标记：
       • 先检查全局会话登录尝试是否达上限：
         • 是：为会话添加全局login_lockout并更新数据库，返回TRUE
         • 否：
           • 若传入userid>0：检查账户级锁定状态
             • 账户级锁定时间未过期（处于未来）：返回TRUE（存在恶意锁定合法用户的风险）
             • 账户级锁定时间已过期：重置账户的登录尝试次数和锁定时间，返回FALSE
           • 未传入userid：返回FALSE
     • 已设置锁定标记：
       • 检查锁定超时是否已过期：
         • 已过期：移除标记后，继续检查账户级锁定状态
           • 账户级锁定未过期：返回TRUE
           • 账户级锁定已过期：重置会话和账户的相关参数，返回FALSE
         • 未过期：返回TRUE

实现代码（PHP） #

// Check if the user has exceeded their login attempts, and if they are locked 
// out return TRUE, and if the lockout has expired, remove it from the session 
// AND the user account if it is set - Optional return time to unlock 
function check_login_attempts_exceeded($database, $config, $userid = 0, $return_time = FALSE) {
 // Set the time of now for checking if they are past the lockout expiry 
 $time = CURRENT_TIME;
 $userid = (int) $userid;
 $unauthenticated_user = FALSE;

 // Check if this user is logged in - If they are, we can return false 
 // since this user is already logged in and doesn't need to have attempts 
 // logged for them anymore.
 if ($userid > 0 && $userid == $this->userid) {
 return FALSE;
 } else {
 $unauthenticated_user = TRUE;
 }

 // If the user is a guest, we can process the request since it's not important 
 // otherwise. This condition should always be true, but better safe than sorry?
 if ($unauthenticated_user) {
 // If the login_timeout isn't set we can check if the attempts are over 
 // and set it otherwise, it must be set so we will see if its expired
 if (!$this->login_timeout || $this->login_timeout == 0) {
 // Check the global lockout first - if it above the max then we will 
 // ignore the fact that the user is over since we're not there yet
 if ($this->login_attempts >= $config->general->max_login_trys) {
 $this->login_timeout = (int) $time + ($config->general->lockout_time * 60);
 $database->update_prepared_query("sessions", array("lockout_time" => $this->login_timeout), array("sid" => $this->session_id));
 return TRUE;
 }
 // Now lets check the specific user case, because we know that the 
 // above has been handled and returned already
 if ($userid > 0) {
 $user_info = $database->prepared_select("users", "WHERE uid = ? LIMIT 1", array($userid), "loginattempts, loginlockoutexpiry");
 if ($user_info) {
 $user_specific_loginattempts = $user_info["loginattempts"];
 $user_specific_loginexpiry = $user_info["loginlockoutexpiry"];
 // If this specific user is over the max attempts, then we return 
 // true because they're over the max attempt
 if ($user_specific_loginattempts >= $config->general->max_login_trys) {
 // We also want to see if their expiry is over the max limit 
 // and it if is, we will return true, if it is expired, 
 // lets remove it and set these back to 0 and return false!
 if ($user_specific_loginexpiry > $time) {
 if ($return_time) {
 $secsleft = (int) ($user_specific_loginexpiry - $time);
 $hoursleft = floor($secsleft / 3600);
 $minsleft = floor(($secsleft / 60) % 60);
 $secsleft = floor($secsleft % 60);
 return array("hours" => $hoursleft, "minutes" => $minsleft, "seconds" => $secsleft);
 }
 return TRUE;
 } else {
 // This user specific timeout has expired, so lets 
 // remove it from the system and let the user attempt 
 // a login!
 $database->update_prepared_query("users", array("loginattempts" => 0, "loginlockoutexpiry" => 0), array("uid" => $userid));
 return FALSE;
 }
 }
 } else {
 // There was no user information found for this id, so we can't 
 // return a result and we will just return false instead
 return FALSE;
 }
 }
 // Must not be over yet then, so lets return false as no user was 
 // supplied and the user hasn't hit the limit globally yet either
 return FALSE;
 } else {
 // The login timeout is set, let see if it's expired or not?
 if ($this->login_timeout <= CURRENT_TIME) {
 if ($userid > 0) {
 $user_info = $database->prepared_select("users", "WHERE uid = ? LIMIT 1", array($userid), "loginattempts, loginlockoutexpiry");
 if ($user_info) {
 if ($user_info["loginattempts"] >= $config->general->max_login_trys) {
 if ($user_info["loginlockoutexpiry"] > $time) {
 return TRUE;
 } else {
 $this->login_timeout = 0;
 $this->login_attempts = 0;
 $database->update_prepared_query("sessions", array("login_attempts" => $this->login_attempts, "lockout_time" => $this->login_timeout), array("sid" => $this->session_id));
 $database->update_prepared_query("users", array("loginattempts" => 0, "loginlockoutexpiry" => 0), array("uid" => $userid));
 return FALSE;
 }
 } else {
 // The user they're trying for is not at the max tries so we can return false!
 return FALSE;
 }
 } else {
 // There was no user information found for this id, so we can't 
 // return a result and we will just return false instead
 return FALSE;
 }
 } else {
 // The login timeout has expired for this guest account so 
 // we remove it globally!
 $this->login_timeout = 0;
 $this->login_attempts = 0;
 $database->update_prepared_query("sessions", array("login_attempts" => $this->login_attempts, "lockout_time" => $this->login_timeout), array("sid" => $this->session_id));
 return FALSE;
 }
 }
 // They must still be expired, so lets return true!
 return TRUE;
 }
}
return FALSE;
}

同行校验请求 #

目前该方案已完成调试，所有条件测试均通过，但我已投入近100小时开发，希望获得专业视角的校验：

• 确认该用户流程是否逻辑合理
• 检查函数是否存在可能引发错误的遗漏点
• 若方案无问题，请告知，以便我停止过度开发

内容的提问来源于stack exchange，提问作者Kaboom