From db88a00d48d1dce48b8700e460c06ff7fb344f0a Mon Sep 17 00:00:00 2001
From: Frederic Guillot <fred@kanboard.net>
Date: Sat, 1 Aug 2015 12:14:22 -0400
Subject: Add bruteforce protection
---
app/Api/Auth.php | 2 +-
app/Controller/Auth.php | 18 +++++++
app/Model/Acl.php | 2 +-
app/Model/Authentication.php | 117 +++++++++++++++++++++++++++++++++++--------
app/Model/User.php | 65 ++++++++++++++++++++++++
app/Schema/Mysql.php | 8 ++-
app/Schema/Postgres.php | 8 ++-
app/Schema/Sqlite.php | 8 ++-
app/Template/auth/index.php | 8 ++-
app/constants.php | 5 ++
10 files changed, 213 insertions(+), 28 deletions(-)
(limited to 'app')
diff --git a/app/Api/Auth.php b/app/Api/Auth.php
index 9d401746..18fe9ff9 100644
--- a/app/Api/Auth.php
+++ b/app/Api/Auth.php
@@ -26,7 +26,7 @@ class Auth extends Base
{
$this->container['dispatcher']->dispatch('api.bootstrap', new Event);
- if ($username !== 'jsonrpc' && $this->authentication->authenticate($username, $password)) {
+ if ($username !== 'jsonrpc' && ! $this->authentication->hasCaptcha($username) && $this->authentication->authenticate($username, $password)) {
$this->checkProcedurePermission(true, $method);
$this->userSession->refresh($this->user->getByUsername($username));
}
diff --git a/app/Controller/Auth.php b/app/Controller/Auth.php
index e8889b7f..bb1154e4 100644
--- a/app/Controller/Auth.php
+++ b/app/Controller/Auth.php
@@ -2,6 +2,8 @@
namespace Controller;
+use Gregwar\Captcha\CaptchaBuilder;
+
/**
* Authentication controller
*
@@ -22,6 +24,7 @@ class Auth extends Base
}
$this->response->html($this->template->layout('auth/index', array(
+ 'captcha' => isset($values['username']) && $this->authentication->hasCaptcha($values['username']),
'errors' => $errors,
'values' => $values,
'no_layout' => true,
@@ -64,4 +67,19 @@ class Auth extends Base
$this->session->close();
$this->response->redirect($this->helper->url->to('auth', 'login'));
}
+
+ /**
+ * Display captcha image
+ *
+ * @access public
+ */
+ public function captcha()
+ {
+ $this->response->contentType('image/jpeg');
+
+ $builder = new CaptchaBuilder;
+ $builder->build();
+ $this->session['captcha'] = $builder->getPhrase();
+ $builder->output();
+ }
}
diff --git a/app/Model/Acl.php b/app/Model/Acl.php
index 95056de6..a47886bb 100644
--- a/app/Model/Acl.php
+++ b/app/Model/Acl.php
@@ -17,7 +17,7 @@ class Acl extends Base
* @var array
*/
private $public_acl = array(
- 'auth' => array('login', 'check'),
+ 'auth' => array('login', 'check', 'captcha'),
'task' => array('readonly'),
'board' => array('readonly'),
'webhook' => '*',
diff --git a/app/Model/Authentication.php b/app/Model/Authentication.php
index 31969b57..f09312bd 100644
--- a/app/Model/Authentication.php
+++ b/app/Model/Authentication.php
@@ -5,6 +5,7 @@ namespace Model;
use Core\Request;
use SimpleValidator\Validator;
use SimpleValidator\Validators;
+use Gregwar\Captcha\CaptchaBuilder;
/**
* Authentication model
@@ -75,17 +76,51 @@ class Authentication extends Base
*/
public function authenticate($username, $password)
{
- // Try first the database auth and then LDAP if activated
- if ($this->backend('database')->authenticate($username, $password)) {
+ if ($this->user->isLocked($username)) {
+ $this->container['logger']->error('Account locked: '.$username);
+ return false;
+ }
+ else if ($this->backend('database')->authenticate($username, $password)) {
+ $this->user->resetFailedLogin($username);
return true;
}
else if (LDAP_AUTH && $this->backend('ldap')->authenticate($username, $password)) {
+ $this->user->resetFailedLogin($username);
return true;
}
+ $this->handleFailedLogin($username);
return false;
}
+ /**
+ * Return true if the captcha must be shown
+ *
+ * @access public
+ * @param string $username
+ * @return boolean
+ */
+ public function hasCaptcha($username)
+ {
+ return $this->user->getFailedLogin($username) >= BRUTEFORCE_CAPTCHA;
+ }
+
+ /**
+ * Handle failed login
+ *
+ * @access public
+ * @param string $username
+ */
+ public function handleFailedLogin($username)
+ {
+ $this->user->incrementFailedLogin($username);
+
+ if ($this->user->getFailedLogin($username) >= BRUTEFORCE_LOCKDOWN) {
+ $this->container['logger']->critical('Locking account: '.$username);
+ $this->user->lock($username, BRUTEFORCE_LOCKDOWN_DURATION);
+ }
+ }
+
/**
* Validate user login form
*
@@ -95,27 +130,12 @@ class Authentication extends Base
*/
public function validateForm(array $values)
{
- $v = new Validator($values, array(
- new Validators\Required('username', t('The username is required')),
- new Validators\MaxLength('username', t('The maximum length is %d characters', 50), 50),
- new Validators\Required('password', t('The password is required')),
- ));
-
- $result = $v->execute();
- $errors = $v->getErrors();
+ list($result, $errors) = $this->validateFormCredentials($values);
if ($result) {
- if ($this->authenticate($values['username'], $values['password'])) {
-
- // Setup the remember me feature
- if (! empty($values['remember_me'])) {
-
- $credentials = $this->backend('rememberMe')
- ->create($this->userSession->getId(), Request::getIpAddress(), Request::getUserAgent());
-
- $this->backend('rememberMe')->writeCookie($credentials['token'], $credentials['sequence'], $credentials['expiration']);
- }
+ if ($this->validateFormCaptcha($values) && $this->authenticate($values['username'], $values['password'])) {
+ $this->createRememberMeSession($values);
}
else {
$result = false;
@@ -123,9 +143,62 @@ class Authentication extends Base
}
}
+ return array($result, $errors);
+ }
+
+ /**
+ * Validate credentials syntax
+ *
+ * @access public
+ * @param array $values Form values
+ * @return array $valid, $errors [0] = Success or not, [1] = List of errors
+ */
+ public function validateFormCredentials(array $values)
+ {
+ $v = new Validator($values, array(
+ new Validators\Required('username', t('The username is required')),
+ new Validators\MaxLength('username', t('The maximum length is %d characters', 50), 50),
+ new Validators\Required('password', t('The password is required')),
+ ));
+
return array(
- $result,
- $errors
+ $v->execute(),
+ $v->getErrors(),
);
}
+
+ /**
+ * Validate captcha
+ *
+ * @access public
+ * @param array $values Form values
+ * @return boolean
+ */
+ public function validateFormCaptcha(array $values)
+ {
+ if ($this->hasCaptcha($values['username'])) {
+ $builder = new CaptchaBuilder;
+ $builder->setPhrase($this->session['captcha']);
+ return $builder->testPhrase(isset($values['captcha']) ? $values['captcha'] : '');
+ }
+
+ return true;
+ }
+
+ /**
+ * Create remember me session if necessary
+ *
+ * @access private
+ * @param array $values Form values
+ */
+ private function createRememberMeSession(array $values)
+ {
+ if (! empty($values['remember_me'])) {
+
+ $credentials = $this->backend('rememberMe')
+ ->create($this->userSession->getId(), Request::getIpAddress(), Request::getUserAgent());
+
+ $this->backend('rememberMe')->writeCookie($credentials['token'], $credentials['sequence'], $credentials['expiration']);
+ }
+ }
}
diff --git a/app/Model/User.php b/app/Model/User.php
index b6804abc..8daef3f2 100644
--- a/app/Model/User.php
+++ b/app/Model/User.php
@@ -364,6 +364,71 @@ class User extends Base
->save(array('token' => ''));
}
+ /**
+ * Get the number of failed login for the user
+ *
+ * @access public
+ * @param string $username
+ * @return integer
+ */
+ public function getFailedLogin($username)
+ {
+ return (int) $this->db->table(self::TABLE)->eq('username', $username)->findOneColumn('nb_failed_login');
+ }
+
+ /**
+ * Reset to 0 the counter of failed login
+ *
+ * @access public
+ * @param string $username
+ * @return boolean
+ */
+ public function resetFailedLogin($username)
+ {
+ return $this->db->table(self::TABLE)->eq('username', $username)->update(array('nb_failed_login' => 0, 'lock_expiration_date' => 0));
+ }
+
+ /**
+ * Increment failed login counter
+ *
+ * @access public
+ * @param string $username
+ * @return boolean
+ */
+ public function incrementFailedLogin($username)
+ {
+ return $this->db->execute('UPDATE '.self::TABLE.' SET nb_failed_login=nb_failed_login+1 WHERE username=?', array($username)) !== false;
+ }
+
+ /**
+ * Check if the account is locked
+ *
+ * @access public
+ * @param string $username
+ * @return boolean
+ */
+ public function isLocked($username)
+ {
+ return $this->db->table(self::TABLE)
+ ->eq('username', $username)
+ ->neq('lock_expiration_date', 0)
+ ->gte('lock_expiration_date', time())
+ ->exists();
+ }
+
+ /**
+ * Lock the account for the specified duration
+ *
+ * @access public
+ * @param string $username Username
+ * @param integer $duration Duration in minutes
+ * @return boolean
+ */
+ public function lock($username, $duration = 15)
+ {
+ return $this->db->table(self::TABLE)->eq('username', $username)->update(array('lock_expiration_date' => time() + $duration * 60));
+ }
+
/**
* Common validation rules
*
diff --git a/app/Schema/Mysql.php b/app/Schema/Mysql.php
index 47fb806e..31ffaa32 100644
--- a/app/Schema/Mysql.php
+++ b/app/Schema/Mysql.php
@@ -6,7 +6,13 @@ use PDO;
use Core\Security;
use Model\Link;
-const VERSION = 81;
+const VERSION = 82;
+
+function version_82($pdo)
+{
+ $pdo->exec("ALTER TABLE users ADD COLUMN nb_failed_login INT DEFAULT 0");
+ $pdo->exec("ALTER TABLE users ADD COLUMN lock_expiration_date INT DEFAULT 0");
+}
function version_81($pdo)
{
diff --git a/app/Schema/Postgres.php b/app/Schema/Postgres.php
index 6b85ff57..876df981 100644
--- a/app/Schema/Postgres.php
+++ b/app/Schema/Postgres.php
@@ -6,7 +6,13 @@ use PDO;
use Core\Security;
use Model\Link;
-const VERSION = 61;
+const VERSION = 62;
+
+function version_62($pdo)
+{
+ $pdo->exec("ALTER TABLE users ADD COLUMN nb_failed_login INTEGER DEFAULT 0");
+ $pdo->exec("ALTER TABLE users ADD COLUMN lock_expiration_date INTEGER DEFAULT 0");
+}
function version_61($pdo)
{
diff --git a/app/Schema/Sqlite.php b/app/Schema/Sqlite.php
index 9e0575cf..4b8f5ac6 100644
--- a/app/Schema/Sqlite.php
+++ b/app/Schema/Sqlite.php
@@ -6,7 +6,13 @@ use Core\Security;
use PDO;
use Model\Link;
-const VERSION = 77;
+const VERSION = 78;
+
+function version_78($pdo)
+{
+ $pdo->exec("ALTER TABLE users ADD COLUMN nb_failed_login INTEGER DEFAULT 0");
+ $pdo->exec("ALTER TABLE users ADD COLUMN lock_expiration_date INTEGER DEFAULT 0");
+}
function version_77($pdo)
{
diff --git a/app/Template/auth/index.php b/app/Template/auth/index.php
index ca303df9..efe95185 100644
--- a/app/Template/auth/index.php
+++ b/app/Template/auth/index.php
@@ -10,11 +10,17 @@
<?= $this->form->csrf() ?>
<?= $this->form->label(t('Username'), 'username') ?>
- <?= $this->form->text('username', $values, $errors, array('autofocus', 'required')) ?><br/>
+ <?= $this->form->text('username', $values, $errors, array('autofocus', 'required')) ?>
<?= $this->form->label(t('Password'), 'password') ?>
<?= $this->form->password('password', $values, $errors, array('required')) ?>
+ <?php if ($captcha): ?>
+ <?= $this->form->label(t('Enter the text below'), 'captcha') ?>
+ <img src="<?= $this->url->href('auth', 'captcha') ?>"/>
+ <?= $this->form->text('captcha', $values, $errors, array('required')) ?>
+ <?php endif ?>
+
<?= $this->form->checkbox('remember_me', t('Remember Me'), 1, true) ?><br/>
<div class="form-actions">
diff --git a/app/constants.php b/app/constants.php
index 83fba468..e232aba6 100644
--- a/app/constants.php
+++ b/app/constants.php
@@ -88,3 +88,8 @@ defined('ENABLE_URL_REWRITE') or define('ENABLE_URL_REWRITE', isset($_SERVER['HT
// Hide login form
defined('HIDE_LOGIN_FORM') or define('HIDE_LOGIN_FORM', false);
+
+// Bruteforce protection
+defined('BRUTEFORCE_CAPTCHA') or define('BRUTEFORCE_CAPTCHA', 3);
+defined('BRUTEFORCE_LOCKDOWN') or define('BRUTEFORCE_LOCKDOWN', 6);
+defined('BRUTEFORCE_LOCKDOWN_DURATION') or define('BRUTEFORCE_LOCKDOWN_DURATION', 15);
--
cgit v1.2.3