fix: Add bruteforce protection to federation endpoint

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

diff --git a/apps/federation/lib/Controller/OCSAuthAPIController.php b/apps/federation/lib/Controller/OCSAuthAPIController.php
index 82c3e20da31641edd25572eec2b58af22c608499..63a5fbb31559d32a33b5bdb6152282047aac3976 100644 (file)
--- a/apps/federation/lib/Controller/OCSAuthAPIController.php
+++ b/apps/federation/lib/Controller/OCSAuthAPIController.php
@@ -38,6 +38,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;
 
@@ -56,6 +57,7 @@ class OCSAuthAPIController extends OCSController {
        private DbHandler $dbHandler;
        private LoggerInterface $logger;
        private ITimeFactory $timeFactory;
+       private IThrottler $throttler;
 
        public function __construct(
                string $appName,
@@ -65,7 +67,8 @@ class OCSAuthAPIController extends OCSController {
                TrustedServers $trustedServers,
                DbHandler $dbHandler,
                LoggerInterface $logger,
-               ITimeFactory $timeFactory
+               ITimeFactory $timeFactory,
+               IThrottler $throttler
        ) {
                parent::__construct($appName, $request);
 
@@ -75,6 +78,7 @@ class OCSAuthAPIController extends OCSController {
                $this->dbHandler = $dbHandler;
                $this->logger = $logger;
                $this->timeFactory = $timeFactory;
+               $this->throttler = $throttler;
        }
 
        /**
@@ -82,6 +86,7 @@ class OCSAuthAPIController extends OCSController {
         *
         * @NoCSRFRequired
         * @PublicPage
+        * @BruteForceProtection(action=federationSharedSecret)
         *
         * @param string $url URL of the server
         * @param string $token Token of the server
@@ -100,6 +105,7 @@ class OCSAuthAPIController extends OCSController {
         *
         * @NoCSRFRequired
         * @PublicPage
+        * @BruteForceProtection(action=federationSharedSecret)
         *
         * @param string $url URL of the server
         * @param string $token Token of the server
@@ -117,6 +123,7 @@ class OCSAuthAPIController extends OCSController {
         *
         * @NoCSRFRequired
         * @PublicPage
+        * @BruteForceProtection(action=federationSharedSecret)
         *
         * @param string $url URL of the server
         * @param string $token Token of the server
@@ -127,6 +134,7 @@ class OCSAuthAPIController extends OCSController {
         */
        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();
                }
@@ -159,6 +167,7 @@ class OCSAuthAPIController extends OCSController {
         *
         * @NoCSRFRequired
         * @PublicPage
+        * @BruteForceProtection(action=federationSharedSecret)
         *
         * @param string $url URL of the server
         * @param string $token Token of the server
@@ -169,11 +178,13 @@ class OCSAuthAPIController extends OCSController {
         */
        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 74d09c6d3630bb4e9a89bce5103f852dc7b5a62a..171252460883bd7f2c8a27e16420299b90d635ce 100644 (file)
--- 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;
@@ -59,6 +60,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 */
@@ -74,6 +78,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',
@@ -83,7 +88,8 @@ class OCSAuthAPIControllerTest extends TestCase {
                        $this->trustedServers,
                        $this->dbHandler,
                        $this->logger,
-                       $this->timeFactory
+                       $this->timeFactory,
+                       $this->throttler
                );
 
                $this->timeFactory->method('getTime')
@@ -108,8 +114,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);
@@ -144,7 +156,8 @@ class OCSAuthAPIControllerTest extends TestCase {
                                        $this->trustedServers,
                                        $this->dbHandler,
                                        $this->logger,
-                                       $this->timeFactory
+                                       $this->timeFactory,
+                                       $this->throttler
                                ]
                        )->setMethods(['isValidToken'])->getMock();
 
@@ -162,6 +175,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 {