ability to enable bruteforce protection for ExApp routes (#368)

```xml <route> <url>^api\/w\/nextcloud\/jobs\/.*</url> <verb>GET,POST,PUT,DELETE</verb> <access_level>PUBLIC</access_level> <headers_to_exclude>[]</headers_to_exclude> <bruteforce_protection>[401, 500]</bruteforce_protection> </route> ``` Looks like this. ExApps should not implement its own protection, we should provide a way to enable basic protection from Nextcloud/AppAPI side. --------- Signed-off-by: Alexander Piskun <[email protected]> Signed-off-by: Andrey Borysenko <[email protected]> Co-authored-by: Andrey Borysenko <[email protected]>
nextcloud · Aug 22, 2024 · eda9c80 · eda9c80
1 parent 7c0587e
commit eda9c80
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 26 deletions.
diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -43,7 +43,7 @@ to join us in shaping a more versatile, stable, and secure app landscape.
 *Your insights, suggestions, and contributions are invaluable to us.*
 
 	]]></description>
-	<version>3.1.0</version>
+	<version>3.1.1</version>
 	<licence>agpl</licence>
 	<author mail="[email protected]" homepage="https://github.com/andrey18106">Andrey Borysenko</author>
 	<author mail="[email protected]" homepage="https://github.com/bigcat88">Alexander Piskun</author>

diff --git a/docs/tech_details/api/routes.rst b/docs/tech_details/api/routes.rst
@@ -28,6 +28,7 @@ Example
 			<verb>GET,POST,PUT,DELETE</verb>
 			<access_level>USER</access_level>
 			<headers_to_exclude>[]</headers_to_exclude>
+			<bruteforce_protection>[401, 500]</bruteforce_protection>
 		</route>
 	</routes>
 
@@ -37,6 +38,7 @@ where the fields are:
 - ``verb``: the HTTP verb that the route will accept, can be a comma separated list of verbs
 - ``access_level``: the name of the access level required to access the route, PUBLIC - public access without auth, USER - Nextcloud user auth required, ADMIN - admin user required
 - ``headers_to_exclude``: a json encoded string of an array of strings, the headers that the ExApp wants to be excluded from the request to it
+- ``bruteforce_protection``: a json encoded string of an array of numbers, the HTTP status codes that must trigger the bruteforce protection
 
 
 Unregister

diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php
@@ -24,6 +24,7 @@
 use OCP\Http\Client\IResponse;
 use OCP\IGroupManager;
 use OCP\IRequest;
+use OCP\Security\Bruteforce\IThrottler;
 use Psr\Log\LoggerInterface;
 
 class ExAppProxyController extends Controller {
@@ -37,6 +38,7 @@ public function __construct(
 		private readonly ?string                           $userId,
 		private readonly IGroupManager                     $groupManager,
 		private readonly LoggerInterface                   $logger,
+		private readonly IThrottler              		   $throttler,
 	) {
 		parent::__construct(Application::APP_ID, $request);
 	}
@@ -84,37 +86,45 @@ private function createProxyResponse(string $path, IResponse $response, $cache =
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	public function ExAppGet(string $appId, string $other): Response {
-		$exApp = $this->checkAccess($appId, $other);
+		$route = [];
+		$bruteforceProtection = [];
+		$delay = 0;
+		$exApp = $this->prepareProxy($appId, $other, $route, $bruteforceProtection, $delay);
 		if ($exApp === null) {
 			return new NotFoundResponse();
 		}
 
 		$response = $this->service->requestToExApp2(
 			$exApp, '/' . $other, $this->userId, 'GET', queryParams: $_GET, options: [
 				RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),
-				RequestOptions::HEADERS => $this->buildHeadersWithExclude($exApp, $other, getallheaders()),
+				RequestOptions::HEADERS => $this->buildHeadersWithExclude($route, getallheaders()),
 				RequestOptions::TIMEOUT => 0,
 			],
 			request: $this->request,
 		);
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
 		}
+
+		$this->processBruteforce($bruteforceProtection, $delay, $response->getStatusCode());
 		return $this->createProxyResponse($other, $response);
 	}
 
 	#[PublicPage]
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	public function ExAppPost(string $appId, string $other): Response {
-		$exApp = $this->checkAccess($appId, $other);
+		$route = [];
+		$bruteforceProtection = [];
+		$delay = 0;
+		$exApp = $this->prepareProxy($appId, $other, $route, $bruteforceProtection, $delay);
 		if ($exApp === null) {
 			return new NotFoundResponse();
 		}
 
 		$options = [
 			RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),
-			RequestOptions::HEADERS => $this->buildHeadersWithExclude($exApp, $other, getallheaders()),
+			RequestOptions::HEADERS => $this->buildHeadersWithExclude($route, getallheaders()),
 			RequestOptions::TIMEOUT => 0,
 		];
 		if (str_starts_with($this->request->getHeader('Content-Type'), 'multipart/form-data') || count($_FILES) > 0) {
@@ -136,14 +146,19 @@ public function ExAppPost(string $appId, string $other): Response {
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
 		}
+
+		$this->processBruteforce($bruteforceProtection, $delay, $response->getStatusCode());
 		return $this->createProxyResponse($other, $response);
 	}
 
 	#[PublicPage]
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	public function ExAppPut(string $appId, string $other): Response {
-		$exApp = $this->checkAccess($appId, $other);
+		$route = [];
+		$bruteforceProtection = [];
+		$delay = 0;
+		$exApp = $this->prepareProxy($appId, $other, $route, $bruteforceProtection, $delay);
 		if ($exApp === null) {
 			return new NotFoundResponse();
 		}
@@ -152,7 +167,7 @@ public function ExAppPut(string $appId, string $other): Response {
 		$options = [
 			RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),
 			RequestOptions::BODY => $stream,
-			RequestOptions::HEADERS => $this->buildHeadersWithExclude($exApp, $other, getallheaders()),
+			RequestOptions::HEADERS => $this->buildHeadersWithExclude($route, getallheaders()),
 			RequestOptions::TIMEOUT => 0,
 		];
 		$response = $this->service->requestToExApp2(
@@ -163,14 +178,19 @@ public function ExAppPut(string $appId, string $other): Response {
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
 		}
+
+		$this->processBruteforce($bruteforceProtection, $delay, $response->getStatusCode());
 		return $this->createProxyResponse($other, $response);
 	}
 
 	#[PublicPage]
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	public function ExAppDelete(string $appId, string $other): Response {
-		$exApp = $this->checkAccess($appId, $other);
+		$route = [];
+		$bruteforceProtection = [];
+		$delay = 0;
+		$exApp = $this->prepareProxy($appId, $other, $route, $bruteforceProtection, $delay);
 		if ($exApp === null) {
 			return new NotFoundResponse();
 		}
@@ -179,7 +199,7 @@ public function ExAppDelete(string $appId, string $other): Response {
 		$options = [
 			RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $this->service->getExAppDomain($exApp)),
 			RequestOptions::BODY => $stream,
-			RequestOptions::HEADERS => $this->buildHeadersWithExclude($exApp, $other, getallheaders()),
+			RequestOptions::HEADERS => $this->buildHeadersWithExclude($route, getallheaders()),
 			RequestOptions::TIMEOUT => 0,
 		];
 		$response = $this->service->requestToExApp2(
@@ -190,10 +210,15 @@ public function ExAppDelete(string $appId, string $other): Response {
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
 		}
+
+		$this->processBruteforce($bruteforceProtection, $delay, $response->getStatusCode());
 		return $this->createProxyResponse($other, $response);
 	}
 
-	private function checkAccess(string $appId, string $other): ?ExApp {
+	private function prepareProxy(
+		string $appId, string $other, array &$route, array &$bruteforceProtection, int &$delay
+	): ?ExApp {
+		$delay = 0;
 		$exApp = $this->exAppService->getExApp($appId);
 		if ($exApp === null) {
 			$this->logger->debug(
@@ -205,15 +230,33 @@ private function checkAccess(string $appId, string $other): ?ExApp {
 				sprintf('Returning status 404 for "%s": ExApp is not enabled.', $other)
 			);
 			return null;
-		} elseif (!$this->passesExAppProxyRoutesChecks($exApp, $other)) {
+		}
+		$route = $this->passesExAppProxyRoutesChecks($exApp, $other);
+		if (empty($route)) {
 			$this->logger->debug(
 				sprintf('Returning status 404 for "%s": route does not pass the access check.', $other)
 			);
 			return null;
 		}
+		$bruteforceProtection = isset($route['bruteforce_protection'])
+			? json_decode($route['bruteforce_protection'], true)
+			: [];
+		if (!empty($bruteforceProtection)) {
+			$delay = $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), Application::APP_ID);
+		}
 		return $exApp;
 	}
 
+	private function processBruteforce(array $bruteforceProtection, int $delay, int $status): void {
+		if (!empty($bruteforceProtection)) {
+			if ($delay > 0 && ($status >= 200 && $status < 300)) {
+				$this->throttler->resetDelay($this->request->getRemoteAddress(), Application::APP_ID, []);
+			} elseif (in_array($status, $bruteforceProtection)) {
+				$this->throttler->registerAttempt(Application::APP_ID, $this->request->getRemoteAddress());
+			}
+		}
+	}
+
 	private function buildProxyCookiesJar(array $cookies, string $domain): CookieJar {
 		$cookieJar = new CookieJar();
 		foreach ($cookies as $name => $value) {
@@ -250,16 +293,16 @@ private function buildMultipartFormData(array $bodyParams, array $files): array
 		return $multipart;
 	}
 
-	private function passesExAppProxyRoutesChecks(ExApp $exApp, string $exAppRoute): bool {
+	private function passesExAppProxyRoutesChecks(ExApp $exApp, string $exAppRoute): array {
 		foreach ($exApp->getRoutes() as $route) {
 			if (preg_match('/' . $route['url'] . '/i', $exAppRoute) === 1 &&
 				str_contains(strtolower($route['verb']), strtolower($this->request->getMethod())) &&
 				$this->passesExAppProxyRouteAccessLevelCheck($route['access_level'])
 			) {
-				return true;
+				return $route;
 			}
 		}
-		return false;
+		return [];
 	}
 
 	private function passesExAppProxyRouteAccessLevelCheck(int $accessLevel): bool {
@@ -271,18 +314,8 @@ private function passesExAppProxyRouteAccessLevelCheck(int $accessLevel): bool {
 		};
 	}
 
-	private function buildHeadersWithExclude(ExApp $exApp, string $exAppRoute, array $headers): array {
-		$headersToExclude = [];
-		foreach ($exApp->getRoutes() as $route) {
-			$matchesUrlPattern = preg_match('/' . $route['url'] . '/i', $exAppRoute) === 1;
-			$matchesVerb = str_contains(strtolower($route['verb']), strtolower($this->request->getMethod()));
-			if ($matchesUrlPattern && $matchesVerb) {
-				$headersToExclude = array_map(function ($headerName) {
-					return strtolower($headerName);
-				}, json_decode($route['headers_to_exclude'], true));
-				break;
-			}
-		}
+	private function buildHeadersWithExclude(array $route, array $headers): array {
+		$headersToExclude = json_decode($route['headers_to_exclude'], true);
 		if (!in_array('x-origin-ip', $headersToExclude)) {
 			$headersToExclude[] = 'x-origin-ip';
 		}

diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php
@@ -37,6 +37,7 @@ public function findAll(?int $limit = null, ?int $offset = null): array {
 			'r.verb',
 			'r.access_level',
 			'r.headers_to_exclude',
+			'r.bruteforce_protection',
 		)
 			->from($this->tableName, 'a')
 			->leftJoin('a', 'ex_apps_daemons', 'd', $qb->expr()->eq('a.daemon_config_name', 'd.name'))
@@ -68,6 +69,7 @@ public function findByAppId(string $appId): Entity {
 			'r.verb',
 			'r.access_level',
 			'r.headers_to_exclude',
+			'r.bruteforce_protection',
 		)
 			->from($this->tableName, 'a')
 			->leftJoin('a', 'ex_apps_daemons', 'd', $qb->expr()->eq('a.daemon_config_name', 'd.name'))
@@ -125,6 +127,7 @@ private function buildExAppWithRoutes(array $result): array {
 					'verb' => $row['verb'],
 					'access_level' => $row['access_level'],
 					'headers_to_exclude' => $row['headers_to_exclude'],
+					'bruteforce_protection' => $row['bruteforce_protection'],
 				];
 				$lastAppRoutes = $lastApp->getRoutes();
 				$lastAppRoutes[] = $route;
@@ -195,13 +198,21 @@ public function registerExAppRoutes(ExApp $exApp, array $routes): int {
 		$qb = $this->db->getQueryBuilder();
 		$count = 0;
 		foreach ($routes as $route) {
+			if (isset($route['bruteforce_protection']) && is_string($route['bruteforce_protection'])) {
+				$route['bruteforce_protection'] = json_decode($route['bruteforce_protection'], false);
+			}
 			$qb->insert('ex_apps_routes')
 				->values([
 					'appid' => $qb->createNamedParameter($exApp->getAppid()),
 					'url' => $qb->createNamedParameter($route['url']),
 					'verb' => $qb->createNamedParameter($route['verb']),
 					'access_level' => $qb->createNamedParameter($route['access_level']),
 					'headers_to_exclude' => $qb->createNamedParameter(is_array($route['headers_to_exclude']) ? json_encode($route['headers_to_exclude']) : '[]'),
+					'bruteforce_protection' => $qb->createNamedParameter(
+						isset($route['bruteforce_protection']) && is_array($route['bruteforce_protection'])
+							? json_encode($route['bruteforce_protection'])
+							: '[]'
+					),
 				]);
 			$count += $qb->executeStatement();
 		}

diff --git a/lib/Migration/Version3100Date20240822080316.php b/lib/Migration/Version3100Date20240822080316.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\AppAPI\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version3100Date20240822080316 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('ex_apps_routes');
+		if (!$table->hasColumn('bruteforce_protection')) {
+			$table->addColumn('bruteforce_protection', Types::STRING, [
+				'notnull' => false,
+				'length' => 512,
+			]);
+		}
+
+		return $schema;
+	}
+}