From 5cd50544a8e33662511e9edfae3f1e01a24c56c0 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Mon, 6 Jul 2026 23:54:05 -0600 Subject: [PATCH] fix(remote): harden serve API access control and input handling Bind sessions and download jobs to the API key (not source IP), reject path-traversal cache keys, bound decompression of peer-supplied blobs, gate the CDM device before the vault fallback, and compare API keys in constant time. --- tests/remote/unit/test_errors.py | 1 + unshackle/commands/serve.py | 5 +- unshackle/core/api/compression.py | 17 ++++++ unshackle/core/api/download_manager.py | 8 ++- unshackle/core/api/handlers.py | 77 ++++++++++++++++++++------ unshackle/core/api/sanitize.py | 19 ++++++- unshackle/core/api/session_store.py | 1 + unshackle/core/remote_service.py | 18 ++++-- 8 files changed, 121 insertions(+), 25 deletions(-) diff --git a/tests/remote/unit/test_errors.py b/tests/remote/unit/test_errors.py index b5906431..51f37534 100644 --- a/tests/remote/unit/test_errors.py +++ b/tests/remote/unit/test_errors.py @@ -32,6 +32,7 @@ def test_api_error_default_http_status_per_code() -> None: APIErrorCode.JOB_NOT_FOUND: 404, APIErrorCode.SESSION_NOT_FOUND: 404, APIErrorCode.TRACK_NOT_FOUND: 404, + APIErrorCode.CONFLICT: 409, APIErrorCode.RATE_LIMITED: 429, APIErrorCode.INTERNAL_ERROR: 500, APIErrorCode.DOWNLOAD_ERROR: 500, diff --git a/unshackle/commands/serve.py b/unshackle/commands/serve.py index 57417593..22ad10e3 100644 --- a/unshackle/commands/serve.py +++ b/unshackle/commands/serve.py @@ -1,3 +1,4 @@ +import hmac import logging import subprocess @@ -135,7 +136,9 @@ def serve( secret_key = request.headers.get("X-Secret-Key") if not secret_key: return web.json_response({"status": 401, "message": "Secret Key is Empty."}, status=401) - if secret_key not in request.app["config"]["users"]: + # Constant-time compare against every configured key so a valid key can't be + # recovered byte-by-byte from response timing. + if not any(hmac.compare_digest(secret_key, k) for k in request.app["config"]["users"]): return web.json_response({"status": 401, "message": "Secret Key is Invalid."}, status=401) return await handler(request) diff --git a/unshackle/core/api/compression.py b/unshackle/core/api/compression.py index 104751b8..8b05a607 100644 --- a/unshackle/core/api/compression.py +++ b/unshackle/core/api/compression.py @@ -3,9 +3,26 @@ from __future__ import annotations import gzip +import zlib from aiohttp import web +# Cap the inflated size of a peer-supplied compressed blob (cookies, session cache, +# serialized manifests) so a tiny payload cannot expand without bound (zip-bomb DoS). +MAX_INFLATE_BYTES = 64 * 1024 * 1024 # 64 MiB + + +def safe_inflate(data: bytes, max_size: int = MAX_INFLATE_BYTES) -> bytes: + """zlib-decompress *data*, raising ValueError if the result would exceed *max_size*.""" + decompressor = zlib.decompressobj() + out = decompressor.decompress(data, max_size + 1) + if len(out) > max_size or decompressor.unconsumed_tail: + raise ValueError("compressed payload expands beyond allowed size") + out += decompressor.flush() + if len(out) > max_size: + raise ValueError("compressed payload expands beyond allowed size") + return out + @web.middleware async def compression_middleware(request: web.Request, handler) -> web.Response: diff --git a/unshackle/core/api/download_manager.py b/unshackle/core/api/download_manager.py index aa4aac64..a670ab81 100644 --- a/unshackle/core/api/download_manager.py +++ b/unshackle/core/api/download_manager.py @@ -127,6 +127,9 @@ class DownloadJob: # Guards against writing the same job to the persistent history file twice. history_recorded: bool = False + # X-Secret-Key that created the job; scopes per-caller access, never serialized into to_dict(). + owner_key: Optional[str] = None + def to_dict(self, include_full_details: bool = False) -> Dict[str, Any]: """Convert job to dictionary for JSON response.""" result = { @@ -617,7 +620,9 @@ class DownloadQueueManager: f"Initialized download queue manager: max_concurrent={max_concurrent_downloads}, retention_hours={job_retention_hours}" ) - def create_job(self, service: str, title_id: str, **parameters) -> DownloadJob: + def create_job( + self, service: str, title_id: str, owner_key: Optional[str] = None, **parameters + ) -> DownloadJob: """Create a new download job and add it to the queue.""" job_id = str(uuid.uuid4()) job = DownloadJob( @@ -627,6 +632,7 @@ class DownloadQueueManager: service=service, title_id=title_id, parameters=parameters, + owner_key=owner_key, ) self._jobs[job_id] = job diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 1b9ad0a7..77ade8b6 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -8,9 +8,10 @@ from typing import Any, Dict, List, Optional from aiohttp import web +from unshackle.core.api.compression import safe_inflate from unshackle.core.api.errors import APIError, APIErrorCode, handle_api_exception from unshackle.core.api.input_bridge import AuthStatus, InputBridge -from unshackle.core.api.sanitize import sanitize_log +from unshackle.core.api.sanitize import safe_cache_key, sanitize_log from unshackle.core.config import config from unshackle.core.constants import AUDIO_CODEC_MAP, DYNAMIC_RANGE_MAP, VIDEO_CODEC_MAP from unshackle.core.proxies.resolve import initialize_proxy_providers, resolve_proxy @@ -275,6 +276,25 @@ def get_allowed_services(request: Optional[web.Request] = None) -> Optional[List return list(result) +def caller_key(request: Optional[web.Request] = None) -> str: + """The authenticating X-Secret-Key for a request, or 'anonymous' when unauthenticated.""" + return request.headers.get("X-Secret-Key", "anonymous") if request else "anonymous" + + +def owns_job(job: Any, request: Optional[web.Request] = None) -> bool: + """Whether the calling key owns this job. + + Jobs created without an owner (no-key mode / legacy) stay shared; otherwise a job is + only visible to the key that created it (constant-time compare). + """ + import hmac + + owner = getattr(job, "owner_key", None) + if owner is None: + return True + return hmac.compare_digest(owner, caller_key(request)) + + def validate_service(service_tag: str, request: Optional[web.Request] = None) -> Optional[str]: """Validate, normalize, and check allowlist for service tag.""" try: @@ -985,7 +1005,9 @@ async def download_handler(data: Dict[str, Any], request: Optional[web.Request] **service_specific_defaults, **filtered_params, } - job = manager.create_job(normalized_service, title_id, **params_with_defaults) + job = manager.create_job( + normalized_service, title_id, owner_key=caller_key(request), **params_with_defaults + ) return web.json_response( {"job_id": job.job_id, "status": job.status.value, "created_time": job.created_time.isoformat()}, status=202 @@ -1009,7 +1031,7 @@ async def list_download_jobs_handler(data: Dict[str, Any], request: Optional[web try: manager = get_download_manager() - jobs = manager.list_jobs() + jobs = [job for job in manager.list_jobs() if owns_job(job, request)] status_filter = data.get("status") if status_filter: @@ -1080,7 +1102,7 @@ async def get_download_job_handler(job_id: str, request: Optional[web.Request] = manager = get_download_manager() job = manager.get_job(job_id) - if not job: + if not job or not owns_job(job, request): raise APIError( APIErrorCode.JOB_NOT_FOUND, "Job not found", @@ -1109,7 +1131,7 @@ async def cancel_download_job_handler(job_id: str, request: Optional[web.Request manager = get_download_manager() job = manager.get_job(job_id) - if not job: + if not job or not owns_job(job, request): raise APIError( APIErrorCode.JOB_NOT_FOUND, "Job not found", @@ -1173,7 +1195,7 @@ async def retry_download_job_handler(job_id: str, request: Optional[web.Request] manager = get_download_manager() job = manager.get_job(job_id) - if not job: + if not job or not owns_job(job, request): raise APIError( APIErrorCode.JOB_NOT_FOUND, "Job not found", @@ -1200,7 +1222,7 @@ async def retry_download_job_handler(job_id: str, request: Optional[web.Request] await manager.start_workers() # Reuse the raw in-memory parameters; redaction only ever applies to serialized copies. - new_job = manager.create_job(job.service, job.title_id, **job.parameters) + new_job = manager.create_job(job.service, job.title_id, owner_key=caller_key(request), **job.parameters) return web.json_response( {"job_id": new_job.job_id, "status": new_job.status.value, "created_time": new_job.created_time.isoformat()}, @@ -1227,7 +1249,7 @@ async def prioritize_download_job_handler(job_id: str, request: Optional[web.Req manager = get_download_manager() job = manager.get_job(job_id) - if not job: + if not job or not owns_job(job, request): raise APIError( APIErrorCode.JOB_NOT_FOUND, "Job not found", @@ -1626,10 +1648,9 @@ def _create_service_instance( if cookie_text and isinstance(cookie_text, str): import base64 import tempfile - import zlib from http.cookiejar import MozillaCookieJar - cookie_str = zlib.decompress(base64.b64decode(cookie_text)).decode("utf-8") + cookie_str = safe_inflate(base64.b64decode(cookie_text)).decode("utf-8") with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False, encoding="utf-8") as f: f.write(cookie_str) tmp_path = f.name @@ -1700,13 +1721,16 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req cache_data = data.get("cache", {}) if cache_data: import base64 - import zlib cache_dir = app_config.directories.cache / session_cache_tag cache_dir.mkdir(parents=True, exist_ok=True) for key, content in cache_data.items(): - decompressed = zlib.decompress(base64.b64decode(content)).decode("utf-8") - (cache_dir / key).with_suffix(".json").write_text(decompressed, encoding="utf-8") + safe_name = safe_cache_key(key) + if not safe_name: + log.warning(f"Rejecting unsafe session cache key: {sanitize_log(key)}") + continue + decompressed = safe_inflate(base64.b64decode(content)).decode("utf-8") + (cache_dir / safe_name).with_suffix(".json").write_text(decompressed, encoding="utf-8") bridge = InputBridge() service_instance._input_bridge = bridge @@ -1718,6 +1742,7 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req session_id=session_id, ) session.creator_ip = request.remote if request else None + session.owner_key = api_key session.cache_tag = session_cache_tag session.input_bridge = bridge session.auth_status = AuthStatus.AUTHENTICATING @@ -2138,7 +2163,14 @@ async def session_prompt_post_handler( async def _get_validated_session(session_id: str, request: Optional[web.Request]) -> Any: - """Fetch a session and verify the requesting IP matches the creator.""" + """Fetch a session and verify the caller owns it. + + Ownership is bound to the authenticating X-Secret-Key rather than the source IP: + behind a reverse proxy every caller shares the proxy's address, so the IP check + (kept as defence in depth) cannot distinguish users on its own. + """ + import hmac + from unshackle.core.api.session_store import get_session_store store = get_session_store() @@ -2149,6 +2181,13 @@ async def _get_validated_session(session_id: str, request: Optional[web.Request] f"Session not found or expired: {session_id}", details={"session_id": session_id}, ) + if session.owner_key is not None and request is not None: + caller_key = request.headers.get("X-Secret-Key", "anonymous") + if not hmac.compare_digest(caller_key, session.owner_key): + raise APIError( + APIErrorCode.FORBIDDEN, + "Session access denied", + ) if session.creator_ip and request and request.remote != session.creator_ip: raise APIError( APIErrorCode.FORBIDDEN, @@ -2407,11 +2446,14 @@ def _handle_single_server_cdm( pr_pssh = PlayReadyPSSH(base64.b64decode(pssh_b64)) pr_drm = PlayReady(pssh=pr_pssh, pssh_b64=pssh_b64) + # Gate on the caller's CDM device first so a caller with no device cannot + # harvest server-side keys from the vault fallback below. + device_name = _resolve_device_name(user_config, drm_type, service.__class__.__name__) + vault_keys = _check_vaults(pr_drm.kids, service.__class__.__name__) if vault_keys: return vault_keys - device_name = _resolve_device_name(user_config, drm_type, service.__class__.__name__) cdm = load_cdm(device_name, service_name=service.__class__.__name__) pr_drm.get_content_keys( cdm=cdm, @@ -2427,11 +2469,14 @@ def _handle_single_server_cdm( wv_pssh = WvPSSH(pssh_b64) wv_drm = Widevine(pssh=wv_pssh) + # Gate on the caller's CDM device first so a caller with no device cannot + # harvest server-side keys from the vault fallback below. + device_name = _resolve_device_name(user_config, drm_type, service.__class__.__name__) + vault_keys = _check_vaults(wv_drm.kids, service.__class__.__name__) if vault_keys: return vault_keys - device_name = _resolve_device_name(user_config, drm_type, service.__class__.__name__) cdm = load_cdm(device_name, service_name=service.__class__.__name__) wv_drm.get_content_keys( cdm=cdm, diff --git a/unshackle/core/api/sanitize.py b/unshackle/core/api/sanitize.py index 81e4a0e1..f62bcf33 100644 --- a/unshackle/core/api/sanitize.py +++ b/unshackle/core/api/sanitize.py @@ -1,8 +1,25 @@ -"""Helpers for safely logging user-provided values.""" +"""Helpers for safely logging and handling user-provided values.""" from __future__ import annotations +from pathlib import Path +from typing import Optional + def sanitize_log(value: object) -> str: """Sanitize a value for safe logging by removing newlines and control characters.""" return str(value).replace("\n", "").replace("\r", "").replace("\x00", "") + + +def safe_cache_key(key: object) -> Optional[str]: + """Return a bare filename for a peer-supplied cache key, or None if it escapes its directory. + + Cache keys are written as ``.json`` inside a fixed directory. A key containing a + path separator, ``..``, or an absolute path would let the peer write outside that + directory, so anything that is not already a plain filename is rejected. + """ + text = str(key) + name = Path(text).name + if not name or name != text or name in {".", ".."}: + return None + return name diff --git a/unshackle/core/api/session_store.py b/unshackle/core/api/session_store.py index 0ba1a0ec..71a657e4 100644 --- a/unshackle/core/api/session_store.py +++ b/unshackle/core/api/session_store.py @@ -35,6 +35,7 @@ class SessionEntry: tracks_by_title: Dict[str, Dict[str, Track]] = field(default_factory=dict) # title_key -> {track_id -> Track} chapters_by_title: Dict[str, List[Any]] = field(default_factory=dict) # title_key -> [Chapter] creator_ip: Optional[str] = None + owner_key: Optional[str] = None # X-Secret-Key that owns this session cache_tag: Optional[str] = None # per-session cache directory tag input_bridge: Optional[InputBridge] = None auth_status: AuthStatus = AuthStatus.AUTHENTICATED diff --git a/unshackle/core/remote_service.py b/unshackle/core/remote_service.py index a1805130..ade9643e 100644 --- a/unshackle/core/remote_service.py +++ b/unshackle/core/remote_service.py @@ -207,7 +207,8 @@ def _resolve_manifest_data(tracks: Tracks, manifests: list) -> None: to copy track.data. HLS is skipped as it re-fetches from track.url. """ import base64 as b64 - import zlib + + from unshackle.core.api.compression import safe_inflate if not manifests: return @@ -223,7 +224,7 @@ def _resolve_manifest_data(tracks: Tracks, manifests: list) -> None: continue try: - raw = zlib.decompress(b64.b64decode(m_data)) + raw = safe_inflate(b64.b64decode(m_data)) if m_type == "dash": from lxml import etree @@ -836,17 +837,22 @@ class RemoteService: if not cache_data: return - import zlib + from unshackle.core.api.compression import safe_inflate + from unshackle.core.api.sanitize import safe_cache_key cache_dir = config.directories.cache / self.service_tag cache_dir.mkdir(parents=True, exist_ok=True) for key, content in cache_data.items(): + safe_name = safe_cache_key(key) + if not safe_name: + self.log.warning(f"Rejecting unsafe cache filename from server: {key!r}") + continue try: - decompressed = zlib.decompress(base64.b64decode(content)) - (cache_dir / key).with_suffix(".json").write_bytes(decompressed) + decompressed = safe_inflate(base64.b64decode(content)) + (cache_dir / safe_name).with_suffix(".json").write_bytes(decompressed) except Exception as e: - self.log.warning(f"Failed to save returned cache file '{key}': {e}") + self.log.warning(f"Failed to save returned cache file '{safe_name}': {e}") self.log.info(f"Saved {len(cache_data)} cache file(s) from server")