Skip to content

Use public interface of throttler #7506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nickvergessen wants to merge 6 commits into from
Closed

Use public interface of throttler #7506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
99846f5
Add brute-force protection to conversation passwords
nickvergessen Jun 13, 2022
3a9ed27
Also throttle signaling secret guessing
nickvergessen Jun 13, 2022
a05e5f6
Show warning about the throttling
nickvergessen Jun 13, 2022
15f8e48
Reset the password attempts on succcessful answer
nickvergessen Jun 30, 2022
6a17079
Fix psalm
nickvergessen Jul 1, 2022
92ded31
Use public IThrottler interface
nickvergessen Jul 1, 2022
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
38 changes: 32 additions & 6 deletions lib/Controller/PageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
use OCP\IUser;
use OCP\IUserSession;
use OCP\Notification\IManager as INotificationManager;
use OCP\Security\Bruteforce\IThrottler;
use Psr\Log\LoggerInterface;

class PageController extends Controller {
Expand All @@ -73,6 +74,7 @@ class PageController extends Controller {
private INotificationManager $notificationManager;
private IAppManager $appManager;
private IRootFolder $rootFolder;
private IThrottler $throttler;

public function __construct(string $appName,
IRequest $request,
Expand All @@ -90,6 +92,7 @@ public function __construct(string $appName,
IInitialState $initialState,
ICacheFactory $memcacheFactory,
IRootFolder $rootFolder,
IThrottler $throttler,
Config $talkConfig,
IConfig $serverConfig) {
parent::__construct($appName, $request);
Expand All @@ -107,6 +110,7 @@ public function __construct(string $appName,
$this->initialState = $initialState;
$this->memcacheFactory = $memcacheFactory;
$this->rootFolder = $rootFolder;
$this->throttler = $throttler;
$this->talkConfig = $talkConfig;
$this->serverConfig = $serverConfig;
}
Expand All @@ -129,6 +133,7 @@ public function showCall(string $token): Response {
* @PublicPage
* @NoCSRFRequired
* @UseSession
* @BruteForceProtection(action=talkRoomPassword)
*
* @param string $token
* @param string $password
Expand Down Expand Up @@ -177,6 +182,7 @@ public function index(string $token = '', string $callUser = '', string $passwor
return $this->guestEnterRoom($token, $password);
}

$throttle = false;
if ($token !== '') {
$room = null;
try {
Expand Down Expand Up @@ -205,6 +211,7 @@ public function index(string $token = '', string $callUser = '', string $passwor
} catch (RoomNotFoundException $e) {
// Room not found, redirect to main page
$token = '';
$throttle = true;
}

if ($room instanceof Room && $room->hasPassword()) {
Expand All @@ -224,15 +231,22 @@ public function index(string $token = '', string $callUser = '', string $passwor
if ($passwordVerification['result']) {
$this->talkSession->renewSessionId();
$this->talkSession->setPasswordForRoom($token, $password);
$this->throttler->resetDelay($this->request->getRemoteAddress(), 'talkRoomPassword', ['token' => $token]);
} else {
$this->talkSession->removePasswordForRoom($token);
$showBruteForceWarning = $this->throttler->getDelay($this->request->getRemoteAddress(), 'talkRoomPassword') > 5000;

if ($passwordVerification['url'] === '') {
return new TemplateResponse($this->appName, 'authenticate', [
$response = new TemplateResponse($this->appName, 'authenticate', [
'wrongpw' => $password !== '',
'showBruteForceWarning' => $showBruteForceWarning,
], 'guest');
} else {
$response = new RedirectResponse($passwordVerification['url']);
}

return new RedirectResponse($passwordVerification['url']);
$response->throttle(['token' => $token]);
return $response;
}
}
}
Expand Down Expand Up @@ -268,6 +282,10 @@ public function index(string $token = '', string $callUser = '', string $passwor
$csp->addAllowedConnectDomain("'self'");
$csp->addAllowedImageDomain('https://*.tile.openstreetmap.org');
$response->setContentSecurityPolicy($csp);
if ($throttle) {
// Logged-in user tried to access a chat they can not access
$response->throttle();
}
return $response;
}

Expand All @@ -288,9 +306,11 @@ protected function guestEnterRoom(string $token, string $password): Response {
if ($token) {
$redirectUrl = $this->url->linkToRoute('spreed.Page.showCall', ['token' => $token]);
}
return new RedirectResponse($this->url->linkToRoute('core.login.showLoginForm', [
$response = new RedirectResponse($this->url->linkToRoute('core.login.showLoginForm', [
'redirect_url' => $redirectUrl,
]));
$response->throttle();
return $response;
}

if ($room->hasPassword()) {
Expand All @@ -300,15 +320,21 @@ protected function guestEnterRoom(string $token, string $password): Response {
if ($passwordVerification['result']) {
$this->talkSession->renewSessionId();
$this->talkSession->setPasswordForRoom($token, $password);
$this->throttler->resetDelay($this->request->getRemoteAddress(), 'talkRoomPassword', ['token' => $token]);
} else {
$this->talkSession->removePasswordForRoom($token);
$showBruteForceWarning = $this->throttler->getDelay($this->request->getRemoteAddress(), 'talkRoomPassword') > 5000;

if ($passwordVerification['url'] === '') {
return new TemplateResponse($this->appName, 'authenticate', [
$response = new TemplateResponse($this->appName, 'authenticate', [
'wrongpw' => $password !== '',
'showBruteForceWarning' => $showBruteForceWarning,
], 'guest');
} else {
$response = new RedirectResponse($passwordVerification['url']);
}

return new RedirectResponse($passwordVerification['url']);
$response->throttle(['token' => $token]);
return $response;
}
}

Expand Down
23 changes: 18 additions & 5 deletions lib/Controller/RoomController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ public function getListedRooms(string $searchTerm = ''): DataResponse {

/**
* @PublicPage
* @BruteForceProtection(action=sipBridgeSecret)
*
* @param string $token
* @return DataResponse
Expand All @@ -268,7 +269,9 @@ public function getSingleRoom(string $token): DataResponse {
try {
$isSIPBridgeRequest = $this->validateSIPBridgeRequest($token);
} catch (UnauthorizedException $e) {
return new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response = new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response->throttle();
return $response;
}

// The SIP bridge only needs room details (public, sip enabled, lobby state, etc)
Expand Down Expand Up @@ -1330,6 +1333,7 @@ public function setPassword(string $password): DataResponse {
/**
* @PublicPage
* @UseSession
* @BruteForceProtection(action=talkRoomPassword)
*
* @param string $token
* @param string $password
Expand Down Expand Up @@ -1389,9 +1393,13 @@ public function joinRoom(string $token, string $password = '', bool $force = tru
$participant = $this->participantService->joinRoomAsNewGuest($this->roomService, $room, $password, $result['result'], $previousParticipant);
}
} catch (InvalidPasswordException $e) {
return new DataResponse([], Http::STATUS_FORBIDDEN);
$response = new DataResponse([], Http::STATUS_FORBIDDEN);
$response->throttle(['token' => $token]);
return $response;
} catch (UnauthorizedException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
$response = new DataResponse([], Http::STATUS_NOT_FOUND);
$response->throttle(['token' => $token]);
return $response;
}

$this->session->removePasswordForRoom($token);
Expand All @@ -1407,17 +1415,22 @@ public function joinRoom(string $token, string $password = '', bool $force = tru
/**
* @PublicPage
* @RequireRoom
* @BruteForceProtection(action=sipBridgeSecret)
*
* @param string $pin
* @return DataResponse
*/
public function getParticipantByDialInPin(string $pin): DataResponse {
try {
if (!$this->validateSIPBridgeRequest($this->room->getToken())) {
return new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response = new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response->throttle();
return $response;
}
} catch (UnauthorizedException $e) {
return new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response = new DataResponse([], Http::STATUS_UNAUTHORIZED);
$response->throttle();
return $response;
}

try {
Expand Down
5 changes: 4 additions & 1 deletion lib/Controller/SignalingController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,19 +461,22 @@ protected function getInputStream(): string {
* https://nextcloud-spreed-signaling.readthedocs.io/en/latest/standalone-signaling-api-v1/#backend-requests
*
* @PublicPage
* @BruteForceProtection(action=signalingSecret)
*
* @return DataResponse
*/
public function backend(): DataResponse {
$json = $this->getInputStream();
if (!$this->validateBackendRequest($json)) {
return new DataResponse([
$response = new DataResponse([
'type' => 'error',
'error' => [
'code' => 'invalid_request',
'message' => 'The request could not be authenticated.',
],
]);
$response->throttle();
return $response;
}

$message = json_decode($json, true);
Expand Down
9 changes: 8 additions & 1 deletion templates/authenticate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@
<form method="post">
<fieldset class="warning">
<?php if (!$_['wrongpw']) { ?>
<div class="warning-info"><?php p($l->t('This conversation is password-protected')); ?></div>
<div class="warning-info">
<?php p($l->t('This conversation is password-protected.')); ?>
<?php if ($_['showBruteForceWarning']) { ?>
<?php p($l->t('We have detected multiple invalid password attempts from your IP. Therefore your next attempt is throttled up to 30 seconds.')); ?>
<?php } ?>
</div>
<?php } elseif ($_['showBruteForceWarning']) { ?>
<div class="warning-info"><?php p($l->t('We have detected multiple invalid password attempts from your IP. Therefore your next attempt is throttled up to 30 seconds.')); ?></div>
<?php } else { ?>
<div class="warning"><?php p($l->t('The password is wrong. Try again.')); ?></div>
<?php } ?>
Expand Down