Skip to content

fix(gateway): move localhost routing behind an insecure debug flag - #831

Open
kvinwang wants to merge 1 commit into
masterfrom
fix/gateway-insecure-localhost-backend
Open

fix(gateway): move localhost routing behind an insecure debug flag#831
kvinwang wants to merge 1 commit into
masterfrom
fix/gateway-insecure-localhost-backend

Conversation

@kvinwang

Copy link
Copy Markdown
Collaborator

Problem

core.proxy.localhost_enabled reads like a proxy convenience — "let the app address localhost resolve to 127.0.0.1" — and it sits next to ordinary tuning like listen_port and buffer_size. Its blast radius is much wider than that placement suggests.

App addresses do not only come from the platform's own <app_id>.<base_domain> grammar. select_top_n_hosts is reached from both routing paths:

gateway/src/proxy/tls_terminate.rs:290   <- base domain
gateway/src/proxy/tls_passthough.rs:243  <- custom domain, resolved from an unauthenticated
                                            _dstack-app-address TXT record

So with the flag on, anyone who controls any DNS zone can publish

_dstack-app-address.evil.example  TXT  "localhost:8111"

and have the gateway open a connection to 127.0.0.1:8111 on the gateway host, then pass their TLS through to it. Port 8111 is the default admin listener, which binds loopback precisely because unreachability is its access control; the debug RPC and the gateway's own RPC are next door.

port_policy does not constrain it either — the shortcut is not a registered instance, and the code says so:

/// Instances not present in state (e.g. the `localhost` shortcut) bypass the
/// check — the policy machinery only applies to registered CVMs.

so the caller picks the port.

The flag is off by default, which is the saving grace. But an operator turning it on for the base-domain case it advertises has no way to see that they are also opening the gateway's loopback to every domain owner on the internet.

Fix

Move it to [core.debug] insecure_localhost_backend, next to insecure_skip_attestation and insecure_enable_debug_rpc, and warn at startup when it is set.

The insecure_ prefix is earned here in a way it would not be for every switch: this one makes a relying party — the gateway — accept a routing target it would otherwise reject, and skips a policy check on the way. That is the same shape as insecure_no_auth and insecure_allow_external_trust_anchors, and unlike, say, a switch that only narrows what a component asserts about itself.

The doc comment and the gateway.toml comment both state the custom-domain coupling explicitly, since that is the part that is not obvious from the name alone.

No compatibility shim: an old config with core.proxy.localhost_enabled now leaves the feature off, which is the safe direction, and this is a development-only switch.

Not changed here, but worth a follow-up decision: the shortcut could be restricted to the base-domain path so the TXT path can never name it. That would keep the development use case while removing the exposure entirely, rather than relying on the operator reading a flag name.

Verification

test-run/test_proxy.sh — the gateway's proxy data-path integration suite — runs entirely on this shortcut (localhost-<port>[s].<base_domain> is how it avoids registering a CVM), so it exercises the renamed flag end to end:

66 passed, 0 failed, 2 skipped

(the two skips are environmental: the TLS ULP is not removable on this host, and the Status RPC needs a real WireGuard device.)

cargo build, cargo fmt and cargo clippy -p dstack-gateway are clean.

Copilot AI review requested due to automatic review settings July 28, 2026 04:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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