Skip to content

[security] fix(proxy): require auth for node management#4579

Open
Hinotoi-agent wants to merge 1 commit into
InternLM:mainfrom
Hinotoi-agent:fix/proxy-node-management-auth
Open

[security] fix(proxy): require auth for node management#4579
Hinotoi-agent wants to merge 1 commit into
InternLM:mainfrom
Hinotoi-agent:fix/proxy-node-management-auth

Conversation

@Hinotoi-agent
Copy link
Copy Markdown

Summary

This PR hardens the proxy node-management boundary so /nodes/* routes are no longer exempt from bearer-token authentication when proxy API keys are configured.

  • Removes the broad /nodes authentication bypass from AuthenticationMiddleware.
  • Preserves authenticated API-server registration by forwarding the configured API key when an API server registers with a protected proxy.
  • Normalizes string api_keys values before middleware setup so a single configured key is treated as one token, not as an iterable of characters.
  • Adds focused regression tests proving node-management routes require auth while passive health checks remain public.

Security issues covered

Issue Impact Severity
Proxy node-management routes bypass API-key auth An unauthenticated caller can register/remove/terminate backend nodes and steer model traffic to an attacker-controlled node Critical

Before this PR

  • AuthenticationMiddleware skipped every path with the /nodes prefix.
  • /nodes/add, /nodes/remove, /nodes/terminate, and /nodes/status were reachable without a bearer token when proxy API keys were enabled.
  • A caller could register a node by sending a non-empty status.models list, causing the proxy to route matching model requests to the supplied URL.
  • The behavior was not covered by focused middleware tests.

After this PR

  • Only passive public routes (/health, /docs, /redoc) bypass bearer-token auth.
  • /nodes/* routes now require the configured bearer token whenever proxy API keys are enabled.
  • API servers registering with a protected proxy include the configured bearer token in the registration request.
  • Focused tests lock in the expected route-auth behavior.

Why this matters

The proxy node registry is a model-serving control plane. It decides which backend receives /v1/chat/completions and /v1/completions traffic. If unauthenticated callers can mutate that registry, they can steer prompts, generation parameters, and proxied request headers to an arbitrary backend, tamper with completions, or disrupt service by removing/terminating legitimate nodes.

How this differs from related PRs

PR #4338 introduced the shared authentication middleware and documented the /nodes skip prefix as an operational exception for proxy-to-node coordination. This PR is a follow-up hardening change for the management-route trust boundary: it keeps health/docs public, but does not treat mutating node-management endpoints as public coordination traffic.

PR #4354 is an open proxy refactor. This PR is intentionally narrow and applies to current main: it fixes the authentication boundary in the existing middleware and adds a focused regression test for the route behavior.

Attack flow

Unauthenticated network caller
    -> POST /nodes/add with an attacker-controlled backend URL and model name
        -> proxy stores the supplied node as a model backend
            -> later client inference traffic can be routed to the attacker-controlled node
                -> prompt/request disclosure, response tampering, and denial of service

Affected code

Issue Files
Node-management auth bypass lmdeploy/serve/utils/server_utils.py, lmdeploy/serve/proxy/proxy.py, lmdeploy/serve/openai/api_server.py

Root cause

Issue: proxy node-management routes bypass API-key auth

  • The auth middleware exempted the whole /nodes prefix.
  • That prefix includes privileged mutating routes, not only passive health/status endpoints.
  • API-server registration did not include a bearer token, so the previous exemption was used to keep registration working when proxy auth was enabled.

CVSS assessment

Issue CVSS v3.1 Vector
Proxy node-management auth bypass 10.0 Critical CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H

Rationale:

  • A network attacker needs no credentials to mutate a model-serving routing plane on vulnerable deployments.
  • Successful exploitation can disclose confidential prompts/request headers, tamper with model responses, and disrupt service.
  • Scope changes because the attacker-controlled backend receives traffic from users that authenticated to the proxy.

Safe reproduction steps

  1. Start a proxy configuration with API keys enabled.
  2. Send an unauthenticated request to POST /nodes/add with a JSON body containing a backend URL and status.models for an existing model.
  3. Observe vulnerable behavior on pre-patch code: the request is accepted and the supplied node appears in /nodes/status without a bearer token.
  4. Observe fixed behavior after this PR: unauthenticated /nodes/add and /nodes/status return 401; the same requests succeed with Authorization: Bearer <configured-key>.

The included regression test reproduces this boundary locally with FastAPI TestClient and the real AuthenticationMiddleware implementation.

Expected vulnerable behavior

  • Pre-patch: unauthenticated /nodes/add returns success and mutates the node registry.
  • Pre-patch: unauthenticated /nodes/status discloses registered backend nodes.
  • Post-patch: both routes require the configured bearer token.

Changes in this PR

  • Removes /nodes from the authentication middleware skip-prefix list.
  • Keeps /health, /docs, and /redoc public as passive endpoints.
  • Stores normalized API keys on the API-server interface so startup registration can authenticate to a protected proxy.
  • Normalizes string api_keys values in proxy setup before creating AuthenticationMiddleware.
  • Adds regression tests for protected /nodes/* routes and public /health behavior.

Files changed

Category Files What changed
Auth boundary lmdeploy/serve/utils/server_utils.py Removed /nodes from unauthenticated skip prefixes
Proxy setup lmdeploy/serve/proxy/proxy.py Normalized string api_keys before middleware creation
API-server registration lmdeploy/serve/openai/api_server.py Reuses configured API key for authenticated proxy registration
Tests tests/test_lmdeploy/serve/test_authentication_middleware.py Adds focused route-auth regression tests

Maintainer impact

  • Deployments without --api-keys are unchanged.
  • Deployments with proxy API keys now require the same bearer-token boundary for node-management routes as for inference routes.
  • API servers launched with matching API keys can still register with the proxy at startup.
  • This PR does not refactor proxy routing or DistServe behavior; it only narrows the public route exceptions.

Fix rationale

Node registration, removal, and termination are privileged control-plane operations. They should not share the same unauthenticated treatment as health checks or documentation routes. The narrowest durable boundary is to keep passive endpoints public and require bearer-token authentication for all node-management routes when API keys are configured.

Type of change

  • Security fix
  • Tests
  • Documentation update
  • Refactor with no behavior change

Test plan

  • Focused regression test for AuthenticationMiddleware route behavior.
  • Python syntax compilation for touched files.
  • Whitespace/diff validation.
  • Full test suite was not run locally because this checkout does not have all heavy runtime dependencies installed; focused tests were used for the touched middleware behavior.
  • pre-commit was not run locally because pre-commit is not installed in this environment.

Executed with:

python3 -m pytest tests/test_lmdeploy/serve/test_authentication_middleware.py -q
python3 -m compileall -q lmdeploy/serve/openai/api_server.py lmdeploy/serve/proxy/proxy.py lmdeploy/serve/utils/server_utils.py tests/test_lmdeploy/serve/test_authentication_middleware.py
git diff --check

Result:

2 passed in 0.10s

Token usage

  • discovery tokens: partial/unknown
  • validation tokens: partial/unknown
  • duplicate-check tokens: partial/unknown
  • PR/writeup tokens: partial/unknown
  • total tokens: partial/unknown
  • notes: exact usage unavailable from the local agent session telemetry.

Disclosure notes

This PR is bounded to the proxy node-management authentication boundary. It does not claim to change model execution behavior, prompt handling, or the broader proxy refactor in #4354. No unrelated files were intentionally changed.

'/health', # Health check endpoints
'/docs', # Swagger UI documentation
'/redoc', # ReDoc documentation
'/nodes', # Endpoints about node operation between proxy and api_server
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These node management endpoints are only intended for internal communication between the api_server and the proxy. In that context, do we really need to enforce authentication on them?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising this. I agree these endpoints are intended for api_server ↔ proxy coordination, but today that is only an architectural convention, not an enforced boundary.

The proxy defaults to binding on 0.0.0.0, and when api_keys are configured the public inference routes are protected while /nodes/* was still skipped by auth. If the proxy port is reachable, an unauthenticated caller can:

  • read /nodes/status to learn the current node URLs/models;
  • call /nodes/remove or /nodes/terminate* to disrupt serving; and
  • call /nodes/add with a non-empty status.models, which NodeManager.add() accepts directly without first querying the backend.

That last case lets an attacker register their own backend for an existing model and potentially receive subsequent inference requests. Since the proxy forwards the original request headers to the selected backend, that can also expose client request metadata/authorization headers to the malicious node.

So the issue is exploitable whenever the proxy listener is reachable outside the trusted internal network. If deployments enforce isolation externally (localhost/private listener/firewall/service mesh), the risk is reduced, but when operators configure api_keys on a network-reachable proxy, leaving /nodes/* unauthenticated creates a separate unauthenticated management plane.

This patch keeps unauthenticated behavior for deployments without api_keys, and for authenticated deployments it makes the existing api_server registration path send the configured bearer token so the intended internal flow still works.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants