mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-13 12:35:05 +00:00
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.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 ``<key>.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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user