amadan
edge: make cross-account cookies host-only on the apps domain (#217)
oncarlos.com is not on the Public Suffix List, so every tenant app at <app>.<sqid>.oncarlos.com shares one registrable domain. An app could answer with Set-Cookie: ...; Domain=oncarlos.com (or another account's <sqid>.oncarlos.com) and browsers would hand that cookie to other accounts' apps: cookie tossing and session fixation across accounts. The tenant proxies' ModifyResponse hook (now responseModifier: the existing Carlos-Purge hook plus cookie scoping) removes the Domain attribute from any Set-Cookie whose Domain is neither the request host nor within the account's own <sqid>.<apps domain>. The namespace is the sqid the host names, and only when the route's Account agrees. The rest of the line is kept byte-for-byte. Custom domains, and every host when PLATFORM_APPS_DOMAIN is unset, are untouched. Editing the upstream response's own header map means no writer wrapper and a shared map, and the cache recorder only ever sees the rewritten form. Trailers are rewritten when the proxy closes the body, before it copies them out. Each route's first rewrite is logged once per process. maybeStore now also refuses a response carrying a Set-Cookie trailer. An undeclared trailer reaches the recorder under the raw key "Trailer:Set-Cookie", which the header Set-Cookie refusal never saw, so the response was cached and writeCached replayed one visitor's cookie to every later visitor as a trailer. Same-site CSRF between accounts stays open until the PSL entry lands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+519
−6
docs/site/cli.md+8 −0internal/edge/cookiescope.go+232 −0internal/edge/cookiescope_test.go+249 −0internal/edge/edge.go+30 −6
CI passed — run details
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 1ea8c232e..d9240df7b 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -850,6 +850,14 @@ owns the whole zone. A deployment that hosts apps for other people sets |
| refuses every name under the apps domain and under its own hostname, because |
| those names belong to the operator and not to a tenant. |
| +On a platform hostname, an app can set cookies for its own hostname or for |
| +its own account's platform domain (`<your-account>.<apps-domain>`), so your |
| +apps can share a cookie. Browsers would otherwise let an app set a cookie |
| +for the whole apps domain, and every other account's apps would get it. So |
| +when a cookie's `Domain` names anything wider, or another account, the edge |
| +removes the `Domain` and the cookie only goes back to the hostname that set |
| +it. Custom domains are left as they are. |
| + |
| #### One name per instance: `*.<parent>` |
| For an app with instances, attaching `*.<parent>` gives every instance a |
diff --git a/internal/edge/cookiescope.go b/internal/edge/cookiescope.go| new file mode 100644 |
| index 000000000..ca9aa374c |
| --- /dev/null |
| +++ b/internal/edge/cookiescope.go |
| @@ -0,0 +1,232 @@ |
| +package edge |
| + |
| +import ( |
| + "io" |
| + "log" |
| + "net" |
| + "net/http" |
| + "os" |
| + "strings" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// Cookie scoping on the platform apps domain (discussion #217). |
| +// |
| +// Every tenant app lives at <app>.<sqid>.<apps domain>, and the apps domain |
| +// (oncarlos.com on flagship) is not on the Public Suffix List — Carloku is |
| +// below the list's user threshold. So to a browser every tenant shares ONE |
| +// registrable domain, and an app that answers |
| +// |
| +// Set-Cookie: sid=x; Domain=oncarlos.com |
| +// |
| +// plants that cookie on every other account's apps too: cookie tossing, and |
| +// with it session fixation across accounts. Until a PSL entry lands, the edge |
| +// is the only party in a position to stop it, so it rewrites such a cookie to |
| +// a host-only one on the way out. |
| +// |
| +// The rule, for a response to a request whose host is under the apps domain: |
| +// |
| +// - Domain equal to the request host passes. |
| +// - Domain equal to, or under, the account's own namespace <sqid>.<apps |
| +// domain> passes, so an account can still share cookies across its own |
| +// apps. The sqid is the one the request host names in its own labels |
| +// (release.HostAccountLabel), and only when the serving route's Account |
| +// agrees with it: a route whose row names a different account — or none — |
| +// gets no namespace at all, only its host. Strict on purpose; the only |
| +// cost of a mismatch is a cookie scoped narrower than asked. |
| +// - Anything else — the apps domain itself, another account's namespace, |
| +// an unrelated domain — loses its Domain attribute and becomes |
| +// host-only. The cookie is otherwise byte-for-byte what the app sent: |
| +// name, value, every other attribute, their order and spelling. |
| +// |
| +// Hosts NOT under the apps domain — custom domains, and the platform's own |
| +// console and gate hosts — are left alone: a custom domain is its owner's |
| +// registrable domain, and nothing in the platform itself sets a Domain |
| +// attribute (checked for #217), so there is nothing platform-side to break. |
| +// |
| +// Where it runs: the ModifyResponse hook every tenant reverse proxy already |
| +// carries (responseModifier, below). That edits the upstream response's OWN |
| +// header map before httputil.ReverseProxy copies it to the writer stack, so |
| +// the client, the cache recorder above the compressor and the 101 upgrade |
| +// path all see only the rewritten form — and no ResponseWriter wrapper with a |
| +// shared header map is involved (AGENTS.md). Static and status routes have no |
| +// hook: their headers are the edge's own and never carry a tenant cookie. |
| +// |
| +// Trailers: a Set-Cookie sent as an HTTP trailer arrives after |
| +// ModifyResponse, so the body is wrapped and the trailer map rewritten when |
| +// the proxy closes it — which it does before copying trailers out. Browsers |
| +// ignore cookies in trailers; other clients need not. (A response carrying a |
| +// Set-Cookie trailer is never cached: maybeStore's hasTrailerSetCookie.) |
| +// |
| +// Not covered: Set-Cookie on a 1xx informational response (103 Early Hints), |
| +// which httputil forwards without ModifyResponse. Browsers do not apply |
| +// cookies from 1xx responses. |
| + |
| +// appsDomainFromEnv reads PLATFORM_APPS_DOMAIN the way the rest of the edge |
| +// does: trimmed, lowercased, no trailing dot. "" disables cookie scoping. |
| +func appsDomainFromEnv() string { |
| + return strings.ToLower(strings.TrimSuffix(strings.TrimSpace(os.Getenv("PLATFORM_APPS_DOMAIN")), ".")) |
| +} |
| + |
| +// responseModifier is the ModifyResponse hook for a tenant route's reverse |
| +// proxy: the Carlos-Purge channel (purgeModifier) and the cookie scoping |
| +// above, in that order. Neither depends on the other. |
| +func (e *Edge) responseModifier(rt registry.Route) func(*http.Response) error { |
| + purge := e.purgeModifier(rt) |
| + return func(resp *http.Response) error { |
| + if err := purge(resp); err != nil { |
| + return err |
| + } |
| + e.scopeCookies(rt, resp) |
| + return nil |
| + } |
| +} |
| + |
| +// scopeCookies applies the cookie rule to resp's headers, and arranges for |
| +// its trailers to get the same treatment once the body has been read. |
| +func (e *Edge) scopeCookies(rt registry.Route, resp *http.Response) { |
| + host := rt.Host |
| + if resp.Request != nil && resp.Request.Host != "" { |
| + // The host the visitor asked for: every tenant proxy's Rewrite sets |
| + // Out.Host = In.Host, and a path-routed front host differs from the |
| + // target route's own Host. The browser scopes cookies by this one. |
| + host = resp.Request.Host |
| + } |
| + allowed, ok := e.cookieScope(rt, host) |
| + if !ok { |
| + return |
| + } |
| + if n := rewriteSetCookies(resp.Header, allowed); n > 0 { |
| + e.noteCookieRewrite(rt, host) |
| + } |
| + // 101 Switching Protocols keeps its body: httputil needs it to be the |
| + // raw io.ReadWriteCloser, and an upgraded stream has no trailers. Every |
| + // other response is wrapped, announced trailers or not: net/http fills |
| + // resp.Trailer with UNdeclared trailers too, on HTTP/1.1 and HTTP/2. |
| + if resp.StatusCode != http.StatusSwitchingProtocols && resp.Body != nil { |
| + resp.Body = &trailerCookieBody{ReadCloser: resp.Body, close: func() { |
| + if n := rewriteSetCookies(resp.Trailer, allowed); n > 0 { |
| + e.noteCookieRewrite(rt, host) |
| + } |
| + }} |
| + } |
| +} |
| + |
| +// cookieScope reports which Domain values a response for host may keep, or |
| +// ok=false when host is not under the apps domain (nothing to enforce). |
| +func (e *Edge) cookieScope(rt registry.Route, host string) (allowed func(string) bool, ok bool) { |
| + apps := e.appsDomain |
| + if apps == "" { |
| + return nil, false |
| + } |
| + if h, _, err := net.SplitHostPort(host); err == nil { |
| + host = h |
| + } |
| + host = strings.ToLower(strings.TrimSuffix(host, ".")) |
| + if host != apps && !strings.HasSuffix(host, "."+apps) { |
| + return nil, false |
| + } |
| + ns := "" |
| + if sq := release.HostAccountLabel(host, apps); sq != "" && sq == rt.Account { |
| + ns = sq + "." + apps |
| + } |
| + return func(d string) bool { |
| + return d == host || (ns != "" && (d == ns || strings.HasSuffix(d, "."+ns))) |
| + }, true |
| +} |
| + |
| +// rewriteSetCookies drops the Domain attribute from every Set-Cookie line in |
| +// h whose Domain is not allowed, in place, and returns how many it changed. |
| +func rewriteSetCookies(h http.Header, allowed func(string) bool) int { |
| + lines := h["Set-Cookie"] |
| + n := 0 |
| + for i, line := range lines { |
| + if out, changed := scopeSetCookie(line, allowed); changed { |
| + lines[i] = out |
| + n++ |
| + } |
| + } |
| + return n |
| +} |
| + |
| +// scopeSetCookie returns line without its Domain attribute(s) when any of |
| +// them names a domain allowed rejects, and line unchanged otherwise. |
| +// |
| +// Parsing follows RFC 6265 §5.2: attributes are the ";"-separated pieces |
| +// after the name-value pair, names compare case-insensitively, whitespace |
| +// around name and value is ignored, and a leading dot on the value is |
| +// dropped. An empty Domain value is ignored by user agents (§5.2.3) — the |
| +// cookie is host-only already — so it is left as sent. Browsers disagree on |
| +// which of several Domain attributes wins, so one bad one strips them all; |
| +// that can only narrow the cookie, never widen it. A trailing dot is trimmed |
| +// before comparing, so "oncarlos.com." is judged as "oncarlos.com". |
| +func scopeSetCookie(line string, allowed func(string) bool) (string, bool) { |
| + parts := strings.Split(line, ";") |
| + var domains []int |
| + bad := false |
| + for i := 1; i < len(parts); i++ { |
| + name, value, _ := strings.Cut(parts[i], "=") |
| + if !strings.EqualFold(strings.TrimSpace(name), "domain") { |
| + continue |
| + } |
| + domains = append(domains, i) |
| + d := strings.ToLower(strings.TrimSpace(value)) |
| + d = strings.TrimPrefix(d, ".") |
| + d = strings.TrimSuffix(d, ".") |
| + if d == "" { |
| + continue |
| + } |
| + if !allowed(d) { |
| + bad = true |
| + } |
| + } |
| + if !bad { |
| + return line, false |
| + } |
| + kept := make([]string, 0, len(parts)-len(domains)) |
| + next := 0 |
| + for i, p := range parts { |
| + if next < len(domains) && domains[next] == i { |
| + next++ |
| + continue |
| + } |
| + kept = append(kept, p) |
| + } |
| + return strings.Join(kept, ";"), true |
| +} |
| + |
| +// noteCookieRewrite logs the first rewrite for each route this process |
| +// serves, and never again for it: enough for an operator to see which app |
| +// does it, without a line per response. Keyed by the route's own host — a |
| +// value the box controls, so the set is bounded by the routing table. |
| +func (e *Edge) noteCookieRewrite(rt registry.Route, host string) { |
| + if _, seen := e.cookieLogged.LoadOrStore(rt.Host, true); seen { |
| + return |
| + } |
| + acct := rt.Account |
| + if acct == "" { |
| + acct = "-" |
| + } |
| + log.Printf("edge: route %s (app %q, account %s): a Set-Cookie on %s named a Domain outside the app's own account; removed it so the cookie is host-only (logged once per route)", |
| + rt.Host, rt.App, acct, host) |
| +} |
| + |
| +// trailerCookieBody runs close once, when the proxy closes the upstream |
| +// body: after the last read, so the trailer map is complete, and before |
| +// httputil copies the trailers to the client. |
| +type trailerCookieBody struct { |
| + io.ReadCloser |
| + close func() |
| + done bool |
| +} |
| + |
| +func (b *trailerCookieBody) Close() error { |
| + err := b.ReadCloser.Close() |
| + if !b.done { |
| + b.done = true |
| + b.close() |
| + } |
| + return err |
| +} |
diff --git a/internal/edge/cookiescope_test.go b/internal/edge/cookiescope_test.go| new file mode 100644 |
| index 000000000..c75b4f245 |
| --- /dev/null |
| +++ b/internal/edge/cookiescope_test.go |
| @@ -0,0 +1,249 @@ |
| +package edge |
| + |
| +import ( |
| + "bytes" |
| + "fmt" |
| + "log" |
| + "net/http" |
| + "net/http/httptest" |
| + "os" |
| + "slices" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// Two accounts on the apps domain: kmqa (the one under test) and xqzb (its |
| +// neighbour, the victim of a tossed cookie). |
| +const ( |
| + cookieApps = "oncarlos.com" |
| + cookieAcct = "kmqa" |
| + cookieOther = "xqzb" |
| + cookieHost = "shop.kmqa.oncarlos.com" |
| +) |
| + |
| +// cookieEdge builds an edge (with a cache) over one addr route per host, all |
| +// backed by h, under PLATFORM_APPS_DOMAIN=oncarlos.com. Each route's Account |
| +// is acct. |
| +func cookieEdge(t *testing.T, acct string, h http.HandlerFunc, hosts ...string) *Edge { |
| + t.Helper() |
| + if !release.ValidAccount(cookieAcct) || !release.ValidAccount(cookieOther) { |
| + t.Fatal("test sqids must be valid account names") |
| + } |
| + t.Setenv("PLATFORM_APPS_DOMAIN", "OnCarlos.com.") |
| + backend := httptest.NewServer(h) |
| + t.Cleanup(backend.Close) |
| + reg := openReg(t) |
| + for _, host := range hosts { |
| + rt := registry.Route{Host: host, Kind: "instance", App: "shop", Account: acct, Addr: backend.Listener.Addr().String(), Status: "ready"} |
| + if err := reg.Upsert(rt); 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)) |
| + return e |
| +} |
| + |
| +func setCookies(lines ...string) http.HandlerFunc { |
| + return func(w http.ResponseWriter, r *http.Request) { |
| + for _, l := range lines { |
| + w.Header().Add("Set-Cookie", l) |
| + } |
| + w.Write([]byte("ok")) |
| + } |
| +} |
| + |
| +func getCookies(t *testing.T, e *Edge, url string) []string { |
| + t.Helper() |
| + rec := httptest.NewRecorder() |
| + e.ServeHTTP(rec, httptest.NewRequest("GET", url, nil)) |
| + if rec.Code != 200 { |
| + t.Fatalf("GET %s: status %d", url, rec.Code) |
| + } |
| + return rec.Header().Values("Set-Cookie") |
| +} |
| + |
| +// The headline: a cookie scoped to the apps domain itself, or to another |
| +// account's namespace, comes out host-only; everything else about it — and |
| +// every cookie on the response, not just the first — is byte-for-byte. |
| +func TestCookieScopeRewritesDomainsOutsideTheAccount(t *testing.T) { |
| + sent := []string{ |
| + "a=1; Domain=oncarlos.com; Path=/; Secure", |
| + "b=2; path=/; DOMAIN=.OnCarlos.Com; HttpOnly", |
| + "c=3; Domain=xqzb.oncarlos.com", |
| + "d=4; Max-Age=60; domain = blog.xqzb.oncarlos.com ; SameSite=Lax", |
| + "e=5; Domain=example.org", |
| + "f=6; Domain=oncarlos.com.", |
| + "g=7; Domain=kmqa.oncarlos.com; Domain=oncarlos.com", |
| + } |
| + e := cookieEdge(t, cookieAcct, setCookies(sent...), cookieHost) |
| + got := getCookies(t, e, "http://"+cookieHost+"/") |
| + want := []string{ |
| + "a=1; Path=/; Secure", |
| + "b=2; path=/; HttpOnly", |
| + "c=3", |
| + "d=4; Max-Age=60; SameSite=Lax", |
| + "e=5", |
| + "f=6", |
| + "g=7", |
| + } |
| + if !slices.Equal(got, want) { |
| + t.Fatalf("Set-Cookie:\n got %q\nwant %q", got, want) |
| + } |
| +} |
| + |
| +// The account's own namespace (so its apps can share a cookie), the host |
| +// itself, a host-only cookie and an empty Domain all pass untouched. |
| +func TestCookieScopeKeepsTheAccountsOwnScope(t *testing.T) { |
| + sent := []string{ |
| + "a=1; Domain=kmqa.oncarlos.com; Path=/", |
| + "b=2; Domain=.KMQA.oncarlos.com", |
| + "c=3; Domain=shop.kmqa.oncarlos.com; Secure", |
| + "d=4; Domain=other.kmqa.oncarlos.com", |
| + "e=5; Path=/; HttpOnly", |
| + "f=6; Domain=; Path=/", |
| + "domain=oncarlos.com; Path=/", |
| + } |
| + e := cookieEdge(t, cookieAcct, setCookies(sent...), cookieHost) |
| + if got := getCookies(t, e, "http://"+cookieHost+":443/"); !slices.Equal(got, sent) { |
| + t.Fatalf("Set-Cookie changed:\n got %q\nwant %q", got, sent) |
| + } |
| +} |
| + |
| +// The namespace comes from the host's own sqid label AND the route's account |
| +// agreeing. A route whose row names another account (or none) keeps only its |
| +// host: strict, and narrower is the safe way to be wrong. |
| +func TestCookieScopeNamespaceNeedsTheRoutesAccount(t *testing.T) { |
| + for _, acct := range []string{cookieOther, ""} { |
| + e := cookieEdge(t, acct, setCookies("a=1; Domain=kmqa.oncarlos.com", "b=2; Domain=shop.kmqa.oncarlos.com"), cookieHost) |
| + got := getCookies(t, e, "http://"+cookieHost+"/") |
| + want := []string{"a=1", "b=2; Domain=shop.kmqa.oncarlos.com"} |
| + if !slices.Equal(got, want) { |
| + t.Fatalf("account %q: got %q, want %q", acct, got, want) |
| + } |
| + } |
| +} |
| + |
| +// A custom domain is its owner's registrable domain: nothing to enforce. |
| +func TestCookieScopeLeavesCustomDomainsAlone(t *testing.T) { |
| + sent := []string{"a=1; Domain=example.com; Path=/", "b=2; Domain=oncarlos.com"} |
| + e := cookieEdge(t, cookieAcct, setCookies(sent...), "shop.example.com") |
| + if got := getCookies(t, e, "http://shop.example.com/"); !slices.Equal(got, sent) { |
| + t.Fatalf("custom domain cookies changed: got %q", got) |
| + } |
| +} |
| + |
| +// No apps domain configured: the edge changes nothing anywhere. |
| +func TestCookieScopeOffWithoutAppsDomain(t *testing.T) { |
| + e := cookieEdge(t, cookieAcct, setCookies("a=1; Domain=oncarlos.com"), cookieHost) |
| + e.appsDomain = "" |
| + if got := getCookies(t, e, "http://"+cookieHost+"/"); !slices.Equal(got, []string{"a=1; Domain=oncarlos.com"}) { |
| + t.Fatalf("got %q", got) |
| + } |
| +} |
| + |
| +// A Set-Cookie sent as an undeclared trailer on a cacheable response reaches |
| +// the recorder as "Trailer:Set-Cookie", which the plain Set-Cookie refusal |
| +// never saw: the response was stored and its trailer replayed to every later |
| +// visitor. It is delivered rewritten, and never stored; a second visitor |
| +// gets a fresh response from the app, with its own cookie, not the first's. |
| +func TestCookieScopeTrailerCookieIsNeverReplayed(t *testing.T) { |
| + n := 0 |
| + e := cookieEdge(t, cookieAcct, func(w http.ResponseWriter, r *http.Request) { |
| + n++ |
| + w.Header().Set("Cache-Control", "max-age=600") |
| + w.Write([]byte("ok")) |
| + w.(http.Flusher).Flush() // chunked, so a trailer can follow |
| + w.Header().Set(http.TrailerPrefix+"Set-Cookie", fmt.Sprintf("t=%d; Domain=oncarlos.com; Path=/", n)) |
| + }, cookieHost) |
| + for i, want := range []string{"t=1; Path=/", "t=2; Path=/"} { |
| + rec := httptest.NewRecorder() |
| + e.ServeHTTP(rec, httptest.NewRequest("GET", "http://"+cookieHost+"/t", nil)) |
| + if got := rec.Result().Trailer.Values("Set-Cookie"); !slices.Equal(got, []string{want}) { |
| + t.Fatalf("visitor %d trailer: got %q, want %q", i+1, got, want) |
| + } |
| + if cachedAt(e, cookieHost, "/t") { |
| + t.Fatal("a response with a Set-Cookie trailer must never be cached") |
| + } |
| + } |
| +} |
| + |
| +// The same refusal, at maybeStore, for an app that names the trailer in any |
| +// spelling and on a response with no other per-visitor marker. |
| +func TestMaybeStoreRefusesTrailerSetCookie(t *testing.T) { |
| + e := cookieEdge(t, cookieAcct, setCookies(), cookieHost) |
| + for _, k := range []string{http.TrailerPrefix + "Set-Cookie", http.TrailerPrefix + "set-cookie"} { |
| + req := httptest.NewRequest("GET", "http://"+cookieHost+"/m", nil) |
| + rec := &recordingResponseWriter{ResponseWriter: httptest.NewRecorder(), request: req, cacheable: true, maxBody: 1 << 20} |
| + rec.Header().Set("Cache-Control", "max-age=600") |
| + rec.Header()[k] = []string{"sid=abc"} |
| + rec.WriteHeader(200) |
| + rec.Write([]byte("ok")) |
| + key := CacheKey{Host: cookieHost, Method: "GET", Path: "/m"} |
| + e.maybeStore(key, rec) |
| + if _, ok := e.cache.get(key); ok { |
| + t.Fatalf("stored a response carrying %q", k) |
| + } |
| + } |
| +} |
| + |
| +// The cache path delivers the rewritten cookie, and (an older rule) never |
| +// stores a Set-Cookie response at all. |
| +func TestCookieScopeOnTheCachePath(t *testing.T) { |
| + e := cookieEdge(t, cookieAcct, func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("Cache-Control", "max-age=600") |
| + w.Header().Add("Set-Cookie", "a=1; Domain=oncarlos.com") |
| + w.Header().Add("Set-Cookie", "b=2; Domain=kmqa.oncarlos.com") |
| + w.Write([]byte("ok")) |
| + }, cookieHost) |
| + got := getCookies(t, e, "http://"+cookieHost+"/c") |
| + if want := []string{"a=1", "b=2; Domain=kmqa.oncarlos.com"}; !slices.Equal(got, want) { |
| + t.Fatalf("got %q, want %q", got, want) |
| + } |
| + if cachedAt(e, cookieHost, "/c") { |
| + t.Fatal("a Set-Cookie response must never be cached") |
| + } |
| +} |
| + |
| +// One log line per route per process, naming the route. |
| +func TestCookieScopeLogsOncePerRoute(t *testing.T) { |
| + var buf bytes.Buffer |
| + log.SetOutput(&buf) |
| + t.Cleanup(func() { log.SetOutput(os.Stderr) }) |
| + e := cookieEdge(t, cookieAcct, setCookies("a=1; Domain=oncarlos.com", "b=2; Domain=oncarlos.com"), cookieHost, "blog.kmqa.oncarlos.com") |
| + for range 3 { |
| + getCookies(t, e, "http://"+cookieHost+"/") |
| + } |
| + getCookies(t, e, "http://blog.kmqa.oncarlos.com/") |
| + out := buf.String() |
| + if n := strings.Count(out, "route "+cookieHost+" "); n != 1 { |
| + t.Fatalf("want one line for %s, got %d:\n%s", cookieHost, n, out) |
| + } |
| + if n := strings.Count(out, "route blog.kmqa.oncarlos.com "); n != 1 { |
| + t.Fatalf("want one line for blog, got %d:\n%s", n, out) |
| + } |
| +} |
| + |
| +func TestScopeSetCookieUnit(t *testing.T) { |
| + allowed := func(d string) bool { return d == "kmqa.oncarlos.com" } |
| + for _, tc := range []struct{ in, want string }{ |
| + {"a=1", "a=1"}, |
| + {"a=1;Domain=oncarlos.com;Path=/", "a=1;Path=/"}, |
| + {"a=1; Path=/; Domain=oncarlos.com", "a=1; Path=/"}, |
| + {"a=1; Domain=\t.oncarlos.com\t; Secure", "a=1; Secure"}, |
| + {"a=1; Domain=.; Secure", "a=1; Domain=.; Secure"}, |
| + {"a=1; Domain", "a=1; Domain"}, |
| + {"a=Domain=oncarlos.com; Secure", "a=Domain=oncarlos.com; Secure"}, |
| + {"a=1; Domain=kmqa.oncarlos.com", "a=1; Domain=kmqa.oncarlos.com"}, |
| + } { |
| + if got, _ := scopeSetCookie(tc.in, allowed); got != tc.want { |
| + t.Errorf("scopeSetCookie(%q) = %q, want %q", tc.in, got, tc.want) |
| + } |
| + } |
| +} |
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index a4222f36b..94fe9afb3 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -130,6 +130,13 @@ type Edge struct { |
| // environment; see steeringprobe.go for why it is a serving concern at |
| // all. |
| steeringHost string |
| + // appsDomain is the platform apps domain (PLATFORM_APPS_DOMAIN, |
| + // normalized), read once in New; cookieLogged is the set of route hosts |
| + // whose cookie rewrite has been logged. Both serve cookiescope.go. "" |
| + // disables the rewrite, which is every test Edge and every deployment |
| + // without an apps domain. |
| + appsDomain string |
| + cookieLogged sync.Map // route host → true |
| // Static serving config (EnableStatic): the deployment store the site |
| // tars are fetched from, the release pubkey channel pointers must verify |
| // against, and the on-box cache the trees are extracted into. staticStore |
| @@ -334,7 +341,7 @@ type upstream struct { |
| // New builds an Edge over reg and performs the startup reload. act may be nil |
| // (no hibernation). |
| func New(reg *registry.Registry, act Activator) (*Edge, error) { |
| - e := &Edge{reg: reg, act: act, gate: gate.New(), probers: newProberSet(), gzipOn: gzipEnabled(), steeringHost: steeringProbeHostFromEnv()} |
| + e := &Edge{reg: reg, act: act, gate: gate.New(), probers: newProberSet(), gzipOn: gzipEnabled(), steeringHost: steeringProbeHostFromEnv(), appsDomain: appsDomainFromEnv()} |
| if err := e.Reload(); err != nil { |
| return nil, err |
| } |
| @@ -641,7 +648,7 @@ func (e *Edge) Reload() error { |
| // wins outright rather than silently falling back to a stale Addr. |
| case t.Tunnel != "" && e.tunnels != nil: |
| p := tunnelProxy(t.Tunnel, e.tunnels) |
| - p.ModifyResponse = e.purgeModifier(t) |
| + p.ModifyResponse = e.responseModifier(t) |
| proxy = p |
| viaTunnel = true |
| // Fleet placement (Region == "fleet/<name>", registry.FleetPlacement) is |
| @@ -656,12 +663,12 @@ func (e *Edge) Reload() error { |
| // locally as a `-tunnel`-bound one; see the hibernation comment below. |
| case isFleetPlaced && e.tunnels != nil: |
| p := fleetProxy(fleet, t.FleetBox, e.tunnels) |
| - p.ModifyResponse = e.purgeModifier(t) |
| + p.ModifyResponse = e.responseModifier(t) |
| proxy = p |
| viaTunnel = true |
| case t.Socket != "": |
| p := unixProxy(t.Socket) |
| - p.ModifyResponse = e.purgeModifier(t) |
| + p.ModifyResponse = e.responseModifier(t) |
| proxy = p |
| // TCP upstreams. The gate is AddrList (registry), not t.Addr, so a |
| // multi-upstream route — which writes the Addrs column and leaves the |
| @@ -713,7 +720,7 @@ func (e *Edge) Reload() error { |
| probeSpecs[t.Host] = probeSpec{Host: t.Host, Addrs: addrs, Health: h} |
| p = multiProxy(t.Host, e.probers) |
| } |
| - p.ModifyResponse = e.purgeModifier(t) |
| + p.ModifyResponse = e.responseModifier(t) |
| proxy = p |
| case t.Kind == "static": |
| proxy = e.staticHandlerFor(t) |
| @@ -1727,6 +1734,8 @@ func (e *Edge) cacheMaxBody() int { |
| // - Cache-Control: private — the app itself saying "one visitor only". |
| // - Set-Cookie — the response carries per-visitor state (a session id, a |
| // CSRF token); replaying it would hand that cookie to later visitors. |
| +// As a trailer too (hasTrailerSetCookie): the cached header map replays |
| +// trailers on every hit. |
| // - an Authorization header on the REQUEST — the response is presumed |
| // private per RFC 9111 §3.5, unless Cache-Control explicitly says public, |
| // which is the app asserting the response is safe to share. |
| @@ -1753,7 +1762,7 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| if !ok || ttl <= 0 { |
| return |
| } |
| - if len(rec.Header().Values("Set-Cookie")) > 0 { |
| + if len(rec.Header().Values("Set-Cookie")) > 0 || hasTrailerSetCookie(rec.Header()) { |
| return |
| } |
| if len(rec.request.Header.Values("Authorization")) > 0 && !hasCCDirective(cc, "public") { |
| @@ -1794,6 +1803,21 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| }) |
| } |
| +// hasTrailerSetCookie reports whether h carries a Set-Cookie TRAILER. An |
| +// undeclared trailer reaches the recorder under the raw key |
| +// "Trailer:Set-Cookie" (httputil re-adds it with http.TrailerPrefix after the |
| +// body), which the Set-Cookie check above never sees, and writeCached would |
| +// replay it to every later visitor as a trailer. The key is not canonical |
| +// (the colon stops CanonicalMIMEHeaderKey), so it is matched by hand. |
| +func hasTrailerSetCookie(h http.Header) bool { |
| + for k, v := range h { |
| + if name, ok := strings.CutPrefix(k, http.TrailerPrefix); ok && strings.EqualFold(name, "Set-Cookie") && len(v) > 0 { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| // joinHeader returns every line of one header field as a single comma-joined |
| // value. RFC 9110 §5.3: a field may legally appear on multiple lines and that |
| // is identical in meaning to one comma-joined line — which is exactly what |