Merge pull request #3902 from plotly/fix/callback-args-hardening

callback args hardening.

Author: T4rk1n

CHANGELOG.md

@@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ## [Unreleased]
 
 ## Fixed
+- [#3902](https://github.com/plotly/dash/pull/3902) Fix background callbacks trusting the client-supplied `job`/`oldJob`/`cancelJob` and `cacheKey` query parameters verbatim, which let an unauthenticated client terminate an arbitrary process (with `DiskcacheManager`, by sending its PID) or read and delete arbitrary result-cache entries. The `cacheKey`/`job` handles are now HMAC-signed by the server and bound to a per-page-load token, so forged or replayed values are rejected. Handles stay opaque to the renderer, so no app or callback code changes are required.
 - [3883](https://github.com/plotly/dash/pull/3883) Fix callbacks being registered twice when running the app file as a script with a server that loads it by import string, e.g. `uvicorn.run("app:server", reload=True)` with `backend="fastapi"`. The spawned worker re-executes the main module as `__mp_main__` and the import string then executed the same file a second time, duplicating every callback in `_dash-dependencies` and triggering `Duplicate callback outputs` errors in the renderer. `Dash()` now pre-registers the running main module in `sys.modules` under its canonical import name so the second import reuses it instead of re-executing the file. Fixes [#3818](https://github.com/plotly/dash/issues/3818).
 - [3885](https://github.com/plotly/dash/pull/3885) Fix Flask-WTF `CSRFProtect` (and Quart-WTF) exemptions breaking after the backend refactor. The callback dispatch view is now exposed with the fully-qualified name `dash.dash.dispatch` again, so `csrf._exempt_views.add("dash.dash.dispatch")` works as it did in Dash 4.1.0. Fixes [#3827](https://github.com/plotly/dash/issues/3827).
 - [#3882](https://github.com/plotly/dash/pull/3882) Fix `dcc.Graph` user interactions (pan, zoom, edited shapes & annotations, ...) being reverted by a subsequent `Patch` update, by syncing all relayout changes back to the `figure` prop instead of only shapes. Fixes [#3810](https://github.com/plotly/dash/issues/3810).

dash/_callback.py

@@ -24,6 +24,7 @@
     ImportedInsideCallbackError,
 )
 from ._get_app import get_app
+from . import _callback_signing
 from ._grouping import (
     flatten_grouping,
     make_grouping_by_index,
@@ -387,6 +388,24 @@ def _initialize_context(args, kwargs, inputs_state_indices, has_output, insert_o
     )
 
 
+def get_request_end_id(secret: bytes):
+    """Return the verified end_id for the current request, or ``None``.
+
+    The renderer echoes the server-signed ``endId`` token on every callback
+    request; this verifies the signature and returns the underlying end_id so
+    background handles can be checked against it.
+    """
+    adapter = get_app().backend.request_adapter()
+    if not adapter:
+        return None
+    token = adapter.args.get("endId")
+    return _callback_signing.unsign(secret, _callback_signing.END_SCOPE, token)
+
+
+def _get_signing_secret() -> bytes:
+    return get_app()._get_signing_secret()  # pylint: disable=protected-access
+
+
 def _get_callback_manager(
     kwargs: dict, background: dict
 ) -> BaseBackgroundCallbackManager:
@@ -409,8 +428,14 @@ def _get_callback_manager(
     old_job = adapter.args.getlist("oldJob") if hasattr(adapter.args, "getlist") else []
 
     if old_job:
-        for job in old_job:
-            callback_manager.terminate_job(job)
+        secret = _get_signing_secret()
+        scope = _callback_signing.job_scope(get_request_end_id(secret))
+        for signed_job in old_job:
+            # Only terminate jobs whose handle we actually signed for this page
+            # load; ignore forged or replayed pids.
+            job = _callback_signing.unsign(secret, scope, signed_job)
+            if job is not None:
+                callback_manager.terminate_job(job)
 
     return callback_manager
 
@@ -450,9 +475,18 @@ def _setup_background_callback(
         ctx_value,
     )
 
+    # Sign the handles before handing them to the browser so they cannot be
+    # forged (arbitrary pid kill / arbitrary cache read) or replayed from
+    # another page load. The renderer treats them as opaque strings.
+    secret = _get_signing_secret()
+    end_id = get_request_end_id(secret)
     data = {
-        "cacheKey": cache_key,
-        "job": job,
+        "cacheKey": _callback_signing.sign(
+            secret, _callback_signing.cache_scope(end_id), cache_key
+        ),
+        "job": _callback_signing.sign(
+            secret, _callback_signing.job_scope(end_id), str(job)
+        ),
     }
 
     cancel = background.get("cancel")
@@ -467,15 +501,38 @@ def _setup_background_callback(
     return to_json(data)
 
 
+def _read_request_handles():
+    """Read and verify the signed cacheKey/job handles from the current request.
+
+    Returns ``(cache_key, job_id)`` as the raw (unsigned) values, or ``None`` for
+    any handle that is missing or whose signature does not verify against this
+    page load's end_id. This is what prevents a client from supplying an
+    arbitrary cache key (read/delete) or an arbitrary pid (kill).
+    """
+    adapter = get_app().backend.request_adapter()
+    if not adapter:
+        return None, None
+    secret = _get_signing_secret()
+    end_id = get_request_end_id(secret)
+    cache_key = _callback_signing.unsign(
+        secret,
+        _callback_signing.cache_scope(end_id),
+        adapter.args.get("cacheKey"),
+    )
+    job_id = _callback_signing.unsign(
+        secret,
+        _callback_signing.job_scope(end_id),
+        adapter.args.get("job"),
+    )
+    return cache_key, job_id
+
+
 def _progress_background_callback(
     response, callback_manager, background, cache_key=None
 ):
     progress_outputs = background.get("progress")
-    if cache_key is None:
-        adapter = get_app().backend.request_adapter()
-        cache_key = adapter.args.get("cacheKey")
 
-    if progress_outputs:
+    if progress_outputs and cache_key is not None:
         # Get the progress before the result as it would be erased after the results.
         progress = callback_manager.get_progress(cache_key)
         if progress:
@@ -498,15 +555,19 @@ def _update_background_callback(
     callback_manager = _get_callback_manager(kwargs, background)
 
     if cache_key is None or job_id is None:
-        adapter = get_app().backend.request_adapter()
-        cache_key = cache_key or (adapter.args.get("cacheKey") if adapter else None)
-        job_id = job_id or (adapter.args.get("job") if adapter else None)
+        req_cache_key, req_job_id = _read_request_handles()
+        cache_key = cache_key or req_cache_key
+        job_id = job_id or req_job_id
 
     _progress_background_callback(
         response, callback_manager, background, cache_key=cache_key
     )
 
-    output_value = callback_manager.get_result(cache_key, job_id)
+    if cache_key is None:
+        # No valid handle for this page load -> read nothing, delete nothing.
+        output_value = callback_manager.UNDEFINED
+    else:
+        output_value = callback_manager.get_result(cache_key, job_id)
 
     return _handle_rest_background_callback(
         output_value,
@@ -531,12 +592,8 @@ def _handle_rest_background_callback(
     cache_key=None,
     job_id=None,
 ):
-    if cache_key is None or job_id is None:
-        adapter = get_app().backend.request_adapter()
-        cache_key = cache_key or (adapter.args.get("cacheKey") if adapter else None)
-        job_id = job_id or (adapter.args.get("job") if adapter else None)
     # Must get job_running after get_result since get_results terminates it.
-    job_running = callback_manager.job_running(job_id)
+    job_running = callback_manager.job_running(job_id) if job_id is not None else False
     if not job_running and output_value is callback_manager.UNDEFINED:
         # Job canceled -> no output to close the loop.
         output_value = NoUpdate()

dash/_callback_signing.py

@@ -0,0 +1,68 @@
+"""Signing helpers for background-callback handles.
+
+Background callbacks round-trip two values through the browser: the ``cacheKey``
+(the diskcache/celery result key) and the ``job`` (the OS PID or Celery task id).
+Historically these were sent to the client verbatim and trusted verbatim when
+they came back, which let any HTTP client ask the server to read/delete an
+arbitrary cache entry or to ``SIGKILL`` an arbitrary process id.
+
+To close that, the server issues a per-page-load ``end_id`` token (signed so the
+server can recognise the ones it minted), and every ``cacheKey``/``job`` handle
+is HMAC-signed with a server-held secret and bound to that ``end_id``. A forged
+PID or arbitrary cache key never carries a valid signature, and a handle minted
+for one page load cannot be replayed from another. The client treats all of
+these as opaque strings, so nothing about the handshake is visible to or
+forgeable by the browser.
+
+(This ``end_id`` is unrelated to the client-generated ``rendererId`` used for
+SharedWorker routing in ``dash-renderer/src/utils/rendererId.ts``.)
+"""
+
+import hmac
+from hashlib import sha256
+from typing import Optional
+
+# Separates the payload from its signature in a signed token. Chosen because it
+# never appears in the values we sign (hex digests, integers, and gen_salt
+# output are all limited to ``[A-Za-z0-9]``).
+_SEP = "~"
+
+
+def _mac(secret: bytes, scope: str, value: str) -> str:
+    msg = f"{scope}\x00{value}".encode("utf-8")
+    return hmac.new(secret, msg, sha256).hexdigest()
+
+
+def sign(secret: bytes, scope: str, value: str) -> str:
+    """Return ``value`` with an HMAC signature appended, bound to ``scope``."""
+    return f"{value}{_SEP}{_mac(secret, scope, value)}"
+
+
+def unsign(secret: bytes, scope: str, signed: Optional[str]) -> Optional[str]:
+    """Verify a signed token and return the original value, or ``None``.
+
+    Returns ``None`` when the token is missing, malformed, or the signature does
+    not match ``scope`` under ``secret``. Uses a constant-time comparison so the
+    check does not leak the expected signature.
+    """
+    if not signed or _SEP not in signed:
+        return None
+
+    value, _, mac = signed.rpartition(_SEP)
+    expected = _mac(secret, scope, value)
+    if hmac.compare_digest(mac, expected):
+        return value
+    return None
+
+
+# Scope strings. The end_id is folded into the job/cache scopes so a handle is
+# only valid for the page load it was issued to.
+END_SCOPE = "end"
+
+
+def job_scope(end_id: Optional[str]) -> str:
+    return f"job:{end_id or ''}"
+
+
+def cache_scope(end_id: Optional[str]) -> str:
+    return f"cacheKey:{end_id or ''}"

dash/background_callback/managers/__init__.py

@@ -55,6 +55,20 @@ def get_result(self, key, job):
     def get_updated_props(self, key):
         raise NotImplementedError
 
+    # Key under which the shared background-callback signing secret is stored in
+    # the result store, so all workers agree on one secret without a configured
+    # server secret_key. See Dash._get_signing_secret.
+    SIGNING_SECRET_KEY = "__dash_background_signing_secret__"
+
+    def get_or_create_signing_secret(self, generate):
+        """Return a signing secret shared across workers via the result store.
+
+        ``generate`` is a zero-argument callable returning fresh ``bytes``. The
+        first worker to reach an empty store wins; every other worker reads back
+        the stored value, so all workers converge on a single secret.
+        """
+        raise NotImplementedError
+
     def build_cache_key(self, fn, args, cache_args_to_ignore, triggered):
         try:
             fn_source = inspect.getsource(fn)

dash/background_callback/managers/celery_manager.py

@@ -92,6 +92,16 @@ def get_task(self, job):
     def clear_cache_entry(self, key):
         self.handle.backend.delete(key)
 
+    def get_or_create_signing_secret(self, generate):
+        backend = self.handle.backend
+        existing = backend.get(self.SIGNING_SECRET_KEY)
+        if existing is not None:
+            return existing
+        secret = generate()
+        backend.set(self.SIGNING_SECRET_KEY, secret)
+        # Re-read so workers that raced on the initial set converge on one value.
+        return backend.get(self.SIGNING_SECRET_KEY) or secret
+
     def call_job_fn(self, key, job_fn, args, context):
         task = job_fn.delay(key, self._make_progress_key(key), args, context)
         return task.task_id

dash/background_callback/managers/diskcache_manager.py

@@ -118,6 +118,12 @@ def make_job_fn(self, fn, progress, key=None):
     def clear_cache_entry(self, key):
         self.handle.delete(key)
 
+    def get_or_create_signing_secret(self, generate):
+        # ``add`` only sets the value if the key is absent (atomic in diskcache),
+        # so the first worker wins and the rest read back the stored secret.
+        self.handle.add(self.SIGNING_SECRET_KEY, generate())
+        return self.handle.get(self.SIGNING_SECRET_KEY)
+
     # noinspection PyUnresolvedReferences
     def call_job_fn(self, key, job_fn, args, context):
         """

dash/dash-renderer/src/actions/callbacks.ts

@@ -499,6 +499,11 @@ function handleServerside(
             }
             url = `${url}${delim}${name}=${value}`;
         };
+        // Echo the server-issued token so the server can bind/verify the
+        // background-callback handles (cacheKey/job/oldJob/cancelJob) it signs.
+        if (config.end_id) {
+            addArg('endId', config.end_id);
+        }
         if (cacheKey || job) {
             if (cacheKey) addArg('cacheKey', cacheKey);
             if (job) addArg('job', job);

dash/dash-renderer/src/config.ts

@@ -31,6 +31,11 @@ export type DashConfig = {
     };
     csrf_token_name?: string;
     csrf_header_name?: string;
+    // Server-issued, server-signed token for this page load. Echoed on every
+    // callback request so the server can bind/verify background-callback
+    // handles (cacheKey/job) to this page load. Treated as an opaque string.
+    // (Unrelated to the client-side rendererId used for SharedWorker routing.)
+    end_id?: string;
 };
 
 export default function getConfigFromDOM(): DashConfig {