amadan

carlos / platform Public

edge: split-line Authorization and Vary: * closed the last two shared-cache leaks

maybeStore read the request's Authorization with Header.Get (first line
only), so a client splitting it across two lines (empty first line, the real
credential second — which Go's server accepts) made the RFC 9111 §3.5
refusal see an unauthenticated request and cache a credentialed response for
everyone. Switch to Header.Values, matching the neighboring Set-Cookie check.

Separately, the Vary-token scan treated "*" as an ordinary header name
instead of the RFC 9111 §4.1 / RFC 9110 §12.5.5 "never reusable from a
shared cache" signal, so every response carrying Vary: * computed the same
constant variant and was replayed across visitors. Refuse on "*" alongside
the existing Cookie check.

Both covered by regression tests that fail at HEAD for the right reason
(entry actually cached; for Vary: *, a second visitor served the first
visitor's body) and pass after the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Paul Campbell f78bde6bbe00e6dce776f802c033275dff109936 parent f0ca540
2 files changed, +87 −2
  • internal/edge/edge.go +6 −2
  • internal/edge/edge_test.go +81 −0
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index 50cb0090a..ed9ec5a03 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -364,6 +364,9 @@ func (e *Edge) cacheMaxBody() int {
// private per RFC 9111 §3.5, unless Cache-Control explicitly says public,
// which is the app asserting the response is safe to share.
// - Vary: Cookie — varying by cookie would defeat correctness per-visitor.
+// - Vary: * — RFC 9111 §4.1 / RFC 9110 §12.5.5: the response depends on
+// something request headers can't capture, so it is never reusable from
+// a shared cache.
//
// The stored TTL is clamped to DefaultCacheMaxTTL so a lost purge always
// self-heals within a bounded window.
@@ -382,7 +385,7 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) {
if len(rec.Header().Values("Set-Cookie")) > 0 {
return
}
- if rec.request.Header.Get("Authorization") != "" && !hasCCDirective(cc, "public") {
+ if len(rec.request.Header.Values("Authorization")) > 0 && !hasCCDirective(cc, "public") {
return
}
if ttl > DefaultCacheMaxTTL {
@@ -391,7 +394,8 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) {
vary := joinHeader(rec.Header(), "Vary")
if vary != "" {
for _, h := range strings.Split(vary, ",") {
- if strings.EqualFold(strings.TrimSpace(h), "Cookie") {
+ h = strings.TrimSpace(h)
+ if h == "*" || strings.EqualFold(h, "Cookie") {
return
}
}
diff --git a/internal/edge/edge_test.go b/internal/edge/edge_test.go
index c09526e08..492479c7e 100644
--- a/internal/edge/edge_test.go
+++ b/internal/edge/edge_test.go
@@ -1007,6 +1007,87 @@ func TestServeHTTPConditionalVaryDoesNotPoisonURL(t *testing.T) {
}
}
+// The same root cause on the Authorization rule, but in the read of the
+// REQUEST'S Authorization header rather than the response's Cache-Control:
+// http.Header.Get returns only the first line, so a request that splits
+// Authorization across two lines (an empty first line, the real credential on
+// the second — Go's HTTP server accepts exactly this from a client) made Get
+// see "" and the request looked unauthenticated. The RFC 9111 §3.5 refusal
+// never fired and a credentialed response got cached at the shared edge for
+// everyone.
+func TestServeHTTPRefusesAuthorizationSplitAcrossHeaderLines(t *testing.T) {
+ e := cacheProbeEdge(t, "splitauthreq.example.com", func(path string, h http.Header) {
+ if path == "/public" {
+ h.Set("Cache-Control", "max-age=60, public")
+ return
+ }
+ h.Set("Cache-Control", "max-age=60")
+ })
+
+ req := httptest.NewRequest("GET", "http://splitauthreq.example.com/private", nil)
+ req.Header.Add("Authorization", "")
+ req.Header.Add("Authorization", "Bearer secret-token")
+ e.ServeHTTP(httptest.NewRecorder(), req)
+
+ if n := cachedVariants(e, "splitauthreq.example.com", "/private"); n != 0 {
+ t.Fatalf("%d variants cached for a request whose Authorization arrived on a second header line, want 0 — a multi-line Authorization is still an Authorization", n)
+ }
+
+ // Positive control: the same split-Authorization request, but the
+ // response says public — proving the refusal isn't just breaking caching
+ // wholesale.
+ pubReq := httptest.NewRequest("GET", "http://splitauthreq.example.com/public", nil)
+ pubReq.Header.Add("Authorization", "")
+ pubReq.Header.Add("Authorization", "Bearer secret-token")
+ e.ServeHTTP(httptest.NewRecorder(), pubReq)
+
+ if !cachedAt(e, "splitauthreq.example.com", "/public") {
+ t.Fatal("Cache-Control: public must still be cached even with a split-line Authorization present")
+ }
+}
+
+// RFC 9111 §4.1 / RFC 9110 §12.5.5: Vary: * means the response is NEVER
+// reusable from a shared cache — it signals that the response depends on
+// something not captured by request headers at all. The token-scanning loop
+// treated "*" as if it were an ordinary header name: varyValue computed the
+// constant string "*=;" for every request regardless of who they were, so one
+// variant was stored and replayed to everyone.
+func TestServeHTTPRefusesVaryStar(t *testing.T) {
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Cache-Control", "max-age=60")
+ w.Header().Set("Vary", "*")
+ w.WriteHeader(200)
+ w.Write([]byte("dashboard for " + r.Header.Get("Cookie")))
+ }))
+ t.Cleanup(backend.Close)
+
+ reg := openReg(t)
+ if err := reg.Upsert(registry.Route{Host: "varystar.example.com", Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil {
+ t.Fatalf("upsert: %v", err)
+ }
+ e, err := New(reg, nil)
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes))
+ const url = "http://varystar.example.com/dashboard"
+
+ alice := getWith(t, e, url, "Cookie", "sid=alice").Body.String()
+ if alice != "dashboard for sid=alice" {
+ t.Fatalf("alice body = %q, want %q", alice, "dashboard for sid=alice")
+ }
+ if n := cachedVariants(e, "varystar.example.com", "/dashboard"); n != 0 {
+ // Errorf, not Fatalf: the replay assertion below is the live
+ // consequence and is worth reporting in the same run.
+ t.Errorf("%d variants cached for a response carrying Vary: *, want 0 — Vary: * means never reusable from a shared cache", n)
+ }
+
+ bob := getWith(t, e, url, "Cookie", "sid=bob").Body.String()
+ if bob != "dashboard for sid=bob" {
+ t.Fatalf("bob was served %q — one visitor's response was replayed to another despite Vary: *", bob)
+ }
+}
+
// hijackableRecorder embeds httptest.ResponseRecorder (which deliberately
// does NOT implement http.Hijacker) and adds a no-op Hijack, standing in for
// a real connection's ResponseWriter — the minimum shape needed to prove