diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 1c145b3a0..e578efa5d 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,11 @@ **Vulnerability:** User-provided string fields (like project and connection names) lacked strict validation against control characters, only relying on length constraints. **Learning:** This could potentially lead to Log Injection (CRLF injection), Null Byte Injection, or terminal escape injection if these strings are subsequently logged or rendered directly. **Prevention:** Use explicit regex validation `pattern=r'^[^\x00-\x1F\x7F]+$'` on Pydantic string fields to strictly reject control characters. +## 2025-02-18 - JWT crit 헤더 검증 강제 +**Vulnerability:** JWT `crit` (critical) 헤더가 존재할 때, RFC 7515에 따라 이를 명시적으로 검증하지 않으면 인지하지 못하는 중요 확장이 포함된 토큰을 허용하게 되어 보안 스캔(STRIX)을 통과하지 못합니다. +**Learning:** `PyJWT`나 `python-jose`를 사용할 때 `crit` 헤더의 타입 및 길이를 엄격히 검증(길이 제한 리스트 형태)해야 하고, 알 수 없는 확장에 대해서는 반드시 토큰을 거부해야 합니다. +**Prevention:** `_validate_jwt_header` 함수에서 `crit` 헤더 존재 여부를 확인하고, 길이가 제한된 문자열 리스트인지 확인하며, 지원하지 않는 확장(현재 0개)이 있으면 즉시 401을 반환하도록 수정합니다. +## 2026-08-23 - JWT Token Validation Error Message Leakage Fix +**Vulnerability:** The API returned highly specific error messages (e.g., "unknown signing key", "algorithm/key type mismatch", "unsupported token algorithm") during JWT validation, leaking internal implementation details to potential attackers. +**Learning:** Returning specific token validation errors allows attackers to perform reconnaissance on the auth mechanism, testing different vectors to map out the exact token requirements and libraries in use. +**Prevention:** Standardize all token validation failure responses to return a generic HTTP 401 with `detail="invalid token"` to provide no useful feedback to unauthorized requests. diff --git a/backend/app/auth.py b/backend/app/auth.py index d328aa3a7..70d5799b0 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -166,7 +166,7 @@ def _jwt_expiry(claims: dict[str, Any]) -> dt.datetime: exp = claims.get("exp") if not isinstance(exp, int | float): - raise HTTPException(status_code=401, detail="token missing exp") + raise HTTPException(status_code=401, detail="invalid token") return dt.datetime.fromtimestamp(float(exp), tz=dt.timezone.utc) @@ -179,15 +179,27 @@ def _validate_jwt_header(header: dict[str, Any]) -> str: not isinstance(token_type, str) or token_type.strip().lower() not in OIDC_ALLOWED_TOKEN_TYPES ): - raise HTTPException(status_code=401, detail="unsupported token type") + raise HTTPException(status_code=401, detail="invalid token") content_type = header.get("cty") if content_type is not None: - raise HTTPException(status_code=401, detail="unsupported token content type") + raise HTTPException(status_code=401, detail="invalid token") + + crit = header.get("crit") + if crit is not None: + if not isinstance(crit, list): + raise HTTPException(status_code=401, detail="invalid token") + if len(crit) > 5: + raise HTTPException(status_code=401, detail="invalid token") + for param in crit: + if not isinstance(param, str): + raise HTTPException(status_code=401, detail="invalid token") + # Reject all unrecognized parameters. We currently do not support any critical extensions. + raise HTTPException(status_code=401, detail="invalid token") header_alg_raw = header.get("alg") if not isinstance(header_alg_raw, str) or not header_alg_raw: - raise HTTPException(status_code=401, detail="token missing alg") + raise HTTPException(status_code=401, detail="invalid token") return header_alg_raw.upper() @@ -240,13 +252,13 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]: try: header = cast(dict[str, Any], jwt.get_unverified_header(token)) except Exception: # noqa: BLE001 - raise HTTPException(status_code=401, detail="invalid token header") + raise HTTPException(status_code=401, detail="invalid token") header_alg = _validate_jwt_header(header) if header_alg not in OIDC_ALLOWED_ALGORITHMS: raise HTTPException( status_code=401, - detail="unsupported token algorithm", + detail="invalid token", ) jwks = await _get_jwks() @@ -255,20 +267,20 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]: jwks = await _get_jwks(force_refresh=True) jwk = _pick_jwk(jwks, header.get("kid")) if jwk is None: - raise HTTPException(status_code=401, detail="unknown signing key") + raise HTTPException(status_code=401, detail="invalid token") kty = jwk.get("kty") if not isinstance(kty, str): - raise HTTPException(status_code=401, detail="algorithm/key type mismatch") + raise HTTPException(status_code=401, detail="invalid token") jwk_kty = kty.upper() if jwk_kty == "RSA": if not (header_alg.startswith("RS") or header_alg.startswith("PS")): - raise HTTPException(status_code=401, detail="algorithm/key type mismatch") + raise HTTPException(status_code=401, detail="invalid token") elif jwk_kty == "EC": if not header_alg.startswith("ES"): - raise HTTPException(status_code=401, detail="algorithm/key type mismatch") + raise HTTPException(status_code=401, detail="invalid token") else: - raise HTTPException(status_code=401, detail="algorithm/key type mismatch") + raise HTTPException(status_code=401, detail="invalid token") try: claims = jwt.decode( @@ -288,7 +300,7 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]: ) except Exception as err: raise HTTPException( - status_code=401, detail="token verification failed" + status_code=401, detail="invalid token" ) from err return cast(dict[str, Any], claims) @@ -303,13 +315,13 @@ async def _verified_token_from_claims( jwt_id = claims.get("jti") name = claims.get("name") or claims.get("preferred_username") if not isinstance(sub, str): - raise HTTPException(status_code=401, detail="token missing sub") + raise HTTPException(status_code=401, detail="invalid token") if not isinstance(jwt_id, str) or not jwt_id.strip(): - raise HTTPException(status_code=401, detail="token missing jti") + raise HTTPException(status_code=401, detail="invalid token") expires_at = _jwt_expiry(claims) if verify_revocation and await is_token_jti_revoked(jwt_id): - raise HTTPException(status_code=401, detail="token revoked") + raise HTTPException(status_code=401, detail="invalid token") return VerifiedToken( subject=sub, diff --git a/backend/tests/test_auth_security.py b/backend/tests/test_auth_security.py index 9f93eb4f1..598198f38 100644 --- a/backend/tests/test_auth_security.py +++ b/backend/tests/test_auth_security.py @@ -207,7 +207,7 @@ def fail_decode(*_: object, **__: object) -> dict: ) assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "unsupported token algorithm" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio @@ -244,7 +244,7 @@ def fail_decode(*_: object, **__: object) -> dict: await auth._decode_verified_oidc_token("ey...fake...") assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "algorithm/key type mismatch" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio @@ -303,21 +303,19 @@ async def mock_is_token_revoked(jti): @pytest.mark.parametrize( - ("header", "detail"), + ("header",), [ ( {"kid": "key-1", "alg": "RS256", "typ": "nested+jwt"}, - "unsupported token type", ), ( {"kid": "key-1", "alg": "RS256", "cty": "JWT"}, - "unsupported token content type", ), ], ) @pytest.mark.asyncio async def test_oidc_rejects_unsupported_header_types( - monkeypatch: pytest.MonkeyPatch, header: dict[str, str], detail: str + monkeypatch: pytest.MonkeyPatch, header: dict[str, str] ) -> None: monkeypatch.setattr(settings, "oidc_issuer", "https://issuer.example") monkeypatch.setattr(settings, "oidc_audience", "pg-erd") @@ -334,7 +332,7 @@ async def fail_jwks() -> dict: ) assert exc_info.value.status_code == 401 - assert exc_info.value.detail == detail + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio @@ -417,7 +415,7 @@ async def mock_is_token_revoked2(jti): ) assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "token missing jti" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio @@ -464,7 +462,7 @@ async def mock_revoke(jti, ext): ) assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "token revoked" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio @@ -560,7 +558,7 @@ def mock_get_unverified_header(token): await auth._decode_verified_oidc_token("invalid_token") assert excinfo.value.status_code == 401 - assert excinfo.value.detail == "invalid token header" + assert excinfo.value.detail == "invalid token" @pytest.mark.asyncio @@ -592,7 +590,7 @@ async def mock_is_token_revoked2(jti): await auth._decode_verified_oidc_token("Bearer token") assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "token verification failed" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio async def test_oidc_rejects_algorithm_key_type_mismatch( @@ -625,7 +623,7 @@ def fail_decode(*_: object, **__: object) -> dict: await auth._decode_verified_oidc_token("ey...") assert exc_info.value.status_code == 401 - assert exc_info.value.detail == "algorithm/key type mismatch" + assert exc_info.value.detail == "invalid token" @pytest.mark.asyncio async def test_oidc_jwks_refresh_rate_limiting( monkeypatch: pytest.MonkeyPatch, @@ -733,3 +731,16 @@ async def get(self, url: str) -> _FakeHttpResponse: {"keys": [{"kid": "new-key", "kty": "RSA"}]}, ] assert request_count == before_concurrent_refresh + 1 + +def test_crit_header_validation(): + with pytest.raises(HTTPException, match="invalid token"): + auth._validate_jwt_header({"alg": "RS256", "crit": "not-a-list"}) + + with pytest.raises(HTTPException, match="invalid token"): + auth._validate_jwt_header({"alg": "RS256", "crit": ["a", "b", "c", "d", "e", "f"]}) + + with pytest.raises(HTTPException, match="invalid token"): + auth._validate_jwt_header({"alg": "RS256", "crit": [123]}) + + with pytest.raises(HTTPException, match="invalid token"): + auth._validate_jwt_header({"alg": "RS256", "crit": ["unknown"]})