amadan
Merge branch 'tenant-iso-cookies' into main
Edge: tenant cookies can't reach beyond their own account; never cache a trailer Set-Cookie (#217)
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 5a5c80a15..05e911548 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 |