From e8610a498921d56399c64a2549e8180c2fb4ead2 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:22:54 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20JWT=20=EA=B2=80=EC=A6=9D=20=EC=A0=95=EB=B3=B4=20?= =?UTF-8?q?=EB=85=B8=EC=B6=9C=20=EB=B0=8F=20crit=20=ED=97=A4=EB=8D=94=20?= =?UTF-8?q?=EA=B2=80=EC=A6=9D=20=EB=88=84=EB=9D=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 5 ++++ backend/app/auth.py | 41 ++++++++++++++++++----------- backend/tests/test_auth_crit.py | 33 +++++++++++++++++++++++ backend/tests/test_auth_security.py | 18 ++++++------- 4 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 backend/tests/test_auth_crit.py diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 1c145b3a0..b68987031 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,8 @@ **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-27 - JWT 검증 시 Critical (crit) 헤더 필수 검증 및 에러 메시지 정보 노출 방지 +**Vulnerability:** JWT 파싱 중 `crit` 헤더를 검증하지 않아 RFC 7515 요구사항과 STRIX 보안 점검을 우회할 수 있었고, JWT 검증 실패 시 자세한 에러 메시지가 반환되어 정보가 유출될 위험이 존재했습니다. +**Learning:** `crit` 헤더는 안전하게 길이와 내용이 제한된 문자열 리스트로 검증되어야 하며, 인가 실패 시 공격자에게 너무 자세한 검증 실패 사유(예: "token missing exp" 등)를 노출해서는 안 됩니다. +**Prevention:** `_validate_jwt_header`에서 `crit` 배열의 존재와 형태를 엄격하게 검증하고, 모든 JWT 예외 처리에서 자세한 사유 대신 HTTP 401 `"invalid token"`을 반환하여 내부 검증 로직 노출을 차단합니다. diff --git a/backend/app/auth.py b/backend/app/auth.py index d328aa3a7..c04582431 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,26 @@ 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) or len(crit) == 0 or len(crit) > 10: + raise HTTPException(status_code=401, detail="invalid token") + for item in crit: + if not isinstance(item, str): + raise HTTPException(status_code=401, detail="invalid token") + # For this app, we don't recognize ANY critical extensions. + # If `crit` is present and specifies any extensions, we MUST reject the token. + 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 +251,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 +266,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 +299,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 +314,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_crit.py b/backend/tests/test_auth_crit.py new file mode 100644 index 000000000..2cf0c190a --- /dev/null +++ b/backend/tests/test_auth_crit.py @@ -0,0 +1,33 @@ +import pytest +from fastapi import HTTPException +from app.auth import _validate_jwt_header + +def test_crit_validates_as_list(): + with pytest.raises(HTTPException) as exc_info: + _validate_jwt_header({"alg": "RS256", "crit": "not-a-list"}) + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "invalid token" + +def test_crit_rejects_empty_list(): + with pytest.raises(HTTPException) as exc_info: + _validate_jwt_header({"alg": "RS256", "crit": []}) + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "invalid token" + +def test_crit_rejects_long_list(): + with pytest.raises(HTTPException) as exc_info: + _validate_jwt_header({"alg": "RS256", "crit": ["item"] * 11}) + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "invalid token" + +def test_crit_rejects_non_string_items(): + with pytest.raises(HTTPException) as exc_info: + _validate_jwt_header({"alg": "RS256", "crit": [123]}) + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "invalid token" + +def test_crit_rejects_unrecognized_items(): + with pytest.raises(HTTPException) as exc_info: + _validate_jwt_header({"alg": "RS256", "crit": ["b64"]}) + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "invalid token" diff --git a/backend/tests/test_auth_security.py b/backend/tests/test_auth_security.py index 9f93eb4f1..24485d6bb 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 @@ -307,11 +307,11 @@ async def mock_is_token_revoked(jti): [ ( {"kid": "key-1", "alg": "RS256", "typ": "nested+jwt"}, - "unsupported token type", + "invalid token", ), ( {"kid": "key-1", "alg": "RS256", "cty": "JWT"}, - "unsupported token content type", + "invalid token", ), ], ) @@ -417,7 +417,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 +464,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 +560,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 +592,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 +625,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,