From 77d55abb7e66d45aa1acc980f59dc60a19967136 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 26 Oct 2026 14:10:20 +0100 Subject: [PATCH] Derive every key from the signing key in one place (closes #315) seal.DeriveKeys derives the session hash and block keys, the encrypted URL key and the CSRF key from the signing key, with their salts declared beside it, and returns them as one seal.Keys. internal/providers calls it once at startup and provides the value to the fx graph. The session manager, the encrypted URL generator and the CSRF protection each take their own key. session.NewManager, encurl.NewGenerator, newCSRFProtect and handlers.New return no error, as nothing in them can fail now. Each key is derived as before, with seal.DeriveKey and the same salt. The signing key itself stays the HMAC key of signed URLs and the login password. README.md says under signing_key what it protects. Model: opus-5-5 --- README.md | 12 +++- TODO.md | 11 ++++ internal/encurl/encurl.go | 16 ++---- internal/encurl/encurl_test.go | 28 ++++++--- internal/handlers/auth_csrf_internal_test.go | 21 ++----- .../handlers/auth_login_log_internal_test.go | 7 ++- .../handlers/auth_session_internal_test.go | 8 ++- internal/handlers/bodylimit_internal_test.go | 21 ++----- internal/handlers/cache_hit_internal_test.go | 7 ++- internal/handlers/csrf.go | 20 ++----- .../handlers/first_bytes_internal_test.go | 7 ++- internal/handlers/handlers.go | 13 ++--- .../image_cache_control_internal_test.go | 7 ++- internal/handlers/imageenc_internal_test.go | 7 ++- .../handlers/imageenc_token_internal_test.go | 7 ++- .../referer_blocklist_internal_test.go | 7 ++- internal/providers/providers.go | 30 ++++++---- internal/seal/keys.go | 57 +++++++++++++++++++ internal/seal/keys_test.go | 42 ++++++++++++++ .../server/computed_side_internal_test.go | 7 ++- internal/server/fit_outside_internal_test.go | 7 ++- internal/server/source_url_internal_test.go | 7 ++- internal/server/svg_as_avif_internal_test.go | 7 ++- internal/session/session.go | 37 +++--------- .../session/session_cookie_attributes_test.go | 7 ++- internal/session/session_logout_test.go | 19 +++++-- internal/session/session_test.go | 28 ++++++--- 27 files changed, 295 insertions(+), 152 deletions(-) create mode 100644 internal/seal/keys.go create mode 100644 internal/seal/keys_test.go diff --git a/README.md b/README.md index d2170b9..787d958 100644 --- a/README.md +++ b/README.md @@ -765,7 +765,17 @@ Key settings in more detail: duration; default `60s`. The upstream fetch counts toward it, and so do the waits for an upstream connection and for a processing slot (up to 10 seconds each), so keep it longer than `upstream_fetch_timeout` plus 20 seconds -- `signing_key` — HMAC secret for URL signatures +- `signing_key` — the secret that protects signed URLs, encrypted URLs and + logins. pixa uses it as it is for two things: + - the HMAC key of signed URLs (see Signature Specification) + - the password of the login page + + pixa derives every other key it uses from it at startup, with HKDF-SHA256: + - a key that signs the session cookie + - a key that encrypts the session cookie + - a key that encrypts the token of an encrypted URL + - a key that signs the CSRF token cookie of the login and generator pages + - `db_url` — the SQLite database to open; omitted, it is `file:/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the database in WAL mode. `state_dir` is percent-encoded in it, so a `%`, `?` or diff --git a/TODO.md b/TODO.md index f02b7c0..85667d1 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,17 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-26 every key derived from the signing key is derived in one place + (closes #315): `seal.DeriveKeys` returns them as one `seal.Keys`, with their + salts declared beside it. `internal/providers` calls it once at startup. Each + key is derived as before, with `seal.DeriveKey` and the same salt. + - The session manager, the encrypted URL generator and the CSRF protection + each take their own key instead of the signing key. + - `session.NewManager`, `encurl.NewGenerator`, `newCSRFProtect` and + `handlers.New` return no error, as nothing in them can fail now. + - The signing key itself stays the HMAC key of signed URLs and the login + password. + - `README.md` says under `signing_key` what it protects. - 2026-10-26 the image request holds only what names the cached object (closes #314): its source and its transform, which the variant key covers. The signed URL format, the signing algorithm and every route's answers are unchanged. diff --git a/internal/encurl/encurl.go b/internal/encurl/encurl.go index 2f06726..9d67d35 100644 --- a/internal/encurl/encurl.go +++ b/internal/encurl/encurl.go @@ -16,9 +16,6 @@ const ( DefaultQuality = 85 DefaultFormat = imagerequest.FormatJXL DefaultFitMode = imagerequest.FitCover - - // HKDF salt for URL encryption key derivation - urlKeySalt = "pixa-urlenc-v1" ) // Errors returned by encurl operations. @@ -48,15 +45,10 @@ type Generator struct { key [seal.KeySize]byte } -// NewGenerator creates an encrypted URL generator with a key derived -// from the signing key. -func NewGenerator(signingKey string) (*Generator, error) { - key, err := seal.DeriveKey([]byte(signingKey), urlKeySalt) - if err != nil { - return nil, err - } - - return &Generator{key: key}, nil +// NewGenerator creates an encrypted URL generator that encrypts its tokens +// with key. +func NewGenerator(key [seal.KeySize]byte) *Generator { + return &Generator{key: key} } // Generate creates an encrypted URL token from the request's source and diff --git a/internal/encurl/encurl_test.go b/internal/encurl/encurl_test.go index 9ce3100..7b19068 100644 --- a/internal/encurl/encurl_test.go +++ b/internal/encurl/encurl_test.go @@ -7,6 +7,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // Shared test fixture strings. @@ -19,11 +20,13 @@ const ( func TestGenerator_GenerateAndParse(t *testing.T) { t.Parallel() - gen, err := encurl.NewGenerator("test-signing-key-12345") + keys, err := seal.DeriveKeys("test-signing-key-12345") if err != nil { - t.Fatalf("NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + gen := encurl.NewGenerator(keys.EncryptedURLKey) + req := &imagerequest.Request{ Source: imagerequest.Source{ Host: testSourceHost, @@ -99,7 +102,8 @@ func TestGenerator_GenerateAndParse(t *testing.T) { func TestGenerator_Parse_Expired(t *testing.T) { t.Parallel() - gen, _ := encurl.NewGenerator("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + gen := encurl.NewGenerator(keys.EncryptedURLKey) req := &imagerequest.Request{ Source: imagerequest.Source{ @@ -126,7 +130,8 @@ func TestGenerator_Parse_Expired(t *testing.T) { func TestGenerator_Parse_InvalidToken(t *testing.T) { t.Parallel() - gen, _ := encurl.NewGenerator("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + gen := encurl.NewGenerator(keys.EncryptedURLKey) _, _, err := gen.Parse("not-a-valid-token") if err == nil { @@ -137,7 +142,8 @@ func TestGenerator_Parse_InvalidToken(t *testing.T) { func TestGenerator_Parse_TamperedToken(t *testing.T) { t.Parallel() - gen, _ := encurl.NewGenerator("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + gen := encurl.NewGenerator(keys.EncryptedURLKey) req := &imagerequest.Request{ Source: imagerequest.Source{ @@ -163,8 +169,10 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) { func TestGenerator_Parse_WrongKey(t *testing.T) { t.Parallel() - gen1, _ := encurl.NewGenerator("signing-key-1") - gen2, _ := encurl.NewGenerator("signing-key-2") + keys1, _ := seal.DeriveKeys("signing-key-1") + keys2, _ := seal.DeriveKeys("signing-key-2") + gen1 := encurl.NewGenerator(keys1.EncryptedURLKey) + gen2 := encurl.NewGenerator(keys2.EncryptedURLKey) req := &imagerequest.Request{ Source: imagerequest.Source{ @@ -184,7 +192,8 @@ func TestGenerator_Parse_WrongKey(t *testing.T) { func TestGenerator_Parse_Defaults(t *testing.T) { t.Parallel() - gen, _ := encurl.NewGenerator("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + gen := encurl.NewGenerator(keys.EncryptedURLKey) // Request with only required fields - should get defaults token, err := gen.Generate(&imagerequest.Request{ @@ -221,7 +230,8 @@ func TestGenerator_Parse_Defaults(t *testing.T) { func TestGenerator_TokenIsURLSafe(t *testing.T) { t.Parallel() - gen, _ := encurl.NewGenerator("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + gen := encurl.NewGenerator(keys.EncryptedURLKey) req := &imagerequest.Request{ Source: imagerequest.Source{ diff --git a/internal/handlers/auth_csrf_internal_test.go b/internal/handlers/auth_csrf_internal_test.go index 161253b..6237798 100644 --- a/internal/handlers/auth_csrf_internal_test.go +++ b/internal/handlers/auth_csrf_internal_test.go @@ -14,6 +14,7 @@ import ( "sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -44,27 +45,17 @@ func newCSRFTestRouter(t *testing.T) (*Handlers, http.Handler) { cfg := &config.Config{SigningKey: testSigningKey, Debug: true} - sessMgr, err := session.NewManager(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("session.NewManager() error = %v", err) - } - - encGen, err := encurl.NewGenerator(testSigningKey) - if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) - } - - protect, err := newCSRFProtect(testSigningKey, cfg.Debug) - if err != nil { - t.Fatalf("newCSRFProtect() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } h := &Handlers{ log: slog.New(slog.DiscardHandler), config: cfg, - sessMgr: sessMgr, - encGen: encGen, - csrfProtect: protect, + sessMgr: session.NewManager(keys.SessionHashKey, keys.SessionBlockKey), + encGen: encurl.NewGenerator(keys.EncryptedURLKey), + csrfProtect: newCSRFProtect(keys.CSRFKey, cfg.Debug), } r := chi.NewRouter() diff --git a/internal/handlers/auth_login_log_internal_test.go b/internal/handlers/auth_login_log_internal_test.go index 2c8e49f..71943a8 100644 --- a/internal/handlers/auth_login_log_internal_test.go +++ b/internal/handlers/auth_login_log_internal_test.go @@ -10,6 +10,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -22,15 +23,15 @@ func TestLoginLogLeavesOutSubmittedKey(t *testing.T) { var buf bytes.Buffer - sessMgr, err := session.NewManager(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("session.NewManager() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } h := &Handlers{ log: slog.New(slog.NewJSONHandler(&buf, nil)), config: &config.Config{SigningKey: testSigningKey}, - sessMgr: sessMgr, + sessMgr: session.NewManager(keys.SessionHashKey, keys.SessionBlockKey), } submittedKeys := []string{wrongKey, testSigningKey} diff --git a/internal/handlers/auth_session_internal_test.go b/internal/handlers/auth_session_internal_test.go index d99ca86..8920ae5 100644 --- a/internal/handlers/auth_session_internal_test.go +++ b/internal/handlers/auth_session_internal_test.go @@ -11,6 +11,7 @@ import ( "time" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -232,8 +233,13 @@ func TestHandleLogout_SaveFails_Answers500(t *testing.T) { db := setupTestDB(t) + keys, err := seal.DeriveKeys(testSigningKey) + if err != nil { + t.Fatalf("seal.DeriveKeys() error = %v", err) + } + sessMgr, err := session.NewManagerWithDatabase( - t.Context(), testSigningKey, db) + t.Context(), keys.SessionHashKey, keys.SessionBlockKey, db) if err != nil { t.Fatalf("session.NewManagerWithDatabase() error = %v", err) } diff --git a/internal/handlers/bodylimit_internal_test.go b/internal/handlers/bodylimit_internal_test.go index 59e5fd7..496eccd 100644 --- a/internal/handlers/bodylimit_internal_test.go +++ b/internal/handlers/bodylimit_internal_test.go @@ -11,6 +11,7 @@ import ( "sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -32,27 +33,17 @@ func newBodyLimitTestRouter( cfg := &config.Config{SigningKey: testSigningKey, Debug: true} - sessMgr, err := session.NewManager(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("session.NewManager() error = %v", err) - } - - encGen, err := encurl.NewGenerator(testSigningKey) - if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) - } - - protect, err := newCSRFProtect(testSigningKey, cfg.Debug) - if err != nil { - t.Fatalf("newCSRFProtect() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } h := &Handlers{ log: slog.New(slog.DiscardHandler), config: cfg, - sessMgr: sessMgr, - encGen: encGen, - csrfProtect: protect, + sessMgr: session.NewManager(keys.SessionHashKey, keys.SessionBlockKey), + encGen: encurl.NewGenerator(keys.EncryptedURLKey), + csrfProtect: newCSRFProtect(keys.CSRFKey, cfg.Debug), } r := chi.NewRouter() diff --git a/internal/handlers/cache_hit_internal_test.go b/internal/handlers/cache_hit_internal_test.go index bbc16e1..f62a2b0 100644 --- a/internal/handlers/cache_hit_internal_test.go +++ b/internal/handlers/cache_hit_internal_test.go @@ -25,6 +25,7 @@ import ( "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/signature" ) @@ -146,11 +147,13 @@ func newCountingDatabaseServer( tb.Fatalf("imgcache.NewService() error = %v", err) } - encGen, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - tb.Fatalf("encurl.NewGenerator() error = %v", err) + tb.Fatalf("seal.DeriveKeys() error = %v", err) } + encGen := encurl.NewGenerator(keys.EncryptedURLKey) + token, err := encGen.Generate(&imagerequest.Request{ Source: imagerequest.Source{Host: countingHost, Path: photoPath}, Transform: imagerequest.Transform{ diff --git a/internal/handlers/csrf.go b/internal/handlers/csrf.go index 166abdc..03e4de7 100644 --- a/internal/handlers/csrf.go +++ b/internal/handlers/csrf.go @@ -9,15 +9,10 @@ import ( "sneak.berlin/go/pixa/internal/seal" ) -// csrfKeySalt provides domain separation for the CSRF authentication key, -// derived from the signing key so tokens survive restarts without extra -// configuration and never reuse the session or encrypted-URL key material. -const csrfKeySalt = "pixa-csrf-v1" - // newCSRFProtect builds the CSRF-protection middleware for the // state-mutating HTML form routes. The token lives in its own cookie, // independent of the session cookie, so it also protects the login POST -// where no session exists yet (login CSRF). +// where no session exists yet (login CSRF). key signs that cookie. // // When plaintext is true (local HTTP development), requests are marked // plaintext so the library neither demands an https Referer nor sets a @@ -25,13 +20,8 @@ const csrfKeySalt = "pixa-csrf-v1" // service runs behind a TLS-terminating proxy, so plaintext is false and // the library enforces its https Referer origin check. func newCSRFProtect( - signingKey string, plaintext bool, -) (func(http.Handler) http.Handler, error) { - key, err := seal.DeriveKey([]byte(signingKey), csrfKeySalt) - if err != nil { - return nil, err - } - + key [seal.KeySize]byte, plaintext bool, +) func(http.Handler) http.Handler { protect := csrf.Protect( key[:], csrf.Path("/"), @@ -40,7 +30,7 @@ func newCSRFProtect( ) if !plaintext { - return protect, nil + return protect } return func(next http.Handler) http.Handler { @@ -49,7 +39,7 @@ func newCSRFProtect( return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { protected.ServeHTTP(w, csrf.PlaintextHTTPRequest(r)) }) - }, nil + } } // CSRF returns the CSRF-protection middleware for the login and generator diff --git a/internal/handlers/first_bytes_internal_test.go b/internal/handlers/first_bytes_internal_test.go index 69a3751..3a25b79 100644 --- a/internal/handlers/first_bytes_internal_test.go +++ b/internal/handlers/first_bytes_internal_test.go @@ -19,6 +19,7 @@ import ( "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/signature" ) @@ -60,11 +61,13 @@ func newRoutesFetchingThrough( t.Fatalf("imgcache.NewService() error = %v", err) } - encGen, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + encGen := encurl.NewGenerator(keys.EncryptedURLKey) + h := &Handlers{ log: log, imgSvc: svc, diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index db24c4f..50c4f1e 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -15,6 +15,7 @@ import ( "sneak.berlin/go/pixa/internal/healthcheck" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/logger" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" "sneak.berlin/go/pixa/internal/signature" ) @@ -26,6 +27,7 @@ type Params struct { Logger *logger.Logger Healthcheck *healthcheck.Healthcheck Config *config.Config + Keys seal.Keys ImageService *imgcache.Service SessionManager *session.Manager @@ -53,12 +55,7 @@ type Handlers struct { } // New creates a new Handlers instance. -func New(_ fx.Lifecycle, params Params) (*Handlers, error) { - csrfProtect, err := newCSRFProtect(params.Config.SigningKey, params.Config.Debug) - if err != nil { - return nil, err - } - +func New(_ fx.Lifecycle, params Params) *Handlers { return &Handlers{ log: params.Logger.Get(), hc: params.Healthcheck, @@ -66,11 +63,11 @@ func New(_ fx.Lifecycle, params Params) (*Handlers, error) { imgSvc: params.ImageService, sessMgr: params.SessionManager, encGen: params.EncryptedURLGenerator, - csrfProtect: csrfProtect, + csrfProtect: newCSRFProtect(params.Keys.CSRFKey, params.Config.Debug), signer: signature.New(params.Config.SigningKey), allowlist: allowlist.New(params.Config.AllowlistHosts), refererBlocklist: allowlist.New(params.Config.RefererBlocklist), - }, nil + } } // WaitForProcessing waits until no image is being processed, or until ctx diff --git a/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go index 7059ac7..a561c8d 100644 --- a/internal/handlers/image_cache_control_internal_test.go +++ b/internal/handlers/image_cache_control_internal_test.go @@ -19,6 +19,7 @@ import ( "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/signature" ) @@ -61,11 +62,13 @@ func newSignedHostServer( t.Fatalf("imgcache.NewService() error = %v", err) } - encGen, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + encGen := encurl.NewGenerator(keys.EncryptedURLKey) + h := &Handlers{ log: log, imgSvc: svc, diff --git a/internal/handlers/imageenc_internal_test.go b/internal/handlers/imageenc_internal_test.go index e939e13..2da0124 100644 --- a/internal/handlers/imageenc_internal_test.go +++ b/internal/handlers/imageenc_internal_test.go @@ -12,6 +12,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // newEncTestServer builds a router serving the encrypted-URL route with a @@ -21,11 +22,13 @@ import ( func newEncTestServer(t *testing.T) (*encurl.Generator, http.Handler) { t.Helper() - encGen, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + encGen := encurl.NewGenerator(keys.EncryptedURLKey) + h := &Handlers{ log: slog.New(slog.DiscardHandler), encGen: encGen, diff --git a/internal/handlers/imageenc_token_internal_test.go b/internal/handlers/imageenc_token_internal_test.go index 920cc12..09e449f 100644 --- a/internal/handlers/imageenc_token_internal_test.go +++ b/internal/handlers/imageenc_token_internal_test.go @@ -10,6 +10,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // requireServedPhoto requires that rec answers 200 with the JPEG at photoPath @@ -80,11 +81,13 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) { t.Fatalf("Generate() error = %v", err) } - otherGen, err := encurl.NewGenerator("another-signing-key-fedcba9876543210") + keys, err := seal.DeriveKeys("another-signing-key-fedcba9876543210") if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + otherGen := encurl.NewGenerator(keys.EncryptedURLKey) + otherKey, err := otherGen.Generate(&photo, time.Time{}) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/referer_blocklist_internal_test.go b/internal/handlers/referer_blocklist_internal_test.go index a81a7f5..51fa8ec 100644 --- a/internal/handlers/referer_blocklist_internal_test.go +++ b/internal/handlers/referer_blocklist_internal_test.go @@ -17,6 +17,7 @@ import ( "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/signature" ) @@ -73,11 +74,13 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher) t.Fatalf("imgcache.NewService() error = %v", err) } - encGen, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + encGen := encurl.NewGenerator(keys.EncryptedURLKey) + h := &Handlers{ log: slog.New(slog.DiscardHandler), imgSvc: svc, diff --git a/internal/providers/providers.go b/internal/providers/providers.go index 7d2a951..2a11e61 100644 --- a/internal/providers/providers.go +++ b/internal/providers/providers.go @@ -1,5 +1,6 @@ // Package providers builds from the config the parts pixad serves images // and logins with, and provides them to the fx graph: +// - the keys derived from the signing key // - the image cache, with its default limit // - the fetcher // - the processor @@ -27,6 +28,7 @@ import ( "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/logger" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -35,6 +37,7 @@ import ( // the cache registers its counters on. func Constructors() []any { return []any{ + newKeys, newCache, newFetcherConfig, httpfetcher.New, @@ -45,6 +48,12 @@ func Constructors() []any { } } +// newKeys derives the keys of seal.Keys from the signing key. fx calls it +// once, at startup. +func newKeys(cfg *config.Config) (seal.Keys, error) { + return seal.DeriveKeys(cfg.SigningKey) +} + // cacheParams defines dependencies for newCache. type cacheParams struct { fx.In @@ -229,22 +238,23 @@ func newImageService( type sessionManagerParams struct { fx.In - Config *config.Config + Keys seal.Keys Database *database.Database } -// newSessionManager builds the session manager. Session cookies are -// always Secure, HttpOnly and SameSite=Strict. The random value that each -// logout replaces is saved in the database, so it outlasts a restart. +// newSessionManager builds the session manager with its two keys. Session +// cookies are always Secure, HttpOnly and SameSite=Strict. The random value +// that each logout replaces is saved in the database, so it outlasts a +// restart. func newSessionManager( _ fx.Lifecycle, params sessionManagerParams, ) (*session.Manager, error) { - return session.NewManagerWithDatabase( - context.Background(), params.Config.SigningKey, params.Database.DB()) + return session.NewManagerWithDatabase(context.Background(), + params.Keys.SessionHashKey, params.Keys.SessionBlockKey, + params.Database.DB()) } -// newEncryptedURLGenerator builds the encrypted URL generator, with a key -// derived from the signing key. -func newEncryptedURLGenerator(cfg *config.Config) (*encurl.Generator, error) { - return encurl.NewGenerator(cfg.SigningKey) +// newEncryptedURLGenerator builds the encrypted URL generator with its key. +func newEncryptedURLGenerator(keys seal.Keys) *encurl.Generator { + return encurl.NewGenerator(keys.EncryptedURLKey) } diff --git a/internal/seal/keys.go b/internal/seal/keys.go new file mode 100644 index 0000000..b84b416 --- /dev/null +++ b/internal/seal/keys.go @@ -0,0 +1,57 @@ +package seal + +// The salt of each key that DeriveKeys derives. Each key has a salt of its +// own, so no two uses share a key. +const ( + sessionHashKeySalt = "pixa-session-hash-v1" + sessionBlockKeySalt = "pixa-session-block-v1" + encryptedURLKeySalt = "pixa-urlenc-v1" + csrfKeySalt = "pixa-csrf-v1" +) + +// Keys holds the keys derived from the signing key, one for each use. The +// signing key itself is used for two things only: it is the HMAC key of signed +// URLs and the password of the login page. +type Keys struct { + // SessionHashKey signs the session cookie. + SessionHashKey [KeySize]byte + // SessionBlockKey encrypts the session cookie. + SessionBlockKey [KeySize]byte + // EncryptedURLKey encrypts the token of an encrypted URL. + EncryptedURLKey [KeySize]byte + // CSRFKey signs the CSRF token cookie of the login and generator pages. + CSRFKey [KeySize]byte +} + +// DeriveKeys derives each key of Keys from signingKey, with DeriveKey and the +// salt of that key. +func DeriveKeys(signingKey string) (Keys, error) { + masterKey := []byte(signingKey) + + sessionHashKey, err := DeriveKey(masterKey, sessionHashKeySalt) + if err != nil { + return Keys{}, err + } + + sessionBlockKey, err := DeriveKey(masterKey, sessionBlockKeySalt) + if err != nil { + return Keys{}, err + } + + encryptedURLKey, err := DeriveKey(masterKey, encryptedURLKeySalt) + if err != nil { + return Keys{}, err + } + + csrfKey, err := DeriveKey(masterKey, csrfKeySalt) + if err != nil { + return Keys{}, err + } + + return Keys{ + SessionHashKey: sessionHashKey, + SessionBlockKey: sessionBlockKey, + EncryptedURLKey: encryptedURLKey, + CSRFKey: csrfKey, + }, nil +} diff --git a/internal/seal/keys_test.go b/internal/seal/keys_test.go new file mode 100644 index 0000000..33a6183 --- /dev/null +++ b/internal/seal/keys_test.go @@ -0,0 +1,42 @@ +package seal_test + +import ( + "bytes" + "testing" + + "sneak.berlin/go/pixa/internal/seal" +) + +// TestDeriveKeys_KeysDiffer verifies that the keys DeriveKeys derives differ +// from each other and from the signing key. +func TestDeriveKeys_KeysDiffer(t *testing.T) { + t.Parallel() + + // 32 bytes, as long as a derived key, so that no key differs from it by + // its length alone + const signingKey = "0123456789abcdef0123456789abcdef" + + keys, err := seal.DeriveKeys(signingKey) + if err != nil { + t.Fatalf("DeriveKeys() error = %v", err) + } + + named := []struct { + name string + key []byte + }{ + {"the signing key", []byte(signingKey)}, + {"SessionHashKey", keys.SessionHashKey[:]}, + {"SessionBlockKey", keys.SessionBlockKey[:]}, + {"EncryptedURLKey", keys.EncryptedURLKey[:]}, + {"CSRFKey", keys.CSRFKey[:]}, + } + + for i, a := range named { + for _, b := range named[i+1:] { + if bytes.Equal(a.key, b.key) { + t.Errorf("%s equals %s", a.name, b.name) + } + } + } +} diff --git a/internal/server/computed_side_internal_test.go b/internal/server/computed_side_internal_test.go index 95fb696..157f0c3 100644 --- a/internal/server/computed_side_internal_test.go +++ b/internal/server/computed_side_internal_test.go @@ -11,6 +11,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600 @@ -48,11 +49,13 @@ func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) { s, _, _ := startImageProxy(t, upstream) - generator, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator: %v", err) + t.Fatalf("seal.DeriveKeys: %v", err) } + generator := encurl.NewGenerator(keys.EncryptedURLKey) + tests := []struct { path string // size is the size of the /v1/image/ URL, and width and height are diff --git a/internal/server/fit_outside_internal_test.go b/internal/server/fit_outside_internal_test.go index 1c095fd..55e3acd 100644 --- a/internal/server/fit_outside_internal_test.go +++ b/internal/server/fit_outside_internal_test.go @@ -9,6 +9,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // TestImageProxyRefusesFitOutsideAboveMaximum requests sizes with fit=outside @@ -42,11 +43,13 @@ func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) { s, _, _ := startImageProxy(t, upstream) - generator, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator: %v", err) + t.Fatalf("seal.DeriveKeys: %v", err) } + generator := encurl.NewGenerator(keys.EncryptedURLKey) + tests := []struct { path string width, height int diff --git a/internal/server/source_url_internal_test.go b/internal/server/source_url_internal_test.go index 68676d8..f897262 100644 --- a/internal/server/source_url_internal_test.go +++ b/internal/server/source_url_internal_test.go @@ -16,6 +16,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" "sneak.berlin/go/pixa/internal/signature" ) @@ -540,11 +541,13 @@ func signedImageURL(t *testing.T, sourceHost string) string { func encryptedImageURL(t *testing.T, sourceHost string) string { t.Helper() - generator, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator: %v", err) + t.Fatalf("seal.DeriveKeys: %v", err) } + generator := encurl.NewGenerator(keys.EncryptedURLKey) + token, err := generator.Generate(&imagerequest.Request{ Source: imagerequest.Source{Host: sourceHost, Path: ipv6SourcePath}, Transform: imagerequest.Transform{Format: imagerequest.FormatPNG}, diff --git a/internal/server/svg_as_avif_internal_test.go b/internal/server/svg_as_avif_internal_test.go index 0993c16..b1ce81d 100644 --- a/internal/server/svg_as_avif_internal_test.go +++ b/internal/server/svg_as_avif_internal_test.go @@ -10,6 +10,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/seal" ) // svgServedAsAVIF is an SVG whose first bytes pass the first-bytes check for @@ -34,11 +35,13 @@ func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) { s, _, _ := startImageProxy(t, upstream) - generator, err := encurl.NewGenerator(testSigningKey) + keys, err := seal.DeriveKeys(testSigningKey) if err != nil { - t.Fatalf("encurl.NewGenerator: %v", err) + t.Fatalf("seal.DeriveKeys: %v", err) } + generator := encurl.NewGenerator(keys.EncryptedURLKey) + encryptedURL := func(format imagerequest.Format) string { token, err := generator.Generate(&imagerequest.Request{ Source: imagerequest.Source{Host: upstreamHost, Path: "/drawing.avif"}, diff --git a/internal/session/session.go b/internal/session/session.go index 788c114..ad1359e 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -22,10 +22,6 @@ import ( const ( CookieName = "pixa_session" SessionTTL = 30 * 24 * time.Hour // 30 days - - // HKDF salts for key derivation - hashKeySalt = "pixa-session-hash-v1" - blockKeySalt = "pixa-session-block-v1" ) // Errors returned by session operations. @@ -66,35 +62,23 @@ type Manager struct { logoutValue string } -// NewManager creates a session manager with keys derived from the signing key. -// 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. +// NewManager creates a session manager whose session cookies hashKey signs +// and blockKey encrypts. 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 // trustworthy origin and accept Secure cookies there, so local development // keeps working. -func NewManager(signingKey string) (*Manager, error) { - masterKey := []byte(signingKey) - - // Derive separate keys for HMAC (hash) and encryption (block) - hashKey, err := seal.DeriveKey(masterKey, hashKeySalt) - if err != nil { - return nil, err - } - - blockKey, err := seal.DeriveKey(masterKey, blockKeySalt) - if err != nil { - return nil, err - } - +func NewManager(hashKey, blockKey [seal.KeySize]byte) *Manager { sc := securecookie.New(hashKey[:], blockKey[:]) sc.MaxAge(int(SessionTTL.Seconds())) return &Manager{ sc: sc, logoutValue: rand.Text(), - }, nil + } } // NewManagerWithDatabase creates a session manager like NewManager that keeps @@ -102,14 +86,11 @@ func NewManager(signingKey string) (*Manager, error) { // NewManager made when db has none, and otherwise starts from the one saved // there. func NewManagerWithDatabase( - ctx context.Context, signingKey string, db *sql.DB, + ctx context.Context, hashKey, blockKey [seal.KeySize]byte, db *sql.DB, ) (*Manager, error) { - m, err := NewManager(signingKey) - if err != nil { - return nil, err - } + m := NewManager(hashKey, blockKey) - _, err = db.ExecContext(ctx, + _, err := db.ExecContext(ctx, "INSERT OR IGNORE INTO logout (id, logout_value) VALUES (1, ?)", m.logoutValue) if err != nil { diff --git a/internal/session/session_cookie_attributes_test.go b/internal/session/session_cookie_attributes_test.go index 58a17a6..f016034 100644 --- a/internal/session/session_cookie_attributes_test.go +++ b/internal/session/session_cookie_attributes_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -20,11 +21,13 @@ import ( func TestSessionCookieAttributesAlwaysSecure(t *testing.T) { t.Parallel() - mgr, err := session.NewManager("test-signing-key-12345") + keys, err := seal.DeriveKeys("test-signing-key-12345") if err != nil { - t.Fatalf("NewManager() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) + writePaths := []struct { name string setCookie func(t *testing.T, w http.ResponseWriter) diff --git a/internal/session/session_logout_test.go b/internal/session/session_logout_test.go index b86dbb7..604cb15 100644 --- a/internal/session/session_logout_test.go +++ b/internal/session/session_logout_test.go @@ -8,6 +8,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) @@ -80,8 +81,13 @@ func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) { db := newTestDatabase(t) + keys, err := seal.DeriveKeys(logoutTestSigningKey) + if err != nil { + t.Fatalf("seal.DeriveKeys() error = %v", err) + } + mgr, err := session.NewManagerWithDatabase( - t.Context(), logoutTestSigningKey, db) + t.Context(), keys.SessionHashKey, keys.SessionBlockKey, db) if err != nil { t.Fatalf("NewManagerWithDatabase() error = %v", err) } @@ -109,7 +115,7 @@ func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) { } restarted, err := session.NewManagerWithDatabase( - t.Context(), logoutTestSigningKey, db) + t.Context(), keys.SessionHashKey, keys.SessionBlockKey, db) if err != nil { t.Fatalf("NewManagerWithDatabase() after restart error = %v", err) } @@ -129,8 +135,13 @@ func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) { func TestLogout_WithoutSession_EndsNoSession(t *testing.T) { t.Parallel() - mgr, err := session.NewManagerWithDatabase( - t.Context(), logoutTestSigningKey, newTestDatabase(t)) + keys, err := seal.DeriveKeys(logoutTestSigningKey) + if err != nil { + t.Fatalf("seal.DeriveKeys() error = %v", err) + } + + mgr, err := session.NewManagerWithDatabase(t.Context(), + keys.SessionHashKey, keys.SessionBlockKey, newTestDatabase(t)) if err != nil { t.Fatalf("NewManagerWithDatabase() error = %v", err) } diff --git a/internal/session/session_test.go b/internal/session/session_test.go index c0a2822..b994496 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -7,17 +7,20 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/seal" "sneak.berlin/go/pixa/internal/session" ) func TestManager_CreateAndValidate(t *testing.T) { t.Parallel() - mgr, err := session.NewManager("test-signing-key-12345") + keys, err := seal.DeriveKeys("test-signing-key-12345") if err != nil { - t.Fatalf("NewManager() error = %v", err) + t.Fatalf("seal.DeriveKeys() error = %v", err) } + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) + // Create a session w := httptest.NewRecorder() @@ -69,7 +72,8 @@ func TestManager_CreateAndValidate(t *testing.T) { func TestManager_ValidateSession_NoCookie(t *testing.T) { t.Parallel() - mgr, _ := session.NewManager("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) @@ -86,7 +90,8 @@ func TestManager_ValidateSession_NoCookie(t *testing.T) { func TestManager_ValidateSession_TamperedCookie(t *testing.T) { t.Parallel() - mgr, _ := session.NewManager("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) req.AddCookie(&http.Cookie{ @@ -110,8 +115,10 @@ func TestManager_ValidateSession_TamperedCookie(t *testing.T) { func TestManager_ValidateSession_WrongKey(t *testing.T) { t.Parallel() - mgr1, _ := session.NewManager("signing-key-1") - mgr2, _ := session.NewManager("signing-key-2") + keys1, _ := seal.DeriveKeys("signing-key-1") + keys2, _ := seal.DeriveKeys("signing-key-2") + mgr1 := session.NewManager(keys1.SessionHashKey, keys1.SessionBlockKey) + mgr2 := session.NewManager(keys2.SessionHashKey, keys2.SessionBlockKey) // Create session with mgr1 w := httptest.NewRecorder() @@ -142,7 +149,8 @@ func TestManager_ValidateSession_WrongKey(t *testing.T) { func TestManager_ClearSession(t *testing.T) { t.Parallel() - mgr, _ := session.NewManager("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) w := httptest.NewRecorder() mgr.ClearSession(w) @@ -172,7 +180,8 @@ func TestManager_ClearSession(t *testing.T) { func TestManager_IsAuthenticated(t *testing.T) { t.Parallel() - mgr, _ := session.NewManager("test-signing-key-12345") + keys, _ := seal.DeriveKeys("test-signing-key-12345") + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) // No session - should return false req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) @@ -208,7 +217,8 @@ func TestManager_IsAuthenticated(t *testing.T) { func TestManager_CookieAttributes(t *testing.T) { t.Parallel() - mgr, _ := session.NewManager("test-key") + keys, _ := seal.DeriveKeys("test-key") + mgr := session.NewManager(keys.SessionHashKey, keys.SessionBlockKey) w := httptest.NewRecorder() _ = mgr.CreateSession(w)