diff --git a/README.md b/README.md index e6d0a57..44b4d94 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,10 @@ The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the database and the disk cache: - It needs a persistent volume: without one, every restart starts with an empty - cache and forgets the count of logouts, so login sessions that a logout ended - can work again (see Encrypted URLs). In the container, the startup script - gives the directory to the user pixa runs as (uid 65532) and sets its mode to - `750`; outside it, that user must be able to write the directory. + cache. Losing the directory also ends every login session (see Encrypted + URLs). In the container, the startup script gives the directory to the user + pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user + must be able to write the directory. - `cache_max_bytes` limits the source and transformed images together. The database, the metadata files, the `.meta` file beside each transformed image and files still being written come on top, and eviction runs in the @@ -382,11 +382,12 @@ nor change what it asks for. lasts 30 days, or until someone logs out with the generator page's Logout button. Every login is made with the one signing key, so logging out ends every login session, in every browser, a copy of the session cookie included. - pixa keeps a count of logouts in its database: each logout adds one, a login - session carries the count at its login, and a login session whose count is - not the current one is refused, also after a restart that keeps the state - directory (see Deployment). No time is compared, so a login after a logout - works whatever the host's clock says. + pixa keeps a random value in its database, which it makes when the database + has none and replaces at each logout. A login session carries the value + current at its login and is refused once the value has changed, also after a + restart (see Deployment). The check for a logout compares no time, so a login + after a logout works whatever the host's clock says; the 30 days are still + measured by the host's clock. 2. On the generator page, give the source image's URL, the width and height, the format, quality and fit, and how long the URL lasts, then submit the form (`POST /generate`). The format is JPEG XL unless another is chosen; a form diff --git a/TODO.md b/TODO.md index 0d54024..2677deb 100644 --- a/TODO.md +++ b/TODO.md @@ -48,6 +48,23 @@ P2: security: per-origin rate limiting times and no longer says the work stops when `downstream_timeout` ends. The settings of the WebP, AVIF and JPEG XL encoders, which can hold a slot past the default `downstream_timeout` of 60 seconds, are unchanged. +- 2026-10-15 logging out ends every login session (closes #256): a login session + was a cookie with nothing kept on the server, so `/logout` removed it from one + browser and a copy of it kept working for its 30 days. Every login is made + with the one signing key, so logging out now ends them all: the session + manager keeps a random value, made with `crypto/rand` when the new `logout` + table has none and replaced by each logout, saves it there so that it outlasts + a restart, puts the value at login in each session, and refuses a session + carrying another value. The check for a logout compares no time, so a login + after a logout works whatever the host's clock does; a session's 30 days are + still checked against the clock. Losing the state directory ends every login + session, and no logout can make an ended one work again. Logging out is now + `POST /logout` from a form on the generator page, behind the CSRF check the + other forms have, so a link or image elsewhere on the same site cannot end the + sessions. A logout without a valid login session ends none and only removes + the cookie, as anyone can take a CSRF token from the login page. A logout that + cannot be saved answers 500. `session.NewManager`, which the tests use, keeps + the value in memory only; pixad uses `session.NewManagerWithDatabase`. - 2026-10-15 "Running under upaas" in `README.md` tells the operator to set `PIXA_CACHE_MAX_BYTES` (closes #251): it listed the variable as optional, but under upaas the volume is a directory on the host's filesystem. The default, @@ -64,22 +81,6 @@ P2: security: per-origin rate limiting 405, or a CORS preflight. The metrics library takes the label before the request is routed, so the metrics middleware asks the router which route matches. -- 2026-10-15 logging out ends every login session (closes #256): a login session - was a cookie with nothing kept on the server, so `/logout` removed it from one - browser and a copy of it kept working for its 30 days. Every login is made - with the one signing key, so logging out now ends them all: the session - manager counts logouts, saves the count in the new `logout` table so that it - outlasts a restart, puts the count at login in each session, and refuses a - session whose count is not the current one. No time is compared, so a login - after a logout works whatever the host's clock does. A restart without a - persistent volume for the state directory forgets the count. Logging out is - now `POST /logout` from a form on the generator page, behind the CSRF check - the other forms have, so a link or image elsewhere on the same site cannot end - the sessions. A logout without a valid login session ends none and only - removes the cookie, as anyone can take a CSRF token from the login page. A - logout that cannot be saved answers 500. `session.NewManager`, which the tests - use, keeps the count in memory only; pixad uses - `session.NewManagerWithDatabase`. - 2026-10-15 expired negative cache rows are removed on a schedule (closes #244): the row of a failed fetch, which makes pixa answer 502 for that source URL for 5 minutes, was deleted only when the same URL was requested again diff --git a/internal/db/migrations/001_schema.sql b/internal/db/migrations/001_schema.sql index 2e018af..234a141 100644 --- a/internal/db/migrations/001_schema.sql +++ b/internal/db/migrations/001_schema.sql @@ -177,11 +177,10 @@ CREATE TABLE IF NOT EXISTS cache_stats ( ); INSERT OR IGNORE INTO cache_stats (id) VALUES (1); --- Count of logouts. Each logout adds one; a login session carries the count --- at its login and is refused once the count has changed, so logging out ends --- every login session. +-- A random value that a login session carries from its login. pixa makes it +-- when the table has no row, and each logout replaces it; a login session +-- carrying another value is refused, so logging out ends every login session. CREATE TABLE IF NOT EXISTS logout ( id INTEGER PRIMARY KEY CHECK (id = 1), - logout_count INTEGER NOT NULL DEFAULT 0 + logout_value TEXT NOT NULL ); -INSERT OR IGNORE INTO logout (id) VALUES (1); diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index c82b621..9cfc3bb 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -164,8 +164,8 @@ func (s *Handlers) initImageService() error { // Initialize session manager (signing key is validated at config load // time). Session cookies are always Secure/HttpOnly/SameSite=Strict. - // The count of logouts is saved in the database, so it outlasts a - // restart. + // The random value that each logout replaces is saved in the database, + // so it outlasts a restart. sessMgr, err := session.NewManagerWithDatabase( context.Background(), s.config.SigningKey, s.db.DB()) if err != nil { diff --git a/internal/session/session.go b/internal/session/session.go index 06c77bd..788c114 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -1,10 +1,11 @@ -// Package session provides encrypted session cookie management, and keeps a -// count of logouts in the database so that a logout ends every session made -// before it. +// Package session provides encrypted session cookie management, and keeps in +// the database a random value that each logout replaces, so that a logout +// ends every session made before it. package session import ( "context" + "crypto/rand" "database/sql" "errors" "fmt" @@ -39,33 +40,35 @@ type Data struct { CreatedAt time.Time `json:"created"` ExpiresAt time.Time `json:"expires"` - // LogoutCount is the count of logouts when the session was made. - LogoutCount int64 `json:"logoutCount"` + // LogoutValue is the Manager's random value, which each logout + // replaces, when the session was made. + LogoutValue string `json:"logoutValue"` } // Manager handles session creation and validation using encrypted cookies. // // Every login is made with the one signing key, so a logout ends every -// session made before it, in every browser. The Manager counts logouts: each -// logout adds one, a session carries the count at its login, and a session -// whose count is not the current one is refused. No clock is compared, so a -// login after a logout works whatever the host's clock does. The Manager -// keeps the count in memory, so checking a session reads no database; a -// Manager from NewManagerWithDatabase also saves it in the database, so that -// it outlasts a restart. +// session made before it, in every browser. The Manager keeps a random +// value, which each logout replaces: a session carries the value current at +// its login, and a session carrying another value is refused. The check for +// a logout compares no time, so a login after a logout works whatever the +// host's clock does; the session's expiry is still checked against the +// clock. The Manager keeps the value in memory, so checking a session reads +// no database; a Manager from NewManagerWithDatabase also saves it in the +// database, so that it outlasts a restart. type Manager struct { sc *securecookie.SecureCookie db *sql.DB - // mu guards logoutCount, and is held while a logout is saved, so that - // the database and logoutCount agree. + // mu guards logoutValue, and is held while a logout is saved, so that + // the database and logoutValue agree. mu sync.Mutex - logoutCount int64 + logoutValue string } // NewManager creates a session manager with keys derived from the signing key. -// It keeps the count of logouts in memory only, so a restart forgets it; -// NewManagerWithDatabase keeps it in the database. +// It makes a new random value and keeps it in memory only, so a restart ends +// every session; NewManagerWithDatabase keeps the value in the database. // // Session cookies always carry the Secure, HttpOnly, and SameSite=Strict // attributes; this cannot be configured. Browsers treat http://localhost as a @@ -89,13 +92,15 @@ func NewManager(signingKey string) (*Manager, error) { sc.MaxAge(int(SessionTTL.Seconds())) return &Manager{ - sc: sc, + sc: sc, + logoutValue: rand.Text(), }, nil } -// NewManagerWithDatabase creates a session manager like NewManager that saves -// the count of logouts in db, which must have pixa's schema, and starts from -// the count saved there. +// NewManagerWithDatabase creates a session manager like NewManager that keeps +// its random value in db, which must have pixa's schema: it saves the value +// NewManager made when db has none, and otherwise starts from the one saved +// there. func NewManagerWithDatabase( ctx context.Context, signingKey string, db *sql.DB, ) (*Manager, error) { @@ -104,11 +109,20 @@ func NewManagerWithDatabase( return nil, err } - err = db.QueryRowContext(ctx, - "SELECT logout_count FROM logout WHERE id = 1", - ).Scan(&m.logoutCount) + _, err = db.ExecContext(ctx, + "INSERT OR IGNORE INTO logout (id, logout_value) VALUES (1, ?)", + m.logoutValue) if err != nil { - return nil, fmt.Errorf("failed to read the count of logouts: %w", err) + return nil, fmt.Errorf( + "failed to save the random value in the logout table: %w", err) + } + + err = db.QueryRowContext(ctx, + "SELECT logout_value FROM logout WHERE id = 1", + ).Scan(&m.logoutValue) + if err != nil { + return nil, fmt.Errorf( + "failed to read the random value in the logout table: %w", err) } m.db = db @@ -123,7 +137,7 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error { Authenticated: true, CreatedAt: now, ExpiresAt: now.Add(SessionTTL), - LogoutCount: m.currentLogoutCount(), + LogoutValue: m.currentLogoutValue(), } encoded, err := m.sc.Encode(CookieName, data) @@ -173,7 +187,7 @@ func (m *Manager) ValidateSession(r *http.Request) (*Data, error) { } // A logout since the session was made has ended it. - if data.LogoutCount != m.currentLogoutCount() { + if data.LogoutValue != m.currentLogoutValue() { return nil, ErrInvalidSession } @@ -217,30 +231,31 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool { return err == nil && data != nil && data.Authenticated } -// saveLogout adds one to the count of logouts, saving the new count in the +// saveLogout replaces the random value with a new one, saving it in the // database first when the Manager has one. func (m *Manager) saveLogout(ctx context.Context) error { + value := rand.Text() + m.mu.Lock() defer m.mu.Unlock() if m.db != nil { _, err := m.db.ExecContext(ctx, - "UPDATE logout SET logout_count = ? WHERE id = 1", - m.logoutCount+1) + "UPDATE logout SET logout_value = ? WHERE id = 1", value) if err != nil { return fmt.Errorf("failed to save the logout: %w", err) } } - m.logoutCount++ + m.logoutValue = value return nil } -// currentLogoutCount returns the count of logouts. -func (m *Manager) currentLogoutCount() int64 { +// currentLogoutValue returns the Manager's random value. +func (m *Manager) currentLogoutValue() string { m.mu.Lock() defer m.mu.Unlock() - return m.logoutCount + return m.logoutValue }