Mitigate race condition

This commit is contained in:
Lukas Reschke 2016-07-20 23:09:27 +02:00
parent adf67fac96
commit c1589f163c
No known key found for this signature in database
GPG key ID: B9F6980CF6E759B1
4 changed files with 57 additions and 11 deletions

View file

@ -178,6 +178,7 @@ class LoginController extends Controller {
* @return RedirectResponse
*/
public function tryLogin($user, $password, $redirect_url) {
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress());
$this->throttler->sleepDelay($this->request->getRemoteAddress());
$originalUser = $user;
@ -194,7 +195,9 @@ class LoginController extends Controller {
}
if ($loginResult === false) {
$this->throttler->registerAttempt('login', $this->request->getRemoteAddress(), ['user' => $originalUser]);
if($currentDelay === 0) {
$this->throttler->sleepDelay($this->request->getRemoteAddress());
}
$this->session->set('loginMessages', [
['invalidpassword']
]);

View file

@ -310,6 +310,7 @@ class Session implements IUserSession, Emitter {
$password,
IRequest $request,
OC\Security\Bruteforce\Throttler $throttler) {
$currentDelay = $throttler->getDelay($request->getRemoteAddress());
$throttler->sleepDelay($request->getRemoteAddress());
$isTokenPassword = $this->isTokenPassword($password);
@ -326,6 +327,9 @@ class Session implements IUserSession, Emitter {
}
$throttler->registerAttempt('login', $request->getRemoteAddress(), ['uid' => $user]);
if($currentDelay === 0) {
$throttler->sleepDelay($request->getRemoteAddress());
}
return false;
}
@ -405,7 +409,6 @@ class Session implements IUserSession, Emitter {
public function tryBasicAuthLogin(IRequest $request,
OC\Security\Bruteforce\Throttler $throttler) {
if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) {
$throttler->sleepDelay(\OC::$server->getRequest()->getRemoteAddress());
try {
if ($this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW'], $request, $throttler)) {
/**

View file

@ -289,13 +289,18 @@ class LoginControllerTest extends TestCase {
$loginPageUrl = 'some url';
$this->request
->expects($this->exactly(2))
->expects($this->exactly(4))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->expects($this->exactly(2))
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(0);
$this->throttler
->expects($this->once())
->method('registerAttempt')
@ -322,13 +327,18 @@ class LoginControllerTest extends TestCase {
$indexPageUrl = 'some url';
$this->request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPassword')
->will($this->returnValue($user));
@ -362,13 +372,18 @@ class LoginControllerTest extends TestCase {
$redirectUrl = 'http://localhost/another url';
$this->request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPassword')
->with('Jane', $password)
@ -399,13 +414,18 @@ class LoginControllerTest extends TestCase {
$challengeUrl = 'challenge/url';
$this->request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPassword')
->will($this->returnValue($user));
@ -456,9 +476,14 @@ class LoginControllerTest extends TestCase {
->with('core.login.showLoginForm', ['user' => 'john@doe.com'])
->will($this->returnValue(''));
$this->request
->expects($this->exactly(2))
->expects($this->exactly(3))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->throttler
->expects($this->once())
->method('sleepDelay')

View file

@ -371,13 +371,18 @@ class SessionTest extends \Test\TestCase {
->with('token_auth_enforced', false)
->will($this->returnValue(true));
$request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(0);
$userSession->logClientIn('john', 'doe', $request, $this->throttler);
}
@ -407,13 +412,18 @@ class SessionTest extends \Test\TestCase {
->method('set')
->with('app_password', 'I-AM-AN-APP-PASSWORD');
$request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(0);
$this->assertTrue($userSession->logClientIn('john', 'I-AM-AN-APP-PASSWORD', $request, $this->throttler));
}
@ -449,13 +459,18 @@ class SessionTest extends \Test\TestCase {
->will($this->returnValue(true));
$request
->expects($this->once())
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(0);
$userSession->logClientIn('john', 'doe', $request, $this->throttler);
}