fix: Fix the broken bruteforce protection

- Moved the bruteforce protection directly to the login action
- Fixed the datetime format used by the protection
- Cleaned up the logging of failed attempts
pull/1527/head
Iglocska 2016-09-12 11:20:26 +02:00
parent 6eb6bfb10b
commit ab50d00b15
4 changed files with 28 additions and 89 deletions

View File

@ -72,9 +72,7 @@ class AppController extends Controller {
'authError' => 'Unauthorised access.',
'loginRedirect' => array('controller' => 'users', 'action' => 'routeafterlogin'),
'logoutRedirect' => array('controller' => 'users', 'action' => 'login', 'admin' => false),
//'authorize' => array('Controller', // Added this line
//'Actions' => array('actionPath' => 'controllers')) // TODO ACL, 4: tell actionPath
),
),
'Security',
'ACL'
);
@ -108,7 +106,6 @@ class AppController extends Controller {
)
);
} else {
$this->Auth->className = 'SecureAuth';
$this->Auth->authenticate = array(
'Form' => array(
'fields' => array('username' => 'email'),
@ -138,7 +135,6 @@ class AppController extends Controller {
$userLoggedIn = false;
if (Configure::read('Plugin.CustomAuth_enable')) $userLoggedIn = $this->__customAuthentication($_SERVER);
if (!$userLoggedIn) {
// REST authentication
if ($this->_isRest() || $this->_isAutomation()) {

View File

@ -1,65 +0,0 @@
<?php
App::uses('AuthComponent', 'Controller/Component');
class SecureAuthComponent extends AuthComponent {
/**
* Log a user in using anti-brute-force protection.
* If a $user is provided that data will be stored as the logged in user. If `$user` is empty or not
* specified, the request will be used to identify a user. If the identification was successful,
* the user record is written to the session key specified in AuthComponent::$sessionKey. Logging in
* will also change the session id in order to help mitigate session replays.
*
* @param mixed $user Either an array of user data, or null to identify a user using the current request.
* @return boolean True on login success, false on failure
* @link http://book.cakephp.org/2.0/en/core-libraries/components/authentication.html#identifying-users-and-logging-them-in
* @throws ForbiddenException
*/
public function login($user = null) {
$this->_setDefaults();
if (empty($user)) {
$this->Bruteforce = ClassRegistry::init('Bruteforce');
// do the anti-bruteforce checks
$usernameField = $this->authenticate['Form']['fields']['username'];
if (isset($this->request->data['User'][$usernameField])) {
$username = $this->request->data['User'][$usernameField];
if (!$this->Bruteforce->isBlacklisted($_SERVER['REMOTE_ADDR'], $username)) {
// user - ip combination is not blacklisted
// check if the user credentials are valid
$user = $this->identify($this->request, $this->response);
unset($user['gpgkey']);
unset($user['certif_public']);
if ($user === false) {
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$log = array(
'org' => 'SYSTEM',
'model' => 'User',
'model_id' => 0,
'email' => $username,
'action' => 'login_fail',
'title' => 'Failed login attempt',
'change' => null,
);
$this->Log->save($log);
// insert row in Bruteforce table
$this->Bruteforce->insert($_SERVER['REMOTE_ADDR'], $username);
// do nothing as user is not logged in
}
} else {
// user - ip combination has reached the amount of maximum attempts in the timeframe
throw new ForbiddenException('You have reached the maximum number of login attempts. Please wait ' . Configure::read('SecureAuth.expire') . ' seconds and try again.');
}
} else {
// user didn't fill in all the form fields, nothing to do
}
}
if ($user) {
$this->Session->renew();
$this->Session->write(self::$sessionKey, $user);
}
return $this->loggedIn();
}
}

View File

@ -565,6 +565,12 @@ class UsersController extends AppController {
}
public function login() {
$this->Bruteforce = ClassRegistry::init('Bruteforce');
if ($this->request->is('post') && isset($this->request->data['User']['email'])) {
if ($this->Bruteforce->isBlacklisted($_SERVER['REMOTE_ADDR'], $this->request->data['User']['email'])) {
throw new ForbiddenException('You have reached the maximum number of login attempts. Please wait ' . Configure::read('SecureAuth.expire') . ' seconds and try again.');
}
}
if ($this->Auth->login()) {
$this->__extralog("login"); // TODO Audit, __extralog, check: customLog i.s.o. __extralog, no auth user?: $this->User->customLog('login', $this->Auth->user('id'), array('title' => '','user_id' => $this->Auth->user('id'),'email' => $this->Auth->user('email'),'org' => 'IN2'));
$this->User->Behaviors->disable('SysLogLogable.SysLogLogable');
@ -585,6 +591,9 @@ class UsersController extends AppController {
// don't display "invalid user" before first login attempt
if ($this->request->is('post')) {
$this->Session->setFlash(__('Invalid username or password, try again'));
if (isset($this->request->data['User']['email'])) {
$this->Bruteforce->insert($_SERVER['REMOTE_ADDR'], $this->request->data['User']['email']);
}
}
// populate the DB with the first role (site admin) if it's empty
$this->loadModel('Role');

View File

@ -6,30 +6,29 @@ App::uses('Sanitize', 'Utility');
class Bruteforce extends AppModel {
public function insert($ip, $username) {
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$expire = time() + Configure::read('SecureAuth.expire');
$dataSourceConfig = ConnectionManager::getDataSource('default')->config;
$dataSource = $dataSourceConfig['datasource'];
// sanitize fields
$ip = Sanitize::clean($ip);
$username = Sanitize::clean($username);
if ($dataSource == 'Database/Mysql') {
$sql = "INSERT INTO bruteforces (ip, username, `expire`) VALUES ('$ip', '$username', '$expire');";
} else if ($dataSource == 'Database/Postgres') {
$sql = "INSERT INTO bruteforces (ip, username, expire) VALUES ('$ip', '$username', '$expire');";
}
$this->query($sql);
$expire = date('Y-m-d H:i:s', $expire);
$bruteforceEntry = array(
'ip' => $ip,
'username' => $username,
'expire' => $expire
);
$this->save($bruteforceEntry);
$title = 'Failed login attempt using username ' . $username . ' from IP: ' . $_SERVER['REMOTE_ADDR'] . '.';
if ($this->isBlacklisted($ip, $username)) {
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$this->Log->save(array(
$title .= 'This has tripped the bruteforce protection after ' . Configure::read('SecureAuth.amount') . ' failed attempts. The user is now blacklisted for ' . Configure::read('SecureAuth.expire') . ' seconds.';
}
$log = array(
'org' => 'SYSTEM',
'model' => 'Blacklist',
'model' => 'User',
'model_id' => 0,
'email' => $username,
'action' => 'blacklist',
'title' => 'User from ' . $ip . ' claiming to be ' . $username . ' has been blacklisted after ' . Configure::read('SecureAuth.amount') . ' failed attempts'
));
}
'action' => 'login_fail',
'title' => $title
);
$this->Log->save($log);
}
public function clean() {