Merge pull request #479 from nextcloud/add-bruteforce-throttler

Implement brute force protection

5 files changed, 275 insertions, 25 deletions

diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php
index d6fa772d38b..f09f3c98118 100644
--- a/tests/Core/Controller/LoginControllerTest.php
+++ b/tests/Core/Controller/LoginControllerTest.php
@@ -23,6 +23,7 @@ namespace Tests\Core\Controller;
 
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Core\Controller\LoginController;
+use OC\Security\Bruteforce\Throttler;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
@@ -51,6 +52,8 @@ class LoginControllerTest extends TestCase {
 	private $urlGenerator;
 	/** @var Manager | \PHPUnit_Framework_MockObject_MockObject */
 	private $twoFactorManager;
+	/** @var Throttler */
+	private $throttler;
 
 	public function setUp() {
 		parent::setUp();
@@ -65,6 +68,9 @@ class LoginControllerTest extends TestCase {
 		$this->twoFactorManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->throttler = $this->getMockBuilder('\OC\Security\Bruteforce\Throttler')
+			->disableOriginalConstructor()
+			->getMock();
 
 		$this->loginController = new LoginController(
 			'core',
@@ -74,7 +80,8 @@ class LoginControllerTest extends TestCase {
 			$this->session,
 			$this->userSession,
 			$this->urlGenerator,
-			$this->twoFactorManager
+			$this->twoFactorManager,
+			$this->throttler
 		);
 	}
 
@@ -277,10 +284,27 @@ class LoginControllerTest extends TestCase {
 	}
 
 	public function testLoginWithInvalidCredentials() {
-		$user = $this->getMock('\OCP\IUser');
+		$user = 'MyUserName';
 		$password = 'secret';
 		$loginPageUrl = 'some url';
 
+		$this->request
+			->expects($this->exactly(4))
+			->method('getRemoteAddress')
+			->willReturn('192.168.0.1');
+		$this->throttler
+			->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')
+			->with('login', '192.168.0.1', ['user' => 'MyUserName']);
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->will($this->returnValue(false));
@@ -302,6 +326,19 @@ class LoginControllerTest extends TestCase {
 		$password = 'secret';
 		$indexPageUrl = 'some url';
 
+		$this->request
+			->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));
@@ -334,6 +371,19 @@ class LoginControllerTest extends TestCase {
 		$originalUrl = 'another%20url';
 		$redirectUrl = 'http://localhost/another url';
 
+		$this->request
+			->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)
@@ -363,6 +413,19 @@ class LoginControllerTest extends TestCase {
 		$password = 'secret';
 		$challengeUrl = 'challenge/url';
 
+		$this->request
+			->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));
@@ -412,6 +475,23 @@ class LoginControllerTest extends TestCase {
 			->method('linkToRoute')
 			->with('core.login.showLoginForm', ['user' => 'john@doe.com'])
 			->will($this->returnValue(''));
+		$this->request
+			->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')
+			->with('192.168.0.1');
+		$this->throttler
+			->expects($this->once())
+			->method('registerAttempt')
+			->with('login', '192.168.0.1', ['user' => 'john@doe.com']);
 
 		$expected = new RedirectResponse('');
 		$this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', 'just wrong', null));
diff --git a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php
index 0edf96dd5a4..2e450d897bd 100644
--- a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php
+++ b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php
@@ -29,6 +29,9 @@ namespace Test\AppFramework\DependencyInjection;
 
 use \OC\AppFramework\Http\Request;
 
+/**
+ * @group DB
+ */
 class DIContainerTest extends \Test\TestCase {
 
 	private $container;
@@ -74,7 +77,6 @@ class DIContainerTest extends \Test\TestCase {
 		$this->assertEquals('name', $this->container['AppName']);
 	}
 
-
 	public function testMiddlewareDispatcherIncludesSecurityMiddleware(){
 		$this->container['Request'] = new Request(
 			['method' => 'GET'],
diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
index a0dbcc6872a..d0096d43f3d 100644
--- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
@@ -16,6 +16,7 @@ use OC\AppFramework\Http\Request;
 use OC\AppFramework\Middleware\Security\CORSMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
+use OC\Security\Bruteforce\Throttler;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 
@@ -24,6 +25,8 @@ class CORSMiddlewareTest extends \Test\TestCase {
 
 	private $reflector;
 	private $session;
+	/** @var Throttler */
+	private $throttler;
 
 	protected function setUp() {
 		parent::setUp();
@@ -31,6 +34,9 @@ class CORSMiddlewareTest extends \Test\TestCase {
 		$this->session = $this->getMockBuilder('\OC\User\Session')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->throttler =  $this->getMockBuilder('\OC\Security\Bruteforce\Throttler')
+			->disableOriginalConstructor()
+			->getMock();
 	}
 
 	/**
@@ -47,7 +53,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -65,7 +71,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock(),
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -83,7 +89,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -106,7 +112,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$response = new Response();
 		$response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE');
@@ -124,7 +130,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 		$this->session->expects($this->never())
 			->method('logout');
 		$this->session->expects($this->never())
@@ -155,7 +161,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			->with($this->equalTo('user'), $this->equalTo('pass'))
 			->will($this->returnValue(true));
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$middleware->beforeController($this, __FUNCTION__, new Response());
 	}
@@ -180,7 +186,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			->with($this->equalTo('user'), $this->equalTo('pass'))
 			->will($this->throwException(new \OC\Authentication\Exceptions\PasswordLoginForbiddenException));
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$middleware->beforeController($this, __FUNCTION__, new Response());
 	}
@@ -205,7 +211,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			->with($this->equalTo('user'), $this->equalTo('pass'))
 			->will($this->returnValue(false));
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 
 		$middleware->beforeController($this, __FUNCTION__, new Response());
 	}
@@ -219,7 +225,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock(),
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 		$response = $middleware->afterException($this, __FUNCTION__, new SecurityException('A security exception'));
 
 		$expected = new JSONResponse(['message' => 'A security exception'], 500);
@@ -235,7 +241,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock(),
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 		$response = $middleware->afterException($this, __FUNCTION__, new SecurityException('A security exception', 501));
 
 		$expected = new JSONResponse(['message' => 'A security exception'], 501);
@@ -255,7 +261,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock(),
 			$this->getMockBuilder('\OCP\IConfig')->getMock()
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
 		$middleware->afterException($this, __FUNCTION__, new \Exception('A regular exception'));
 	}
 
diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php
new file mode 100644
index 00000000000..9b7a47ceec8
--- /dev/null
+++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php
@@ -0,0 +1,123 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016 Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace Test\Security\Bruteforce;
+
+use OC\AppFramework\Utility\TimeFactory;
+use OC\Security\Bruteforce\Throttler;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\ILogger;
+use Test\TestCase;
+
+/**
+ * Based on the unit tests from Paragonie's Airship CMS
+ * Ref: https://github.com/paragonie/airship/blob/7e5bad7e3c0fbbf324c11f963fd1f80e59762606/test/unit/Engine/Security/AirBrakeTest.php
+ *
+ * @package Test\Security\Bruteforce
+ */
+class ThrottlerTest extends TestCase {
+	/** @var Throttler */
+	private $throttler;
+	/** @var IDBConnection */
+	private $dbConnection;
+	/** @var ILogger */
+	private $logger;
+	/** @var IConfig */
+	private $config;
+
+	public function setUp() {
+		$this->dbConnection = $this->getMock('\OCP\IDBConnection');
+		$this->logger = $this->getMock('\OCP\ILogger');
+		$this->config = $this->getMock('\OCP\IConfig');
+
+		$this->throttler = new Throttler(
+			$this->dbConnection,
+			new TimeFactory(),
+			$this->logger,
+			$this->config
+		);
+		return parent::setUp();
+	}
+
+	public function testCutoff() {
+		// precisely 31 second shy of 12 hours
+		$cutoff = $this->invokePrivate($this->throttler, 'getCutoff', [43169]);
+		$this->assertSame(0, $cutoff->y);
+		$this->assertSame(0, $cutoff->m);
+		$this->assertSame(0, $cutoff->d);
+		$this->assertSame(11, $cutoff->h);
+		$this->assertSame(59, $cutoff->i);
+		$this->assertSame(29, $cutoff->s);
+		$cutoff = $this->invokePrivate($this->throttler, 'getCutoff', [86401]);
+		$this->assertSame(0, $cutoff->y);
+		$this->assertSame(0, $cutoff->m);
+		$this->assertSame(1, $cutoff->d);
+		$this->assertSame(0, $cutoff->h);
+		$this->assertSame(0, $cutoff->i);
+		// Leap second tolerance:
+		$this->assertLessThan(2, $cutoff->s);
+	}
+
+	public function testSubnet() {
+		// IPv4
+		$this->assertSame(
+			'64.233.191.254/32',
+			$this->invokePrivate($this->throttler, 'getIPv4Subnet', ['64.233.191.254', 32])
+		);
+		$this->assertSame(
+			'64.233.191.252/30',
+			$this->invokePrivate($this->throttler, 'getIPv4Subnet', ['64.233.191.254', 30])
+		);
+		$this->assertSame(
+			'64.233.191.240/28',
+			$this->invokePrivate($this->throttler, 'getIPv4Subnet', ['64.233.191.254', 28])
+		);
+		$this->assertSame(
+			'64.233.191.0/24',
+			$this->invokePrivate($this->throttler, 'getIPv4Subnet', ['64.233.191.254', 24])
+		);
+		$this->assertSame(
+			'64.233.188.0/22',
+			$this->invokePrivate($this->throttler, 'getIPv4Subnet', ['64.233.191.254', 22])
+		);
+		// IPv6
+		$this->assertSame(
+			'2001:db8:85a3::8a2e:370:7334/127',
+			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7334', 127])
+		);
+		$this->assertSame(
+			'2001:db8:85a3::8a2e:370:7300/120',
+			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7300', 120])
+		);
+		$this->assertSame(
+			'2001:db8:85a3::/64',
+			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7334', 64])
+		);
+		$this->assertSame(
+			'2001:db8:85a3::/48',
+			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7334', 48])
+		);
+		$this->assertSame(
+			'2001:db8:8500::/40',
+			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7334', 40])
+		);
+	}
+}
diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index 9bde2c664b6..379c7e39442 100644
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -9,6 +9,7 @@
 
 namespace Test\User;
 
+use OC\Security\Bruteforce\Throttler;
 use OC\Session\Memory;
 use OC\User\User;
 
@@ -17,15 +18,14 @@ use OC\User\User;
  * @package Test\User
  */
 class SessionTest extends \Test\TestCase {
-
 	/** @var \OCP\AppFramework\Utility\ITimeFactory */
 	private $timeFactory;
-
 	/** @var \OC\Authentication\Token\DefaultTokenProvider */
 	protected $tokenProvider;
-
 	/** @var \OCP\IConfig */
 	private $config;
+	/** @var Throttler */
+	private $throttler;
 
 	protected function setUp() {
 		parent::setUp();
@@ -36,6 +36,8 @@ class SessionTest extends \Test\TestCase {
 			->will($this->returnValue(10000));
 		$this->tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider');
 		$this->config = $this->getMock('\OCP\IConfig');
+		$this->throttler = $this->getMockBuilder('\OC\Security\Bruteforce\Throttler')
+			->disableOriginalConstructor()->getMock();
 	}
 
 	public function testGetUser() {
@@ -353,7 +355,6 @@ class SessionTest extends \Test\TestCase {
 			->getMock();
 		$session = $this->getMock('\OCP\ISession');
 		$request = $this->getMock('\OCP\IRequest');
-		$user = $this->getMock('\OCP\IUser');
 
 		/** @var \OC\User\Session $userSession */
 		$userSession = $this->getMockBuilder('\OC\User\Session')
@@ -369,8 +370,21 @@ class SessionTest extends \Test\TestCase {
 			->method('getSystemValue')
 			->with('token_auth_enforced', false)
 			->will($this->returnValue(true));
-
-		$userSession->logClientIn('john', 'doe', $request);
+		$request
+			->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);
 	}
 
 	public function testLogClientInWithTokenPassword() {
@@ -379,7 +393,6 @@ class SessionTest extends \Test\TestCase {
 			->getMock();
 		$session = $this->getMock('\OCP\ISession');
 		$request = $this->getMock('\OCP\IRequest');
-		$user = $this->getMock('\OCP\IUser');
 
 		/** @var \OC\User\Session $userSession */
 		$userSession = $this->getMockBuilder('\OC\User\Session')
@@ -398,8 +411,21 @@ class SessionTest extends \Test\TestCase {
 		$session->expects($this->once())
 			->method('set')
 			->with('app_password', 'I-AM-AN-APP-PASSWORD');
-
-		$this->assertTrue($userSession->logClientIn('john', 'I-AM-AN-APP-PASSWORD', $request));
+		$request
+			->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));
 	}
 
 	/**
@@ -410,7 +436,6 @@ class SessionTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 		$session = $this->getMock('\OCP\ISession');
-		$user = $this->getMock('\OCP\IUser');
 		$request = $this->getMock('\OCP\IRequest');
 
 		/** @var \OC\User\Session $userSession */
@@ -433,7 +458,21 @@ class SessionTest extends \Test\TestCase {
 			->with('john')
 			->will($this->returnValue(true));
 
-		$userSession->logClientIn('john', 'doe', $request);
+		$request
+			->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);
 	}
 
 	public function testRememberLoginValidToken() {