Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions apps/dav/lib/Connector/Sabre/PublicAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OCP\IRequest;
use OCP\ISession;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\Bruteforce\MaxDelayReached;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;
use OCP\Share\IShare;
Expand Down Expand Up @@ -56,6 +57,7 @@ public function __construct(
*
* @return array
* @throws NotAuthenticated
* @throws MaxDelayReached
* @throws ServiceUnavailable
*/
public function check(RequestInterface $request, ResponseInterface $response): array {
Expand All @@ -75,7 +77,8 @@ public function check(RequestInterface $request, ResponseInterface $response): a
}

return $this->checkToken();
} catch (NotAuthenticated $e) {
} catch (NotAuthenticated|MaxDelayReached $e) {
$this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress());
throw $e;
} catch (\Exception $e) {
$class = get_class($e);
Expand All @@ -94,7 +97,7 @@ private function getToken(): string {
$path = $this->request->getPathInfo() ?: '';
// ['', 'dav', 'files', 'token']
$splittedPath = explode('/', $path);

if (count($splittedPath) < 4 || $splittedPath[3] === '') {
throw new NotFound();
}
Expand Down Expand Up @@ -176,7 +179,7 @@ protected function validateUserPass($username, $password) {
}
return true;
}

if ($this->session->exists(PublicAuth::DAV_AUTHENTICATED)
&& $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/Direct/DirectHome.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function getChild($name): DirectFile {
} catch (DoesNotExistException $e) {
// Since the token space is so huge only throttle on non-existing token
$this->throttler->registerAttempt('directlink', $this->request->getRemoteAddress());
$this->throttler->sleepDelay($this->request->getRemoteAddress(), 'directlink');
$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), 'directlink');

throw new NotFound();
}
Expand Down
6 changes: 5 additions & 1 deletion apps/dav/lib/Files/BrowserErrorPagePlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IRequest;
use OCP\Security\Bruteforce\MaxDelayReached;
use OCP\Template\ITemplateManager;
use Sabre\DAV\Exception;
use Sabre\DAV\Server;
Expand Down Expand Up @@ -60,6 +61,9 @@ public function logException(\Throwable $ex): void {
if ($ex instanceof Exception) {
$httpCode = $ex->getHTTPCode();
$headers = $ex->getHTTPHeaders($this->server);
} elseif ($ex instanceof MaxDelayReached) {
$httpCode = 429;
$headers = [];
} else {
$httpCode = 500;
$headers = [];
Expand All @@ -81,7 +85,7 @@ public function generateBody(int $httpCode) {
$request = \OCP\Server::get(IRequest::class);

$templateName = 'exception';
if ($httpCode === 403 || $httpCode === 404) {
if ($httpCode === 403 || $httpCode === 404 || $httpCode === 429) {
$templateName = (string)$httpCode;
}

Expand Down
2 changes: 1 addition & 1 deletion apps/dav/tests/unit/Direct/DirectHomeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public function testGetChildInvalid(): void {
'1.2.3.4'
);
$this->throttler->expects($this->once())
->method('sleepDelay')
->method('sleepDelayOrThrowOnMax')
->with(
'1.2.3.4',
'directlink'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private function isLinkSharingEnabled(): bool {

private function throttle($bruteforceProtectionAction, $token): void {
$ip = $this->request->getRemoteAddress();
$this->throttler->sleepDelay($ip, $bruteforceProtectionAction);
$this->throttler->sleepDelayOrThrowOnMax($ip, $bruteforceProtectionAction);
$this->throttler->registerAttempt($bruteforceProtectionAction, $ip, ['token' => $token]);
}
}
29 changes: 20 additions & 9 deletions lib/private/Security/Bruteforce/Throttler.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ public function getAttempts(string $ip, string $action = '', float $maxAgeHours
*/
public function getDelay(string $ip, string $action = ''): int {
$attempts = $this->getAttempts($ip, $action);
return $this->calculateDelay($attempts);
}

/**
* {@inheritDoc}
*/
public function calculateDelay(int $attempts): int {
if ($attempts === 0) {
return 0;
}
Expand Down Expand Up @@ -199,25 +206,29 @@ public function sleepDelay(string $ip, string $action = ''): int {
* {@inheritDoc}
*/
public function sleepDelayOrThrowOnMax(string $ip, string $action = ''): int {
$delay = $this->getDelay($ip, $action);
if (($delay === self::MAX_DELAY_MS) && $this->getAttempts($ip, $action, 0.5) > $this->config->getSystemValueInt('auth.bruteforce.max-attempts', self::MAX_ATTEMPTS)) {
$this->logger->info('IP address blocked because it reached the maximum failed attempts in the last 30 minutes [action: {action}, ip: {ip}]', [
$attempts = $this->getAttempts($ip, $action, 0.5);
if ($attempts > $this->config->getSystemValueInt('auth.bruteforce.max-attempts', self::MAX_ATTEMPTS)) {
$this->logger->info('IP address blocked because it reached the maximum failed attempts in the last 30 minutes [action: {action}, attempts: {attempts}, ip: {ip}]', [
'action' => $action,
'ip' => $ip,
'attempts' => $attempts,
]);
// If the ip made too many attempts within the last 30 mins we don't execute anymore
throw new MaxDelayReached('Reached maximum delay');
}
if ($delay > 100) {
$this->logger->info('IP address throttled because it reached the attempts limit in the last 30 minutes [action: {action}, delay: {delay}, ip: {ip}]', [

$attempts = $this->getAttempts($ip, $action);
if ($attempts > 10) {
$this->logger->info('IP address throttled because it reached the attempts limit in the last 12 hours [action: {action}, attempts: {attempts}, ip: {ip}]', [
'action' => $action,
'ip' => $ip,
'delay' => $delay,
'attempts' => $attempts,
]);
}
if (!$this->config->getSystemValueBool('auth.bruteforce.protection.testing')) {
usleep($delay * 1000);
if ($attempts > 0) {
return $this->calculateDelay($attempts);
}
return $delay;

return 0;
}
}
Loading