carlos / platform Public

gate: close the tab open-redirect, the decoy cookie lockout and the cross-site attempt drain

Review round 1 on internal/gate.

safeNext refused "//" and "/\" but not "/\t/evil.example/": net/http's
header writer rewrites only CR and LF, so the tab reached the wire and the
browser stripped it, resolving an off-origin "//evil.example/". Refuse any
control byte before the shape is judged.

Admit took only the first carlos_gate. Tenant hosts share a wildcard apex,
so a sibling host — or an XSS on one — could plant a decoy scoped to .apex
that sorts first and locked the visitor out for good. Accept any cookie by
that name that verifies; stripCookie already removes them all.

The login POST needs no cookie, so SameSite does not cover it and a hidden
form elsewhere could auto-submit eleven guesses and park a visitor at 429.
A present Origin must now match the request's own host — r.Host, not the
route's, so an alias posts from its own name — and a mismatch or "null" is
403 with the plain login page, before the bucket is touched and before
bcrypt runs. An absent Origin is still allowed, for curl and older clients.

Also: safeNext sends the login path itself to "/", the 303 carries
X-Robots-Tag, Verify requires the canonical expiry spelling, Admit
documents that an empty hash fails closed, and the page's heading and
label sizes separate.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 8bfd2a96476cff586bbf18e6340abb1df1c367d0 parent 017594e

4 files changed, +208 −17

internal/gate/cookie.go +3 −1
internal/gate/gate.go +68 −4
internal/gate/gate_test.go +135 −10
internal/gate/login.html +2 −2

diff --git a/internal/gate/cookie.go b/internal/gate/cookie.go

index 88f15480b..764adc720 100644

--- a/internal/gate/cookie.go

+++ b/internal/gate/cookie.go

@@ -48,7 +48,9 @@ func Verify(value, host, hash string, now time.Time) bool {

return false

}

exp, err := strconv.ParseInt(parts[1], 10, 64)

- if err != nil || exp <= now.Unix() {

+ // One spelling per expiry: ParseInt accepts "+123" and "-0", which

+ // would MAC differently from what Mint wrote for the same instant.

+ if err != nil || parts[1] != strconv.FormatInt(exp, 10) || exp <= now.Unix() {

return false

}

got, err := base64.RawURLEncoding.DecodeString(parts[2])

diff --git a/internal/gate/gate.go b/internal/gate/gate.go

index 160921145..963ac66e3 100644

--- a/internal/gate/gate.go

+++ b/internal/gate/gate.go

@@ -14,6 +14,7 @@ import (

"fmt"

"net"

"net/http"

+ "net/url"

"strings"

"time"

@@ -88,15 +89,27 @@ func New() *Gate {

// true: the request carries a valid cookie, now stripped from the Cookie

// header; the caller proxies it exactly as it would have. false: Admit has

// written the whole response — the login page (401), the redirect after a

-// successful POST (303), or the too-many-attempts page (429).

+// successful POST (303), the cross-origin refusal (403), or the

+// too-many-attempts page (429).

+//

+// Do not call Admit for a row with an empty hash. An empty hash fails

+// closed, not open — no password can match it and no cookie verifies

+// against it, so the host would become unreachable rather than public.

+// The edge gates on up.route.Gate != "" before it gets here.

func (g *Gate) Admit(w http.ResponseWriter, r *http.Request, host, hash string) bool {

if r.Method == http.MethodPost && r.URL.Path == LoginPath {

g.login(w, r, host, hash)

return false

}

- if c, err := r.Cookie(CookieName); err == nil && Verify(c.Value, host, hash, g.now()) {

- stripCookie(r)

- return true

+ // Any cookie by this name will do. Tenant hosts share a wildcard apex,

+ // so a sibling host — or an XSS on one — can plant a second

+ // carlos_gate for .apex that sorts ahead of the real one; taking only

+ // the first would let that lock the visitor out for good.

+ for _, c := range r.Cookies() {

+ if c.Name == CookieName && Verify(c.Value, host, hash, g.now()) {

+ stripCookie(r)

+ return true

+ }

}

writePage(w, r, host, http.StatusUnauthorized, "", safeNext(r.URL.RequestURI()))

return false

@@ -109,6 +122,15 @@ func (g *Gate) login(w http.ResponseWriter, r *http.Request, host, hash string)

return

}

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

+ // park a visitor at 429 on a site they can otherwise reach. Refuse

+ // before the bucket is touched and before bcrypt runs, and say

+ // nothing about the password — none was judged.

+ writePage(w, r, host, http.StatusForbidden, "", next)

+ return

+ }

if !g.limiter.Allow(clientKey(r) + "\x1f" + host) {

w.Header().Set("Retry-After", "60")

writePage(w, r, host, http.StatusTooManyRequests, msgTooMany, next)

@@ -128,14 +150,56 @@ func (g *Gate) login(w http.ResponseWriter, r *http.Request, host, hash string)

SameSite: http.SameSiteLaxMode,

})

w.Header().Set("Cache-Control", "no-store")

+ w.Header().Set("X-Robots-Tag", "noindex, nofollow")

http.Redirect(w, r, next, http.StatusSeeOther)

}

+// sameOrigin reports whether the POST came from the host it is posting to.

+// An absent Origin is allowed — curl and older clients send none — but a

+// present one must match, and "null" (a sandboxed frame, a file:// page)

+// never does. The comparison is against r.Host, not the route's host: an

+// alias posts from its own name, and the route's primary is a different

+// string.

+func sameOrigin(r *http.Request) bool {

+ o := r.Header.Get("Origin")

+ if o == "" {

+ return true

+ }

+ u, err := url.Parse(o)

+ if err != nil || u.Host == "" {

+ return false

+ }

+ return strings.EqualFold(hostOnly(u.Host), hostOnly(r.Host))

+// hostOnly drops the port, if there is one, so :443 and an implicit 443

+// are the same origin here.

+func hostOnly(h string) string {

+ if x, _, err := net.SplitHostPort(h); err == nil {

+ return x

+ }

+ return h

// safeNext is the entire open-redirect surface: a path on this host, or "/".

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

+ // as "//evil.example/". Refuse the lot before the shape is judged.

+ if strings.IndexFunc(next, func(r rune) bool { return r < 0x20 || r == 0x7f }) >= 0 {

+ return "/"

+ }

if !strings.HasPrefix(next, "/") || strings.HasPrefix(next, "//") || strings.HasPrefix(next, "/\\") {

return "/"

}

+ // Bouncing back to the login path lands on a page the app behind the

+ // curtain does not serve; root is the honest destination.

+ p, _, _ := strings.Cut(next, "?")

+ p, _, _ = strings.Cut(p, "#")

+ if p == LoginPath {

+ return "/"

+ }

return next

}

diff --git a/internal/gate/gate_test.go b/internal/gate/gate_test.go

index 879763d09..7c10f67d7 100644

--- a/internal/gate/gate_test.go

+++ b/internal/gate/gate_test.go

@@ -74,14 +74,14 @@ func TestLimiterTenPerMinute(t *testing.T) {

}

-func testGate(hash string) *Gate {

+func testGate() *Gate {

g := New()

g.now = func() time.Time { return time.Unix(1_800_000_000, 0) }

return g

}

func TestAdmitNoCookieIsTheLoginPage(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodHead} {

rec := httptest.NewRecorder()

@@ -128,7 +128,7 @@ func postLogin(g *Gate, host, hash, password, next string) *httptest.ResponseRec

}

func TestAdmitRightPasswordSetsCookieAndRedirects(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

rec := postLogin(g, "site.example", h, "correct horse", "/docs?x=1")

if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/docs?x=1" {

@@ -162,7 +162,7 @@ func TestAdmitRightPasswordSetsCookieAndRedirects(t *testing.T) {

}

func TestAdmitBadNextBecomesRoot(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

for _, next := range []string{"//evil.example/", "https://evil.example/", "docs", ""} {

rec := postLogin(g, "site.example", h, "correct horse", next)

@@ -173,7 +173,7 @@ func TestAdmitBadNextBecomesRoot(t *testing.T) {

}

func TestAdmitWrongPasswordThenLimit(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

compares := 0

g.compare = func(hash, password string) bool { compares++; return false }

@@ -196,7 +196,7 @@ func TestAdmitWrongPasswordThenLimit(t *testing.T) {

}

func TestLoginPathOnGetIsJustAGatedRequest(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

rec := httptest.NewRecorder()

r := httptest.NewRequest(http.MethodGet, "https://site.example"+LoginPath, nil)

@@ -208,7 +208,19 @@ func TestLoginPathOnGetIsJustAGatedRequest(t *testing.T) {

// 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) {

- for _, bad := range []string{"//evil.example/", `/\evil.example/`, `/\/evil.example/`, "https://evil.example/", "javascript:alert(1)", `\\evil.example\`, "", "docs", "?x=1", "#frag"} {

+ bad := []string{

+ "//evil.example/", `/\evil.example/`, `/\/evil.example/`,

+ "https://evil.example/", "javascript:alert(1)", `\\evil.example\`,

+ "", "docs", "?x=1", "#frag",

+ // Control bytes: net/http rewrites only CR and LF, and a browser

+ // strips the rest before resolving, so a tab here would land as

+ // "//evil.example/".

+ "/\t/evil.example/", "/\r\n/evil.example/", "/\x00/evil.example/",

+ "/\x7f/evil.example/", "/\x1f/evil.example/",

+ // The login path itself: the app behind the curtain does not serve it.

+ LoginPath, LoginPath + "?x=1", LoginPath + "#f",

+ }

+ for _, bad := range bad {

if got := safeNext(bad); got != "/" {

t.Errorf("safeNext(%q) = %q, want /", bad, got)

}

@@ -250,10 +262,12 @@ func TestStripCookieDropsTheHeaderWhenTheGateCookieIsTheOnlyOne(t *testing.T) {

}

// A body over maxFormBody fails to parse; that must cost no bcrypt and no

-// limiter token, so a flood of junk cannot lock a tenant out of their own

-// site.

+// limiter token. The bucket is keyed on the sender's own address, so a junk

+// flood only ever starves the flooder — except behind the backhaul fold,

+// where a sibling edge's address stands in for every visitor through it,

+// and there this is what keeps a flood from reaching them.

func TestOversizedFormBodyIsRefusedWithoutSpendingAnAttempt(t *testing.T) {

- g := testGate("")

+ g := testGate()

h, _ := Hash("correct horse")

compares := 0

g.compare = func(hash, password string) bool { compares++; return true }

@@ -272,3 +286,114 @@ func TestOversizedFormBodyIsRefusedWithoutSpendingAnAttempt(t *testing.T) {

t.Fatal("an unparseable body spent an attempt")

}

+// Tenant hosts share a wildcard apex, so a sibling host — or an XSS on one

+// — can plant a second carlos_gate scoped to .apex, and the browser sends

+// it first. Taking only the first cookie would lock the visitor out of a

+// site they hold a good cookie for, with no way to clear it.

+func TestAdmitAcceptsAnyValidGateCookieNotJustTheFirst(t *testing.T) {

+ g := testGate()

+ h, _ := Hash("correct horse")

+ good := Mint("site.example", h, g.now().Add(Lifetime))

+ rec := httptest.NewRecorder()

+ r := httptest.NewRequest(http.MethodGet, "https://site.example/", nil)

+ r.Header.Set("Cookie", CookieName+"=v1.9999999999.junk; theme=dark; "+CookieName+"="+good)

+ if !g.Admit(rec, r, "site.example", h) {

+ t.Fatal("a planted decoy cookie locked the visitor out")

+ }

+ got := r.Header.Get("Cookie")

+ if strings.Contains(got, CookieName) {

+ t.Fatalf("upstream Cookie = %q, want every gate cookie stripped", got)

+ }

+ if got != "theme=dark" {

+ t.Fatalf("upstream Cookie = %q, want the other cookies untouched", got)

+ }

+func postLoginFrom(g *Gate, reqHost, host, hash, password, next, origin string) *httptest.ResponseRecorder {

+ rec := httptest.NewRecorder()

+ form := url.Values{"password": {password}, "next": {next}}

+ r := httptest.NewRequest(http.MethodPost, "https://"+reqHost+LoginPath, strings.NewReader(form.Encode()))

+ r.Header.Set("Content-Type", "application/x-www-form-urlencoded")

+ if origin != "" {

+ r.Header.Set("Origin", origin)

+ }

+ r.RemoteAddr = "203.0.113.9:4242"

+ g.Admit(rec, r, host, hash)

+ return rec

+// The login POST needs no cookie, so SameSite does not cover it: without an

+// Origin check a hidden form on an attacker's page could auto-submit eleven

+// guesses and park a visitor at 429.

+func TestLoginRefusesACrossOriginPostWithoutSpendingAnAttempt(t *testing.T) {

+ g := testGate()

+ h, _ := Hash("correct horse")

+ compares := 0

+ g.compare = func(hash, password string) bool { compares++; return true }

+ for _, origin := range []string{"https://evil.example", "null", "https://evil.example:443", "http://site.example.evil.example"} {

+ rec := postLoginFrom(g, "site.example", "site.example", h, "correct horse", "/docs", origin)

+ if rec.Code != http.StatusForbidden {

+ t.Fatalf("Origin %q -> %d, want 403", origin, rec.Code)

+ }

+ if strings.Contains(rec.Body.String(), msgWrong) || strings.Contains(rec.Body.String(), msgTooMany) {

+ t.Fatalf("Origin %q: the 403 page judged the password: %s", origin, rec.Body.String())

+ }

+ if !strings.Contains(rec.Body.String(), "Password required") {

+ t.Fatalf("Origin %q: the 403 is not the login page: %s", origin, rec.Body.String())

+ }

+ if compares != 0 {

+ t.Fatalf("bcrypt compared %d times on cross-origin posts, want 0", compares)

+ }

+ // The bucket is untouched: a visitor who was targeted can still make

+ // all ten of their own attempts.

+ for i := 0; i < 10; i++ {

+ if rec := postLogin(g, "site.example", h, "correct horse", "/"); rec.Code == http.StatusTooManyRequests {

+ t.Fatalf("attempt %d was rate-limited — the cross-origin posts drained the bucket", i+1)

+ }

+func TestLoginAllowsSameOriginAndAbsentOrigin(t *testing.T) {

+ g := testGate()

+ h, _ := Hash("correct horse")

+ cases := []struct{ name, reqHost, origin string }{

+ {"same origin", "site.example", "https://site.example"},

+ {"absent origin (curl)", "site.example", ""},

+ {"origin and host both carry the port", "site.example:8443", "https://site.example:8443"},

+ {"port on one side only", "site.example:8443", "https://site.example"},

+ {"case-insensitive host", "site.example", "https://SITE.example"},

+ // An alias posts from its own name, so the comparison is against

+ // r.Host and not the route's primary hostname.

+ {"alias posts from the alias", "alias.example", "https://alias.example"},

+ }

+ for _, c := range cases {

+ rec := postLoginFrom(g, c.reqHost, "site.example", h, "correct horse", "/docs", c.origin)

+ if rec.Code != http.StatusSeeOther {

+ t.Errorf("%s: %d, want 303", c.name, rec.Code)

+ }

+func TestSuccessfulLoginIsNoStoreAndNoindex(t *testing.T) {

+ g := testGate()

+ h, _ := Hash("correct horse")

+ rec := postLogin(g, "site.example", h, "correct horse", "/docs")

+ if cc := rec.Header().Get("Cache-Control"); cc != "no-store" {

+ t.Errorf("Cache-Control = %q", cc)

+ }

+ if rt := rec.Header().Get("X-Robots-Tag"); rt != "noindex, nofollow" {

+ t.Errorf("X-Robots-Tag = %q", rt)

+ }

+func TestCookieExpiryEncodingIsCanonical(t *testing.T) {

+ now := time.Unix(1_800_000_000, 0)

+ v := Mint("site.example", "$2a$10$hash", now.Add(Lifetime))

+ parts := strings.Split(v, ".")

+ if Verify(parts[0]+".+"+parts[1]+"."+parts[2], "site.example", "$2a$10$hash", now) {

+ t.Fatal(`a "+"-signed expiry was accepted`)

+ }

diff --git a/internal/gate/login.html b/internal/gate/login.html

index 670bdd09c..8c379db9d 100644

--- a/internal/gate/login.html

+++ b/internal/gate/login.html

@@ -23,10 +23,10 @@ body {

min-height: 100vh; display: flex; align-items: center; justify-content: center; padding: 1.25rem;

}

main { width: 100%; max-width: 22rem; }

-h1 { font-size: 1.25rem; margin-bottom: 0.5rem; }

+h1 { font-size: 1.5rem; margin-bottom: 0.5rem; }

p { color: var(--muted); margin-bottom: 1.25rem; }

p code { color: var(--ink); font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; font-size: 0.95em; }

-label { display: block; font-size: 0.85rem; color: var(--muted); margin-bottom: 0.35rem; }

+label { display: block; font-size: 0.8rem; color: var(--muted); margin-bottom: 0.35rem; }

input[type=password] {

width: 100%; font: inherit; color: var(--ink); background: var(--bg);

border: 1px solid var(--rule); border-radius: 0.4rem; padding: 0.55rem 0.7rem; margin-bottom: 0.9rem;