upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-09 00:32:29 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 35

fix(federation): Add bruteforce protection to federation endpoint

Browse source 
Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2024-02-02 16:38:10 +01:00
parent dd291e1a16
commit c932916a82
No known key found for this signature in database
GPG key ID: 74434EFE0D2E2205
2 changed files with 30 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
apps/federation/lib/Controller/OCSAuthAPIController.php
View file

@ -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',

20
apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
View file

@ -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 {