mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(gateway): reject non-ASCII tokens instead of raising in constant-time compares (#6076)
* fix(gateway): reject non-ASCII tokens instead of raising in constant-time compares
hmac.compare_digest raises TypeError when a str operand holds non-ASCII
characters. Starlette decodes header bytes as latin-1 and query values as
UTF-8, so a single 0xE9 byte in a CSRF token, GitHub webhook signature,
internal auth token, or OIDC state turned the expected 403/401 into a 500.
Route the five client-facing comparisons through a new
app.gateway.utils.constant_time_equals helper that compares the UTF-8
encodings ("surrogatepass" so even a lone surrogate cannot raise). ASCII
results are unchanged, and no bypass was possible before the fix.
* fix(provisioner): reject non-ASCII API keys instead of raising; drive OIDC test over HTTP
The standalone provisioner compared X-API-Key with secrets.compare_digest on
str operands, so a non-ASCII header raised TypeError (500) the same way the
Gateway sites did. It cannot import app.gateway, so encode inline.
The OIDC state regression test now mounts the auth router and sends the
percent-encoded state through a real signed state cookie instead of stubbing
get_state_cookie and calling the handler directly.
* docs(changelog): reference #6076
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
1e462ae4a2
commit
14cd840bbf
10
CHANGELOG.md
10
CHANGELOG.md
@ -327,6 +327,15 @@ This release closes that milestone with **181 merged pull requests**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **gateway:** A non-ASCII CSRF token, GitHub webhook signature, internal auth
|
||||
token, OIDC `state`, or provisioner `X-API-Key` is now rejected with the
|
||||
usual 403/401 instead of a 500. `hmac.compare_digest` raises `TypeError` for
|
||||
`str` operands with non-ASCII characters, and Starlette decodes header bytes
|
||||
as latin-1, so a single `0xE9` byte crashed the comparison. The Gateway now
|
||||
compares the UTF-8 bytes through one helper,
|
||||
`app.gateway.utils.constant_time_equals`, and the standalone provisioner
|
||||
encodes inline. No bypass was possible; the request was already failing, just
|
||||
with the wrong status. ([#6076])
|
||||
- **agents:** Context-compaction fraction triggers and fraction-based retention
|
||||
now use the active run model's context profile; a separate
|
||||
`summarization.model_name` remains generation-only. This prevents mismatched
|
||||
@ -6293,5 +6302,6 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#6066]: https://github.com/bytedance/deer-flow/pull/6066
|
||||
[#6069]: https://github.com/bytedance/deer-flow/pull/6069
|
||||
[#6070]: https://github.com/bytedance/deer-flow/pull/6070
|
||||
[#6076]: https://github.com/bytedance/deer-flow/pull/6076
|
||||
[#6088]: https://github.com/bytedance/deer-flow/pull/6088
|
||||
|
||||
|
||||
@ -332,6 +332,12 @@
|
||||
|
||||
### 修复
|
||||
|
||||
- **Gateway:** 含非 ASCII 字符的 CSRF token、GitHub webhook 签名、内部认证 token、
|
||||
OIDC `state` 或 provisioner `X-API-Key` 现在按常规返回 403/401,而不是 500。
|
||||
`hmac.compare_digest` 遇到含非 ASCII 字符的 `str` 参数会抛出 `TypeError`,而
|
||||
Starlette 以 latin-1 解码请求头字节,一个 `0xE9` 字节就能让比较崩溃。Gateway 现在通过统一的
|
||||
`app.gateway.utils.constant_time_equals` 比较 UTF-8 字节,独立部署的 provisioner
|
||||
则在原处编码后比较。此前不存在绕过,请求本就会失败,只是状态码错误。([#6076])
|
||||
- **智能体:** 上下文压缩的 fraction 触发器与 fraction 保留量现在使用当前运行
|
||||
模型的上下文 profile;单独配置的 `summarization.model_name` 只负责生成摘要。
|
||||
这避免运行模型与摘要模型的窗口不一致时压缩过晚或过早。中间件发布身份现在
|
||||
@ -5336,4 +5342,5 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#6066]: https://github.com/bytedance/deer-flow/pull/6066
|
||||
[#6069]: https://github.com/bytedance/deer-flow/pull/6069
|
||||
[#6070]: https://github.com/bytedance/deer-flow/pull/6070
|
||||
[#6076]: https://github.com/bytedance/deer-flow/pull/6076
|
||||
[#6088]: https://github.com/bytedance/deer-flow/pull/6088
|
||||
|
||||
@ -18,6 +18,7 @@ from app.gateway.auth.config import get_auth_config
|
||||
from app.gateway.auth.session_cookie_state import SESSION_COOKIE_ISSUED_STATE_ATTR, SESSION_COOKIE_MAX_AGE_STATE_ATTR, SESSION_COOKIE_SECURE_STATE_ATTR, SKIP_AUTH_CSRF_COOKIE_STATE_ATTR
|
||||
from app.gateway.auth_disabled import is_auth_disabled
|
||||
from app.gateway.request_path import get_request_route_path
|
||||
from app.gateway.utils import constant_time_equals
|
||||
from deerflow.trace_context import TRACE_ID_HEADER
|
||||
|
||||
CSRF_COOKIE_NAME = "csrf_token"
|
||||
@ -245,7 +246,7 @@ class CSRFMiddleware(BaseHTTPMiddleware):
|
||||
content={"detail": "CSRF token missing. Include X-CSRF-Token header."},
|
||||
)
|
||||
|
||||
if not secrets.compare_digest(cookie_token, header_token):
|
||||
if not constant_time_equals(cookie_token, header_token):
|
||||
return JSONResponse(
|
||||
status_code=403,
|
||||
content={"detail": "CSRF token mismatch."},
|
||||
|
||||
@ -7,6 +7,7 @@ import secrets
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
|
||||
from app.gateway.utils import constant_time_equals
|
||||
from deerflow.config.paths import make_safe_user_id
|
||||
from deerflow.runtime.user_context import DEFAULT_USER_ID
|
||||
|
||||
@ -36,7 +37,7 @@ def create_internal_auth_headers(*, owner_user_id: str | None = None) -> dict[st
|
||||
|
||||
def is_valid_internal_auth_token(token: str | None) -> bool:
|
||||
"""Return True when *token* matches this Gateway worker's internal token."""
|
||||
return bool(token) and secrets.compare_digest(token, _INTERNAL_AUTH_TOKEN)
|
||||
return bool(token) and constant_time_equals(token, _INTERNAL_AUTH_TOKEN)
|
||||
|
||||
|
||||
def get_internal_user(owner_user_id: str | None = None):
|
||||
|
||||
@ -14,7 +14,6 @@ Two layers:
|
||||
2. @auth.on — returns metadata filter so each user only sees own threads
|
||||
"""
|
||||
|
||||
import secrets
|
||||
from contextvars import ContextVar
|
||||
from uuid import uuid4
|
||||
|
||||
@ -25,6 +24,7 @@ from app.gateway.auth.errors import TokenError
|
||||
from app.gateway.auth.jwt import decode_token
|
||||
from app.gateway.auth_disabled import AUTH_DISABLED_USER_ID, is_auth_disabled
|
||||
from app.gateway.deps import get_local_provider
|
||||
from app.gateway.utils import constant_time_equals
|
||||
from deerflow.mcp_scope import (
|
||||
THREAD_INCARNATION_CONTEXT_KEY,
|
||||
THREAD_INCARNATION_METADATA_GUARD_KEY,
|
||||
@ -234,7 +234,7 @@ def _check_csrf(request) -> None:
|
||||
detail="CSRF token missing. Include X-CSRF-Token header.",
|
||||
)
|
||||
|
||||
if not secrets.compare_digest(cookie_token, header_token):
|
||||
if not constant_time_equals(cookie_token, header_token):
|
||||
raise Auth.exceptions.HTTPException(
|
||||
status_code=403,
|
||||
detail="CSRF token mismatch.",
|
||||
|
||||
@ -4,7 +4,6 @@ import asyncio
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import secrets
|
||||
import time
|
||||
import urllib.parse
|
||||
from ipaddress import ip_address, ip_network
|
||||
@ -37,6 +36,7 @@ from app.gateway.auth.session_cookie_state import SKIP_AUTH_CSRF_COOKIE_STATE_AT
|
||||
from app.gateway.auth.user_provisioning import get_or_provision_oidc_user
|
||||
from app.gateway.csrf_middleware import CSRF_COOKIE_NAME, _request_origin, auth_csrf_cookie_settings, generate_csrf_token, is_secure_request
|
||||
from app.gateway.deps import get_current_user_from_request, get_local_provider
|
||||
from app.gateway.utils import constant_time_equals
|
||||
from deerflow.config.auth_config import OIDCProviderConfig
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@ -1031,7 +1031,7 @@ async def oauth_callback(
|
||||
if not state_payload:
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Missing or expired OIDC state cookie")
|
||||
|
||||
if not secrets.compare_digest(state_payload.state, state):
|
||||
if not constant_time_equals(state_payload.state, state):
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="OIDC state mismatch")
|
||||
|
||||
# ── Resolve redirect URI ─────────────────────────────────────────
|
||||
|
||||
@ -35,6 +35,7 @@ from typing import Any
|
||||
from fastapi import APIRouter, Header, HTTPException, Request
|
||||
|
||||
from app.gateway.github.dispatcher import fanout_event
|
||||
from app.gateway.utils import constant_time_equals
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -110,7 +111,7 @@ def _verify_signature(secret: str, body: bytes, signature_header: str | None) ->
|
||||
return False
|
||||
provided = signature_header.removeprefix("sha256=").strip()
|
||||
expected = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest()
|
||||
return hmac.compare_digest(provided, expected)
|
||||
return constant_time_equals(provided, expected)
|
||||
|
||||
|
||||
def _summarise_event(event: str, payload: dict[str, Any]) -> str:
|
||||
|
||||
@ -1,6 +1,20 @@
|
||||
"""Shared utility helpers for the Gateway layer."""
|
||||
|
||||
import hmac
|
||||
|
||||
|
||||
def sanitize_log_param(value: str) -> str:
|
||||
"""Strip control characters to prevent log injection."""
|
||||
return value.replace("\n", "").replace("\r", "").replace("\x00", "")
|
||||
|
||||
|
||||
def constant_time_equals(a: str, b: str) -> bool:
|
||||
"""Compare two secret strings in constant time, never raising on content.
|
||||
|
||||
``hmac.compare_digest`` raises ``TypeError`` for ``str`` operands holding
|
||||
non-ASCII characters, and headers, cookies and query values are client
|
||||
controlled, so a stray byte would turn a rejection into a 500. Comparing
|
||||
the UTF-8 encodings keeps the same result for ASCII input; ``surrogatepass``
|
||||
lets even a lone surrogate encode instead of raising.
|
||||
"""
|
||||
return hmac.compare_digest(a.encode("utf-8", "surrogatepass"), b.encode("utf-8", "surrogatepass"))
|
||||
|
||||
@ -447,6 +447,17 @@ def test_protected_post_with_internal_auth_header_passes():
|
||||
assert res.status_code == 200
|
||||
|
||||
|
||||
def test_protected_post_with_non_ascii_internal_auth_header_returns_401(client):
|
||||
from app.gateway.internal_auth import INTERNAL_AUTH_HEADER_NAME
|
||||
|
||||
res = client.post(
|
||||
"/api/threads/abc/runs/stream",
|
||||
headers={INTERNAL_AUTH_HEADER_NAME: "forged-tok\xe9n".encode("latin-1")},
|
||||
)
|
||||
|
||||
assert res.status_code == 401
|
||||
|
||||
|
||||
# ── Method matrix: PUT/DELETE/PATCH also protected ────────────────────────
|
||||
|
||||
|
||||
|
||||
@ -276,6 +276,24 @@ def test_non_auth_mutation_rejects_mismatched_double_submit_token():
|
||||
assert response.json()["detail"] == "CSRF token mismatch."
|
||||
|
||||
|
||||
def test_non_auth_mutation_rejects_non_ascii_double_submit_token():
|
||||
# Starlette decodes header bytes as latin-1, so a raw 0xE9 reaches the
|
||||
# comparison as "é"; that must be a 403 mismatch, not a 500.
|
||||
client = TestClient(_make_app(), base_url="https://deerflow.example")
|
||||
client.cookies.set("csrf_token", "cookie-token")
|
||||
|
||||
response = client.post(
|
||||
"/api/threads/abc/runs/stream",
|
||||
headers={
|
||||
"Origin": "https://deerflow.example",
|
||||
"X-CSRF-Token": "cookie-tok\xe9n".encode("latin-1"),
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json()["detail"] == "CSRF token mismatch."
|
||||
|
||||
|
||||
def test_channel_posts_require_double_submit_csrf():
|
||||
client = TestClient(_make_app(), base_url="https://deerflow.example")
|
||||
|
||||
|
||||
21
backend/tests/test_gateway_utils.py
Normal file
21
backend/tests/test_gateway_utils.py
Normal file
@ -0,0 +1,21 @@
|
||||
"""Tests for shared Gateway utility helpers."""
|
||||
|
||||
import pytest
|
||||
|
||||
from app.gateway.utils import constant_time_equals
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("a", "b", "expected"),
|
||||
[
|
||||
("token", "token", True),
|
||||
("token", "other", False),
|
||||
("tok\xe9n", "tok\xe9n", True),
|
||||
("token", "tok\xe9n", False),
|
||||
("tok\xe9n", "token", False),
|
||||
("tok\ud800n", "token", False),
|
||||
("", "", True),
|
||||
],
|
||||
)
|
||||
def test_constant_time_equals_never_raises_on_non_ascii(a, b, expected):
|
||||
assert constant_time_equals(a, b) is expected
|
||||
@ -315,6 +315,21 @@ def test_signature_mismatch_returns_401(client: TestClient) -> None:
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
def test_non_ascii_signature_returns_401(client: TestClient) -> None:
|
||||
body = b'{"zen": "x"}'
|
||||
response = client.post(
|
||||
"/api/webhooks/github",
|
||||
content=body,
|
||||
headers={
|
||||
"X-GitHub-Event": "ping",
|
||||
"X-GitHub-Delivery": DELIVERY_ID,
|
||||
"X-Hub-Signature-256": ("sha256=" + "\xe9" * 64).encode("latin-1"),
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
def test_signature_verified_against_exact_bytes(client: TestClient) -> None:
|
||||
"""Signature must be computed over the request body bytes, not
|
||||
re-serialised JSON. Whitespace and key ordering matter."""
|
||||
|
||||
@ -764,6 +764,22 @@ def test_csrf_post_mismatched_token():
|
||||
assert "mismatch" in str(exc.value.detail)
|
||||
|
||||
|
||||
def test_csrf_post_non_ascii_token_is_a_mismatch():
|
||||
"""POST with a non-ASCII CSRF header → 403 mismatch, not TypeError."""
|
||||
with pytest.raises(Auth.exceptions.HTTPException) as exc:
|
||||
asyncio.run(
|
||||
authenticate(
|
||||
_req(
|
||||
method="POST",
|
||||
cookies={"access_token": "some-jwt", "csrf_token": "real-token"},
|
||||
headers={"x-csrf-token": "real-tok\xe9n"},
|
||||
)
|
||||
)
|
||||
)
|
||||
assert exc.value.status_code == 403
|
||||
assert "mismatch" in str(exc.value.detail)
|
||||
|
||||
|
||||
def test_csrf_post_matching_token_proceeds_to_jwt():
|
||||
"""POST with matching CSRF tokens passes CSRF check, then fails on JWT."""
|
||||
with pytest.raises(Auth.exceptions.HTTPException) as exc:
|
||||
|
||||
@ -405,3 +405,33 @@ def test_oidc_redirect_uri_fallback_plain_host_when_no_proxy_headers():
|
||||
result = _resolve_oidc_redirect_uri(req, "keycloak", cfg)
|
||||
|
||||
assert result == "http://localhost:8001/api/v1/auth/callback/keycloak"
|
||||
|
||||
|
||||
def test_oidc_callback_rejects_non_ascii_state_as_mismatch(monkeypatch):
|
||||
"""A percent-encoded non-ASCII ``state`` is a 403 mismatch, not a 500."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
import deerflow.config.app_config as app_config_module
|
||||
from app.gateway.auth.config import AuthConfig, get_auth_config, set_auth_config
|
||||
from app.gateway.auth.oidc_state import OIDCStatePayload, _sign_state_payload
|
||||
from app.gateway.routers import auth as auth_router
|
||||
|
||||
previous_auth_config = get_auth_config()
|
||||
set_auth_config(AuthConfig(jwt_secret="test-secret-key-for-oidc-state-mismatch-min-32"))
|
||||
try:
|
||||
oidc_config = SimpleNamespace(enabled=True, providers={"keycloak": _provider_config()})
|
||||
monkeypatch.setattr(app_config_module, "get_app_config", lambda: SimpleNamespace(auth=SimpleNamespace(oidc=oidc_config)))
|
||||
app = FastAPI()
|
||||
app.include_router(auth_router.router)
|
||||
client = TestClient(app)
|
||||
client.cookies.set("df_oidc_state_keycloak", _sign_state_payload(OIDCStatePayload(provider="keycloak", state="expected-state")))
|
||||
|
||||
response = client.get("/api/v1/auth/callback/keycloak", params={"code": "code", "state": "expected-st\xe4te"})
|
||||
finally:
|
||||
set_auth_config(previous_auth_config)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json()["detail"] == "OIDC state mismatch"
|
||||
|
||||
@ -558,6 +558,10 @@ async def test_auth_middleware(monkeypatch: pytest.MonkeyPatch, provisioner_modu
|
||||
r = await client.get("/api/sandboxes", headers={"X-API-Key": "wrong-key"})
|
||||
assert r.status_code == 401
|
||||
|
||||
# /api/* with a non-ASCII key → 401, not a TypeError from compare_digest
|
||||
r = await client.get("/api/sandboxes", headers={"X-API-Key": "test-secr\xe9t".encode("latin-1")})
|
||||
assert r.status_code == 401
|
||||
|
||||
# /api/* with correct key → not 401 (auth passed; handler runs with the K8s mock)
|
||||
r = await client.get("/api/sandboxes", headers={"X-API-Key": "test-secret"})
|
||||
assert r.status_code != 401
|
||||
|
||||
@ -450,8 +450,10 @@ app = FastAPI(title="DeerFlow Sandbox Provisioner", lifespan=lifespan)
|
||||
@app.middleware("http")
|
||||
async def verify_api_key(request: Request, call_next):
|
||||
if request.url.path.startswith("/api/"):
|
||||
key = request.headers.get("X-API-Key", "")
|
||||
if not PROVISIONER_API_KEY or not secrets.compare_digest(key, PROVISIONER_API_KEY):
|
||||
# Compare bytes: compare_digest raises TypeError for non-ASCII str,
|
||||
# and header values are client-controlled (decoded as latin-1).
|
||||
key = request.headers.get("X-API-Key", "").encode("utf-8", "surrogatepass")
|
||||
if not PROVISIONER_API_KEY or not secrets.compare_digest(key, PROVISIONER_API_KEY.encode("utf-8", "surrogatepass")):
|
||||
logger.warning("provisioner auth rejected: %s %s", request.method, request.url.path)
|
||||
return Response(status_code=401, content="Unauthorized")
|
||||
return await call_next(request)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user