public/platform

Fork 0

gateway: harden internal-route block (segment-aware check, config normalization, shared client) #385

Merged

vkobel merged 0 commits from refs/pull/385/head into main

2026-07-27 22:37:37 +00:00

vkobel commented

2026-07-16 15:17:13 +00:00

(Migrated from codeberg.org)

Summary

Fixes a cross-tenant authorization bypass in the gateway.

Previously, any authenticated user could request /api/internal/*. The gateway would automatically attach X-Internal-Service-Secret, causing the API to trust the request as an internal service call.

Impact

An authenticated user could potentially:

Suspend another organization and stop its resources.
Suspend fully managed resources.
Restart suspended resources and clear dunning state.
Trigger legal-notice delivery.
Invoke app cleanup if the unsafe E2E feature were deployed.

Organization operations required knowing the target UUID, but did not enforce membership or ownership.

Fix

Block canonical /internal and /internal/* targets in the public gateway proxy.
Return 404 before reading the body, injecting credentials or contacting the API.
Prevent dot-segment, encoded-path and backslash bypasses.
Log blocked requests without sensitive data.

Direct service-to-service calls to private api:8080/internal/* remain unchanged.

Tests

Gateway unit tests cover blocked paths, normalization bypasses and allowed lookalikes.
E2E coverage verifies authenticated requests return 404, normal API traffic still works and direct service-authenticated calls remain available.
Gateway unit suite: 96 passed, 0 failed.

Follow-up

Require the gateway and legal E2E suites in CI.
Ensure API port 8080 remains private in every environment.
Longer-term, remove the service secret from the public gateway or move internal routes to a separate listener.

## Summary Fixes a _cross-tenant authorization bypass_ in the gateway. Previously, any authenticated user could request `/api/internal/*`. The gateway would automatically attach `X-Internal-Service-Secret`, causing the API to trust the request as an internal service call. ## Impact An authenticated user could potentially: - Suspend another organization and stop its resources. - Suspend fully managed resources. - Restart suspended resources and clear dunning state. - Trigger legal-notice delivery. - Invoke app cleanup if the unsafe E2E feature were deployed. Organization operations required knowing the target UUID, but did not enforce membership or ownership. ## Fix - Block canonical `/internal` and `/internal/*` targets in the public gateway proxy. - Return `404` before reading the body, injecting credentials or contacting the API. - Prevent dot-segment, encoded-path and backslash bypasses. - Log blocked requests without sensitive data. Direct service-to-service calls to private `api:8080/internal/*` remain unchanged. ## Tests - Gateway unit tests cover blocked paths, normalization bypasses and allowed lookalikes. - E2E coverage verifies authenticated requests return `404`, normal API traffic still works and direct service-authenticated calls remain available. - Gateway unit suite: **96 passed, 0 failed**. ## Follow-up - Require the gateway and legal E2E suites in CI. - Ensure API port `8080` remains private in every environment. - Longer-term, remove the service secret from the public gateway or move internal routes to a separate listener.

caution-bot commented

2026-07-16 17:12:41 +00:00

(Migrated from codeberg.org)

Security review notes (focused on gateway/internal-route isolation):

Thanks for jumping on this — the main /api/internal/* exposure is real, and the fix is directionally good: the proxy now blocks canonical internal targets before reading the body or attaching service credentials, uses URL path-segment normalization instead of a raw prefix check, does not forward user-supplied internal-secret headers, and adds useful unit/e2e coverage.

I would still address one proxy-hardening issue before treating this as fully closed:

Medium/High: secret-bearing proxy requests still follow redirects

src/gateway/src/main.rs constructs the shared proxy client with reqwest::Client::new(), and src/gateway/src/proxy.rs attaches X-Internal-Service-Secret for authenticated proxied API requests before calling .send(). reqwest follows redirects by default, and custom headers are preserved across redirects. I verified this locally with a two-server test: a request carrying X-Internal-Service-Secret redirected cross-origin and the second server received the header.

If any proxied API endpoint can return, or be induced to return, an attacker-controlled redirect, the gateway may follow it server-side with the internal service secret/request body attached. That is a secret-exposure and SSRF-hardening gap adjacent to the issue this PR fixes.

Recommended fix: use a proxy client with redirects disabled, e.g.:

let http_client = reqwest::Client::builder()
    .redirect(reqwest::redirect::Policy::none())
    .build()?;

Then the gateway should return backend 3xx responses to the caller instead of following them while carrying internal credentials. I’d apply this to the API proxy client at minimum; disabling redirects for gateway service proxying generally is probably safest unless there is a specific trusted redirect requirement.

Lower-severity notes

The internal-route block currently runs inside proxy_handler, after the /api auth/sign/username-gate middleware. That means authenticated users are blocked correctly, but unauthenticated requests get 401 before the internal-path 404, and placeholder-username users may see gate behavior before 404. This does not make the route executable, but if the invariant is “public gateway never acknowledges internal paths,” consider moving the rejection into middleware that runs before auth/gating, or document/test the current behavior.
The path canonicalization tests are good, but I’d add a few more regression cases for expected ingress/intermediary behavior: /%69nternal/..., /%2finternal/..., /%5Cinternal/..., /internal;param/..., and repeated/double-encoded variants. I did not confirm these as Axum bypasses; this is mainly assurance hardening.
git diff --check origin/main...HEAD reports trailing whitespace in changed CLI lines (src/cli/src/lib.rs around 4236, 4244, 4254, 7099, 7314, 8086, 8133, 8198).

Checks I ran locally:

cargo test -p gateway proxy::tests -- --nocapture
# 3 passed; 0 failed

Security review notes (focused on gateway/internal-route isolation): Thanks for jumping on this — the main `/api/internal/*` exposure is real, and the fix is directionally good: the proxy now blocks canonical internal targets before reading the body or attaching service credentials, uses URL path-segment normalization instead of a raw prefix check, does not forward user-supplied internal-secret headers, and adds useful unit/e2e coverage. I would still address one proxy-hardening issue before treating this as fully closed: ### Medium/High: secret-bearing proxy requests still follow redirects `src/gateway/src/main.rs` constructs the shared proxy client with `reqwest::Client::new()`, and `src/gateway/src/proxy.rs` attaches `X-Internal-Service-Secret` for authenticated proxied API requests before calling `.send()`. `reqwest` follows redirects by default, and custom headers are preserved across redirects. I verified this locally with a two-server test: a request carrying `X-Internal-Service-Secret` redirected cross-origin and the second server received the header. If any proxied API endpoint can return, or be induced to return, an attacker-controlled redirect, the gateway may follow it server-side with the internal service secret/request body attached. That is a secret-exposure and SSRF-hardening gap adjacent to the issue this PR fixes. Recommended fix: use a proxy client with redirects disabled, e.g.: ```rust let http_client = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) .build()?; ``` Then the gateway should return backend 3xx responses to the caller instead of following them while carrying internal credentials. I’d apply this to the API proxy client at minimum; disabling redirects for gateway service proxying generally is probably safest unless there is a specific trusted redirect requirement. ### Lower-severity notes - The internal-route block currently runs inside `proxy_handler`, after the `/api` auth/sign/username-gate middleware. That means authenticated users are blocked correctly, but unauthenticated requests get `401` before the internal-path `404`, and placeholder-username users may see gate behavior before `404`. This does not make the route executable, but if the invariant is “public gateway never acknowledges internal paths,” consider moving the rejection into middleware that runs before auth/gating, or document/test the current behavior. - The path canonicalization tests are good, but I’d add a few more regression cases for expected ingress/intermediary behavior: `/%69nternal/...`, `/%2finternal/...`, `/%5Cinternal/...`, `/internal;param/...`, and repeated/double-encoded variants. I did not confirm these as Axum bypasses; this is mainly assurance hardening. - `git diff --check origin/main...HEAD` reports trailing whitespace in changed CLI lines (`src/cli/src/lib.rs` around 4236, 4244, 4254, 7099, 7314, 8086, 8133, 8198). Checks I ran locally: ```text cargo test -p gateway proxy::tests -- --nocapture # 3 passed; 0 failed ```

vkobel commented

2026-07-17 09:45:00 +00:00

(Migrated from codeberg.org)

Thanks — all addressed in d1e759c.

Redirect leak (Med/High): proxy client now built with redirect::Policy::none(); Location added to relayed response headers so callers follow 3xx themselves without the internal secret.
Encoded-path tripwire (P2): you're right that testing the backend's 404 wasn't a real guarantee. Instead of relying on matchit staying non-decoding, the gateway now blocks conservatively itself — folds percent-encoding + backslashes and resolves ./.. with a root-clamped stack, so /%2finternal, /%69nternal, /foo%2f..%2f%69nternal, and ..;matrix variants are all rejected regardless of downstream routing. Double-encoding is the documented stopping point.
Middleware ordering: left the block after auth, documented why in-code — every unauth /api/* already returns a uniform 401, so moving it pre-auth would make internal paths the one surface answering 404 while everything else answers 401, which acknowledges them more, not less.

rebased

Thanks — all addressed in `d1e759c`. - **Redirect leak (Med/High):** proxy client now built with `redirect::Policy::none()`; `Location` added to relayed response headers so callers follow 3xx themselves without the internal secret. - **Encoded-path tripwire (P2):** you're right that testing the backend's 404 wasn't a real guarantee. Instead of relying on `matchit` staying non-decoding, the gateway now blocks conservatively itself — folds percent-encoding + backslashes and resolves `.`/`..` with a root-clamped stack, so `/%2finternal`, `/%69nternal`, `/foo%2f..%2f%69nternal`, and `..;matrix` variants are all rejected regardless of downstream routing. Double-encoding is the documented stopping point. - **Middleware ordering:** left the block after auth, documented why in-code — every unauth `/api/*` already returns a uniform `401`, so moving it pre-auth would make internal paths the one surface answering `404` while everything else answers `401`, which *acknowledges* them more, not less. + rebased

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

public/platform!385

No description provided.

Rows
Columns