From 0608a41a077dfe6f79e8dcdc1668cbb2702e622b Mon Sep 17 00:00:00 2001 From: yucheng-berri Date: Wed, 11 Nov 2026 16:22:14 -0800 Subject: [PATCH] fix(fips): hash MCP advisory-lock keys with SHA-256 in fips_mode and compute the OCI body digest as a security hash (#51165) * fix(fips): hash MCP advisory-lock keys with SHA-256 in fips_mode and compute the OCI body digest as a security hash * test(fips): move the lock-key tests into test_mcp_env_vars.py and type the OCI digest test --- litellm/llms/oci/common_utils.py | 12 ++---- litellm/proxy/mcp_server/db.py | 24 ++++++----- tests/unit/llms/oci/test_oci_common_utils.py | 14 ++++++ .../proxy/mcp_server/test_mcp_env_vars.py | 43 +++++++++++++++++++ 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/litellm/llms/oci/common_utils.py b/litellm/llms/oci/common_utils.py index fe8d0e1c1e7..a545fc26481 100644 --- a/litellm/llms/oci/common_utils.py +++ b/litellm/llms/oci/common_utils.py @@ -97,15 +97,11 @@ class OCIRequestWrapper: def sha256_base64(data: bytes) -> str: - # SHA-256 is used here to compute the x-content-sha256 header required by the - # OCI HTTP signing specification (RSA-SHA256 request signing), not for password - # or secret hashing. This is the correct and mandated algorithm for this purpose. + # Computes the x-content-sha256 header required by the OCI HTTP signing + # specification. The RSA-SHA256 request signature covers this header, so the + # digest is a security use of the hash. # See: https://docs.oracle.com/en-us/iaas/Content/API/Concepts/signingrequests.htm - # - # ``usedforsecurity=False`` declares non-security intent to static analyzers - # (CodeQL ``py/weak-sensitive-data-hashing``) — without it the request body - # gets flagged as "password-like data" via taint tracking. - digest: Final = hashlib.sha256(data, usedforsecurity=False).digest() # noqa: S324 + digest: Final = hashlib.sha256(data).digest() return base64.b64encode(digest).decode() diff --git a/litellm/proxy/mcp_server/db.py b/litellm/proxy/mcp_server/db.py index f29acc3b380..08e1ffd5333 100644 --- a/litellm/proxy/mcp_server/db.py +++ b/litellm/proxy/mcp_server/db.py @@ -35,6 +35,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( # noqa: F401 # encrypt_value_helper, get_salt_key, ) +from litellm.proxy.common_utils.fips import fips_mode_active from litellm.proxy.mcp_server.oauth_identity_binding import ( RefreshTokenPresented, credential_binding_matches, @@ -690,15 +691,22 @@ async def find_mcp_server_identifier_conflict( ) +def _advisory_lock_key(name: str) -> int: + """Signed 64-bit Postgres advisory-lock key for ``name``: BLAKE2b-64, or the + first 8 bytes of SHA-256 when general_settings.fips_mode is on.""" + digest: Final = ( + hashlib.sha256(name.encode()).digest()[:8] + if fips_mode_active() + else hashlib.blake2b(name.encode(), digest_size=8).digest() + ) + return int.from_bytes(digest, "big", signed=True) + + def _mcp_identifier_lock_keys(*identifiers: str | None) -> tuple[int, ...]: """Deterministic advisory-lock keys for the lowercased identifiers, sorted so concurrent requests for the same pair always lock in the same order.""" return tuple( - int.from_bytes( - hashlib.blake2b(f"mcp_identifier:{normalized}".encode(), digest_size=8).digest(), - "big", - signed=True, - ) + _advisory_lock_key(f"mcp_identifier:{normalized}") for normalized in sorted(frozenset(value.lower() for value in identifiers if value)) ) @@ -2515,11 +2523,7 @@ async def merge_user_env_vars( so an admin retiring a user-scoped variable also clears its stored value. """ allowed: Final = set(allowed_names) - lock_key: Final = int.from_bytes( - hashlib.blake2b(f"{user_id}:{server_id}".encode(), digest_size=8).digest(), - "big", - signed=True, - ) + lock_key: Final = _advisory_lock_key(f"{user_id}:{server_id}") async with _db_transaction_manager(prisma_client) as tx: await tx.execute_raw("SELECT pg_advisory_xact_lock($1::bigint)", lock_key) row: Final[prisma_db_models.LiteLLM_MCPUserEnvVars | None] = await tx.litellm_mcpuserenvvars.find_unique( diff --git a/tests/unit/llms/oci/test_oci_common_utils.py b/tests/unit/llms/oci/test_oci_common_utils.py index e66645c4dcd..34e66629120 100644 --- a/tests/unit/llms/oci/test_oci_common_utils.py +++ b/tests/unit/llms/oci/test_oci_common_utils.py @@ -54,6 +54,20 @@ def test_sha256_base64_known_value(): assert sha256_base64(data) == expected +def test_sha256_base64_asks_for_a_security_digest(monkeypatch: pytest.MonkeyPatch) -> None: + import hashlib + + real_sha256: Final = hashlib.sha256 + + def security_only_sha256(data: bytes = b"", *, usedforsecurity: bool = True) -> "hashlib._Hash": + assert usedforsecurity, "usedforsecurity=False lets a provider outside the FIPS module serve the digest" + return real_sha256(data) + + monkeypatch.setattr(hashlib, "sha256", security_only_sha256) + + assert sha256_base64(b"hello") == "LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ=" + + def test_sha256_base64_empty(): result = sha256_base64(b"") assert isinstance(result, str) diff --git a/tests/unit/proxy/mcp_server/test_mcp_env_vars.py b/tests/unit/proxy/mcp_server/test_mcp_env_vars.py index 57ac6c0be2d..b3aebe49968 100644 --- a/tests/unit/proxy/mcp_server/test_mcp_env_vars.py +++ b/tests/unit/proxy/mcp_server/test_mcp_env_vars.py @@ -1748,3 +1748,46 @@ async def test_missing_user_env_vars_error_renders_in_mcp_call_tool(): assert "CorporateDB" in text assert "CORP_USERNAME" in text assert "fill_env_vars=srv-99" in text + + +@pytest.fixture +def blake2b_refused(monkeypatch: pytest.MonkeyPatch) -> None: + """hashlib.blake2b as FIPS Python builds ship it: the call raises.""" + import hashlib + from typing import NoReturn + + def blake2b(*_args: object, **_kwargs: object) -> NoReturn: + raise ValueError("blake2b is blocked when usedforsecurity=True") + + monkeypatch.setattr(hashlib, "blake2b", blake2b) + + +def test_identifier_lock_keys_are_unchanged_without_fips_mode() -> None: + from litellm.proxy.mcp_server.db import _mcp_identifier_lock_keys + + assert _mcp_identifier_lock_keys("GitHub", None, "gh") == (-1720930101670657375, -1498181317228972674) + + +@pytest.mark.usefixtures("fips_mode", "blake2b_refused") +def test_fips_mode_identifier_lock_keys_are_truncated_sha256() -> None: + import hashlib + + from litellm.proxy.mcp_server.db import _mcp_identifier_lock_keys + + assert _mcp_identifier_lock_keys("GitHub", None, "gh") == tuple( + int.from_bytes(hashlib.sha256(name).digest()[:8], "big", signed=True) + for name in (b"mcp_identifier:gh", b"mcp_identifier:github") + ) + assert _mcp_identifier_lock_keys("GitHub") == (-7731150984991732978,) + + +@pytest.mark.asyncio +@pytest.mark.usefixtures("fips_mode", "blake2b_refused", "env_vars_salt_key") +async def test_fips_mode_merge_user_env_vars_takes_its_lock_without_blake2b() -> None: + from litellm.proxy.mcp_server.db import merge_user_env_vars + + merged: Final = await merge_user_env_vars( + _transactional_env_vars_prisma(), "alice", "srv-1", {"TOKEN": "abc"}, allowed_names=["TOKEN"] + ) + + assert merged == {"TOKEN": "abc"}