amadan
edge: route a login POST by next, and 404 one that lands on an ungated route
Review findings on the gate step (spec §4.2, amended in 2f0e570). A hostname that routes by path prefix resolves each request on its first path segment. "/.carlos/gate" has no segment that can ever be a prefix, so a login POST fell through to the FRONT host's own route: against an ungated front the password was proxied to an app that never asked for it, and against a gated one the cookie was minted for the front's host and never verified against the target's row — a gated prefix target could not be logged into at all. So for exactly POST /.carlos/gate the edge now parses the form (bounded by gate.MaxFormBody) before resolving the upstream and looks up by the path in `next`, which is the path the visitor was refused on. gate.safeNext is exported as gate.SafeNext so the path routed by and the path redirected to are produced by one sanitiser. On a host with no prefix map this resolves to exactly the route the request path would have. A form that will not parse falls back to resolving by r.URL.Path, as before. And a login POST that lands on an ungated route is now 404, never proxied: the form is only ever rendered by a gated route, so forwarding a field named "password" to an app that did not ask for it is ruled out. Every other method leaves the path ordinary. Admit no longer wraps the body when the edge has already parsed the form; ParseForm is idempotent, so its own parse is unchanged. Tests: a front host whose "paulca" prefix reaches a gated route can be logged into and admits on the front hostname, with the front app never reached; a login POST to an ungated route is 404 and the app sees nothing; the 401 refusal is metered to the route (the meter neighbour of the gate step had no test); and the 401 asserts X-Robots-Tag alongside no-store.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+220
−12
internal/edge/edge.go+35 −1internal/edge/gate_test.go+160 −0internal/gate/gate.go+20 −6internal/gate/gate_test.go+5 −5
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index 1795d43f8..4a213f7bb 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -1200,7 +1200,31 @@ func (e *Edge) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| // nothing else. |
| r, hopped := e.acceptHop(r, host) |
| - up, ok := e.lookupUpstreamPath(host, r.URL.Path) |
| + // A login POST resolves its upstream by the path in `next`, not by its |
| + // own (spec §4.2). On a hostname that routes by path prefix, |
| + // lookupUpstreamPath keys on the first segment; "/.carlos/gate" has no |
| + // segment that can ever be a prefix, so a login POST would fall through |
| + // to the FRONT host's route — the password would be judged by the wrong |
| + // row (or proxied to an app that never asked for it, which the |
| + // ungated-route branch below then refuses), and the cookie would be |
| + // signed for the wrong host. `next` is the path the visitor was refused |
| + // on, so resolving by it lands on the route that put up the page. |
| + // |
| + // Sanitised with gate.SafeNext before it is used, so the path routed by |
| + // and the path redirected to are the same string. A form that will not |
| + // parse (a truncated body, a bad encoding) falls back to today's |
| + // behaviour; Admit then answers it. On a host with no prefix map this |
| + // resolves to exactly the route r.URL.Path would have. |
| + loginPost := r.Method == http.MethodPost && r.URL.Path == gate.LoginPath |
| + lookupPath := r.URL.Path |
| + if loginPost { |
| + r.Body = http.MaxBytesReader(w, r.Body, gate.MaxFormBody) |
| + if err := r.ParseForm(); err == nil { |
| + lookupPath = gate.SafeNext(r.PostForm.Get("next")) |
| + } |
| + } |
| + |
| + up, ok := e.lookupUpstreamPath(host, lookupPath) |
| if !ok { |
| if hopped { |
| // A PROVEN hop from a sibling edge, for a host that has moved (or |
| @@ -1259,6 +1283,16 @@ func (e *Edge) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| // before the activator, so a stranger — a crawler, a scanner, a guessed |
| // hostname — never wakes a sleeping instance. Admit strips the cookie |
| // on the way through, so the app never sees it. |
| + // |
| + // A login POST that lands on an UNGATED route is a 404 and is never |
| + // proxied (spec §4.2): the form is only ever rendered by a gated route, |
| + // so this is a stray or a probe, and forwarding a field named |
| + // "password" to an app that never asked for it is the one outcome to |
| + // rule out. Every other method leaves the path ordinary. |
| + if loginPost && up.route.Gate == "" { |
| + http.NotFound(w, r) |
| + return |
| + } |
| if up.route.Gate != "" && !e.gate.Admit(w, r, up.route.Host, up.route.Gate) { |
| return |
| } |
diff --git a/internal/edge/gate_test.go b/internal/edge/gate_test.go| index 24645886b..59b83cd06 100644 |
| --- a/internal/edge/gate_test.go |
| +++ b/internal/edge/gate_test.go |
| @@ -67,6 +67,10 @@ func doHost(t *testing.T, srv *httptest.Server, method, host, path, cookie strin |
| } |
| func TestGatedRouteWithoutCookieNeverWakesOrProxies(t *testing.T) { |
| + // refusingActivator's Fatal message talks about caching — that is where |
| + // it came from — but here it proves the gate: a route flagged |
| + // Hibernate:true whose activator is never invoked is one a stranger |
| + // could not have woken. |
| _, srv, _, seen := gatedEdge(t, refusingActivator{t: t}, true) |
| resp := doHost(t, srv, http.MethodGet, "gated.example.com", "/docs", "") |
| if resp.StatusCode != http.StatusUnauthorized { |
| @@ -79,6 +83,9 @@ func TestGatedRouteWithoutCookieNeverWakesOrProxies(t *testing.T) { |
| if resp.Header.Get("Cache-Control") != "no-store" { |
| t.Fatalf("Cache-Control = %q", resp.Header.Get("Cache-Control")) |
| } |
| + if resp.Header.Get("X-Robots-Tag") != "noindex, nofollow" { |
| + t.Fatalf("X-Robots-Tag = %q", resp.Header.Get("X-Robots-Tag")) |
| + } |
| if *seen != "" || strings.Contains(string(b), "secret page") { |
| t.Fatal("upstream was reached without a cookie") |
| } |
| @@ -169,6 +176,159 @@ func TestGatedLoginPostOnAliasKeepsTheInboundHost(t *testing.T) { |
| } |
| } |
| +// gatedPrefixEdge stands up the shape §4.2 is about: a hostname whose own |
| +// route is UNGATED and whose "paulca" prefix reaches a GATED route on |
| +// another host. A login POST to /.carlos/gate has no first segment that can |
| +// ever be a prefix, so the edge must route it by `next` to land on the |
| +// gated target rather than on the front. |
| +func gatedPrefixEdge(t *testing.T) (*httptest.Server, string, *string, *string) { |
| + t.Helper() |
| + front, frontSeen := hostEchoBackend(t, "the front app") |
| + target, targetCookie := cookieEchoBackend(t, "secret page") |
| + h, err := gate.Hash("correct horse") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + reg := openReg(t) |
| + if err := reg.Upsert(registry.Route{Host: "front.example.com", Kind: "service", |
| + Addr: front.Listener.Addr().String(), Status: "ready"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.Upsert(registry.Route{Host: "gated.example.com", Kind: "service", |
| + Addr: target.Listener.Addr().String(), Status: "ready", Gate: h}); err != nil { |
| + t.Fatal(err) |
| + } |
| + upsertPath(t, reg, "front.example.com", "paulca", "gated.example.com") |
| + e, err := New(reg, &fakeActivator{}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + srv := httptest.NewServer(e) |
| + t.Cleanup(srv.Close) |
| + return srv, h, frontSeen, targetCookie |
| +} |
| + |
| +// TestGatedPrefixTargetCanBeLoggedInto is the §4.2 finding: routing the |
| +// login POST by its own path would resolve it to the FRONT host's ungated |
| +// route, which either proxies the password to an app that never asked for |
| +// it or mints a cookie signed for the wrong host. Routed by `next`, the |
| +// POST reaches the target whose hash judges it and whose host signs the |
| +// cookie — and that cookie then admits the visitor on the front hostname, |
| +// because the prefix resolves to the same route either way. |
| +func TestGatedPrefixTargetCanBeLoggedInto(t *testing.T) { |
| + srv, h, frontSeen, targetCookie := gatedPrefixEdge(t) |
| + |
| + // The refusal itself: the prefix reaches the gated route, so the front |
| + // app never sees the request. |
| + resp := doHost(t, srv, http.MethodGet, "front.example.com", "/paulca/x", "") |
| + b, _ := io.ReadAll(resp.Body) |
| + if resp.StatusCode != http.StatusUnauthorized || !strings.Contains(string(b), "Password required") { |
| + t.Fatalf("prefix GET without a cookie = %d %s, want the 401 login page", resp.StatusCode, b) |
| + } |
| + if *frontSeen != "" { |
| + t.Fatalf("the front app was reached: Host %q", *frontSeen) |
| + } |
| + |
| + form := url.Values{"password": {"correct horse"}, "next": {"/paulca/x"}} |
| + req, _ := http.NewRequest(http.MethodPost, srv.URL+gate.LoginPath, strings.NewReader(form.Encode())) |
| + req.Host = "front.example.com" |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + c := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} |
| + post, err := c.Do(req) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer post.Body.Close() |
| + if post.StatusCode != http.StatusSeeOther || post.Header.Get("Location") != "/paulca/x" { |
| + t.Fatalf("login POST = %d -> %q, want 303 -> /paulca/x", post.StatusCode, post.Header.Get("Location")) |
| + } |
| + if *frontSeen != "" { |
| + t.Fatalf("the login POST reached the front app: Host %q — the password must never be proxied", *frontSeen) |
| + } |
| + var got string |
| + for _, k := range post.Cookies() { |
| + if k.Name == gate.CookieName { |
| + got = k.Value |
| + } |
| + } |
| + if got == "" || !gate.Verify(got, "gated.example.com", h, time.Now()) { |
| + t.Fatalf("cookie %q does not verify for the target host", got) |
| + } |
| + |
| + resp = doHost(t, srv, http.MethodGet, "front.example.com", "/paulca/x", gate.CookieName+"="+got) |
| + b, _ = io.ReadAll(resp.Body) |
| + if resp.StatusCode != 200 || string(b) != "secret page" { |
| + t.Fatalf("prefix GET with the cookie = %d %q", resp.StatusCode, b) |
| + } |
| + if *targetCookie != "" { |
| + t.Fatalf("target Cookie = %q, want the gate cookie stripped", *targetCookie) |
| + } |
| +} |
| + |
| +// TestLoginPostToUngatedRouteIs404 is the other half of §4.2: the form is |
| +// only ever rendered by a gated route, so a login POST that lands on an |
| +// ungated one is a stray or a probe. It is answered 404 and the body — a |
| +// field literally named "password" — is never forwarded to an app that did |
| +// not ask for it. |
| +func TestLoginPostToUngatedRouteIs404(t *testing.T) { |
| + backend, seenHost := hostEchoBackend(t, "open page") |
| + reg := openReg(t) |
| + if err := reg.Upsert(registry.Route{Host: "open.example.com", Kind: "service", |
| + Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + e, err := New(reg, &fakeActivator{}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + srv := httptest.NewServer(e) |
| + defer srv.Close() |
| + |
| + form := url.Values{"password": {"correct horse"}, "next": {"/"}} |
| + req, _ := http.NewRequest(http.MethodPost, srv.URL+gate.LoginPath, strings.NewReader(form.Encode())) |
| + req.Host = "open.example.com" |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + resp, err := http.DefaultClient.Do(req) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer resp.Body.Close() |
| + if resp.StatusCode != http.StatusNotFound { |
| + t.Fatalf("login POST to an ungated route = %d, want 404", resp.StatusCode) |
| + } |
| + if *seenHost != "" { |
| + t.Fatalf("the app was reached with the password form: Host %q", *seenHost) |
| + } |
| + // Every other method on that path stays ordinary and reaches the app. |
| + if resp := doHost(t, srv, http.MethodGet, "open.example.com", gate.LoginPath, ""); resp.StatusCode != 200 { |
| + t.Fatalf("GET %s on an ungated route = %d, want the app's 200", gate.LoginPath, resp.StatusCode) |
| + } |
| +} |
| + |
| +// TestGatedRefusalIsMeteredToTheRoute pins the meter neighbour of the gate |
| +// step: the 401 is the tenant's page, served by the route, and its bytes |
| +// bill to the route like any other response it caused. Placing the gate |
| +// above the meter's wrap would make refusals free. |
| +func TestGatedRefusalIsMeteredToTheRoute(t *testing.T) { |
| + e, srv, _, _ := gatedEdge(t, &fakeActivator{}, false) |
| + m := NewUsageMeter() |
| + e.EnableUsage(m) |
| + resp := doHost(t, srv, http.MethodGet, "gated.example.com", "/docs", "") |
| + if resp.StatusCode != http.StatusUnauthorized { |
| + t.Fatalf("status = %d, want 401", resp.StatusCode) |
| + } |
| + row, ok := m.Drain()["gated.example.com"] |
| + if !ok { |
| + t.Fatal("no usage row for the gated route — the refusal was not metered") |
| + } |
| + if row.Requests != 1 { |
| + t.Fatalf("Requests = %d, want 1", row.Requests) |
| + } |
| + if row.BytesOut <= 0 { |
| + t.Fatalf("BytesOut = %d, want the login page's bytes", row.BytesOut) |
| + } |
| +} |
| + |
| func TestGatedRouteCacheIsNeverConsultedBeforeTheGate(t *testing.T) { |
| e, srv, _, _ := gatedEdge(t, &fakeActivator{}, false) |
| e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes)) |
diff --git a/internal/gate/gate.go b/internal/gate/gate.go| index 963ac66e3..217b1c0f7 100644 |
| --- a/internal/gate/gate.go |
| +++ b/internal/gate/gate.go |
| @@ -32,10 +32,14 @@ const ( |
| // value, so a stolen cookie is bounded even if the browser keeps it. |
| Lifetime = 7 * 24 * time.Hour |
| + // MaxFormBody bounds the login POST's body. Exported because the edge |
| + // parses that form too — it routes a login POST by the `next` field |
| + // (spec §4.2) — and the two must agree on one number. |
| + MaxFormBody = 4 << 10 |
| + |
| bcryptCost = 10 |
| minPassword = 8 |
| maxPassword = 72 // bcrypt's input limit; longer is refused, never truncated |
| - maxFormBody = 4 << 10 |
| ) |
| // CheckPassword is the door's rule, applied by the console before hashing. |
| @@ -111,17 +115,23 @@ func (g *Gate) Admit(w http.ResponseWriter, r *http.Request, host, hash string) |
| return true |
| } |
| } |
| - writePage(w, r, host, http.StatusUnauthorized, "", safeNext(r.URL.RequestURI())) |
| + writePage(w, r, host, http.StatusUnauthorized, "", SafeNext(r.URL.RequestURI())) |
| return false |
| } |
| func (g *Gate) login(w http.ResponseWriter, r *http.Request, host, hash string) { |
| - r.Body = http.MaxBytesReader(w, r.Body, maxFormBody) |
| + // The edge may already have parsed this form — it routes a login POST |
| + // by the `next` field (spec §4.2) — in which case the body is spent |
| + // and was bounded there. ParseForm itself is idempotent: with PostForm |
| + // already set it returns the cached values. |
| + if r.PostForm == nil { |
| + r.Body = http.MaxBytesReader(w, r.Body, MaxFormBody) |
| + } |
| if err := r.ParseForm(); err != nil { |
| writePage(w, r, host, http.StatusUnauthorized, msgWrong, "/") |
| return |
| } |
| - next := safeNext(r.PostForm.Get("next")) |
| + next := SafeNext(r.PostForm.Get("next")) |
| if !sameOrigin(r) { |
| // The POST needs no cookie, so SameSite does not cover it: a hidden |
| // form on an attacker's page could auto-submit eleven guesses and |
| @@ -181,8 +191,12 @@ func hostOnly(h string) string { |
| return h |
| } |
| -// safeNext is the entire open-redirect surface: a path on this host, or "/". |
| -func safeNext(next string) string { |
| +// SafeNext is the entire open-redirect surface: a path on this host, or |
| +// "/". Exported because the edge sanitises the login POST's `next` with it |
| +// before resolving the upstream (spec §4.2) — one sanitiser, so the path |
| +// the edge routes a login POST by and the path it redirects to cannot |
| +// differ. |
| +func SafeNext(next string) string { |
| // A control byte — a tab above all — survives net/http's header writer, |
| // which rewrites only CR and LF, and browsers strip it before they |
| // resolve the URL: "/\t/evil.example/" would leave here intact and land |
diff --git a/internal/gate/gate_test.go b/internal/gate/gate_test.go| index 7c10f67d7..76e8e5fd7 100644 |
| --- a/internal/gate/gate_test.go |
| +++ b/internal/gate/gate_test.go |
| @@ -205,7 +205,7 @@ func TestLoginPathOnGetIsJustAGatedRequest(t *testing.T) { |
| } |
| } |
| -// safeNext is the whole open-redirect surface, so pin it directly rather |
| +// SafeNext is the whole open-redirect surface, so pin it directly rather |
| // than only through the three shapes the POST test happens to send. |
| func TestSafeNextIsAPathOnThisHostOrRoot(t *testing.T) { |
| bad := []string{ |
| @@ -221,13 +221,13 @@ func TestSafeNextIsAPathOnThisHostOrRoot(t *testing.T) { |
| LoginPath, LoginPath + "?x=1", LoginPath + "#f", |
| } |
| for _, bad := range bad { |
| - if got := safeNext(bad); got != "/" { |
| - t.Errorf("safeNext(%q) = %q, want /", bad, got) |
| + if got := SafeNext(bad); got != "/" { |
| + t.Errorf("SafeNext(%q) = %q, want /", bad, got) |
| } |
| } |
| for _, ok := range []string{"/", "/docs?x=1", "/a/b#c"} { |
| - if got := safeNext(ok); got != ok { |
| - t.Errorf("safeNext(%q) = %q, want it unchanged", ok, got) |
| + if got := SafeNext(ok); got != ok { |
| + t.Errorf("SafeNext(%q) = %q, want it unchanged", ok, got) |
| } |
| } |
| } |