fix(federation): Add bruteforce protection to federation endpoint

Signed-off-by: Joas Schilling <coding@schilljs.com>

2 files changed, 30 insertions, 3 deletions

diff --git a/apps/federation/lib/Controller/OCSAuthAPIController.php b/apps/federation/lib/Controller/OCSAuthAPIController.php
index 5a976720b04..be4466919f6 100644
--- a/apps/federation/lib/Controller/OCSAuthAPIController.php
+++ b/apps/federation/lib/Controller/OCSAuthAPIController.php
@@ -36,6 +36,7 @@ use OCP\AppFramework\OCSController;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\IRequest;
+use OCP\Security\Bruteforce\IThrottler;
 use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 
@@ -53,6 +54,7 @@ class OCSAuthAPIController extends OCSController {
 	private DbHandler $dbHandler;
 	private LoggerInterface $logger;
 	private ITimeFactory $timeFactory;
+	private IThrottler $throttler;
 
 	public function __construct(
 		string $appName,
@@ -62,7 +64,8 @@ class OCSAuthAPIController extends OCSController {
 		TrustedServers $trustedServers,
 		DbHandler $dbHandler,
 		LoggerInterface $logger,
-		ITimeFactory $timeFactory
+		ITimeFactory $timeFactory,
+		IThrottler $throttler
 	) {
 		parent::__construct($appName, $request);
 
@@ -72,6 +75,7 @@ class OCSAuthAPIController extends OCSController {
 		$this->dbHandler = $dbHandler;
 		$this->logger = $logger;
 		$this->timeFactory = $timeFactory;
+		$this->throttler = $throttler;
 	}
 
 	/**
@@ -79,6 +83,7 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * @NoCSRFRequired
 	 * @PublicPage
+	 * @BruteForceProtection(action=federationSharedSecret)
 	 * @throws OCSForbiddenException
 	 */
 	public function requestSharedSecretLegacy(string $url, string $token): DataResponse {
@@ -91,6 +96,7 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * @NoCSRFRequired
 	 * @PublicPage
+	 * @BruteForceProtection(action=federationSharedSecret)
 	 * @throws OCSForbiddenException
 	 */
 	public function getSharedSecretLegacy(string $url, string $token): DataResponse {
@@ -102,10 +108,12 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * @NoCSRFRequired
 	 * @PublicPage
+	 * @BruteForceProtection(action=federationSharedSecret)
 	 * @throws OCSForbiddenException
 	 */
 	public function requestSharedSecret(string $url, string $token): DataResponse {
 		if ($this->trustedServers->isTrustedServer($url) === false) {
+			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
 			$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']);
 			throw new OCSForbiddenException();
 		}
@@ -138,15 +146,18 @@ class OCSAuthAPIController extends OCSController {
 	 *
 	 * @NoCSRFRequired
 	 * @PublicPage
+	 * @BruteForceProtection(action=federationSharedSecret)
 	 * @throws OCSForbiddenException
 	 */
 	public function getSharedSecret(string $url, string $token): DataResponse {
 		if ($this->trustedServers->isTrustedServer($url) === false) {
+			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
 			$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']);
 			throw new OCSForbiddenException();
 		}
 
 		if ($this->isValidToken($url, $token) === false) {
+			$this->throttler->registerAttempt('federationSharedSecret', $this->request->getRemoteAddress());
 			$expectedToken = $this->dbHandler->getToken($url);
 			$this->logger->error(
 				'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret',
diff --git a/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
index 02e82880f9b..8c9b9b62566 100644
--- a/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
+++ b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
@@ -33,6 +33,7 @@ use OCA\Federation\TrustedServers;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IRequest;
+use OCP\Security\Bruteforce\IThrottler;
 use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -60,6 +61,9 @@ class OCSAuthAPIControllerTest extends TestCase {
 	/** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
 	private $timeFactory;
 
+	/** @var \PHPUnit\Framework\MockObject\MockObject|IThrottler */
+	private $throttler;
+
 	private OCSAuthAPIController $ocsAuthApi;
 
 	/** @var int simulated timestamp */
@@ -75,6 +79,7 @@ class OCSAuthAPIControllerTest extends TestCase {
 		$this->jobList = $this->createMock(JobList::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->throttler = $this->createMock(IThrottler::class);
 
 		$this->ocsAuthApi = new OCSAuthAPIController(
 			'federation',
@@ -84,7 +89,8 @@ class OCSAuthAPIControllerTest extends TestCase {
 			$this->trustedServers,
 			$this->dbHandler,
 			$this->logger,
-			$this->timeFactory
+			$this->timeFactory,
+			$this->throttler
 		);
 
 		$this->timeFactory->method('getTime')
@@ -109,8 +115,14 @@ class OCSAuthAPIControllerTest extends TestCase {
 		} else {
 			$this->jobList->expects($this->never())->method('add');
 			$this->jobList->expects($this->never())->method('remove');
+			if (!$isTrustedServer) {
+				$this->throttler->expects($this->once())
+					->method('registerAttempt')
+					->with('federationSharedSecret');
+			}
 		}
 
+
 		try {
 			$this->ocsAuthApi->requestSharedSecret($url, $token);
 			$this->assertTrue($ok);
@@ -145,7 +157,8 @@ class OCSAuthAPIControllerTest extends TestCase {
 					$this->trustedServers,
 					$this->dbHandler,
 					$this->logger,
-					$this->timeFactory
+					$this->timeFactory,
+					$this->throttler
 				]
 			)->setMethods(['isValidToken'])->getMock();
 
@@ -163,6 +176,9 @@ class OCSAuthAPIControllerTest extends TestCase {
 		} else {
 			$this->secureRandom->expects($this->never())->method('generate');
 			$this->trustedServers->expects($this->never())->method('addSharedSecret');
+			$this->throttler->expects($this->once())
+				->method('registerAttempt')
+				->with('federationSharedSecret');
 		}
 
 		try {