diff --git a/CHANGELOG.md b/CHANGELOG.md index 4573d901f..a19c09acc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Security header Referrer-Policy ([#2519]) - Docs: HTTP Strict Transport Security (HSTS) recommendations ([#2519]) - Virus scan results to metrics ([#2304]) +- Rate limiting to prevent Room-ID enumeration attacks ([#2518]) ### Changed @@ -572,6 +573,7 @@ You can find the changelog for older versions there [here](https://github.com/TH [#2480]: https://github.com/THM-Health/PILOS/pull/2480 [#2496]: https://github.com/THM-Health/PILOS/issues/2496 [#2497]: https://github.com/THM-Health/PILOS/pull/2497 +[#2518]: https://github.com/THM-Health/PILOS/pull/2518 [#2519]: https://github.com/THM-Health/PILOS/pull/2519 [unreleased]: https://github.com/THM-Health/PILOS/compare/v4.7.1...develop [v3.0.0]: https://github.com/THM-Health/PILOS/releases/tag/v3.0.0 diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 21dc575db..8a63ef5f3 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -2,12 +2,14 @@ namespace App\Providers; +use App\Models\Room; use App\Models\User; use Illuminate\Cache\RateLimiting\Limit; use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider; use Illuminate\Http\Request; use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Route; +use Response; class RouteServiceProvider extends ServiceProvider { @@ -70,5 +72,20 @@ protected function configureRateLimiting(): void // If the user is not editing himself, no rate limit (use the default rate limit, see api rate limit) return Limit::none(); }); + + RateLimiter::for('room-enumeration', function (Request $request) { + return Limit::perMinute(10) + ->by($request->user()?->id ?: $request->ip()) + ->after(function (\Symfony\Component\HttpFoundation\Response $response) use ($request) { + // If the response is not a 404, do not count this request + if ($response->getStatusCode() !== 404) { + return false; + } + + // Only count the request if the route parameter 'room' was not resolved to a Room model + // Prevent counting requests that are valid and return a 404 for other reasons + return ! ($request->route('room') instanceof Room); + }); + }); } } diff --git a/routes/api.php b/routes/api.php index 9055225de..855d78445 100644 --- a/routes/api.php +++ b/routes/api.php @@ -73,63 +73,67 @@ Route::get('rooms', [RoomController::class, 'index'])->name('rooms.index'); Route::post('rooms', [RoomController::class, 'store'])->name('rooms.store'); - Route::post('rooms/{room}/favorites', [RoomController::class, 'addToFavorites'])->name('rooms.favorites.add'); - Route::delete('rooms/{room}/favorites', [RoomController::class, 'deleteFromFavorites'])->name('rooms.favorites.delete'); - Route::put('rooms/{room}', [RoomController::class, 'update'])->name('rooms.update'); - Route::delete('rooms/{room}', [RoomController::class, 'destroy'])->name('rooms.destroy'); - Route::get('rooms/{room}/settings', [RoomController::class, 'getSettings'])->name('rooms.settings'); + Route::middleware('throttle:room-enumeration')->group(function () { + Route::post('rooms/{room}/favorites', [RoomController::class, 'addToFavorites'])->name('rooms.favorites.add'); + Route::delete('rooms/{room}/favorites', [RoomController::class, 'deleteFromFavorites'])->name('rooms.favorites.delete'); + Route::put('rooms/{room}', [RoomController::class, 'update'])->name('rooms.update'); + Route::delete('rooms/{room}', [RoomController::class, 'destroy'])->name('rooms.destroy'); + + Route::get('rooms/{room}/settings', [RoomController::class, 'getSettings'])->name('rooms.settings'); + + Route::put('rooms/{room}/description', [RoomController::class, 'updateDescription'])->name('rooms.description.update')->middleware('can:update,room'); + + Route::post('rooms/{room}/transfer', [RoomController::class, 'transferOwnership'])->name('rooms.transfer')->middleware('can:transfer,room'); + + // Membership user self add/remove + Route::post('rooms/{room}/membership', [RoomMemberController::class, 'join'])->name('rooms.membership.join'); + Route::delete('rooms/{room}/membership', [RoomMemberController::class, 'leave'])->name('rooms.membership.leave'); + + // Membership users for mass update & delete + Route::post('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkImport'])->name('rooms.member.bulkImport')->middleware('can:manageMembers,room'); + Route::put('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkUpdate'])->name('rooms.member.bulkUpdate')->middleware('can:manageMembers,room'); + Route::delete('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkDestroy'])->name('rooms.member.bulkDestroy')->middleware('can:manageMembers,room'); + + // Membership operations by room owner + Route::get('rooms/{room}/member', [RoomMemberController::class, 'index'])->name('rooms.member.get')->middleware('can:viewMembers,room'); + Route::post('rooms/{room}/member', [RoomMemberController::class, 'store'])->name('rooms.member.add')->middleware('can:manageMembers,room'); + Route::put('rooms/{room}/member/{user}', [RoomMemberController::class, 'update'])->name('rooms.member.update')->middleware('can:manageMembers,room'); + Route::delete('rooms/{room}/member/{user}', [RoomMemberController::class, 'destroy'])->name('rooms.member.destroy')->middleware('can:manageMembers,room'); + + // Recording operations + Route::middleware('can:manageRecordings,room')->scopeBindings()->group(function () { + Route::put('rooms/{room}/recordings/{recording}', [RecordingController::class, 'update'])->name('rooms.recordings.update'); + Route::delete('rooms/{room}/recordings/{recording}', [RecordingController::class, 'destroy'])->name('rooms.recordings.destroy'); + }); + + // Streaming operations + Route::middleware('can:viewStreaming,room')->scopeBindings()->group(function () { + Route::get('rooms/{room}/streaming/config', [RoomStreamingController::class, 'getConfig'])->name('rooms.streaming.config.get'); + Route::get('rooms/{room}/streaming/status', [RoomStreamingController::class, 'status'])->name('rooms.streaming.status'); + }); + + Route::middleware('can:manageStreaming,room')->scopeBindings()->group(function () { + Route::put('rooms/{room}/streaming/config', [RoomStreamingController::class, 'updateConfig'])->name('rooms.streaming.config.update'); + Route::post('rooms/{room}/streaming/start', [RoomStreamingController::class, 'start'])->name('rooms.streaming.start'); + Route::post('rooms/{room}/streaming/stop', [RoomStreamingController::class, 'stop'])->name('rooms.streaming.stop'); + Route::post('rooms/{room}/streaming/pause', [RoomStreamingController::class, 'pause'])->name('rooms.streaming.pause'); + Route::post('rooms/{room}/streaming/resume', [RoomStreamingController::class, 'resume'])->name('rooms.streaming.resume'); + }); + + // Personalized room tokens + Route::get('rooms/{room}/tokens', [RoomTokenController::class, 'index'])->name('rooms.tokens.get')->middleware('can:viewTokens,room'); + Route::post('rooms/{room}/tokens', [RoomTokenController::class, 'store'])->name('rooms.tokens.add')->middleware('can:manageTokens,room'); + Route::put('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'update'])->name('rooms.tokens.update')->middleware('can:manageTokens,room'); + Route::delete('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'destroy'])->name('rooms.tokens.destroy')->middleware('can:manageTokens,room'); + + // File operations + Route::middleware('can:manageFiles,room')->scopeBindings()->group(function () { + Route::post('rooms/{room}/files', [RoomFileController::class, 'store'])->name('rooms.files.add'); + Route::put('rooms/{room}/files/{file}', [RoomFileController::class, 'update'])->name('rooms.files.update'); + Route::delete('rooms/{room}/files/{file}', [RoomFileController::class, 'destroy'])->name('rooms.files.destroy'); + }); - Route::put('rooms/{room}/description', [RoomController::class, 'updateDescription'])->name('rooms.description.update')->middleware('can:update,room'); - - Route::post('rooms/{room}/transfer', [RoomController::class, 'transferOwnership'])->name('rooms.transfer')->middleware('can:transfer,room'); - - // Membership user self add/remove - Route::post('rooms/{room}/membership', [RoomMemberController::class, 'join'])->name('rooms.membership.join'); - Route::delete('rooms/{room}/membership', [RoomMemberController::class, 'leave'])->name('rooms.membership.leave'); - - // Membership users for mass update & delete - Route::post('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkImport'])->name('rooms.member.bulkImport')->middleware('can:manageMembers,room'); - Route::put('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkUpdate'])->name('rooms.member.bulkUpdate')->middleware('can:manageMembers,room'); - Route::delete('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkDestroy'])->name('rooms.member.bulkDestroy')->middleware('can:manageMembers,room'); - - // Membership operations by room owner - Route::get('rooms/{room}/member', [RoomMemberController::class, 'index'])->name('rooms.member.get')->middleware('can:viewMembers,room'); - Route::post('rooms/{room}/member', [RoomMemberController::class, 'store'])->name('rooms.member.add')->middleware('can:manageMembers,room'); - Route::put('rooms/{room}/member/{user}', [RoomMemberController::class, 'update'])->name('rooms.member.update')->middleware('can:manageMembers,room'); - Route::delete('rooms/{room}/member/{user}', [RoomMemberController::class, 'destroy'])->name('rooms.member.destroy')->middleware('can:manageMembers,room'); - - // Recording operations - Route::middleware('can:manageRecordings,room')->scopeBindings()->group(function () { - Route::put('rooms/{room}/recordings/{recording}', [RecordingController::class, 'update'])->name('rooms.recordings.update'); - Route::delete('rooms/{room}/recordings/{recording}', [RecordingController::class, 'destroy'])->name('rooms.recordings.destroy'); - }); - - // Streaming operations - Route::middleware('can:viewStreaming,room')->scopeBindings()->group(function () { - Route::get('rooms/{room}/streaming/config', [RoomStreamingController::class, 'getConfig'])->name('rooms.streaming.config.get'); - Route::get('rooms/{room}/streaming/status', [RoomStreamingController::class, 'status'])->name('rooms.streaming.status'); - }); - - Route::middleware('can:manageStreaming,room')->scopeBindings()->group(function () { - Route::put('rooms/{room}/streaming/config', [RoomStreamingController::class, 'updateConfig'])->name('rooms.streaming.config.update'); - Route::post('rooms/{room}/streaming/start', [RoomStreamingController::class, 'start'])->name('rooms.streaming.start'); - Route::post('rooms/{room}/streaming/stop', [RoomStreamingController::class, 'stop'])->name('rooms.streaming.stop'); - Route::post('rooms/{room}/streaming/pause', [RoomStreamingController::class, 'pause'])->name('rooms.streaming.pause'); - Route::post('rooms/{room}/streaming/resume', [RoomStreamingController::class, 'resume'])->name('rooms.streaming.resume'); - }); - - // Personalized room tokens - Route::get('rooms/{room}/tokens', [RoomTokenController::class, 'index'])->name('rooms.tokens.get')->middleware('can:viewTokens,room'); - Route::post('rooms/{room}/tokens', [RoomTokenController::class, 'store'])->name('rooms.tokens.add')->middleware('can:manageTokens,room'); - Route::put('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'update'])->name('rooms.tokens.update')->middleware('can:manageTokens,room'); - Route::delete('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'destroy'])->name('rooms.tokens.destroy')->middleware('can:manageTokens,room'); - - // File operations - Route::middleware('can:manageFiles,room')->scopeBindings()->group(function () { - Route::post('rooms/{room}/files', [RoomFileController::class, 'store'])->name('rooms.files.add'); - Route::put('rooms/{room}/files/{file}', [RoomFileController::class, 'update'])->name('rooms.files.update'); - Route::delete('rooms/{room}/files/{file}', [RoomFileController::class, 'destroy'])->name('rooms.files.destroy'); }); Route::get('users/search', [UserController::class, 'search'])->name('users.search'); @@ -156,28 +160,31 @@ Route::get('meetings/{meeting}/attendance', [MeetingController::class, 'attendance'])->name('meetings.attendance'); Route::get('meetings/{meeting}/stats', [MeetingController::class, 'stats'])->name('meetings.stats'); Route::get('meetings', [MeetingController::class, 'index'])->name('meetings.index'); - Route::get('rooms/{room}/meetings', [RoomController::class, 'meetings'])->name('rooms.meetings'); + + Route::get('rooms/{room}/meetings', [RoomController::class, 'meetings'])->middleware('throttle:room-enumeration')->name('rooms.meetings'); Route::get('getTimezones', function () { return response()->json(['data' => timezone_identifiers_list()]); }); }); - Route::get('rooms/{room}', [RoomController::class, 'show'])->name('rooms.show')->middleware('room.authenticate:true'); + Route::middleware('throttle:room-enumeration')->group(function () { + Route::get('rooms/{room}', [RoomController::class, 'show'])->name('rooms.show')->middleware('room.authenticate:true'); - Route::middleware('room.authenticate')->scopeBindings()->group(function () { - Route::options('rooms/{room}/start', [RoomController::class, 'getStartRequirements'])->name('rooms.start-requirements')->middleware('can:start,room'); - Route::options('rooms/{room}/join', [RoomController::class, 'getJoinRequirements'])->name('rooms.join-requirements'); + Route::middleware('room.authenticate')->scopeBindings()->group(function () { + Route::options('rooms/{room}/start', [RoomController::class, 'getStartRequirements'])->name('rooms.start-requirements')->middleware('can:start,room'); + Route::options('rooms/{room}/join', [RoomController::class, 'getJoinRequirements'])->name('rooms.join-requirements'); - Route::post('rooms/{room}/start', [RoomController::class, 'start'])->name('rooms.start')->middleware('can:start,room'); - Route::post('rooms/{room}/join', [RoomController::class, 'join'])->name('rooms.join'); - Route::get('rooms/{room}/files', [RoomFileController::class, 'index'])->name('rooms.files.get'); - Route::get('rooms/{room}/files/{file}', [RoomFileController::class, 'show'])->name('rooms.files.show')->middleware('can:downloadFile,room,file'); - Route::get('rooms/{room}/recordings', [RecordingController::class, 'index'])->name('rooms.recordings.index'); - Route::get('rooms/{room}/recordings/{recording}/formats/{format}', [RecordingFormatController::class, 'show'])->name('rooms.recordings.formats.show')->middleware('can:viewRecordingFormat,room,format'); + Route::post('rooms/{room}/start', [RoomController::class, 'start'])->name('rooms.start')->middleware('can:start,room'); + Route::post('rooms/{room}/join', [RoomController::class, 'join'])->name('rooms.join'); + Route::get('rooms/{room}/files', [RoomFileController::class, 'index'])->name('rooms.files.get'); + Route::get('rooms/{room}/files/{file}', [RoomFileController::class, 'show'])->name('rooms.files.show')->middleware('can:downloadFile,room,file'); + Route::get('rooms/{room}/recordings', [RecordingController::class, 'index'])->name('rooms.recordings.index'); + Route::get('rooms/{room}/recordings/{recording}/formats/{format}', [RecordingFormatController::class, 'show'])->name('rooms.recordings.formats.show')->middleware('can:viewRecordingFormat,room,format'); - }); + }); + }); Route::get('meetings/{meeting}/endCallback', [MeetingController::class, 'endMeetingCallback'])->name('meetings.endcallback'); }); diff --git a/tests/Backend/Feature/api/v1/Room/RoomTest.php b/tests/Backend/Feature/api/v1/Room/RoomTest.php index 78d3dd8b9..c1e7f5703 100644 --- a/tests/Backend/Feature/api/v1/Room/RoomTest.php +++ b/tests/Backend/Feature/api/v1/Room/RoomTest.php @@ -416,6 +416,59 @@ public function test_transfer_room() ->assertJsonValidationErrors(['user']); } + /** + * Test room enumeration attack prevention using a rate limit + */ + public function test_room_404_rate_limit() + { + $room = Room::factory()->create(); + + // Guest trying to access non-existing rooms + for ($i = 0; $i < 10; $i++) { + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertNotFound(); + } + // Check route is now rate limited + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertStatus(429); + // Check other room routes are also rate limited + $this->getJson(route('api.v1.rooms.files.show', ['room' => 999999, 'file' => 999999])) + ->assertStatus(429); + + // User trying to access non-existing rooms + // The rate limit is per user or IP for unauthenticated users, so the user should have its own limit + $this->actingAs($room->owner); + for ($i = 0; $i < 10; $i++) { + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertNotFound(); + } + // Check route is now rate limited + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertStatus(429); + // Check other room routes are also rate limited + $this->getJson(route('api.v1.rooms.files.show', ['room' => 999999, 'file' => 999999])) + ->assertStatus(429); + + // Time travel 1 minute to reset rate limit + $this->travel(1)->minutes(); + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertNotFound(); + + $this->actingAsGuest(); + $this->getJson(route('api.v1.rooms.show', ['room' => 999999])) + ->assertNotFound(); + + // Time travel 1 minute to reset rate limit + $this->travel(1)->minutes(); + + // Check calling routes for an existing room that also result in a 404 + // due to other reasons than the room not existing are not so strictly rate limited + for ($i = 0; $i < 50; $i++) { + $this->actingAs($room->owner)->getJson(route('api.v1.rooms.files.show', ['room' => $room, 'file' => 999999])) + ->assertNotFound(); + } + } + /** * Test if guests can access room */