vole
Clone
Read only, no credentials:
https://rodent.software/vole.git
Write over SSH with a registered key:
git clone ssh://git@rodent.software:2222/vole.git
A free and open-source forge that lives in your Git repo.
Harden the browser sign-in surface
edf9db6e07e5fd676049abcd41e78790b0953652 · Keenan Nemetz · 13d ago · parent 62b59a76
Harden the browser sign-in surface The passkey ceremonies were the one place an anonymous client could make the instance work, and they answered more than they had to. Sign-in told a stranger whether an account existed, nothing bounded how often a client could ask, an invite token sat in the request path where every access log would keep it, and a session lookup hashed whatever cookie it was handed. Sign-in no longer answers the question. An unknown account, a retired one and an account with no passkey all get an ordinary assertion request naming one credential that will not be found, which is a shape WebAuthn cannot distinguish from a real account. Only finishing the ceremony fails, generically. The ceremonies are rate-limited per address (20 begins a minute, 60 finishes) and per account name (10 begins), and registration per address (5), with 429 and Retry-After past the budget. The in-flight ceremony map is capped at 4096 entries, swept on every put, five-minute TTL, so a client that never finishes one cannot grow it. The key is the configured trusted-proxy header when there is one, the connection otherwise. Enrollment tokens moved into the URL fragment: /invite#<token> is never sent to the server, so it reaches no log of ours and no proxy access log, and no referrer carries it. Links in the old /invite/<token> shape are redirected into the fragment, and the paths this instance does log are redacted. The page itself renders before the token is known, so the account name appears only after the ceremony. CSRF checks no longer parse the request body. JSON endpoints require the X-Vole-CSRF header; form endpoints parse a body that readForm has already bounded with MaxBytesReader, and then compare the field or the header. Logout bounds its body before parsing and answers a client with no session the same way it answers a successful sign-out. Sign-in, registration, enrollment, the account page and the admin invite page are no-store, no-cache and no-referrer. HSTS now follows the transport rather than the passkey feature: a request over TLS, or an https public_url, is an HTTPS instance whether or not browser sign-in is configured. Sessions refuse a cookie that is not the shape this store issues (base64url, at most 64 characters) before hashing or querying it, and the stored User-Agent is cut to 256 bytes. A passkey whose signature counter does not advance is refused at sign-in and recorded as auth.clone-warning. The policy is documented; the trade is that an authenticator with a broken counter needs an admin to remove its passkey. Tests: the limiter's window and per-key budgets, 429 with Retry-After on a real begin, the decoy's shape and its silence about account state, the ceremony cap and its sweep, the JSON endpoints refusing a form CSRF field, HSTS across TLS/proxy/http, no-store on the auth pages, the invite token redacted from a logged server error, the legacy invite redirect, logout refusing an oversized body, the account page's headers, session token shape and User-Agent truncation.
server/web/auth_hardening_test.go +271
| @@ -0,0 +1,271 @@ | |||
| 1 | package web | ||
| 2 | |||
| 3 | // Hardening of the authentication surface: what the browser is told to cache, | ||
| 4 | // what HSTS covers, which budget answers a busy client, and what a login for an | ||
| 5 | // account that does not exist looks like. | ||
| 6 | |||
| 7 | import ( | ||
| 8 | "crypto/tls" | ||
| 9 | "fmt" | ||
| 10 | "net/http" | ||
| 11 | "net/http/httptest" | ||
| 12 | "path/filepath" | ||
| 13 | "strings" | ||
| 14 | "testing" | ||
| 15 | "time" | ||
| 16 | |||
| 17 | "github.com/rodent-software/vole/forge/config" | ||
| 18 | "github.com/rodent-software/vole/forge/identity" | ||
| 19 | ) | ||
| 20 | |||
| 21 | // authWeb is a Web with browser sign-in enabled and a real roster, so the | ||
| 22 | // ceremony handlers have the stores they call without a whole server. | ||
| 23 | func authWeb(t *testing.T) *Web { | ||
| 24 | t.Helper() | ||
| 25 | dir := t.TempDir() | ||
| 26 | path := filepath.Join(dir, "users.db") | ||
| 27 | if err := identity.Init(path); err != nil { | ||
| 28 | t.Fatal(err) | ||
| 29 | } | ||
| 30 | ident, err := identity.Open(path) | ||
| 31 | if err != nil { | ||
| 32 | t.Fatal(err) | ||
| 33 | } | ||
| 34 | t.Cleanup(func() { ident.Close() }) | ||
| 35 | ident.AuditPath = filepath.Join(dir, "audit.log") | ||
| 36 | cfg := &config.Config{DataDir: dir, PublicURL: "https://forge.example.com"} | ||
| 37 | w, err := New(nil, nil, ident, nil, cfg) | ||
| 38 | if err != nil { | ||
| 39 | t.Fatal(err) | ||
| 40 | } | ||
| 41 | return w | ||
| 42 | } | ||
| 43 | |||
| 44 | func TestHSTSFollowsTheTransportNotPasskeys(t *testing.T) { | ||
| 45 | t.Parallel() | ||
| 46 | cases := []struct { | ||
| 47 | name string | ||
| 48 | cfg *config.Config | ||
| 49 | tls bool | ||
| 50 | wantHST bool | ||
| 51 | }{ | ||
| 52 | {"plain http with no public url", &config.Config{}, false, false}, | ||
| 53 | {"http public url", &config.Config{PublicURL: "http://forge.example.com"}, false, false}, | ||
| 54 | {"https public url behind a proxy", &config.Config{PublicURL: "https://forge.example.com"}, false, true}, | ||
| 55 | {"ACME listener answering plain HTTP", &config.Config{HTTPSAddr: ":443"}, false, false}, | ||
| 56 | {"request over TLS", &config.Config{}, true, true}, | ||
| 57 | } | ||
| 58 | for _, c := range cases { | ||
| 59 | w := &Web{Cfg: c.cfg} | ||
| 60 | r := httptest.NewRequest("GET", "http://forge.example.com/", nil) | ||
| 61 | if c.tls { | ||
| 62 | r.TLS = &tls.ConnectionState{} | ||
| 63 | } | ||
| 64 | rw := httptest.NewRecorder() | ||
| 65 | w.securityHeaders(rw, r) | ||
| 66 | got := rw.Header().Get("Strict-Transport-Security") | ||
| 67 | if (got != "") != c.wantHST { | ||
| 68 | t.Fatalf("%s: HSTS = %q, want present=%v", c.name, got, c.wantHST) | ||
| 69 | } | ||
| 70 | } | ||
| 71 | } | ||
| 72 | |||
| 73 | // TestAuthPagesAreNotCached: an enrollment page, a sign-in form and an account | ||
| 74 | // page are per-browser state, and a page the browser re-requests from a cache is | ||
| 75 | // a token re-used from a cache. | ||
| 76 | func TestAuthPagesAreNotCached(t *testing.T) { | ||
| 77 | t.Parallel() | ||
| 78 | w := newTestWeb(t) | ||
| 79 | for _, path := range []string{"/login", "/register", "/invite"} { | ||
| 80 | rw := httptest.NewRecorder() | ||
| 81 | switch path { | ||
| 82 | case "/login": | ||
| 83 | w.LoginPage(rw, httptest.NewRequest("GET", path, nil)) | ||
| 84 | case "/register": | ||
| 85 | w.RegisterPage(rw, httptest.NewRequest("GET", path, nil)) | ||
| 86 | case "/invite": | ||
| 87 | w.InvitePage(rw, httptest.NewRequest("GET", path, nil)) | ||
| 88 | } | ||
| 89 | if got := rw.Header().Get("Cache-Control"); got != "no-store" { | ||
| 90 | t.Fatalf("%s: Cache-Control = %q, want no-store", path, got) | ||
| 91 | } | ||
| 92 | if got := rw.Header().Get("Pragma"); got != "no-cache" { | ||
| 93 | t.Fatalf("%s: Pragma = %q, want no-cache", path, got) | ||
| 94 | } | ||
| 95 | if got := rw.Header().Get("Referrer-Policy"); got != "no-referrer" { | ||
| 96 | t.Fatalf("%s: Referrer-Policy = %q, want no-referrer", path, got) | ||
| 97 | } | ||
| 98 | } | ||
| 99 | } | ||
| 100 | |||
| 101 | // TestRateLimiterBudgets is the limiter itself: a window's worth of attempts is | ||
| 102 | // allowed, the next is refused with a wait, and a later window starts over. | ||
| 103 | func TestRateLimiterBudgets(t *testing.T) { | ||
| 104 | t.Parallel() | ||
| 105 | now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) | ||
| 106 | l := newRateLimiter(2, time.Minute) | ||
| 107 | l.now = func() time.Time { return now } | ||
| 108 | |||
| 109 | for i := 0; i < 2; i++ { | ||
| 110 | if ok, _ := l.allow("1.2.3.4"); !ok { | ||
| 111 | t.Fatalf("attempt %d was refused inside the budget", i+1) | ||
| 112 | } | ||
| 113 | } | ||
| 114 | ok, wait := l.allow("1.2.3.4") | ||
| 115 | if ok { | ||
| 116 | t.Fatal("the attempt past the budget was allowed") | ||
| 117 | } | ||
| 118 | if wait <= 0 || wait > time.Minute { | ||
| 119 | t.Fatalf("wait = %s, want a positive wait inside the window", wait) | ||
| 120 | } | ||
| 121 | // Another key has its own budget. | ||
| 122 | if ok, _ := l.allow("5.6.7.8"); !ok { | ||
| 123 | t.Fatal("a different address shared another address's budget") | ||
| 124 | } | ||
| 125 | // The window passing resets it. | ||
| 126 | now = now.Add(2 * time.Minute) | ||
| 127 | if ok, _ := l.allow("1.2.3.4"); !ok { | ||
| 128 | t.Fatal("the budget did not reset after the window") | ||
| 129 | } | ||
| 130 | } | ||
| 131 | |||
| 132 | // TestAuthBeginIsRateLimited: the begin endpoint answers 429 with Retry-After once | ||
| 133 | // an address has spent its budget, and the refusal is a JSON error like every | ||
| 134 | // other failure on that endpoint. | ||
| 135 | func TestAuthBeginIsRateLimited(t *testing.T) { | ||
| 136 | t.Parallel() | ||
| 137 | w := authWeb(t) | ||
| 138 | w.limits.beginAddress = newRateLimiter(2, time.Minute) | ||
| 139 | |||
| 140 | attempt := func(addr string) *httptest.ResponseRecorder { | ||
| 141 | r := httptest.NewRequest("POST", "/auth/login/begin", strings.NewReader(`{"username":"alice"}`)) | ||
| 142 | r.RemoteAddr = addr | ||
| 143 | rw := httptest.NewRecorder() | ||
| 144 | w.LoginBegin(rw, r) | ||
| 145 | return rw | ||
| 146 | } | ||
| 147 | for i := 0; i < 2; i++ { | ||
| 148 | if rw := attempt("203.0.113.9:1234"); rw.Code == http.StatusTooManyRequests { | ||
| 149 | t.Fatalf("attempt %d was limited too early", i+1) | ||
| 150 | } | ||
| 151 | } | ||
| 152 | rw := attempt("203.0.113.9:1234") | ||
| 153 | if rw.Code != http.StatusTooManyRequests { | ||
| 154 | t.Fatalf("status = %d, want 429", rw.Code) | ||
| 155 | } | ||
| 156 | if rw.Header().Get("Retry-After") == "" { | ||
| 157 | t.Fatal("a 429 without Retry-After") | ||
| 158 | } | ||
| 159 | if !strings.Contains(rw.Body.String(), "too many attempts") { | ||
| 160 | t.Fatalf("body = %q", rw.Body.String()) | ||
| 161 | } | ||
| 162 | if rw := attempt("198.51.100.7:1234"); rw.Code == http.StatusTooManyRequests { | ||
| 163 | t.Fatal("one address's budget limited another") | ||
| 164 | } | ||
| 165 | } | ||
| 166 | |||
| 167 | // TestLoginBeginDoesNotSayWhetherAnAccountExists: an unknown account gets the | ||
| 168 | // same assertion request a real one does, so the endpoint cannot be asked whether | ||
| 169 | // somebody has an account here. | ||
| 170 | func TestLoginBeginDoesNotSayWhetherAnAccountExists(t *testing.T) { | ||
| 171 | t.Parallel() | ||
| 172 | w := authWeb(t) | ||
| 173 | r := httptest.NewRequest("POST", "/auth/login/begin", strings.NewReader(`{"username":"nobody"}`)) | ||
| 174 | r.RemoteAddr = "127.0.0.1:1234" | ||
| 175 | rw := httptest.NewRecorder() | ||
| 176 | w.LoginBegin(rw, r) | ||
| 177 | if rw.Code != http.StatusOK { | ||
| 178 | t.Fatalf("unknown account begin = %d %s, want 200 with an assertion request", | ||
| 179 | rw.Code, rw.Body.String()) | ||
| 180 | } | ||
| 181 | body := rw.Body.String() | ||
| 182 | if !strings.Contains(body, `"publicKey"`) || !strings.Contains(body, `"allowCredentials"`) { | ||
| 183 | t.Fatalf("the decoy is not shaped like a real assertion request: %s", body) | ||
| 184 | } | ||
| 185 | for _, leak := range []string{"no passkey", "no such", "not found"} { | ||
| 186 | if strings.Contains(body, leak) { | ||
| 187 | t.Fatalf("the response names the account's state: %s", body) | ||
| 188 | } | ||
| 189 | } | ||
| 190 | } | ||
| 191 | |||
| 192 | // TestCeremonyStoreIsBounded: an anonymous client cannot grow the in-flight | ||
| 193 | // ceremony map past its cap, and expired ceremonies do not count against it. | ||
| 194 | func TestCeremonyStoreIsBounded(t *testing.T) { | ||
| 195 | t.Parallel() | ||
| 196 | s := newCeremonyStore() | ||
| 197 | for i := 0; i < maxCeremonies; i++ { | ||
| 198 | if _, err := s.put(ceremony{Kind: "login", Name: "account"}); err != nil { | ||
| 199 | t.Fatalf("put %d: %v", i, err) | ||
| 200 | } | ||
| 201 | } | ||
| 202 | if _, err := s.put(ceremony{Kind: "login", Name: "account"}); err == nil { | ||
| 203 | t.Fatal("the ceremony map grew past its cap") | ||
| 204 | } | ||
| 205 | // Expiring them makes room again, because a put sweeps first. | ||
| 206 | s.mu.Lock() | ||
| 207 | for k, c := range s.m { | ||
| 208 | c.Expires = time.Now().Add(-time.Second) | ||
| 209 | s.m[k] = c | ||
| 210 | } | ||
| 211 | s.mu.Unlock() | ||
| 212 | if _, err := s.put(ceremony{Kind: "login", Name: "account"}); err != nil { | ||
| 213 | t.Fatalf("a swept store refused a ceremony: %v", err) | ||
| 214 | } | ||
| 215 | } | ||
| 216 | |||
| 217 | // TestJSONEndpointsRequireTheCSRFHeader: the header is the only source for the | ||
| 218 | // JSON ceremonies. A form field is not accepted there, so a cross-site form post | ||
| 219 | // cannot carry a token the endpoint would read. | ||
| 220 | func TestJSONEndpointsRequireTheCSRFHeader(t *testing.T) { | ||
| 221 | t.Parallel() | ||
| 222 | w := authWeb(t) | ||
| 223 | r := httptest.NewRequest("POST", "/account/passkeys/begin", strings.NewReader(`{"label":"x"}`)) | ||
| 224 | r.PostForm = map[string][]string{"csrf": {"from-the-form"}} | ||
| 225 | if w.checkCSRF(r) { | ||
| 226 | t.Fatal("the JSON CSRF check accepted a form field") | ||
| 227 | } | ||
| 228 | } | ||
| 229 | |||
| 230 | // TestClientAddressUsesTheTrustedHeader: the address budget keys on the address | ||
| 231 | // the instance is configured to trust, and on the connection when it is not. | ||
| 232 | func TestClientAddressUsesTheTrustedHeader(t *testing.T) { | ||
| 233 | t.Parallel() | ||
| 234 | r := httptest.NewRequest("POST", "/auth/login/begin", nil) | ||
| 235 | r.RemoteAddr = "10.0.0.1:5000" | ||
| 236 | r.Header.Set("X-Forwarded-For", "203.0.113.9, 10.0.0.1") | ||
| 237 | if got := (&Web{}).clientAddress(r); got != "10.0.0.1" { | ||
| 238 | t.Fatalf("clientAddress without a configured header = %q, want the connection", got) | ||
| 239 | } | ||
| 240 | w := &Web{Cfg: &config.Config{GitHTTPClientIPHeader: "X-Forwarded-For"}} | ||
| 241 | if got := w.clientAddress(r); got != "203.0.113.9" { | ||
| 242 | t.Fatalf("clientAddress with a trusted header = %q, want the first entry", got) | ||
| 243 | } | ||
| 244 | } | ||
| 245 | |||
| 246 | // TestInviteTokensAreRedactedFromLogs: a request path that used to carry an | ||
| 247 | // enrollment token must not put it in a log line, whatever the log is for. | ||
| 248 | func TestInviteTokensAreRedactedFromLogs(t *testing.T) { | ||
| 249 | t.Parallel() | ||
| 250 | if got := loggablePath("/invite/" + "secret-token"); got != "/invite/[redacted]" { | ||
| 251 | t.Fatalf("loggablePath kept the token: %q", got) | ||
| 252 | } | ||
| 253 | if got := loggablePath("/r/demo/issues"); got != "/r/demo/issues" { | ||
| 254 | t.Fatalf("loggablePath changed an ordinary path: %q", got) | ||
| 255 | } | ||
| 256 | var lines []string | ||
| 257 | w := newTestWeb(t) | ||
| 258 | w.Logger = func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) } | ||
| 259 | rw := httptest.NewRecorder() | ||
| 260 | w.serverError(rw, httptest.NewRequest("GET", "/invite/secret-token", nil), "boom") | ||
| 261 | if len(lines) != 1 { | ||
| 262 | t.Fatalf("logged %d lines, want 1", len(lines)) | ||
| 263 | } | ||
| 264 | joined := strings.Join(lines, "\n") | ||
| 265 | if strings.Contains(joined, "secret-token") { | ||
| 266 | t.Fatalf("the log line names the token: %q", joined) | ||
| 267 | } | ||
| 268 | if !strings.Contains(joined, "/invite/[redacted]") { | ||
| 269 | t.Fatalf("the log line does not show where the request went: %q", joined) | ||
| 270 | } | ||
| 271 | } | ||