diff --git a/api/internal/auth/sessions.go b/api/internal/auth/sessions.go index ddc72af33..1e7f3559d 100644 --- a/api/internal/auth/sessions.go +++ b/api/internal/auth/sessions.go @@ -3,7 +3,9 @@ package auth import ( "context" "crypto/rand" + "crypto/sha256" "encoding/base64" + "encoding/hex" "errors" "log/slog" "net/http" @@ -49,15 +51,25 @@ type SessionStore struct { // NewSessionStore returns a new SessionStore. func NewSessionStore(store *db.Store) *SessionStore { return &SessionStore{db: store} } +// sessionDigest returns the hex SHA-256 of a raw session cookie value. The +// sessions table stores only this digest (column "token"), never the cookie +// value, so a copy of the database cannot be replayed as a session cookie. +// The raw value is 256 bits of CSPRNG output, so a plain unsalted hash is +// sufficient and keeps lookup a single equality match. +func sessionDigest(raw string) string { + sum := sha256.Sum256([]byte(raw)) + return hex.EncodeToString(sum[:]) +} + // Create writes a new session row for the given user and returns the -// values that need to go back on the response (session cookie value + -// CSRF token). +// values that need to go back on the response (raw session cookie value + +// CSRF token). Only the digest of the session value is persisted. func (s *SessionStore) Create(ctx context.Context, userID int64) (session, csrf string, err error) { session = randomToken() csrf = randomToken() _, err = s.db.DB.ExecContext(ctx, `INSERT INTO sessions(token, user_id, csrf_token, expires_at) VALUES (?,?,?,?)`, - session, userID, csrf, time.Now().Add(sessionTTL).UTC().Format(time.RFC3339), + sessionDigest(session), userID, csrf, time.Now().Add(sessionTTL).UTC().Format(time.RFC3339), ) return } @@ -141,6 +153,7 @@ func (s *SessionStore) Authenticate(next http.Handler) http.Handler { } func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, error) { + digest := sessionDigest(token) var ( u User csrf string @@ -149,7 +162,7 @@ func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, err := s.db.DB.QueryRowContext(ctx, ` SELECT u.id, u.username, u.display_name, u.email, u.role, s.csrf_token, s.expires_at FROM sessions s JOIN users u ON u.id = s.user_id - WHERE s.token = ?`, token, + WHERE s.token = ?`, digest, ).Scan(&u.ID, &u.Username, &u.DisplayName, &u.Email, &u.Role, &csrf, &expires) if err != nil { return nil, "", err @@ -165,11 +178,11 @@ func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, // session from the client's perspective — delete it and make the // user log in again. Log so it's visible in ops. slog.Warn("session expires_at parse", "err", perr, "value", expires) - _, _ = s.db.DB.ExecContext(ctx, `DELETE FROM sessions WHERE token = ?`, token) + _, _ = s.db.DB.ExecContext(ctx, `DELETE FROM sessions WHERE token = ?`, digest) return nil, "", errors.New("session invalid") } if time.Now().After(exp) { - _, _ = s.db.DB.ExecContext(ctx, `DELETE FROM sessions WHERE token = ?`, token) + _, _ = s.db.DB.ExecContext(ctx, `DELETE FROM sessions WHERE token = ?`, digest) return nil, "", errors.New("session expired") } return &u, csrf, nil @@ -179,7 +192,7 @@ func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, func (s *SessionStore) HandleLogout() http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { if c, err := req.Cookie(sessionCookie); err == nil { - _, _ = s.db.DB.ExecContext(req.Context(), `DELETE FROM sessions WHERE token = ?`, c.Value) + _, _ = s.db.DB.ExecContext(req.Context(), `DELETE FROM sessions WHERE token = ?`, sessionDigest(c.Value)) } clearCookie(w, sessionCookie) // The CSRF cookie is JS-readable while set (see setCSRFCookie), but diff --git a/api/internal/auth/sessions_digest_test.go b/api/internal/auth/sessions_digest_test.go new file mode 100644 index 000000000..65b27b0fa --- /dev/null +++ b/api/internal/auth/sessions_digest_test.go @@ -0,0 +1,345 @@ +package auth + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/ValgulNecron/gameplane/api/internal/db" +) + +// storedSessionToken returns the sessions.token value stored for userID's +// only session. +func storedSessionToken(t *testing.T, s *db.Store, userID int64) string { + t.Helper() + var stored string + if err := s.DB.QueryRowContext(context.Background(), + `SELECT token FROM sessions WHERE user_id = ?`, userID).Scan(&stored); err != nil { + t.Fatalf("read stored session token: %v", err) + } + return stored +} + +// countSessions runs a SELECT COUNT(*) query against the sessions table. +func countSessions(t *testing.T, s *db.Store, query string, args ...any) int { + t.Helper() + var n int + if err := s.DB.QueryRowContext(context.Background(), query, args...).Scan(&n); err != nil { + t.Fatalf("count sessions: %v", err) + } + return n +} + +func TestSessionDigest_Deterministic(t *testing.T) { + a := sessionDigest("a") + if a != sessionDigest("a") { + t.Fatal("sessionDigest is not deterministic") + } + if a == sessionDigest("b") { + t.Fatal("sessionDigest(a) == sessionDigest(b)") + } + sum := sha256.Sum256([]byte("a")) + if want := hex.EncodeToString(sum[:]); a != want { + t.Fatalf("sessionDigest(a) = %q, want %q", a, want) + } + if len(a) != 64 { + t.Fatalf("digest length = %d, want 64", len(a)) + } +} + +func TestSessions_StoresDigestNotCookieValue(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + store := NewSessionStore(s) + tok, _, err := store.Create(context.Background(), 1) + if err != nil { + t.Fatalf("create: %v", err) + } + stored := storedSessionToken(t, s, 1) + if stored == tok { + t.Fatal("sessions.token holds the raw cookie value") + } + if stored != sessionDigest(tok) { + t.Fatalf("sessions.token = %q, want sessionDigest(cookie) = %q", stored, sessionDigest(tok)) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE token = ?`, tok); n != 0 { + t.Fatalf("%d rows match the raw cookie value, want 0", n) + } +} + +func TestSessions_StoredValueCannotBeUsedAsCookie(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + store := NewSessionStore(s) + ctx := context.Background() + tok, csrf, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create: %v", err) + } + stored := storedSessionToken(t, s, 1) + + called := false + h := store.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusNoContent) + })) + serve := func(method, cookie, csrfValue string) int { + called = false + rr := httptest.NewRecorder() + req := httptest.NewRequestWithContext(ctx, method, "/protected", nil) + req.AddCookie(&http.Cookie{Name: sessionCookie, Value: cookie}) + if csrfValue != "" { + req.Header.Set(csrfHeader, csrfValue) + } + h.ServeHTTP(rr, req) + return rr.Code + } + + // The database copy of the session, presented as the cookie, is rejected + // on reads and on writes that carry the session's real CSRF token. + if code := serve(http.MethodGet, stored, ""); code != http.StatusUnauthorized || called { + t.Fatalf("GET with the stored value as cookie: code=%d called=%v, want 401 and not called", code, called) + } + if code := serve(http.MethodPost, stored, csrf); code != http.StatusUnauthorized || called { + t.Fatalf("POST with the stored value as cookie and the valid CSRF token: code=%d called=%v, want 401 and not called", code, called) + } + if _, _, err := store.lookup(ctx, stored); err == nil { + t.Fatal("lookup(stored value) succeeded, want an error") + } + + // The raw cookie value handed out by Create still authenticates. + if code := serve(http.MethodGet, tok, ""); code != http.StatusNoContent || !called { + t.Fatalf("GET with the issued cookie: code=%d called=%v, want 204 and called", code, called) + } + // The rejected attempts left the real session in place. + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE user_id = 1`); n != 1 { + t.Fatalf("%d session rows after the rejected attempts, want 1", n) + } +} + +func TestSessions_LogoutDeletesRowByDigest(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + store := NewSessionStore(s) + ctx := context.Background() + tokA, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create A: %v", err) + } + tokB, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create B: %v", err) + } + logout := func(cookie string) { + rr := httptest.NewRecorder() + req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/logout", nil) + req.AddCookie(&http.Cookie{Name: sessionCookie, Value: cookie}) + store.HandleLogout().ServeHTTP(rr, req) + if rr.Code != http.StatusNoContent { + t.Fatalf("logout code=%d, want 204", rr.Code) + } + } + + logout(tokA) + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE token = ?`, sessionDigest(tokA)); n != 0 { + t.Fatalf("logged-out session: %d rows left, want 0", n) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE token = ?`, sessionDigest(tokB)); n != 1 { + t.Fatalf("other session: %d rows, want 1", n) + } + if _, _, err := store.lookup(ctx, tokB); err != nil { + t.Fatalf("other session no longer authenticates: %v", err) + } + + // Logging out with the stored value as the cookie deletes nothing. + logout(storedSessionToken(t, s, 1)) + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE user_id = 1`); n != 1 { + t.Fatalf("logout with the stored value removed a row: %d rows left, want 1", n) + } + if _, _, err := store.lookup(ctx, tokB); err != nil { + t.Fatalf("session B after a logout with the stored value: %v", err) + } +} + +func TestSessions_ExpiredDigestRowRemovedOnLookupAndGC(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + store := NewSessionStore(s) + ctx := context.Background() + tokA, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create A: %v", err) + } + tokB, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create B: %v", err) + } + past := time.Now().Add(-time.Hour).UTC().Format(time.RFC3339) + for _, tok := range []string{tokA, tokB} { + res, err := s.DB.ExecContext(ctx, `UPDATE sessions SET expires_at = ? WHERE token = ?`, past, sessionDigest(tok)) + if err != nil { + t.Fatalf("expire: %v", err) + } + if n, err := res.RowsAffected(); err != nil || n != 1 { + t.Fatalf("expire matched %d rows (err %v), want 1", n, err) + } + } + + // lookup removes the expired row it was asked about. + if _, _, err := store.lookup(ctx, tokA); err == nil || !strings.Contains(err.Error(), "expired") { + t.Fatalf("lookup of an expired session: err=%v, want expired", err) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE token = ?`, sessionDigest(tokA)); n != 0 { + t.Fatalf("expired session A: %d rows left after lookup, want 0", n) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE token = ?`, sessionDigest(tokB)); n != 1 { + t.Fatalf("expired session B before gc: %d rows, want 1", n) + } + + // The interval GC removes the other one. + store.gcOnce(ctx) + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions`); n != 0 { + t.Fatalf("%d session rows after gc, want 0", n) + } +} + +func TestSessions_DeleteForUserRevokesEveryDigestRow(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + seedUser(t, s, "bob", "pw", "admin") + store := NewSessionStore(s) + ctx := context.Background() + aliceA, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create alice A: %v", err) + } + aliceB, _, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create alice B: %v", err) + } + bob, _, err := store.Create(ctx, 2) + if err != nil { + t.Fatalf("create bob: %v", err) + } + + if err := store.DeleteForUser(ctx, 1); err != nil { + t.Fatalf("DeleteForUser: %v", err) + } + for name, tok := range map[string]string{"alice A": aliceA, "alice B": aliceB} { + if _, _, err := store.lookup(ctx, tok); err == nil { + t.Errorf("%s still authenticates after DeleteForUser", name) + } + } + if _, _, err := store.lookup(ctx, bob); err != nil { + t.Fatalf("bob's session after revoking alice's: %v", err) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE user_id = 1`); n != 0 { + t.Fatalf("alice: %d session rows left, want 0", n) + } + if n := countSessions(t, s, `SELECT COUNT(*) FROM sessions WHERE user_id = 2`); n != 1 { + t.Fatalf("bob: %d session rows, want 1", n) + } +} + +func TestSessions_CSRFStillEnforcedWithDigestStorage(t *testing.T) { + s := newAuthDB(t) + seedUser(t, s, "alice", "pw", "admin") + store := NewSessionStore(s) + ctx := context.Background() + tok, csrf, err := store.Create(ctx, 1) + if err != nil { + t.Fatalf("create: %v", err) + } + stored := storedSessionToken(t, s, 1) + + // The CSRF token is still stored as issued. + var storedCSRF string + if err := s.DB.QueryRowContext(ctx, `SELECT csrf_token FROM sessions WHERE user_id = 1`).Scan(&storedCSRF); err != nil { + t.Fatalf("read csrf_token: %v", err) + } + if storedCSRF != csrf { + t.Fatalf("csrf_token = %q, want the issued token %q", storedCSRF, csrf) + } + + h := store.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + for _, tc := range []struct { + name string + header string + want int + }{ + {"no header", "", http.StatusForbidden}, + {"wrong header", "wrong", http.StatusForbidden}, + {"stored session value as header", stored, http.StatusForbidden}, + {"session cookie value as header", tok, http.StatusForbidden}, + {"issued csrf token", csrf, http.StatusNoContent}, + } { + rr := httptest.NewRecorder() + req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/protected", nil) + req.AddCookie(&http.Cookie{Name: sessionCookie, Value: tok}) + if tc.header != "" { + req.Header.Set(csrfHeader, tc.header) + } + h.ServeHTTP(rr, req) + if rr.Code != tc.want { + t.Errorf("%s: code=%d, want %d", tc.name, rr.Code, tc.want) + } + } +} + +func TestSessions_LoginCookieIsRawValue(t *testing.T) { + s := newAuthDB(t) + // LoginUserLimiter and LoginLimiter are package singletons keyed by + // username and client IP; TestLogin_PerUserRateLimit drains "alice", so + // this test logs in with its own username and address. + seedUser(t, s, "digest-alice", "hunter2", "admin") + if _, err := s.DB.ExecContext(context.Background(), + `INSERT INTO user_role_bindings(user_id, role_name, cluster, namespace) VALUES (1, 'viewer', 'local', '*')`, + ); err != nil { + t.Fatalf("seed binding: %v", err) + } + store := NewSessionStore(s) + rr := httptest.NewRecorder() + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/login", + strings.NewReader(`{"username":"digest-alice","password":"hunter2"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "198.51.100.71:1234" + NewLocal(s).HandleLogin(store, nil).ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("login code=%d body=%s", rr.Code, rr.Body) + } + res := rr.Result() + defer func() { _ = res.Body.Close() }() + var body loginResp + if err := json.NewDecoder(res.Body).Decode(&body); err != nil { + t.Fatalf("decode login response: %v", err) + } + var cookie string + for _, c := range res.Cookies() { + if c.Name == sessionCookie { + cookie = c.Value + } + } + if cookie == "" { + t.Fatal("login set no session cookie") + } + + stored := storedSessionToken(t, s, 1) + if stored == cookie { + t.Fatal("sessions.token holds the login cookie value") + } + if stored != sessionDigest(cookie) { + t.Fatalf("sessions.token = %q, want sessionDigest(login cookie) = %q", stored, sessionDigest(cookie)) + } + u, csrf, err := store.lookup(context.Background(), cookie) + if err != nil || u.Username != "digest-alice" || csrf != body.CSRF { + t.Fatalf("lookup(login cookie) = %+v, %q, %v; want digest-alice with the login CSRF token", u, csrf, err) + } +} diff --git a/api/internal/auth/sessions_test.go b/api/internal/auth/sessions_test.go index 29912d82e..ac48a40fb 100644 --- a/api/internal/auth/sessions_test.go +++ b/api/internal/auth/sessions_test.go @@ -41,7 +41,7 @@ func TestSessions_Lookup_Expired(t *testing.T) { } // Force-expire the row directly. if _, err := s.DB.ExecContext(context.Background(), `UPDATE sessions SET expires_at = ? WHERE token = ?`, - time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), tok); err != nil { + time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), sessionDigest(tok)); err != nil { t.Fatalf("update: %v", err) } if _, _, err := store.lookup(context.Background(), tok); err == nil || @@ -55,7 +55,7 @@ func TestSessions_Lookup_CorruptExpires(t *testing.T) { seedUser(t, s, "alice", "pw", "admin") store := NewSessionStore(s) tok, _, _ := store.Create(context.Background(), 1) - if _, err := s.DB.ExecContext(context.Background(), `UPDATE sessions SET expires_at = 'garbage' WHERE token = ?`, tok); err != nil { + if _, err := s.DB.ExecContext(context.Background(), `UPDATE sessions SET expires_at = 'garbage' WHERE token = ?`, sessionDigest(tok)); err != nil { t.Fatalf("update: %v", err) } if _, _, err := store.lookup(context.Background(), tok); err == nil || @@ -64,7 +64,7 @@ func TestSessions_Lookup_CorruptExpires(t *testing.T) { } // Row should also be deleted. var n int - _ = s.DB.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions WHERE token = ?`, tok).Scan(&n) + _ = s.DB.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions WHERE token = ?`, sessionDigest(tok)).Scan(&n) if n != 0 { t.Fatalf("corrupt row not removed (%d rows remaining)", n) } @@ -182,7 +182,7 @@ func TestSessions_HandleLogout(t *testing.T) { } // Row deleted. var n int - _ = s.DB.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions WHERE token=?`, tok).Scan(&n) + _ = s.DB.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions WHERE token=?`, sessionDigest(tok)).Scan(&n) if n != 0 { t.Fatalf("session row not deleted") } @@ -205,7 +205,7 @@ func TestSessions_GCOnce_DeletesExpired(t *testing.T) { store := NewSessionStore(s) tok, _, _ := store.Create(context.Background(), 1) _, _ = s.DB.ExecContext(context.Background(), `UPDATE sessions SET expires_at = ? WHERE token = ?`, - time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), tok) + time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), sessionDigest(tok)) store.gcOnce(context.Background()) var n int _ = s.DB.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions`).Scan(&n) @@ -220,7 +220,7 @@ func TestSessions_StartGC(t *testing.T) { store := NewSessionStore(s) tok, _, _ := store.Create(context.Background(), 1) _, _ = s.DB.ExecContext(context.Background(), `UPDATE sessions SET expires_at = ? WHERE token = ?`, - time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), tok) + time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), sessionDigest(tok)) ctx, cancel := context.WithCancel(context.Background()) defer cancel() store.StartGC(ctx, 20*time.Millisecond) diff --git a/api/internal/db/migrations/common/014_sessions_digest_reset.sql b/api/internal/db/migrations/common/014_sessions_digest_reset.sql new file mode 100644 index 000000000..57bbd07d9 --- /dev/null +++ b/api/internal/db/migrations/common/014_sessions_digest_reset.sql @@ -0,0 +1,6 @@ +-- Sessions now store a SHA-256 digest of the browser session value instead +-- of the value itself (see auth.sessionDigest). Rows written before this +-- migration hold raw cookie values, which can no longer be matched, so they +-- are deliberately invalidated: every user signs in again once after the +-- upgrade. Rows created afterwards hold digests. +DELETE FROM sessions; diff --git a/api/internal/db/migrations_test.go b/api/internal/db/migrations_test.go index 349380375..765eac393 100644 --- a/api/internal/db/migrations_test.go +++ b/api/internal/db/migrations_test.go @@ -288,3 +288,97 @@ func TestNowTimestamp_Format(t *testing.T) { t.Fatalf("NowTimestamp() = %q is not the current UTC time", ts) } } + +func TestMigrate_SessionDigestResetDeletesExistingSessions(t *testing.T) { + s := newRBACStore(t) + ctx := t.Context() + + // Insert a test user + _, err := s.DB.ExecContext(ctx, + `INSERT INTO users(username, display_name, email, role) VALUES (?,?,?,?)`, + "sess-user", "sess-user", "sess@example.com", "viewer") + if err != nil { + t.Fatalf("insert user: %v", err) + } + + // Insert two sessions with raw-looking tokens + _, err = s.DB.ExecContext(ctx, + `INSERT INTO sessions(token, user_id, csrf_token, expires_at) VALUES (?,?,?,?)`, + "raw-1", 1, "csrf-1", "2999-01-01T00:00:00Z") + if err != nil { + t.Fatalf("insert session 1: %v", err) + } + _, err = s.DB.ExecContext(ctx, + `INSERT INTO sessions(token, user_id, csrf_token, expires_at) VALUES (?,?,?,?)`, + "raw-2", 1, "csrf-2", "2999-01-01T00:00:00Z") + if err != nil { + t.Fatalf("insert session 2: %v", err) + } + + // Delete the migration marker to re-run migration 014 + _, err = s.DB.ExecContext(ctx, `DELETE FROM schema_migrations WHERE version = ?`, "014_sessions_digest_reset.sql") + if err != nil { + t.Fatalf("delete migration marker: %v", err) + } + + // Run migrations - 014 should delete all sessions + if err := s.Migrate(ctx); err != nil { + t.Fatalf("migrate: %v", err) + } + + // Verify sessions were deleted + var count int + err = s.DB.QueryRowContext(ctx, `SELECT COUNT(*) FROM sessions`).Scan(&count) + if err != nil { + t.Fatalf("count sessions: %v", err) + } + if count != 0 { + t.Errorf("session count after 014: %d, want 0", count) + } + + // Verify migration version was recorded + var v string + err = s.DB.QueryRowContext(ctx, `SELECT version FROM schema_migrations WHERE version = ?`, "014_sessions_digest_reset.sql").Scan(&v) + if err != nil { + t.Errorf("migration 014 not recorded: %v", err) + } + + // Insert a new session after migration + _, err = s.DB.ExecContext(ctx, + `INSERT INTO sessions(token, user_id, csrf_token, expires_at) VALUES (?,?,?,?)`, + "digest-row", 1, "csrf-3", "2999-01-01T00:00:00Z") + if err != nil { + t.Fatalf("insert session after 014: %v", err) + } + + // Run migrations again - 014 should not re-run + if err := s.Migrate(ctx); err != nil { + t.Fatalf("migrate again: %v", err) + } + + // Verify the new session still exists (migration not re-run) + err = s.DB.QueryRowContext(ctx, `SELECT COUNT(*) FROM sessions WHERE token = ?`, "digest-row").Scan(&count) + if err != nil { + t.Fatalf("count new session: %v", err) + } + if count != 1 { + t.Errorf("new session count after re-run: %d, want 1", count) + } +} + +func TestSharedMigrations_IncludeSessionDigestReset(t *testing.T) { + files, err := listSQL(migrations, sharedMigrationDir) + if err != nil { + t.Fatalf("list shared migrations: %v", err) + } + var found bool + for _, f := range files { + if f.name == "014_sessions_digest_reset.sql" { + found = true + break + } + } + if !found { + t.Error("014_sessions_digest_reset.sql not found in shared migrations") + } +} diff --git a/api/specs.md b/api/specs.md index 142a7553e..313cf88e1 100644 --- a/api/specs.md +++ b/api/specs.md @@ -485,7 +485,7 @@ audit:read, config:read, config:manage (cluster-scoped) 2. **Every mutating request audited:** audit middleware logs actor, method, path, target, status, IP to database + external sinks 3. **Three-role baseline RBAC:** admin/operator/viewer roles reproduce historical permission matrix exactly 4. **Multi-dimensional RBAC:** namespace + cluster + owner/collaborator dimensions; cluster gating prevents cross-cluster privilege escalation -5. **Append-only migrations:** database schema mutations are irreversible (migrations 001-012); no down-migrations +5. **Append-only migrations:** database schema mutations are irreversible (migrations 001-014); no down-migrations 6. **Login rate limiting:** `/auth/login` is per-IP (burst 10, 5/min, `LoginLimiter`) plus a per-username throttle layered on top (burst 6, 3/min, `LoginUserLimiter` in `auth/local.go`); the OIDC callback routes (`/auth/oidc/{provider}/callback`, legacy `/auth/oidc/callback`) are per-IP only (burst 10, 10/min via `OIDCCallbackLimiter`), no per-user dimension 7. **Audit hash-chain:** each audit_events row includes hash of previous row (prev_hash) + its own content hash (hash); detects DB-level UPDATE/DELETE tampering 8. **Audit pagination is bounded:** The `Auditor.Page(ctx, limit)` method clamps the untrusted `limit` parameter to a maximum of 500 entries (`MaxAuditPageSize`). Clamping occurs at both the API handler layer (api/internal/handlers/audit.go line 25) and the store layer (api/internal/audit/audit.go lines 820–822) so untrusted input is bounded at the earliest opportunity and again at use time. The allocated slice is always created with capacity within the bound (`make([]Event, 0, limit)` after clamping), guaranteeing that no untrusted client input can cause unbounded memory allocation regardless of how the limit value flows through the system. @@ -532,11 +532,11 @@ Verify from `/api/go.mod`. - Runtime SQL is written once with `?` placeholders; the Postgres connector rewrites them to `$n` (`db.Rebind`). Timestamps that used SQLite's `datetime('now')` are generated in Go (`db.NowTimestamp()`, same `YYYY-MM-DD HH:MM:SS` UTC text) and bound as parameters; inserted ids come from `RETURNING id` (pgx has no `LastInsertId`) - Postgres legacy migrations declare the text columns the API sorts or range-compares (`roles.name`, role-binding scope columns, `audit_events.ts`, `sessions.expires_at`, `share_links.created_at`/`expires_at`) `COLLATE "C"` so ordering matches SQLite's byte-wise collation -### Schema (migrations 001-012) +### Schema (migrations 001-014) **001_init.sql:** - `users` — username (unique), email, display_name, pw_hash (argon2id), role (legacy, now via role_bindings), created_at, updated_at -- `sessions` — token (PK), user_id (FK), csrf_token, expires_at +- `sessions` — token (PK; since 014 holds the hex SHA-256 digest of the session cookie value, never the value), user_id (FK), csrf_token, expires_at - `oidc_links` — (issuer, subject) -> user_id (many-to-one); email claim - `audit_events` — ts, actor, method, path, target, status, ip - `api_tokens` — token (PK), user_id, name, last_used @@ -594,6 +594,9 @@ Verify from `/api/go.mod`. - Adds `server_uid TEXT NOT NULL DEFAULT ''` to `share_links`; portable, runs unchanged on SQLite and PostgreSQL. Existing rows get `''` (legacy links, validated by creationTimestamp only); no backfill - New links store the GameServer `metadata.uid`; public share handlers require the live UID to equal it when non-empty (see "Share links are bound to the GameServer incarnation") +**014_sessions_digest_reset.sql:** (session digest storage) +- `DELETE FROM sessions` once. Sessions switched to storing a SHA-256 digest of the cookie value (`auth.sessionDigest`); pre-existing rows hold raw values and are deliberately invalidated, so every user signs in again once after upgrade. Numbered 014 because 013 is used by another shared migration. The runner applies pending files in version (filename) order and skips recorded ones, so 013 and 014 pending together run 013 first; a 013 added after 014 was recorded on an earlier startup still runs on the next startup, after it. Forward-only: a rollback to a pre-digest binary also invalidates digest-era sessions. + Foreign keys are enforced only on Postgres (modernc-sqlite runs with FK OFF); the API layer is authoritative and deletes dependent rows itself, so the Postgres cascades never change the outcome. The one exception is `share_links.created_by`: the Postgres schema declares it as a plain column with no foreign key, so deleting a user keeps that user's share links (revoked) in the audit trail on both drivers, instead of cascading them away. ## Security considerations @@ -601,7 +604,7 @@ Foreign keys are enforced only on Postgres (modernc-sqlite runs with FK OFF); th ### Authentication - **Local:** argon2id (Argon2id13, m=64MiB, t=3, p=2) with per-user random salt; ~200ms per check; operations are bounded by a 1-slot weighted semaphore (`argonConcurrency`) that honours request context, so client disconnects abort pending argon2 operations and prevent stalling under concurrent login attempts. Each operation allocates a 64 MiB block that is garbage on return, so at startup `api/cmd/memlimit.go` also sets the Go soft memory limit to 80% of the cgroup memory limit (256Mi chart default -> ~204 MiB) unless `GOMEMLIMIT` is set, making the GC reclaim those blocks before a burst of concurrent failed logins can OOM-kill the container - **OIDC:** `coreos/go-oidc/v3` discovery + `go-jose/v4` JWT validation; claims mapping (email, groups, roles) -- **Sessions:** cryptographically random token + paired CSRF token; DB-only persistence (no in-memory store), 12-hour TTL from creation, garbage-collected on an interval (`SessionStore.StartGC`) +- **Sessions:** cryptographically random token + paired CSRF token; only the SHA-256 digest (hex) of the token is persisted (the cookie carries the raw value, so a database copy cannot be replayed as a cookie; lookup, logout and cleanup hash the presented value first); DB-only persistence (no in-memory store), 12-hour TTL from creation, garbage-collected on an interval (`SessionStore.StartGC`) - **CSRF cookie is JS-readable by design:** unlike the session cookie (`HttpOnly`), the CSRF cookie is set `HttpOnly: false` so the SPA can read its value and echo it back as `X-Gameplane-CSRF` on mutating requests — the standard double-submit pattern (see `docs/security.md`). Logout's cookie-clear always sends `HttpOnly: true` regardless, since a MaxAge<0 delete carries no value and the browser matches it on Name/Domain/Path alone. - **Bootstrap:** `bootstrap-admin` subcommand hashes password same way as API - **Client IP:** `auth.ClientIPFromTrustedProxies` (`internal/auth/clientip.go`) records the client IP that the rate limiters and audit key on. A TCP peer outside `--trusted-proxies` is the client and its `X-Forwarded-For` is ignored. Behind a trusted peer the header is read right to left up to the first address outside the list, or to the leftmost address when every hop is trusted; an entry that isn't an IP address ends the walk at the last address reached. An empty list makes the peer the client. Tests: `internal/auth/clientip_test.go`. diff --git a/docs/security.md b/docs/security.md index 7c85408ef..c93dc7ff7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -15,7 +15,7 @@ point. Assume: Two modes, configurable independently: - **Local accounts** — argon2id (64 MiB, t=3, p=2) password hashing. - Session cookies are HttpOnly, Secure, SameSite=Lax. CSRF protection + Session cookies are HttpOnly, Secure, SameSite=Lax. The database stores only a SHA-256 digest of the session cookie value, so a leaked database or backup does not yield usable session cookies; the digest itself is rejected if presented as a cookie. CSRF protection via a double-submit `X-Gameplane-CSRF` header on mutating requests. - **OIDC** — Keycloak, Google, GitHub, any RFC-7519 compliant IdP. State validated through a short-lived cookie; `id_token` signature