Skip to content

OAuth metadata issuer/token_endpoint derived from unvalidated X-Forwarded-Host (chart defaults leave SIE_MCP_PUBLIC_URL unpinned) #275

Description

@elfrost

Summary

sie_mcp derives its own public origin from the X-Forwarded-Host / X-Forwarded-Proto request headers when SIE_MCP_PUBLIC_URL is unset, with no allowlist. That origin is stamped into the RFC 8414 and RFC 9728 discovery documents — issuer, authorization_endpoint, token_endpoint, registration_endpoint, authorization_servers — and into the WWW-Authenticate challenge. A caller can therefore choose the authorization server that OAuth clients discover.

mcpEdge.publicUrl defaults to "" and mcpEdge.oauthEnabled defaults to true, so the chart's own defaults produce the unpinned combination.

Where

packages/sie_mcp/src/sie_mcp/auth.py:36

def base_url(config: MCPConfig, *, scheme: str, headers: Headers) -> str:
    if config.public_base_url:
        return config.public_base_url
    proto = headers.get("x-forwarded-proto") or scheme
    host = headers.get("x-forwarded-host") or headers.get("host") or ""
    return f"{proto}://{host}"

Called from oauth.py:233 (_protected_resource) and oauth.py:238 (_authorization_server), whose routes are both in _EXEMPT_PATHS — so they answer before any credential check, and the input needs no credential. Also from auth.py:109, building the WWW-Authenticate challenge on every unauthenticated /mcp request.

Reproduce

Real build_oauth_routes + real ConnectorSecretAuthMiddleware, under the chart's defaults (SIE_MCP_PUBLIC_URL unset, OAuth enabled):

import os, sys
sys.path.insert(0, "packages/sie_mcp/src")
os.environ.pop("SIE_MCP_PUBLIC_URL", None)
os.environ.pop("SIE_MCP_OAUTH_ENABLED", None)
os.environ["SIE_MCP_CONNECTOR_SECRETS"] = "s3cr3t:alice"

from starlette.applications import Starlette
from starlette.testclient import TestClient
from sie_mcp.config import MCPConfig
from sie_mcp.oauth import build_oauth_routes
from sie_mcp.auth import ConnectorSecretAuthMiddleware

config = MCPConfig.from_env()
app = Starlette(routes=list(build_oauth_routes(config)))
app.add_middleware(ConnectorSecretAuthMiddleware, config=config)
client = TestClient(app, base_url="https://mcp.victim.example")

for hdrs in ({}, {"X-Forwarded-Host": "evil.attacker.example", "X-Forwarded-Proto": "https"}):
    body = client.get("/.well-known/oauth-authorization-server", headers=hdrs).json()
    print(hdrs or "(control)", "->", body["issuer"], body["token_endpoint"])
(control)                            -> https://mcp.victim.example       https://mcp.victim.example/token
{'X-Forwarded-Host': 'evil...'}      -> https://evil.attacker.example    https://evil.attacker.example/token

The control matters: with the header absent the code is correct, so this is header-driven rather than a bad config.

Impact

On this bridge the authorize page is where a user types their connector secret. A client that follows poisoned metadata sends its user to an attacker-controlled /authorize to type that credential. The redirect-URI allowlist is implemented correctly (exact match, defaults to claude.ai's callback) but cannot help here, because the victim never reaches the real authorize endpoint.

What already mitigates it

Worth stating, because it bounds the severity: mcp-edge-ingress.yaml marks mcpEdge.ingress.host as required, and ingress-nginx overwrites X-Forwarded-Host with $best_http_host by default — through the chart's default ingress this does not land. The exposed paths are:

  • sie-mcp serve / mise run mcp-serve, which bind 0.0.0.0:8088 under uvicorn with no proxy;
  • an ingress-nginx controller with use-forwarded-headers: "true" (common behind an external LB);
  • any cache in front of the .well-known routes that does not key on X-Forwarded-Host.

I'd call it Medium rather than High for exactly those reasons.

Suggested fix

The docs already say to pin SIE_MCP_PUBLIC_URL — but both places give the reason as URL stability, not security:

Pin SIE_MCP_PUBLIC_URL … so the OAuth metadata URLs are stable (otherwise they are derived per-request from forwarded host/proto headers). — plugin/superlinked.md:93
This ensures that OAuth metadata contains stable public URLs. — README.md:399

An operator whose URLs look fine reads that as optional. Two changes that don't break anything:

  1. Default the chart value from the host it already requires. mcpEdge.ingress.host is mandatory when the ingress is on, so mcp-edge-deployment.yaml can render SIE_MCP_PUBLIC_URL=https://<that host> when mcpEdge.publicUrl is empty. No new setting; the shipped default stops being the unpinned one.
  2. Bound the fallback — validate the derived host against an allowlist, or only honour x-forwarded-* behind an explicit trusted-proxy flag, plus a startup warning naming the consequence.

I deliberately did not propose making SIE_MCP_PUBLIC_URL required when OAuth is enabled: tools/mise_tasks/mcp-serve.bash doesn't set it, so failing closed would break your own documented dev command.

Happy to open a PR for (1) if that's the direction you'd want.


Found while reading the OAuth bridge; the rest of it is notably careful (S256-only PKCE, compare_digest, single-use client-bound codes, exact-match redirect allowlist, and the "never logged" docstring claim holds — oauth.py has zero logging calls). Full write-up: https://elfrost.github.io/ai-patchlab/scans/superlinked-sie.html

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions