From 99846f58ce49d8580440b69132339de99f6651c5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jun 2022 11:30:21 +0200 Subject: [PATCH 1/6] Add brute-force protection to conversation passwords Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 26 ++++++++++++++++++++------ lib/Controller/RoomController.php | 23 ++++++++++++++++++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 0cada87b22c..e8d4beeb604 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -129,6 +129,7 @@ public function showCall(string $token): Response { * @PublicPage * @NoCSRFRequired * @UseSession + * @BruteForceProtection(action=talkRoomPassword) * * @param string $token * @param string $password @@ -177,6 +178,7 @@ public function index(string $token = '', string $callUser = '', string $passwor return $this->guestEnterRoom($token, $password); } + $throttle = false; if ($token !== '') { $room = null; try { @@ -205,6 +207,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()) { @@ -227,12 +230,15 @@ public function index(string $token = '', string $callUser = '', string $passwor } else { $this->talkSession->removePasswordForRoom($token); if ($passwordVerification['url'] === '') { - return new TemplateResponse($this->appName, 'authenticate', [ + $response = new TemplateResponse($this->appName, 'authenticate', [ 'wrongpw' => $password !== '', ], 'guest'); + } else { + $response = new RedirectResponse($passwordVerification['url']); } - return new RedirectResponse($passwordVerification['url']); + $response->throttle(); + return $response; } } } @@ -268,6 +274,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; } @@ -288,9 +298,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()) { @@ -303,12 +315,14 @@ protected function guestEnterRoom(string $token, string $password): Response { } else { $this->talkSession->removePasswordForRoom($token); if ($passwordVerification['url'] === '') { - return new TemplateResponse($this->appName, 'authenticate', [ + $response = new TemplateResponse($this->appName, 'authenticate', [ 'wrongpw' => $password !== '', ], 'guest'); + } else { + $response = new RedirectResponse($passwordVerification['url']); } - - return new RedirectResponse($passwordVerification['url']); + $response->throttle(); + return $response; } } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 40c457de339..3f33f066633 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -260,6 +260,7 @@ public function getListedRooms(string $searchTerm = ''): DataResponse { /** * @PublicPage + * @BruteForceProtection(action=sipBridgeSecret) * * @param string $token * @return DataResponse @@ -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) @@ -1330,6 +1333,7 @@ public function setPassword(string $password): DataResponse { /** * @PublicPage * @UseSession + * @BruteForceProtection(action=talkRoomPassword) * * @param string $token * @param string $password @@ -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(); + return $response; } catch (UnauthorizedException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + $response = new DataResponse([], Http::STATUS_NOT_FOUND); + $response->throttle(); + return $response; } $this->session->removePasswordForRoom($token); @@ -1407,6 +1415,7 @@ public function joinRoom(string $token, string $password = '', bool $force = tru /** * @PublicPage * @RequireRoom + * @BruteForceProtection(action=sipBridgeSecret) * * @param string $pin * @return DataResponse @@ -1414,10 +1423,14 @@ public function joinRoom(string $token, string $password = '', bool $force = tru 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 { From 3a9ed27c1df3ce7c3d0e3a7770f332f0d20a4b43 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jun 2022 12:21:07 +0200 Subject: [PATCH 2/6] Also throttle signaling secret guessing Signed-off-by: Joas Schilling --- lib/Controller/SignalingController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Controller/SignalingController.php b/lib/Controller/SignalingController.php index 4f9bc62ba5c..49bce4bed69 100644 --- a/lib/Controller/SignalingController.php +++ b/lib/Controller/SignalingController.php @@ -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); From a05e5f64ec9c14b7c17d3b9683c884b3a8ccdcea Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jun 2022 12:22:18 +0200 Subject: [PATCH 3/6] Show warning about the throttling Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 10 ++++++++++ templates/authenticate.php | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index e8d4beeb604..4334211bf4b 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -25,6 +25,7 @@ namespace OCA\Talk\Controller; +use OC\Security\Bruteforce\Throttler; use OCA\Talk\AppInfo\Application; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; @@ -73,6 +74,7 @@ class PageController extends Controller { private INotificationManager $notificationManager; private IAppManager $appManager; private IRootFolder $rootFolder; + private Throttler $throttler; public function __construct(string $appName, IRequest $request, @@ -90,6 +92,7 @@ public function __construct(string $appName, IInitialState $initialState, ICacheFactory $memcacheFactory, IRootFolder $rootFolder, + Throttler $throttler, Config $talkConfig, IConfig $serverConfig) { parent::__construct($appName, $request); @@ -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; } @@ -229,9 +233,12 @@ public function index(string $token = '', string $callUser = '', string $passwor $this->talkSession->setPasswordForRoom($token, $password); } else { $this->talkSession->removePasswordForRoom($token); + $showBruteForceWarning = $this->throttler->getDelay($this->request->getRemoteAddress(), 'talkRoomPassword') > 5000; + if ($passwordVerification['url'] === '') { $response = new TemplateResponse($this->appName, 'authenticate', [ 'wrongpw' => $password !== '', + 'showBruteForceWarning' => $showBruteForceWarning, ], 'guest'); } else { $response = new RedirectResponse($passwordVerification['url']); @@ -314,9 +321,12 @@ protected function guestEnterRoom(string $token, string $password): Response { $this->talkSession->setPasswordForRoom($token, $password); } else { $this->talkSession->removePasswordForRoom($token); + $showBruteForceWarning = $this->throttler->getDelay($this->request->getRemoteAddress(), 'talkRoomPassword') > 5000; + if ($passwordVerification['url'] === '') { $response = new TemplateResponse($this->appName, 'authenticate', [ 'wrongpw' => $password !== '', + 'showBruteForceWarning' => $showBruteForceWarning, ], 'guest'); } else { $response = new RedirectResponse($passwordVerification['url']); diff --git a/templates/authenticate.php b/templates/authenticate.php index f62042e1b9c..ba7d5e7a695 100644 --- a/templates/authenticate.php +++ b/templates/authenticate.php @@ -7,7 +7,14 @@
-
t('This conversation is password-protected')); ?>
+
+ t('This conversation is password-protected.')); ?> + + t('We have detected multiple invalid password attempts from your IP. Therefore your next attempt is throttled up to 30 seconds.')); ?> + +
+ +
t('We have detected multiple invalid password attempts from your IP. Therefore your next attempt is throttled up to 30 seconds.')); ?>
t('The password is wrong. Try again.')); ?>
From 15f8e481697ece47f15a9d9b52af1413ea222837 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 30 Jun 2022 17:01:04 +0200 Subject: [PATCH 4/6] Reset the password attempts on succcessful answer Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 6 ++++-- lib/Controller/RoomController.php | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 4334211bf4b..8ade2b01f0e 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -231,6 +231,7 @@ 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; @@ -244,7 +245,7 @@ public function index(string $token = '', string $callUser = '', string $passwor $response = new RedirectResponse($passwordVerification['url']); } - $response->throttle(); + $response->throttle(['token' => $token]); return $response; } } @@ -319,6 +320,7 @@ 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; @@ -331,7 +333,7 @@ protected function guestEnterRoom(string $token, string $password): Response { } else { $response = new RedirectResponse($passwordVerification['url']); } - $response->throttle(); + $response->throttle(['token' => $token]); return $response; } } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 3f33f066633..f8e3eb55a7f 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -1394,11 +1394,11 @@ public function joinRoom(string $token, string $password = '', bool $force = tru } } catch (InvalidPasswordException $e) { $response = new DataResponse([], Http::STATUS_FORBIDDEN); - $response->throttle(); + $response->throttle(['token' => $token]); return $response; } catch (UnauthorizedException $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(); + $response->throttle(['token' => $token]); return $response; } From 6a1707935c6694737a67c1409b4d149419d6c0c7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 1 Jul 2022 11:38:01 +0200 Subject: [PATCH 5/6] Fix psalm Signed-off-by: Joas Schilling --- psalm.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/psalm.xml b/psalm.xml index b82a052fbcd..a6c06de54bb 100644 --- a/psalm.xml +++ b/psalm.xml @@ -26,6 +26,7 @@ + From 92ded3126c93811b7fb0a66715b2b07a8ad8fff6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 1 Jul 2022 11:30:21 +0200 Subject: [PATCH 6/6] Use public IThrottler interface Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 6 +++--- psalm.xml | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 8ade2b01f0e..d9138ffef74 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -25,7 +25,6 @@ namespace OCA\Talk\Controller; -use OC\Security\Bruteforce\Throttler; use OCA\Talk\AppInfo\Application; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; @@ -57,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 { @@ -74,7 +74,7 @@ class PageController extends Controller { private INotificationManager $notificationManager; private IAppManager $appManager; private IRootFolder $rootFolder; - private Throttler $throttler; + private IThrottler $throttler; public function __construct(string $appName, IRequest $request, @@ -92,7 +92,7 @@ public function __construct(string $appName, IInitialState $initialState, ICacheFactory $memcacheFactory, IRootFolder $rootFolder, - Throttler $throttler, + IThrottler $throttler, Config $talkConfig, IConfig $serverConfig) { parent::__construct($appName, $request); diff --git a/psalm.xml b/psalm.xml index a6c06de54bb..b82a052fbcd 100644 --- a/psalm.xml +++ b/psalm.xml @@ -26,7 +26,6 @@ -