Skip to content

Commit 3540fc9

Browse files
committed
Resolve the MCP signing key
Three loose ends from the design's signing-key step. The derived signing and storage keys are only as strong as the client secret they come from, and nothing checked it. The service now refuses to start below 32 characters. Identity providers issue well above that; the check exists to reject a hand-written placeholder. The deployment guide already told operators the service enforced this -- it did not. derive_jwt_key was imported from fastmcp.server.auth.jwt_issuer, a private symbol with no deprecation contract: a minor release could drop it, or change its derivation and silently invalidate every stored registration and token. The derivation is now local, with the same HKDF parameters, so the bytes are identical and existing state stays readable. test_auth asserts that equivalence against FastMCP directly, so a change on their side surfaces as a failure rather than as undecryptable Redis state. The production-readiness warning said the chart exposes no alternative to the derived key and told operators to assess the limitation before rolling out. That framing does not survive reading FastMCP 3.4.7: high-entropy material goes through HKDF, and the PBKDF2 path with a length warning is reserved for low-entropy operator strings. The warning is replaced with the entropy requirement the design states.
1 parent ebc165b commit 3540fc9

3 files changed

Lines changed: 63 additions & 21 deletions

File tree

docs/deployment_guide/advanced_config/mcp.rst

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,16 +147,13 @@ sessions and makes old encrypted state, including DCR registrations, unusable.
147147
Users must authenticate again, and DCR clients might need to remove and re-add
148148
the MCP entry before login.
149149

150-
.. warning::
151-
152-
OSMO currently relies on FastMCP's default derived signing key to avoid a
153-
second operator-managed secret. FastMCP documents that default as a
154-
development or local-testing convenience and recommends an explicit
155-
independent signing key for production. The current OSMO chart does not
156-
expose that independent-key option. Assess this limitation before a
157-
production rollout and require a high-entropy upstream client secret. See
158-
the `FastMCP OIDC proxy signing-key guidance
159-
<https://gofastmcp.com/servers/auth/oidc-proxy#param-jwt-signing-key>`_.
150+
Because both keys come from the client secret, the strength they provide is the
151+
strength of that secret. FastMCP passes it through HKDF as high-entropy key
152+
material, a path distinct from the password-based derivation it reserves for
153+
low-entropy operator-supplied strings. The service therefore requires a client
154+
secret of at least 32 characters and refuses to start below it. Identity
155+
providers issue secrets well above that length; the check exists to reject a
156+
hand-written placeholder.
160157

161158
Configure Helm Values
162159
---------------------

src/service/mcp/auth.py

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616
SPDX-License-Identifier: Apache-2.0
1717
"""
1818

19+
import base64
1920
import dataclasses
2021
from typing import cast
2122
from urllib import parse
2223

2324
from cryptography.fernet import Fernet
24-
from fastmcp.server.auth.jwt_issuer import derive_jwt_key
25+
from cryptography.hazmat.primitives import hashes
26+
from cryptography.hazmat.primitives.kdf.hkdf import HKDF
2527
from fastmcp.server.auth.oidc_proxy import OIDCProxy
2628
from fastmcp.server.auth.providers.jwt import JWTVerifier
2729
from key_value.aio.protocols import AsyncKeyValue
@@ -78,6 +80,11 @@ def get_token_verifier( # pylint: disable=unused-argument
7880
audience=self._access_token_audience,
7981
required_scopes=required_scopes,
8082
)
83+
# The derived signing and storage keys are only as strong as the secret
84+
# they come from. Identity providers issue well above this; the check
85+
# exists to reject a hand-written placeholder.
86+
_MIN_CLIENT_SECRET_LENGTH = 32
87+
8188
_REQUIRED_WHEN_AUTH_ENABLED = (
8289
'resource_url',
8390
'redis_url',
@@ -223,6 +230,13 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime:
223230
cast(str, config.oidc_client_secret_file),
224231
'OIDC client secret',
225232
)
233+
if len(client_secret) < _MIN_CLIENT_SECRET_LENGTH:
234+
raise ValueError(
235+
'OIDC client secret must be at least '
236+
f'{_MIN_CLIENT_SECRET_LENGTH} characters: both the proxy token '
237+
'signing key and the Redis storage key are derived from it, so '
238+
'its entropy is their entropy'
239+
)
226240
redis_client = redis_asyncio.Redis.from_url(
227241
cast(str, config.redis_url),
228242
password=_read_optional_secret(config.redis_password_file),
@@ -281,14 +295,32 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime:
281295
return MCPAuthRuntime(provider, redis_client)
282296

283297

298+
def _derive_fernet_key(material: str, *, salt: str) -> bytes:
299+
"""Derive a Fernet key from high-entropy material.
300+
301+
Reproduces FastMCP's own derivation with the same HKDF parameters rather
302+
than importing ``fastmcp.server.auth.jwt_issuer.derive_jwt_key``, which is
303+
private and carries no deprecation contract. The bytes are identical, so
304+
state encrypted before this change stays readable; ``test_auth`` asserts
305+
that equivalence against FastMCP directly.
306+
"""
307+
derived = HKDF(
308+
algorithm=hashes.SHA256(),
309+
length=32,
310+
salt=salt.encode(),
311+
info=b'Fernet',
312+
).derive(material.encode())
313+
return base64.urlsafe_b64encode(derived)
314+
315+
284316
def _storage_encryption_key(client_secret: str) -> bytes:
285317
"""Mirror FastMCP's default signing and storage key derivation."""
286-
signing_key = derive_jwt_key(
287-
high_entropy_material=client_secret,
318+
signing_key = _derive_fernet_key(
319+
client_secret,
288320
salt='fastmcp-jwt-signing-key',
289321
)
290-
return derive_jwt_key(
291-
high_entropy_material=signing_key.decode('ascii'),
322+
return _derive_fernet_key(
323+
signing_key.decode('ascii'),
292324
salt='fastmcp-storage-encryption-key',
293325
)
294326

src/service/mcp/tests/test_auth.py

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import unittest
2323
from unittest import mock
2424

25+
from fastmcp.server.auth.jwt_issuer import derive_jwt_key
2526
from fastmcp.server.auth.oidc_proxy import OIDCConfiguration, OIDCProxy
2627
from fastmcp.server.auth.providers.jwt import JWTVerifier
2728
from fastmcp.server.auth.oauth_proxy.models import UpstreamTokenSet
@@ -31,6 +32,10 @@
3132

3233
from src.service.mcp import auth, server
3334

35+
# Long enough to satisfy the client-secret entropy check; identity
36+
# providers issue secrets of this order.
37+
_TEST_CLIENT_SECRET = 'client-secret-0123456789abcdef0123456789'
38+
3439

3540
class MCPAuthConfigTest(unittest.TestCase):
3641
def test_authentication_is_not_optional(self) -> None:
@@ -97,7 +102,7 @@ async def test_issuer_falls_back_to_the_discovery_document(self) -> None:
97102
Only an Entra v1 resource application needs it configured, so a
98103
deployment whose discovery issuer is the real one supplies nothing.
99104
"""
100-
with _secret_file('client-secret') as client_secret_file:
105+
with _secret_file(_TEST_CLIENT_SECRET) as client_secret_file:
101106
config = _config(
102107
oidc_client_secret_file=client_secret_file,
103108
oidc_access_token_issuer=None,
@@ -152,7 +157,7 @@ async def test_issuer_falls_back_to_the_discovery_document(self) -> None:
152157
)
153158

154159
async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> None:
155-
with _secret_file('client-secret') as client_secret_file:
160+
with _secret_file(_TEST_CLIENT_SECRET) as client_secret_file:
156161
config = _config(
157162
oidc_client_secret_file=client_secret_file,
158163
)
@@ -208,8 +213,8 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N
208213
self.assertIsInstance(provider, OIDCProxy)
209214
self.assertEqual(
210215
provider._jwt_signing_key, # pylint: disable=protected-access
211-
auth.derive_jwt_key(
212-
high_entropy_material='client-secret',
216+
derive_jwt_key(
217+
high_entropy_material=_TEST_CLIENT_SECRET,
213218
salt='fastmcp-jwt-signing-key',
214219
),
215220
)
@@ -384,6 +389,14 @@ async def __aexit__(self, *args: object) -> None:
384389
await runtime.aclose()
385390
redis_client.aclose.assert_awaited_once()
386391

392+
def test_short_client_secret_fails_at_startup(self) -> None:
393+
"""The derived keys are only as strong as the secret behind them."""
394+
with _secret_file('too-short') as client_secret_file:
395+
config = _config(oidc_client_secret_file=client_secret_file)
396+
with self.assertRaises(ValueError) as caught:
397+
auth.create_auth_runtime(config)
398+
self.assertIn('at least 32 characters', str(caught.exception))
399+
387400
def test_storage_key_matches_fastmcp_default_and_is_deterministic(self) -> None:
388401
first = auth._storage_encryption_key( # pylint: disable=protected-access
389402
'client-secret',
@@ -394,11 +407,11 @@ def test_storage_key_matches_fastmcp_default_and_is_deterministic(self) -> None:
394407
different = auth._storage_encryption_key( # pylint: disable=protected-access
395408
'rotated-client-secret',
396409
)
397-
signing_key = auth.derive_jwt_key(
410+
signing_key = derive_jwt_key(
398411
high_entropy_material='client-secret',
399412
salt='fastmcp-jwt-signing-key',
400413
)
401-
expected = auth.derive_jwt_key(
414+
expected = derive_jwt_key(
402415
high_entropy_material=signing_key.decode('ascii'),
403416
salt='fastmcp-storage-encryption-key',
404417
)

0 commit comments

Comments
 (0)