Implement brute force protection

Class Throttler implements the bruteforce protection for security actions in Nextcloud. It is working by logging invalid login attempts to the database and slowing down all login attempts from the same subnet. The max delay is 30 seconds and the starting delay are 200 milliseconds. (after the first failed login)
nextcloud · Jul 20, 2016 · 98b8ed0 · 98b8ed0
1 parent 7cdf640
commit 98b8ed0
Show file tree

Hide file tree

Showing 25 changed files with 654 additions and 68 deletions.
diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php
@@ -35,6 +35,7 @@
 	\OC::$server->getUserSession(),
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
+	\OC::$server->getBruteForceThrottler(),
 	'principals/'
 );
 $principalBackend = new Principal(

diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php
@@ -36,6 +36,7 @@
 	\OC::$server->getUserSession(),
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
+	\OC::$server->getBruteForceThrottler(),
 	'principals/'
 );
 $principalBackend = new Principal(

diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php
@@ -43,6 +43,7 @@
 	\OC::$server->getUserSession(),
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
+	\OC::$server->getBruteForceThrottler(),
 	'principals/'
 );
 $requestUri = \OC::$server->getRequest()->getRequestUri();

diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php
@@ -33,6 +33,7 @@
 use OC\AppFramework\Http\Request;
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\TwoFactorAuth\Manager;
+use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
 use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
 use OCP\IRequest;
@@ -58,23 +59,28 @@ class Auth extends AbstractBasic {
 	private $currentUser;
 	/** @var Manager */
 	private $twoFactorManager;
+	/** @var Throttler */
+	private $throttler;
 
 	/**
 	 * @param ISession $session
 	 * @param Session $userSession
 	 * @param IRequest $request
 	 * @param Manager $twoFactorManager
+	 * @param Throttler $throttler
 	 * @param string $principalPrefix
 	 */
 	public function __construct(ISession $session,
 								Session $userSession,
 								IRequest $request,
 								Manager $twoFactorManager,
+								Throttler $throttler,
 								$principalPrefix = 'principals/users/') {
 		$this->session = $session;
 		$this->userSession = $userSession;
 		$this->twoFactorManager = $twoFactorManager;
 		$this->request = $request;
+		$this->throttler = $throttler;
 		$this->principalPrefix = $principalPrefix;
 
 		// setup realm
@@ -107,6 +113,7 @@ public function isDavAuthenticated($username) {
 	 * @param string $username
 	 * @param string $password
 	 * @return bool
+	 * @throws PasswordLoginForbidden
 	 */
 	protected function validateUserPass($username, $password) {
 		if ($this->userSession->isLoggedIn() &&
@@ -118,7 +125,7 @@ protected function validateUserPass($username, $password) {
 		} else {
 			\OC_Util::setupFS(); //login hooks may need early access to the filesystem
 			try {
-				if ($this->userSession->logClientIn($username, $password, $this->request)) {
+				if ($this->userSession->logClientIn($username, $password, $this->request, $this->throttler)) {
 					\OC_Util::setupFS($this->userSession->getUser()->getUID());
 					$this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID());
 					$this->session->close();

diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
@@ -64,7 +64,8 @@ public function __construct(IRequest $request, $baseUri) {
 			\OC::$server->getSession(),
 			\OC::$server->getUserSession(),
 			\OC::$server->getRequest(),
-			\OC::$server->getTwoFactorAuthManager()
+			\OC::$server->getTwoFactorAuthManager(),
+			\OC::$server->getBruteForceThrottler()
 		);
 
 		// Set URL explicitly due to reverse-proxy situations

diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
@@ -28,6 +28,7 @@
 namespace OCA\DAV\Tests\unit\Connector\Sabre;
 
 use OC\Authentication\TwoFactorAuth\Manager;
+use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
 use OCP\IRequest;
 use OCP\ISession;
@@ -51,6 +52,8 @@ class AuthTest extends TestCase {
 	private $request;
 	/** @var Manager */
 	private $twoFactorManager;
+	/** @var Throttler */
+	private $throttler;
 
 	public function setUp() {
 		parent::setUp();
@@ -63,11 +66,15 @@ public function setUp() {
 		$this->twoFactorManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->throttler = $this->getMockBuilder('\OC\Security\Bruteforce\Throttler')
+			->disableOriginalConstructor()
+			->getMock();
 		$this->auth = new \OCA\DAV\Connector\Sabre\Auth(
 			$this->session,
 			$this->userSession,
 			$this->request,
-			$this->twoFactorManager
+			$this->twoFactorManager,
+			$this->throttler
 		);
 	}
 

diff --git a/build/integration/run.sh b/build/integration/run.sh
@@ -9,6 +9,9 @@ else
 	exit 1
 fi
 
+# Disable bruteforce protection because the integration tests do trigger them
+../../occ config:system:set auth.bruteforce.protection.enabled --value false --type bool
+
 composer install
 
 SCENARIO_TO_RUN=$1

diff --git a/config/config.sample.php b/config/config.sample.php
@@ -207,6 +207,13 @@
  */
 'token_auth_enforced' => false,
 
+/**
+ * Whether the bruteforce protection shipped with Nextcloud should be enabled or not.
+ *
+ * Disabling this is discouraged for security reasons.
+ */
+'auth.bruteforce.protection.enabled' => true,
+
 /**
  * The directory where the skeleton files are located. These files will be
  * copied to the data directory of new users. Leave empty to not copy any

diff --git a/core/Application.php b/core/Application.php
@@ -103,7 +103,8 @@ public function __construct(array $urlParams=array()){
 				$c->query('Session'),
 				$c->query('UserSession'),
 				$c->query('URLGenerator'),
-				$c->query('TwoFactorAuthManager')
+				$c->query('TwoFactorAuthManager'),
+				$c->query('ServerContainer')->getBruteforceThrottler()
 			);
 		});
 		$container->registerService('TwoFactorChallengeController', function (SimpleContainer $c) {

diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
@@ -22,7 +22,9 @@
 
 namespace OC\Core\Controller;
 
+use OC\AppFramework\Utility\TimeFactory;
 use OC\Authentication\TwoFactorAuth\Manager;
+use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
 use OC_App;
 use OC_Util;
@@ -37,24 +39,20 @@
 use OCP\IUserManager;
 
 class LoginController extends Controller {
-
 	/** @var IUserManager */
 	private $userManager;
-
 	/** @var IConfig */
 	private $config;
-
 	/** @var ISession */
 	private $session;
-
 	/** @var Session */
 	private $userSession;
-
 	/** @var IURLGenerator */
 	private $urlGenerator;
-
 	/** @var Manager */
 	private $twoFactorManager;
+	/** @var Throttler */
+	private $throttler;
 
 	/**
 	 * @param string $appName
@@ -65,16 +63,25 @@ class LoginController extends Controller {
 	 * @param Session $userSession
 	 * @param IURLGenerator $urlGenerator
 	 * @param Manager $twoFactorManager
+	 * @param Throttler $throttler
 	 */
-	function __construct($appName, IRequest $request, IUserManager $userManager, IConfig $config, ISession $session,
-		Session $userSession, IURLGenerator $urlGenerator, Manager $twoFactorManager) {
+	function __construct($appName,
+						 IRequest $request,
+						 IUserManager $userManager,
+						 IConfig $config,
+						 ISession $session,
+						 Session $userSession,
+						 IURLGenerator $urlGenerator,
+						 Manager $twoFactorManager,
+						 Throttler $throttler) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
 		$this->config = $config;
 		$this->session = $session;
 		$this->userSession = $userSession;
 		$this->urlGenerator = $urlGenerator;
 		$this->twoFactorManager = $twoFactorManager;
+		$this->throttler = $throttler;
 	}
 
 	/**
@@ -171,6 +178,8 @@ public function showLoginForm($user, $redirect_url, $remember_login) {
 	 * @return RedirectResponse
 	 */
 	public function tryLogin($user, $password, $redirect_url) {
+		$this->throttler->sleepDelay($this->request->getRemoteAddress());
+
 		$originalUser = $user;
 		// TODO: Add all the insane error handling
 		/* @var $loginResult IUser */
@@ -184,6 +193,8 @@ public function tryLogin($user, $password, $redirect_url) {
 			}
 		}
 		if ($loginResult === false) {
+			$this->throttler->registerAttempt('login', $this->request->getRemoteAddress(), ['user' => $originalUser]);
+
 			$this->session->set('loginMessages', [
 				['invalidpassword']
 			]);

diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php
@@ -1,5 +1,4 @@
 <?php
-
 /**
  * @author Christoph Wurst <[email protected]>
  *
@@ -23,6 +22,7 @@
 namespace OC\Core\Controller;
 
 use OC\AppFramework\Http;
+use OC\AppFramework\Utility\TimeFactory;
 use OC\Authentication\Token\DefaultTokenProvider;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
@@ -35,27 +35,29 @@
 use OCP\Security\ISecureRandom;
 
 class TokenController extends Controller {
-
 	/** @var UserManager */
 	private $userManager;
-
 	/** @var IProvider */
 	private $tokenProvider;
-
 	/** @var TwoFactorAuthManager */
 	private $twoFactorAuthManager;
-
 	/** @var ISecureRandom */
 	private $secureRandom;
 
 	/**
 	 * @param string $appName
 	 * @param IRequest $request
-	 * @param Manager $userManager
-	 * @param DefaultTokenProvider $tokenProvider
+	 * @param UserManager $userManager
+	 * @param IProvider $tokenProvider
+	 * @param TwoFactorAuthManager $twoFactorAuthManager
 	 * @param ISecureRandom $secureRandom
 	 */
-	public function __construct($appName, IRequest $request, UserManager $userManager, IProvider $tokenProvider, TwoFactorAuthManager $twoFactorAuthManager, ISecureRandom $secureRandom) {
+	public function __construct($appName,
+								IRequest $request,
+								UserManager $userManager,
+								IProvider $tokenProvider,
+								TwoFactorAuthManager $twoFactorAuthManager,
+								ISecureRandom $secureRandom) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
 		$this->tokenProvider = $tokenProvider;

diff --git a/db_structure.xml b/db_structure.xml
@@ -1163,6 +1163,84 @@
 		</declaration>
 	</table>
 
+	<table>
+
+		<!--
+		List of usernames, their display name and login password.
+		-->
+		<name>*dbprefix*bruteforce_attempts</name>
+
+		<declaration>
+			<field>
+				<name>id</name>
+				<type>integer</type>
+				<default>0</default>
+				<notnull>true</notnull>
+				<autoincrement>1</autoincrement>
+				<unsigned>true</unsigned>
+				<length>4</length>
+			</field>
+
+			<field>
+				<name>action</name>
+				<type>text</type>
+				<default></default>
+				<notnull>true</notnull>
+				<length>64</length>
+			</field>
+
+			<field>
+				<name>occurred</name>
+				<type>integer</type>
+				<default>0</default>
+				<notnull>true</notnull>
+				<unsigned>true</unsigned>
+				<length>4</length>
+			</field>
+
+			<field>
+				<name>ip</name>
+				<type>text</type>
+				<default></default>
+				<notnull>true</notnull>
+				<length>255</length>
+			</field>
+
+			<field>
+				<name>subnet</name>
+				<type>text</type>
+				<default></default>
+				<notnull>true</notnull>
+				<length>255</length>
+			</field>
+
+			<field>
+				<name>metadata</name>
+				<type>text</type>
+				<default></default>
+				<notnull>true</notnull>
+				<length>255</length>
+			</field>
+
+			<index>
+				<name>bruteforce_attempts_ip</name>
+				<field>
+					<name>ip</name>
+					<sorting>ascending</sorting>
+				</field>
+			</index>
+			<index>
+				<name>bruteforce_attempts_subnet</name>
+				<field>
+					<name>subnet</name>
+					<sorting>ascending</sorting>
+				</field>
+			</index>
+
+		</declaration>
+
+	</table>
+
 	<table>
 
 		<!--

diff --git a/lib/base.php b/lib/base.php
@@ -913,7 +913,7 @@ static function handleLogin(OCP\IRequest $request) {
 		if ($userSession->tryTokenLogin($request)) {
 			return true;
 		}
-		if ($userSession->tryBasicAuthLogin($request)) {
+		if ($userSession->tryBasicAuthLogin($request, \OC::$server->getBruteForceThrottler())) {
 			return true;
 		}
 		return false;

diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -351,7 +351,8 @@ public function __construct($appName, $urlParams = array()){
 			return new CORSMiddleware(
 				$c['Request'],
 				$c['ControllerMethodReflector'],
-				$c['OCP\IUserSession']
+				$c['OCP\IUserSession'],
+				$c->getServer()->getBruteForceThrottler()
 			);
 		});