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)