fix(cache-proxy): scope cache keys by SigV4 credential and lock down relay paths - #1156
Merged
Conversation
…relay paths Security fix for two findings in the unauthenticated forward proxy: (A) Cache keys mixed no credential material, so a warm entry (local or from a peer) was served with zero authorization. In the managed-warehouse topology tenants share a bucket with per-org path prefixes, so one tenant could read another tenant's objects from a warm cache. The tenant scope — the SigV4 access key ID from the Authorization header, unique per issued STS credential set — is now part of every CacheKey and BlockKey hash input (scope + "\x00" + url + ...). A cross-tenant request misses and goes to the origin, where S3 authorization applies. Unsigned requests share the empty-scope namespace, which is correct for public objects. Peer traffic carries only opaque keys, so the peer protocol is unchanged. (B) handleConnect dialed any host:port and the plain-HTTP forward path forwarded to any absolute URL — open relay and SSRF primitives reachable cluster-wide via the hostPort. CONNECT now allows only port 443, refuses loopback/link-local/unspecified IP literals, and honors the new CONNECT_ALLOWED_SUFFIXES hostname list when configured. The plain-HTTP forward path forwards only to hosts matching CACHE_HOST_SUFFIXES when that list is configured (signed S3 traffic matches by definition); legacy no-suffix mode stays unrestricted for backward compatibility. Co-authored-by: Shelley <shelley@exe.dev>
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
Co-authored-by: Shelley <shelley@exe.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security fix for two findings in the cache-proxy DaemonSet, an unauthenticated forward proxy reachable cluster-wide via hostPort:
sha256(url | range)— no credential material. A hit (local NVMe or a peer node) is served with zero authorization. Managed-warehouse tenants share a bucket with per-org path prefixes, so a tenant requesting another org's object URL was served the warm entry despite holding credentials S3 would reject. Cache keys now mix in the request's tenant scope — the SigV4 access key ID, unique per issued STS credential set — so a cross-tenant request misses and goes to origin, where S3 authorization applies. Unsigned requests share one namespace (correct for public objects). Peer traffic carries opaque keys, so the peer protocol is unchanged.CONNECTdialed any host:port and the plain-HTTP forward path forwarded to any absolute URL, from the node's network context (bypassing pod egress policy). CONNECT is now restricted to port 443, refuses loopback/link-local/unspecified IP literals, and honors the newCONNECT_ALLOWED_SUFFIXESlist when set. The plain-HTTP forward path now forwards only to hosts matchingCACHE_HOST_SUFFIXESwhen that list is configured (signed S3 traffic matches by definition; the MW chart setsamazonaws.com). Legacy no-suffix mode is unchanged.Changes
cmd/cache-proxy/scope.go(new):TenantScope(r)extracts the access key ID from the SigV4Authorizationheader (an identifier, not a secret — the secret key and signature never enter the key).CacheKey(scope, url, range)andBlockKey(scope, url, idx, size)take the scope; hash input uses a NUL separator so the scope boundary is unambiguous. All call sites threaded; peer protocol untouched.handleConnect:connectRefusalReasongates the target before dialing.forwardwrapper gates plain-HTTP forwarding onCACHE_HOST_SUFFIXES.CONNECT_ALLOWED_SUFFIXES.Behavior changes / risks
read_csv('http://...')) are refused whenCACHE_HOST_SUFFIXESis set; external HTTPS reads still work via CONNECT:443.Testing
:8816/ link-local / non-443 → 403;:443hostname allowed; plain-HTTP non-suffix host → 403.go test ./cmd/cache-proxy/...green (note: the suite's disk-capacity tests need >5% free disk; verified on tmpfs).Rollback
Reverting the image pin restores the old key space; no persistent state to migrate.