Merge pull request #2095 from nextcloud/bruteforcesetttings

Introduce bruteforce settings
nextcloud · Apr 4, 2017 · e0227cb · e0227cb
2 parents da178db + aee2d63
commit e0227cb
Show file tree

Hide file tree

Showing 6 changed files with 160 additions and 4 deletions.
diff --git a/.gitignore b/.gitignore
@@ -40,7 +40,6 @@
 /apps/files_external/tests/config.*.php
 
 
-
 # ignore themes except the example and the README
 /themes/*
 !/themes/example

diff --git a/core/shipped.json b/core/shipped.json
@@ -2,6 +2,7 @@
   "shippedApps": [
     "activity",
     "admin_audit",
+    "bruteforcesettings",
     "comments",
     "dav",
     "encryption",

diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php
@@ -185,6 +185,67 @@ public function registerAttempt($action,
 		$qb->execute();
 	}
 
+	/**
+	 * Check if the IP is whitelisted
+	 *
+	 * @param string $ip
+	 * @return bool
+	 */
+	private function isIPWhitelisted($ip) {
+		$keys = $this->config->getAppKeys('bruteForce');
+		$keys = array_filter($keys, function($key) {
+			$regex = '/^whitelist_/S';
+			return preg_match($regex, $key) === 1;
+		});
+
+		if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
+			$type = 4;
+		} else if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
+			$type = 6;
+		} else {
+			return false;
+		}
+
+		$ip = inet_pton($ip);
+
+		foreach ($keys as $key) {
+			$cidr = $this->config->getAppValue('bruteForce', $key, null);
+
+			$cx = explode('/', $cidr);
+			$addr = $cx[0];
+			$mask = (int)$cx[1];
+
+			// Do not compare ipv4 to ipv6
+			if (($type === 4 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) ||
+				($type === 6 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6))) {
+				continue;
+			}
+
+			$addr = inet_pton($addr);
+
+			$valid = true;
+			for($i = 0; $i < $mask; $i++) {
+				$part = ord($addr[(int)($i/8)]);
+				$orig = ord($ip[(int)($i/8)]);
+
+				$part = $part & (15 << (1 - ($i % 2)));
+				$orig = $orig & (15 << (1 - ($i % 2)));
+
+				if ($part !== $orig) {
+					$valid = false;
+					break;
+				}
+			}
+
+			if ($valid === true) {
+				return true;
+			}
+		}
+
+		return false;
+
+	}
+
 	/**
 	 * Get the throttling delay (in milliseconds)
 	 *
@@ -193,6 +254,10 @@ public function registerAttempt($action,
 	 * @return int
 	 */
 	public function getDelay($ip, $action = '') {
+		if ($this->isIPWhitelisted($ip)) {
+			return 0;
+		}
+
 		$cutoffTime = (new \DateTime())
 			->sub($this->getCutoff(43200))
 			->getTimestamp();

diff --git a/lib/private/Settings/Manager.php b/lib/private/Settings/Manager.php
@@ -273,6 +273,7 @@ public function getAdminSections() {
 		$sections = [
 			0 => [new Section('server', $this->l->t('Server settings'), 0, $this->url->imagePath('settings', 'admin.svg'))],
 			5 => [new Section('sharing', $this->l->t('Sharing'), 0, $this->url->imagePath('core', 'actions/share.svg'))],
+			10 => [new Section('security', $this->l->t('Security'), 0, $this->url->imagePath('core', 'actions/password.svg'))],
 			45 => [new Section('encryption', $this->l->t('Encryption'), 0, $this->url->imagePath('core', 'actions/password.svg'))],
 			98 => [new Section('additional', $this->l->t('Additional settings'), 0, $this->url->imagePath('core', 'actions/settings-dark.svg'))],
 			99 => [new Section('tips-tricks', $this->l->t('Tips & tricks'), 0, $this->url->imagePath('settings', 'help.svg'))],

diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php
@@ -40,7 +40,7 @@ class ThrottlerTest extends TestCase {
 	private $dbConnection;
 	/** @var ILogger */
 	private $logger;
-	/** @var IConfig */
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	private $config;
 
 	public function setUp() {
@@ -120,4 +120,92 @@ public function testSubnet() {
 			$this->invokePrivate($this->throttler, 'getIPv6Subnet', ['2001:0db8:85a3:0000:0000:8a2e:0370:7334', 40])
 		);
 	}
+
+	public function dataIsIPWhitelisted() {
+		return [
+			[
+				'10.10.10.10',
+				[
+					'whitelist_0' => '10.10.10.0/24',
+				],
+				true,
+			],
+			[
+				'10.10.10.10',
+				[
+					'whitelist_0' => '192.168.0.0/16',
+				],
+				false,
+			],
+			[
+				'10.10.10.10',
+				[
+					'whitelist_0' => '192.168.0.0/16',
+					'whitelist_1' => '10.10.10.0/24',
+				],
+				true,
+			],
+			[
+				'dead:beef:cafe::1',
+				[
+					'whitelist_0' => '192.168.0.0/16',
+					'whitelist_1' => '10.10.10.0/24',
+					'whitelist_2' => 'deaf:beef:cafe:1234::/64'
+				],
+				false,
+			],
+			[
+				'dead:beef:cafe::1',
+				[
+					'whitelist_0' => '192.168.0.0/16',
+					'whitelist_1' => '10.10.10.0/24',
+					'whitelist_2' => 'deaf:beef::/64'
+				],
+				false,
+			],
+			[
+				'dead:beef:cafe::1',
+				[
+					'whitelist_0' => '192.168.0.0/16',
+					'whitelist_1' => '10.10.10.0/24',
+					'whitelist_2' => 'deaf:cafe::/8'
+				],
+				true,
+			],
+			[
+				'invalid',
+				[],
+				false,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataIsIPWhitelisted
+	 *
+	 * @param string $ip
+	 * @param string[] $whitelists
+	 * @param bool $isWhiteListed
+	 */
+	public function testIsIPWhitelisted($ip, $whitelists, $isWhiteListed) {
+		$this->config->method('getAppKeys')
+			->with($this->equalTo('bruteForce'))
+			->willReturn(array_keys($whitelists));
+
+		$this->config->method('getAppValue')
+			->will($this->returnCallback(function($app, $key, $default) use ($whitelists) {
+				if ($app !== 'bruteForce') {
+					return $default;
+				}
+				if (isset($whitelists[$key])) {
+					return $whitelists[$key];
+				}
+				return $default;
+			}));
+
+		$this->assertSame(
+			$isWhiteListed,
+			$this->invokePrivate($this->throttler, 'isIPWhitelisted', [$ip])
+		);
+	}
 }
diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php
@@ -146,7 +146,7 @@ public function testGetAdminSections() {
 				['class' => \OCA\WorkflowEngine\Settings\Section::class, 'priority' => 90]
 			]));
 
-		$this->url->expects($this->exactly(5))
+		$this->url->expects($this->exactly(6))
 			->method('imagePath')
 			->willReturnMap([
 				['settings', 'admin.svg', '1'],
@@ -159,6 +159,7 @@ public function testGetAdminSections() {
 		$this->assertEquals([
 			0 => [new Section('server', 'Server settings', 0, '1')],
 			5 => [new Section('sharing', 'Sharing', 0, '2')],
+			10 => [new Section('security', 'Security', 0, '3')],
 			45 => [new Section('encryption', 'Encryption', 0, '3')],
 			90 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
 			98 => [new Section('additional', 'Additional settings', 0, '4')],
@@ -177,7 +178,7 @@ public function testGetAdminSectionsEmptySection() {
 			->will($this->returnValue([
 			]));
 
-		$this->url->expects($this->exactly(5))
+		$this->url->expects($this->exactly(6))
 			->method('imagePath')
 			->willReturnMap([
 				['settings', 'admin.svg', '1'],
@@ -190,6 +191,7 @@ public function testGetAdminSectionsEmptySection() {
 		$this->assertEquals([
 			0 => [new Section('server', 'Server settings', 0, '1')],
 			5 => [new Section('sharing', 'Sharing', 0, '2')],
+			10 => [new Section('security', 'Security', 0, '3')],
 			45 => [new Section('encryption', 'Encryption', 0, '3')],
 			98 => [new Section('additional', 'Additional settings', 0, '4')],
 			99 => [new Section('tips-tricks', 'Tips & tricks', 0, '5')],