Skip to content
Snippets Groups Projects
Commit 504831fe authored by Jared Hancock's avatar Jared Hancock
Browse files

login: Require CSRF token to login

This patch fixes a vulnerable scenario, where sequential login attempts can
be made without an existing session, and without a valid CSRF token. This
scenario lends itself well for brute force password attempts, because
attackers can avoid using a session and still send requests to determine if
a set of credentials are valid. This vector also avoids the authentication
lockout mechanism, because it requires an ongoing session to shutdown the
requests.

This patch addresses the issue by requiring a session and a valid CSRF token
generated by the server and placed in the session to be submitted with the
credentials. Therefore, an existing session and a Cookie header are required
to process a login attempt. Secondly, the CSRF token will be changed on the
server after each login processed. Therefore, for each session, a subsequent
GET request would be necessary before submitting another login attempt.
parent 5fc563e3
Branches
Tags
No related merge requests found
...@@ -53,12 +53,15 @@ Class CSRF { ...@@ -53,12 +53,15 @@ Class CSRF {
return $this->name; return $this->name;
} }
function getToken() { function rotate() {
$this->csrf['token'] = sha1(session_id().Crypto::random(16).SECRET_SALT);
$this->csrf['time'] = time();
}
if(!$this->csrf['token'] || $this->isExpired()) { function getToken() {
$this->csrf['token'] = sha1(session_id().Crypto::random(16).SECRET_SALT); if (!$this->csrf['token'] || $this->isExpired()) {
$this->csrf['time'] = time(); $this->rotate();
} else { } else {
//Reset the timer //Reset the timer
$this->csrf['time'] = time(); $this->csrf['time'] = time();
......
...@@ -31,6 +31,20 @@ else ...@@ -31,6 +31,20 @@ else
$inc = 'login.inc.php'; $inc = 'login.inc.php';
$suggest_pwreset = false; $suggest_pwreset = false;
// Check the CSRF token, and ensure that future requests will have to use a
// different CSRF token. This will help ward off both parallel and serial
// brute force attacks, because new tokens will have to be requested for
// each attempt.
if ($_POST) {
// Check CSRF token
if (!$ost->checkCSRFToken())
Http::response(400, __('Valid CSRF Token Required'));
// Rotate the CSRF token (original cannot be reused)
$ost->getCSRF()->rotate();
}
if ($_POST && isset($_POST['luser'])) { if ($_POST && isset($_POST['luser'])) {
if (!$_POST['luser']) if (!$_POST['luser'])
$errors['err'] = __('Valid username or email address is required'); $errors['err'] = __('Valid username or email address is required');
......
...@@ -31,6 +31,16 @@ $msg = $msg ?: ($content ? $content->getName() : __('Authentication Required')); ...@@ -31,6 +31,16 @@ $msg = $msg ?: ($content ? $content->getName() : __('Authentication Required'));
$dest=($dest && (!strstr($dest,'login.php') && !strstr($dest,'ajax.php')))?$dest:'index.php'; $dest=($dest && (!strstr($dest,'login.php') && !strstr($dest,'ajax.php')))?$dest:'index.php';
$show_reset = false; $show_reset = false;
if($_POST) { if($_POST) {
// Check the CSRF token, and ensure that future requests will have to
// use a different CSRF token. This will help ward off both parallel and
// serial brute force attacks, because new tokens will have to be
// requested for each attempt.
if (!$ost->checkCSRFToken())
Http::response(400, __('Valid CSRF Token Required'));
// Rotate the CSRF token (original cannot be reused)
$ost->getCSRF()->rotate();
// Lookup support backends for this staff // Lookup support backends for this staff
$username = trim($_POST['userid']); $username = trim($_POST['userid']);
if ($user = StaffAuthenticationBackend::process($username, if ($user = StaffAuthenticationBackend::process($username,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment