From 269acc2dda5a75f872368fdacd24d3256617edad Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Sat, 27 Jun 2026 16:50:44 -0500 Subject: [PATCH] refactor(timeshift): Replace random session ID generation with secure secrets for timeshift sessions. Introduce helper functions for session management and enhance session validation to prevent foreign session reuse. Update tests to cover new session handling logic and ensure proper user ownership checks. --- apps/timeshift/tests/test_views.py | 68 ++++++++++++++++++++++++------ apps/timeshift/views.py | 56 ++++++++++++++++++------ 2 files changed, 99 insertions(+), 25 deletions(-) diff --git a/apps/timeshift/tests/test_views.py b/apps/timeshift/tests/test_views.py index 270dfbe5..1e45f53b 100644 --- a/apps/timeshift/tests/test_views.py +++ b/apps/timeshift/tests/test_views.py @@ -550,7 +550,7 @@ class TimeshiftProxyTimestampWiringTests(TestCase): def _call(self, timestamp, provider_tz="Europe/Brussels"): request = self.factory.get(f"/timeshift/u/p/8/{timestamp}/8.ts?session_id={TEST_SESSION_ID}") sentinel = MagicMock(status_code=200) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -596,7 +596,7 @@ class TimeshiftProxyTimestampWiringTests(TestCase): def test_invalid_timestamp_rejected_before_upstream(self): request = self.factory.get("/timeshift/u/p/8/garbage/8.ts") - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -611,7 +611,7 @@ class TimeshiftProxyTimestampWiringTests(TestCase): def test_network_access_denied_returns_403(self): # Same network gate as other XC API endpoints (player_api, xmltv, etc.). request = self.factory.get(_proxy_url()) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=False) as gate, \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_stream_from_provider") as stream_mock: @@ -634,7 +634,7 @@ class TimeshiftProxyFailoverTests(TestCase): def _call(self, streams, provider_responses): request = self.factory.get(_proxy_url()) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -758,7 +758,7 @@ class _ProxyLoopTestMixin: if build_side_effect is not None else {"return_value": ["http://example.test/x.ts"]} ) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -1359,7 +1359,7 @@ class TimeshiftTakeoverTests(TestCase): # the user's own seek gets denied. call_order = [] request = RequestFactory().get(_proxy_url()) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -1413,7 +1413,9 @@ class TimeshiftSessionReuseTests(TestCase): return_value=profile), \ patch.object(views, "reserve_profile_slot", return_value=(True, 1, None)) as reserve_mock: - acquired = views._acquire_idle_pool_session(self.redis, self.SESSION) + acquired = views._acquire_idle_pool_session( + self.redis, self.SESSION, user_id=5, + ) self.assertIsNotNone(acquired) descriptor, got_profile = acquired self.assertEqual(descriptor["stream_id"], "111") @@ -1425,11 +1427,53 @@ class TimeshiftSessionReuseTests(TestCase): _seed_pool_session(self.redis, session_id=self.SESSION, busy="1") with patch.object(views.M3UAccountProfile.objects, "get") as prof_mock, \ patch.object(views, "reserve_profile_slot") as reserve_mock: - acquired = views._acquire_idle_pool_session(self.redis, self.SESSION) + acquired = views._acquire_idle_pool_session( + self.redis, self.SESSION, user_id=5, + ) self.assertIsNone(acquired) prof_mock.assert_not_called() reserve_mock.assert_not_called() + def test_acquire_rejects_foreign_user(self): + self._make_idle_entry() + profile = MagicMock(id=31) + with patch.object(views.M3UAccountProfile.objects, "get", + return_value=profile), \ + patch.object(views, "reserve_profile_slot", + return_value=(True, 1, None)) as reserve_mock: + acquired = views._acquire_idle_pool_session( + self.redis, self.SESSION, user_id=99, + ) + self.assertIsNone(acquired) + reserve_mock.assert_not_called() + + def test_foreign_session_id_redirects_instead_of_reusing_pool(self): + victim_session = "timeshift_victim_session" + _seed_pool_session(self.redis, session_id=victim_session, user_id=99) + request = self.factory.get(_proxy_url(victim_session)) + attacker = MagicMock(id=5) + with patch.object(views, "_authenticate_user", return_value=attacker), \ + patch.object(views, "network_access_allowed", return_value=True), \ + patch.object(views, "Channel") as channel_cls, \ + patch.object(views, "_user_can_access_channel", return_value=True), \ + patch.object(views, "get_channel_catchup_streams", + return_value=[_make_catchup_stream()]), \ + patch.object(views, "get_programme_duration", return_value=40), \ + patch.object(views, "parse_catchup_timestamp", return_value=True), \ + patch.object(views, "RedisClient") as redis_cls, \ + patch.object(views, "_acquire_idle_pool_session") as acquire_mock, \ + patch.object(views, "_attempt_timeshift_stream") as attempt_mock: + redis_cls.get_client.return_value = self.redis + channel_cls.objects.get.return_value = MagicMock(id=8) + response = views.timeshift_proxy( + request, "u", "p", "8", "2026-06-08:17-00", "8.ts", + ) + self.assertEqual(response.status_code, 301) + self.assertIn("session_id=timeshift_", response["Location"]) + self.assertNotIn(victim_session, response["Location"]) + acquire_mock.assert_not_called() + attempt_mock.assert_not_called() + def test_find_matching_idle_session_requires_ip_and_user_agent(self): _seed_pool_session( self.redis, session_id="timeshift_other", @@ -1729,7 +1773,7 @@ class TimeshiftScrubPreemptTests(TestCase): HTTP_RANGE="bytes=1000-", ) streams = [_make_catchup_stream(account_id=1, stream_id="111", profile_id=31)] - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -1768,7 +1812,7 @@ class TimeshiftScrubPreemptTests(TestCase): HTTP_RANGE="bytes=0-", ) streams = [_make_catchup_stream(account_id=1, stream_id="111", profile_id=31)] - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -1800,7 +1844,7 @@ class TimeshiftScrubPreemptTests(TestCase): HTTP_RANGE="bytes=2527702896-", ) streams = [_make_catchup_stream(account_id=1, stream_id="111", profile_id=31)] - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ @@ -1866,7 +1910,7 @@ class TimeshiftScrubPreemptTests(TestCase): streams = [_make_catchup_stream(account_id=1, stream_id="111", profile_id=31)] profile = MagicMock(id=31) ok = MagicMock(status_code=206) - with patch.object(views, "_authenticate_user", return_value=MagicMock()), \ + with patch.object(views, "_authenticate_user", return_value=MagicMock(id=5)), \ patch.object(views, "network_access_allowed", return_value=True), \ patch.object(views, "Channel") as channel_cls, \ patch.object(views, "_user_can_access_channel", return_value=True), \ diff --git a/apps/timeshift/views.py b/apps/timeshift/views.py index 113cd460..0ba3a5a7 100644 --- a/apps/timeshift/views.py +++ b/apps/timeshift/views.py @@ -3,7 +3,7 @@ import hmac import itertools import logging -import random +import secrets import time from urllib.parse import urlencode @@ -104,19 +104,20 @@ def timeshift_proxy(request, username, password, stream_id, timestamp, duration) session_id = request.GET.get("session_id") if not session_id: - session_id = f"timeshift_{int(time.time() * 1000)}_{random.randint(1000, 9999)}" - query_params = {k: request.GET.getlist(k) for k in request.GET} - query_params["session_id"] = [session_id] - redirect_url = f"{request.path}?{urlencode(query_params, doseq=True)}" - logger.debug("Timeshift session redirect: %s -> %s", request.path, session_id) - return HttpResponse(status=301, headers={"Location": redirect_url}) + logger.debug("Timeshift session redirect: %s (new session)", request.path) + return _redirect_with_new_session(request) + + session_entry = _get_pool_entry(redis_client, session_id) + if session_entry and not _pool_entry_owned_by_user(session_entry, user.id): + logger.info( + "Timeshift: rejecting foreign session_id for user %s", user.id, + ) + return _redirect_with_new_session(request) # Stable client identity for stats, stop keys, and the provider pool. effective_session_id = session_id client_id = session_id - session_entry = _get_pool_entry(redis_client, session_id) - # Reuse an idle pool owned by this session, or fingerprint-match a prior # idle session from the same client (VOD-style) before opening upstream. if not session_entry: @@ -178,11 +179,12 @@ def timeshift_proxy(request, username, password, stream_id, timestamp, duration) acquired = _wait_for_idle_pool_session( redis_client, effective_session_id, + user_id=user.id, wait_seconds=_POOL_PREEMPT_WAIT_SECONDS, ) else: acquired = _acquire_idle_pool_session( - redis_client, effective_session_id, + redis_client, effective_session_id, user_id=user.id, ) if acquired is not None: @@ -466,6 +468,28 @@ def _score_pool_fingerprint(entry, client_ip, client_user_agent): return score +def _mint_timeshift_session_id(): + return f"timeshift_{secrets.token_urlsafe(16)}" + + +def _redirect_with_new_session(request): + session_id = _mint_timeshift_session_id() + query_params = {k: request.GET.getlist(k) for k in request.GET} + query_params["session_id"] = [session_id] + redirect_url = f"{request.path}?{urlencode(query_params, doseq=True)}" + return HttpResponse(status=301, headers={"Location": redirect_url}) + + +def _pool_entry_owned_by_user(entry, user_id): + """True when *entry* is unclaimed or owned by *user_id*.""" + if not entry or not entry.get("profile_id"): + return True + owner = entry.get("user_id") + if owner is None or owner == "": + return False + return str(owner) == str(user_id) + + def _find_matching_idle_session( redis_client, *, media_id, user_id, client_ip, client_user_agent, ): @@ -583,7 +607,7 @@ def _pool_lock(redis_client, session_id): ) -def _acquire_idle_pool_session(redis_client, session_id): +def _acquire_idle_pool_session(redis_client, session_id, *, user_id=None): """Re-reserve an idle session's profile slot and mark it busy.""" if redis_client is None or not session_id: return None @@ -593,6 +617,8 @@ def _acquire_idle_pool_session(redis_client, session_id): data = redis_client.hgetall(key) if not data or not data.get("profile_id"): return None + if user_id is not None and not _pool_entry_owned_by_user(data, user_id): + return None if data.get("busy") == "1": return None try: @@ -614,12 +640,16 @@ def _acquire_idle_pool_session(redis_client, session_id): return None -def _wait_for_idle_pool_session(redis_client, session_id, wait_seconds=_POOL_WAIT_SECONDS): +def _wait_for_idle_pool_session( + redis_client, session_id, *, user_id=None, wait_seconds=_POOL_WAIT_SECONDS, +): if redis_client is None or not session_id: return None deadline = time.time() + wait_seconds while True: - acquired = _acquire_idle_pool_session(redis_client, session_id) + acquired = _acquire_idle_pool_session( + redis_client, session_id, user_id=user_id, + ) if acquired is not None: return acquired if not _get_pool_entry(redis_client, session_id):