diff --git a/README.md b/README.md index 6ec40dc..7418a55 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Session.use_store(Session::MemoryStore(UserSession)) # Configure once Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end @@ -100,7 +100,7 @@ config.store = Session::ClusteredRedisStore(UserSession).new(client: Redis.new) ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] # 32+ chars recommended + # The secret is read from SESSION_SECRET (random per process in development without it) config.use_kdf = true # Enable PBKDF2 config.kdf_iterations = 100_000 # OWASP recommended config.bind_to_ip = true # Prevent session hijacking @@ -237,7 +237,7 @@ class HelloHandler end Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end @@ -269,7 +269,7 @@ end Session::Web.use_store(Session::CookieStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::CookieStore(UserSession).new end @@ -292,7 +292,7 @@ Cookies are always `HttpOnly`, `Secure` and `SameSite=Strict`; `config.cookie_se ```crystal Session.configure do |config| # Core - config.secret = ENV["SESSION_SECRET"] # Required + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour # Session lifetime config.session_key = "_session" # Cookie name config.sliding_expiration = true # Extend on access diff --git a/UPGRADE.md b/UPGRADE.md index 80c1a50..16df1c4 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,25 @@ # Upgrade Guide +## No published default secret + +The secret used to default to a string published in this repository, so an app that never set one signed its +cookies with a key anyone could use to forge a session. Now: + +- `config.secret` defaults to `SESSION_SECRET`. Without it, a random key for this process (one warning), so + sessions end when the process restarts and are not shared between instances. Set `SESSION_SECRET` to keep them. +- In production and staging a missing secret raises `Session::InsecureSecretException` wherever the secret is + used. With a `CookieStore`, or `encrypt_redis_data`, `Session::SessionHandler` then fails every request with `Session::InsecureSecretException` (a 500, logged by the server) instead of answering without a working session; a store that does not encrypt never reads the secret, so nothing is raised. +- A secret shorter than 32 bytes raises `Session::InsecureSecretException` too, instead of an `ArgumentError` from + AES that the handler logged and swallowed. This holds with `use_kdf` on (the `:production`, `:high_security` and + `:clustered` presets): the HMAC signature uses the secret itself, so PBKDF2 does not make a short one safe. +- `Session.configure` checks the secret when the configured store encrypts with it (a `CookieStore`, or + `encrypt_redis_data`), so a missing or short secret fails at boot instead of on every request. Set the secret (or + `SESSION_SECRET`) before, or inside, the `Session.configure` block that sets such a store. +- `require_secure_secret` now reads `CRYSTAL_ENV` (case-insensitive) when it is checked, not when the shard loads; + setting it explicitly still wins. +- `config.secret = nil` goes back to the default. `Configuration::DEFAULT_SECRET` still exists and is still refused + where `require_secure_secret` holds. + ## Crystal 1.22.1 or newer (no code change) `shard.yml` now declares `crystal: ">= 1.22.1"`. It was `crystal: 1.4.1`, a 1.4.1 minimum that no CI job ever built. @@ -98,7 +118,7 @@ end Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end ``` diff --git a/docs/README.md b/docs/README.md index 0e76f5f..26e6d73 100644 --- a/docs/README.md +++ b/docs/README.md @@ -48,7 +48,7 @@ end Session.use_store(Session::RedisStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 24.hours config.store = Session::RedisStore(UserSession).new(client: Redis.new) end diff --git a/docs/api/session.md b/docs/api/session.md index 80fe007..b965f53 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -49,7 +49,7 @@ The `authenticated?` method is abstract and must be implemented by your subclass ```crystal # Configure the session library Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end diff --git a/docs/clustering/configuration.md b/docs/clustering/configuration.md index de5d757..eaa40cd 100644 --- a/docs/clustering/configuration.md +++ b/docs/clustering/configuration.md @@ -20,7 +20,7 @@ This page covers all configuration options for session clustering. ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) # Enable clustering with defaults config.cluster.enabled = true @@ -35,7 +35,7 @@ end ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour # Cluster configuration @@ -183,7 +183,7 @@ config.store = Session::ClusteredRedisStore(UserSession).new( ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) case ENV["APP_ENV"]? when "production" diff --git a/docs/clustering/multi-node-setup.md b/docs/clustering/multi-node-setup.md index aedeb8a..cf417e4 100644 --- a/docs/clustering/multi-node-setup.md +++ b/docs/clustering/multi-node-setup.md @@ -137,7 +137,7 @@ spec: ```crystal # config/session.cr Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 24.hours # Clustering configuration diff --git a/docs/clustering/overview.md b/docs/clustering/overview.md index 3e9c645..d74b363 100644 --- a/docs/clustering/overview.md +++ b/docs/clustering/overview.md @@ -102,7 +102,7 @@ A thread-safe, in-memory cache with: ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) # Enable clustering config.cluster.enabled = true diff --git a/docs/configuration/basic.md b/docs/configuration/basic.md index fe8510c..6dd2b26 100644 --- a/docs/configuration/basic.md +++ b/docs/configuration/basic.md @@ -6,7 +6,7 @@ Configure Session to match your application's requirements. ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end ``` @@ -29,7 +29,7 @@ end ```crystal Session.configure do |config| config.apply_preset(:production) - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::RedisStore(UserSession).new(client: Redis.new) end ``` @@ -56,7 +56,7 @@ Session.configure do |config| config.timeout = 8.hours config.bind_to_user_agent = true - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::RedisStore(UserSession).new end ``` @@ -66,7 +66,7 @@ end ```crystal Session.configure do |config| # Core settings - config.secret = ENV["SESSION_SECRET"] # Required: encryption key + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour # Session lifetime config.session_key = "_session" # Cookie name @@ -81,7 +81,7 @@ end | Property | Type | Default | Description | |----------|------|---------|-------------| -| `secret` | `String` | (default) | Encryption secret (32+ chars recommended) | +| `secret` | `String` | `SESSION_SECRET` | Encryption and signing secret (32+ bytes, also with `use_kdf`) | | `timeout` | `Time::Span` | `1.hour` | Session lifetime | | `session_key` | `String` | `"_session"` | Cookie name | | `store` | `Store` | `nil` | Storage backend (typed by `Session.use_store`) | @@ -93,7 +93,7 @@ end ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) case ENV["APP_ENV"]? when "production" diff --git a/docs/configuration/security.md b/docs/configuration/security.md index e3f09da..6b08104 100644 --- a/docs/configuration/security.md +++ b/docs/configuration/security.md @@ -6,10 +6,9 @@ Configure security features to protect session data. ```crystal Session.configure do |config| - # Required: Secret key for encryption - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) - # Enforce secure secret (raises if using default) + # Raise on a missing or published secret (default: in production and staging) config.require_secure_secret = true # Digest algorithm for HMAC @@ -57,8 +56,8 @@ end | Property | Type | Default | Description | |----------|------|---------|-------------| -| `secret` | `String` | (default) | Encryption key | -| `require_secure_secret` | `Bool` | `false` | Enforce non-default secret | +| `secret` | `String` | `SESSION_SECRET` | Encryption key; random per process when unset, outside production and staging | +| `require_secure_secret` | `Bool` | production, staging | Raise on a missing or published secret | | `digest_algorithm` | `Symbol` | `:sha256` | HMAC algorithm | | `digest_fallback` | `Bool` | `true` | Allow SHA1 fallback | | `use_kdf` | `Bool` | `false` | Enable PBKDF2 | diff --git a/docs/getting-started/installation.md b/docs/getting-started/installation.md index d25d829..dda8144 100644 --- a/docs/getting-started/installation.md +++ b/docs/getting-started/installation.md @@ -39,7 +39,7 @@ end Session.use_store(Session::MemoryStore(TestSession)) Session.configure do |config| - config.secret = "test-secret-for-verification" + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(TestSession).new end diff --git a/docs/getting-started/quick-start.md b/docs/getting-started/quick-start.md index 4a3c2b2..456a13e 100644 --- a/docs/getting-started/quick-start.md +++ b/docs/getting-started/quick-start.md @@ -36,8 +36,9 @@ Declare the store type with `Session.use_store`, then configure: Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| - # Required: Secret key for encryption (use environment variable in production) - config.secret = ENV["SESSION_SECRET"]? || "your-32-character-secret-key-here" + # The key comes from SESSION_SECRET (32 bytes or more). Without it, development gets a random + # key per process; in production and staging, a store that encrypts (CookieStore, or + # encrypt_redis_data) makes Session.configure raise Session::InsecureSecretException at boot. # Session timeout config.timeout = 24.hours @@ -113,7 +114,7 @@ require "session" Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::MemoryStore(UserSession).new end @@ -190,7 +191,7 @@ end Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"]? || "dev-secret-32-characters-long!!" + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour config.sliding_expiration = true config.store = Session::MemoryStore(UserSession).new diff --git a/docs/integrations/azu-framework.md b/docs/integrations/azu-framework.md index ca4fc00..d1d7466 100644 --- a/docs/integrations/azu-framework.md +++ b/docs/integrations/azu-framework.md @@ -39,14 +39,14 @@ end Session::Web.use_store(Session::CookieStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 8.hours config.session_key = "myapp_session" config.store = Session::CookieStore(UserSession).new end ``` -`SESSION_SECRET` is required in production and staging (`CRYSTAL_ENV`): the published default secret is refused there, because anyone could forge a cookie signed with it. Generate one with `Random::Secure.hex(32)` and keep it in your process environment. +`SESSION_SECRET` (32 bytes or more) is required in production and staging (`CRYSTAL_ENV`): without it the cookie store cannot sign: `Session.configure` raises `Session::InsecureSecretException` at boot, and a `Session::SessionHandler` built without `Session.configure` fails every request with it (a 500). Elsewhere a missing secret means a random key for this process (with one warning), so sessions end at a restart. Generate one with `Random::Secure.hex(32)` and keep it in your process environment. This is close to `Azu::Env.secret("SESSION_SECRET")` but not the same: azu refuses the random fallback in its `strict_environments` (`production` unless `AZU_STRICT_ENVS` or `Azu::Env.strict_environments=` widens it), while session refuses it in production and staging (`CRYSTAL_ENV`), whatever `AZU_STRICT_ENVS` says. ## Mount the handler diff --git a/docs/integrations/http-server.md b/docs/integrations/http-server.md index 79ad637..a6756fa 100644 --- a/docs/integrations/http-server.md +++ b/docs/integrations/http-server.md @@ -32,7 +32,7 @@ end Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 24.hours config.session_key = "_myapp_session" config.store = Session::MemoryStore(UserSession).new @@ -143,7 +143,7 @@ end # Configure Session.configure do |config| - config.secret = ENV.fetch("SESSION_SECRET", "dev-secret-change-in-production!") + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour config.sliding_expiration = true config.store = Session::MemoryStore(UserSession).new @@ -222,7 +222,7 @@ server.listen ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 24.hours config.session_key = "_app_session" diff --git a/docs/resilience/error-handling.md b/docs/resilience/error-handling.md index 5cced9d..fe2f915 100644 --- a/docs/resilience/error-handling.md +++ b/docs/resilience/error-handling.md @@ -15,7 +15,7 @@ Session uses specific exception types for precise error handling and graceful de | `SessionEncryptionException` | -- | Encryption or decryption failure | | `CookieSizeExceededException` | `actual_size`, `max_size` | Cookie exceeds 4KB limit | | `SessionBindingException` | `binding_type` | Client fingerprint mismatch | -| `InsecureSecretException` | -- | Default secret with `require_secure_secret` | +| `InsecureSecretException` | -- | Missing or published secret with `require_secure_secret`, or a secret under 32 bytes; `SessionHandler` raises it (a 500) for a store that encrypts | | `CircuitOpenException` | `time_until_retry` | Circuit breaker is open | | `ConnectionPoolTimeoutException` | -- | Pool checkout timeout | | `ClusterException` | -- | General cluster failure | diff --git a/docs/security/encryption.md b/docs/security/encryption.md index 07c421d..57d8b07 100644 --- a/docs/security/encryption.md +++ b/docs/security/encryption.md @@ -14,10 +14,10 @@ Decryption reverses the process: decode, verify HMAC signature, then decrypt. Th ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] # 32+ characters recommended + # The secret is read from SESSION_SECRET (random per process without it, outside production and staging) config.digest_algorithm = :sha256 # HMAC algorithm config.digest_fallback = true # Allow SHA1 fallback for migration - config.require_secure_secret = true # Raise if using default secret + config.require_secure_secret = true # Raise on a missing, published or short secret end ``` @@ -47,16 +47,20 @@ The default digest is `:sha256`. If you're migrating from an older version that ## Secret Key Requirements -- Minimum 32 characters for AES-256 -- Set `require_secure_secret = true` in production to raise `InsecureSecretException` if the default secret is used -- Without `require_secure_secret`, a warning is logged once on first use of the default secret +- At least 32 bytes, with or without `use_kdf` (the HMAC signature uses the secret itself): a shorter one raises + `InsecureSecretException`, from `Session.configure` when the store encrypts with it +- Unset, `secret` is `SESSION_SECRET`. Without that, `InsecureSecretException` where `require_secure_secret` holds + (production and staging by default, read from `CRYSTAL_ENV` each time the secret is read), and a random per-process + key with one warning elsewhere. With a `CookieStore`, or `encrypt_redis_data`, `Session::SessionHandler` then fails every request with `Session::InsecureSecretException` (a 500, logged by the server) instead of answering without a working session; a store that does not encrypt never reads the secret, so nothing is raised. +- The secret older releases used by default (`Configuration::DEFAULT_SECRET`) is public: set explicitly, it is refused + where `require_secure_secret` holds and warned about elsewhere ## Properties | Property | Type | Default | Description | |----------|------|---------|-------------| -| `secret` | `String` | (default) | Encryption key (32+ chars) | -| `require_secure_secret` | `Bool` | `false` | Raise on default secret | +| `secret` | `String` | `SESSION_SECRET` | Encryption key (32+ bytes) | +| `require_secure_secret` | `Bool` | production, staging | Raise on a missing or published secret | | `digest_algorithm` | `Symbol` | `:sha256` | HMAC algorithm | | `digest_fallback` | `Bool` | `true` | Allow SHA1 fallback | | `encrypt_redis_data` | `Bool` | `false` | Encrypt data in Redis | diff --git a/docs/security/key-derivation.md b/docs/security/key-derivation.md index 12f373c..14b4298 100644 --- a/docs/security/key-derivation.md +++ b/docs/security/key-derivation.md @@ -1,6 +1,9 @@ # Key Derivation -PBKDF2-SHA256 derives a strong 32-byte encryption key from your secret, adding protection against brute-force attacks on weak or short secrets. +PBKDF2-SHA256 derives the 32-byte AES key from your secret. It does not make a weak or short secret safe: the HMAC +signature on every cookie uses the secret itself, so a guessable secret can be found offline from one cookie whatever +the KDF does. The secret must still be at least 32 random bytes (`openssl rand -hex 32`); a shorter one raises +`Session::InsecureSecretException`. ## Configuration @@ -14,9 +17,9 @@ end ## How It Works -When `use_kdf` is enabled, the library runs `OpenSSL::PKCS5.pbkdf2_hmac` at initialization to derive a 32-byte key from your secret. This derived key is then used for all AES-256-CBC encryption operations instead of the raw secret. +When `use_kdf` is enabled, `Message::Encryptor` runs `OpenSSL::PKCS5.pbkdf2_hmac` when it is built to derive a 32-byte key from your secret. This derived key is then used for all AES-256-CBC encryption operations instead of the raw secret. -The derived key is computed once and cached, so there is no per-request performance cost. +`Session.config.encryptor` reuses its encryptor while the secret, digest and KDF settings stay the same, so PBKDF2 runs once per setting, not per request. Changing any of them (or `SESSION_SECRET`, when no secret is configured) derives the key again on the next use. An `Encryptor` you build yourself derives its key once, in `new`. ## Properties diff --git a/docs/storage-backends/clustered-redis-store.md b/docs/storage-backends/clustered-redis-store.md index 4f4c882..dfdebe9 100644 --- a/docs/storage-backends/clustered-redis-store.md +++ b/docs/storage-backends/clustered-redis-store.md @@ -34,7 +34,7 @@ end # Configure with clustering Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour # Enable clustering @@ -194,7 +194,7 @@ deleted = store.bulk_delete { |s| s.user_id == compromised_id } ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 24.hours # Cluster settings @@ -223,7 +223,7 @@ end ```crystal Session.configure do |config| - config.secret = "dev-secret-not-for-production" + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 1.hour # Disable clustering in development @@ -241,7 +241,7 @@ end ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) # Aggressive caching for high traffic config.cluster.enabled = true diff --git a/docs/storage-backends/cookie-store.md b/docs/storage-backends/cookie-store.md index 3cb7e3a..4a616cb 100644 --- a/docs/storage-backends/cookie-store.md +++ b/docs/storage-backends/cookie-store.md @@ -16,7 +16,7 @@ flowchart LR ```crystal Session.configure do |config| - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::CookieStore(UserSession).new end ``` diff --git a/examples/configuration_examples.cr b/examples/configuration_examples.cr index a10a001..03a1c92 100644 --- a/examples/configuration_examples.cr +++ b/examples/configuration_examples.cr @@ -9,7 +9,7 @@ Session.configure do |config| config.apply_preset(:development) # Override specific settings - config.secret = ENV["SESSION_SECRET"]? || Configuration::DEFAULT_SECRET + # No secret here: it is read from SESSION_SECRET, or random for this process in development config.timeout = 2.hours # Set your store @@ -22,7 +22,7 @@ Session.configure do |config| config.apply_preset(:production) # Override with environment-specific values - config.secret = ENV["SESSION_SECRET"] + # The secret is read from SESSION_SECRET (random per process in development without it) config.timeout = 4.hours # Use Redis in production @@ -38,8 +38,7 @@ Session.configure do |config| # Load high security preset config.apply_preset(:high_security) - # Must provide secure secret - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) # Use Redis with connection pooling for better performance pool_config = Session::ConnectionPoolConfig.new( @@ -69,8 +68,7 @@ Session.configure do |config| config.cluster.node_id = ENV["NODE_ID"]? || UUID.random.to_s config.cluster.channel = "session:cluster:#{ENV["ENVIRONMENT"]?}" - # Must provide secure secret - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) # Use clustered Redis store redis = Redis.new(host: ENV["REDIS_HOST"]? || "localhost") @@ -90,7 +88,7 @@ Session.configure do |config| # Core settings config.timeout = 30.minutes config.session_key = "_app_session" - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) # Security settings config.require_secure_secret = true @@ -127,7 +125,7 @@ Session.configure do |config| config.bind_to_user_agent = true # Add user agent binding config.compression_threshold = 1024 # Larger threshold - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) config.store = Session::RedisStore(UserSession).new end @@ -146,12 +144,12 @@ Session.configure do |config| config.store = Session::MemoryStore(UserSession).new when "staging" config.apply_preset(:production) - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) redis = Redis.new(host: ENV["REDIS_HOST"]?) config.store = Session::RedisStore(UserSession).new(client: redis) when "production" config.apply_preset(:high_security) - config.secret = ENV.fetch("SESSION_SECRET") + # The secret is read from SESSION_SECRET (random per process in development without it) pool_config = Session::ConnectionPoolConfig.new(size: 20) config.store = Session::RedisStore(UserSession).with_pool(pool_config) else diff --git a/project.mdc b/project.mdc index b9e1bb0..48abb31 100644 --- a/project.mdc +++ b/project.mdc @@ -69,12 +69,12 @@ end Session.configure do |c| c.timeout = 1.hour c.session_key = "_session" - c.secret = "your-secret-key" - c.provider = Session::RedisStore(UserSession).provider(client: Redis.new) + # The secret is read from SESSION_SECRET (32+ bytes, e.g. `openssl rand -hex 32`); never hard-code it + c.store = Session::RedisStore(UserSession).new(client: Redis.new) # Event handlers for monitoring - c.on_started = ->(sid : String, data : SessionData) { ... } - c.on_deleted = ->(sid : String, data : SessionData) { ... } + c.on_started = ->(sid : String, data : Session::Base) { ... } + c.on_deleted = ->(sid : String, data : Session::Base) { ... } end ``` diff --git a/spec/configuration_spec.cr b/spec/configuration_spec.cr index 4f23e16..82d1b24 100644 --- a/spec/configuration_spec.cr +++ b/spec/configuration_spec.cr @@ -7,14 +7,14 @@ describe Session::Configuration do end it "returns false when custom secret is set" do - Session.config.secret = "my-custom-secret-key-32-bytes!!" + Session.config.secret = "my-custom-secret-key-32-bytes!!!" Session.config.using_default_secret?.should be_false end end describe "#validate_secret!" do it "does nothing when custom secret is set" do - Session.config.secret = "my-custom-secret-key-32-bytes!!" + Session.config.secret = "my-custom-secret-key-32-bytes!!!" Session.config.validate_secret! # Should not raise end @@ -27,7 +27,7 @@ describe Session::Configuration do end it "does not raise when require_secure_secret is true and custom secret set" do - Session.config.secret = "my-custom-secret-key-32-bytes!!" + Session.config.secret = "my-custom-secret-key-32-bytes!!!" Session.config.require_secure_secret = true Session.config.validate_secret! # Should not raise @@ -121,7 +121,7 @@ describe Session::Configuration do end it "passes KDF settings to encryptor" do - Session.config.secret = "my-custom-secret-key-32-bytes!!" + Session.config.secret = "my-custom-secret-key-32-bytes!!!" Session.config.use_kdf = true Session.config.kdf_iterations = 50_000 @@ -278,3 +278,156 @@ describe Session::Configuration do end end end + +describe Session::Configuration do + describe "without a configured secret" do + around_each do |example| + previous = {ENV["CRYSTAL_ENV"]?, ENV["SESSION_SECRET"]?} + ENV.delete("SESSION_SECRET") + ENV["CRYSTAL_ENV"] = "development" + example.run + ensure + {"CRYSTAL_ENV", "SESSION_SECRET"}.zip(previous.not_nil!).each do |name, value| + value ? (ENV[name] = value) : ENV.delete(name) + end + end + + it "never signs with the published default secret" do + config = Session::Configuration.new + + config.secret.should_not eq Session::Configuration::DEFAULT_SECRET + config.using_default_secret?.should be_false + config.secret.should eq Session::Configuration.new.secret + end + + it "reads SESSION_SECRET" do + ENV["SESSION_SECRET"] = "from-the-environment-0123456789abcdef" + + Session::Configuration.new.secret.should eq "from-the-environment-0123456789abcdef" + end + + it "is refused in production even when CRYSTAL_ENV is set after the configuration was built" do + config = Session::Configuration.new + ENV["CRYSTAL_ENV"] = "Production" + + expect_raises(Session::InsecureSecretException, /SESSION_SECRET/) { config.encryptor } + end + + it "is refused in production even after the random fallback was used" do + config = Session::Configuration.new + config.encryptor + ENV["CRYSTAL_ENV"] = "production" + + expect_raises(Session::InsecureSecretException, /SESSION_SECRET/) { config.encryptor } + end + + it "refuses a SESSION_SECRET shorter than 32 bytes with a clear message" do + ENV["SESSION_SECRET"] = "short" + + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) { Session::Configuration.new.encryptor } + end + + it "refuses an empty configured secret with a clear message" do + config = Session::Configuration.new + config.secret = "" + + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) { config.encryptor } + end + end +end + +describe Session::SessionHandler do + it "fails the request when the cookie store has no usable secret, instead of answering without a session" do + Session.config.secret = "short" + handler = Session::SessionHandler.new(Session::CookieStore(UserSession).new) + context = HTTP::Server::Context.new(HTTP::Request.new("GET", "/"), HTTP::Server::Response.new(IO::Memory.new)) + + expect_raises(Session::InsecureSecretException) { handler.call(context) } + end +end + +describe Session::Configuration do + describe "a short secret with key derivation on" do + # The HMAC signature uses the raw secret, so PBKDF2 does not make a short one safe. + ["", "a", "abc", "31-bytes-is-still-too-short-xyz"].each do |short| + it "refuses #{short.bytesize} bytes under the production preset" do + config = Session::Configuration.new + config.apply_preset(:production) + config.secret = short + + config.use_kdf.should be_true + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) { config.encryptor } + end + end + end + + describe "#encryptor" do + it "reuses the encryptor (and its derived key) while the settings stay the same" do + config = Session::Configuration.new + config.secret = "a-secret-nobody-else-knows-0123456789" + config.use_kdf = true + config.kdf_iterations = 1_000 + + config.encryptor.should be config.encryptor + end + + it "builds a new encryptor when a setting it depends on changes" do + config = Session::Configuration.new + config.secret = "a-secret-nobody-else-knows-0123456789" + config.use_kdf = true + config.kdf_iterations = 1_000 + first = config.encryptor + + config.kdf_salt = "another-salt" + config.encryptor.should_not be first + second = config.encryptor + + config.secret = "another-secret-nobody-knows-0123456789" + config.encryptor.should_not be second + end + end + + describe "#validate_secret_for!" do + it "refuses a short secret for a store that encrypts with it" do + Session.config.secret = "short" + + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) do + Session.config.validate_secret_for!(Session::CookieStore(UserSession).new) + end + end + + it "does not check the secret for a store that does not use it" do + Session.config.secret = "short" + + Session.config.validate_secret_for!(Session::MemoryStore(UserSession).new) + end + end +end + +describe "Session.configure" do + it "fails at boot when the configured store encrypts with an unusable secret" do + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) do + Session.configure do |c| + c.encrypt_redis_data = true + c.secret = "short" + end + end + end + + it "fails at boot under the production preset with a short secret" do + expect_raises(Session::InsecureSecretException, /at least 32 bytes/) do + Session.configure do |c| + c.apply_preset(:production) + c.cookie_secure = true + c.encrypt_redis_data = true + c.secret = "abc" + end + end + end + + it "does not check the secret when the store does not encrypt" do + Session.configure do |c| + c.secret = "short" + end + end +end diff --git a/spec/presets_spec.cr b/spec/presets_spec.cr index 28a55bc..9f2f457 100644 --- a/spec/presets_spec.cr +++ b/spec/presets_spec.cr @@ -112,10 +112,10 @@ describe Session::Configuration do end it "preserves secret when applying preset" do - Session.config.secret = "my-custom-secret-key-32-bytes!!" + Session.config.secret = "my-custom-secret-key-32-bytes!!!" Session.config.apply_preset(:production) - Session.config.secret.should eq "my-custom-secret-key-32-bytes!!" + Session.config.secret.should eq "my-custom-secret-key-32-bytes!!!" end it "raises for unknown preset" do diff --git a/src/azu.cr b/src/azu.cr index a87b7f6..bffe376 100644 --- a/src/azu.cr +++ b/src/azu.cr @@ -10,7 +10,7 @@ # Session::Web.use_store(Session::CookieStore(UserSession)) # # Session.configure do |config| -# config.secret = ENV["SESSION_SECRET"] +# # The secret is read from SESSION_SECRET (random per process in development without it) # config.store = Session::CookieStore(UserSession).new # end # diff --git a/src/configuration.cr b/src/configuration.cr index 7197e5d..bfaac14 100644 --- a/src/configuration.cr +++ b/src/configuration.cr @@ -23,11 +23,13 @@ module Session end class Configuration + # The secret older releases signed with when none was configured. It is public, so it is never + # used by default any more and is refused where `require_secure_secret` holds. DEFAULT_SECRET = "1sxc1aNxGHTTZKlK5cpCgufJAqGM4G13" property timeout : Time::Span = 1.hour property session_key : String = "_session" - property secret : String = DEFAULT_SECRET + setter secret : String? # Cookie attributes. Cookies are always HttpOnly. Secure keeps them off plain http (browsers # still accept a Secure cookie over http on localhost); turn it off only for an http-only setup. @@ -44,10 +46,49 @@ module Session # Environments that must not run on the published default secret. PRODUCTION_ENVIRONMENTS = %w(production staging) - # Security: require a secure secret (non-default) - raises if default secret is used. - # The default secret is public, so anyone could forge a session cookie signed with it: - # it is refused in production and staging (`CRYSTAL_ENV`) unless turned off explicitly. - property require_secure_secret : Bool = PRODUCTION_ENVIRONMENTS.includes?(ENV["CRYSTAL_ENV"]?) + # Whether a missing or published secret is an error rather than a warning. Unless set, it holds + # in production and staging, read from `CRYSTAL_ENV` (case-insensitive) each time it is checked. + setter require_secure_secret : Bool? + + def require_secure_secret : Bool + @require_secure_secret.nil? ? Configuration.production_environment? : @require_secure_secret == true + end + + def self.production_environment? : Bool + PRODUCTION_ENVIRONMENTS.includes?(ENV["CRYSTAL_ENV"]?.try(&.strip.downcase)) + end + + # The key sessions are encrypted and signed with: the one configured, else `SESSION_SECRET`. + # Without either, `InsecureSecretException` where `require_secure_secret` holds, and otherwise a + # random key for this process only (sessions do not survive a restart or reach another instance). + # Not memoised: the random fallback is refused again as soon as `require_secure_secret` holds. + def secret : String + @secret || ENV["SESSION_SECRET"]?.presence || fallback_secret + end + + # AES-256 takes a 32-byte key, and the secret is that key unless `use_kdf` derives one. The HMAC + # signature always uses the secret itself, so the minimum holds with `use_kdf` too. + MIN_SECRET_BYTES = 32 + + private def fallback_secret : String + if require_secure_secret + raise InsecureSecretException.new( + "SESSION_SECRET is not set: set it, or configure one with Session.configure { |c| c.secret = ... }" + ) + end + Configuration.process_secret + end + + @@process_secret : String = Random::Secure.hex(32) + @@process_secret_warned = Atomic(Bool).new(false) + + # The random fallback, one per process so every configuration agrees on it; warns once. + def self.process_secret : String + unless @@process_secret_warned.swap(true) + Log.warn { "SESSION_SECRET is not set: using a random session secret for this process only" } + end + @@process_secret + end # Digest algorithm for HMAC signatures (:sha256 recommended, :sha1 for legacy) property digest_algorithm : Symbol = :sha256 @@ -192,16 +233,41 @@ module Session raise "Session store not configured. Call Session.use_store(StoreType) before Session.configure." end - def encryptor + # The encryptor for the current settings. It is reused while they stay the same, so PBKDF2 + # (`use_kdf`) runs once per secret, salt and iteration count, not on every encrypt and decrypt. + def encryptor : Message::Encryptor validate_secret! - Message::Encryptor.new( - secret, + key = {secret, digest_algorithm, digest_fallback, use_kdf, kdf_iterations, kdf_salt} + cached = @encryptor_cache + return cached.encryptor if cached && cached.key == key + + encryptor = Message::Encryptor.new( + key[0], digest: digest_algorithm, fallback_digest: digest_fallback ? :sha1 : nil, use_kdf: use_kdf, kdf_iterations: kdf_iterations, kdf_salt: kdf_salt ) + @encryptor_cache = EncryptorCache.new(key, encryptor) + encryptor + end + + # One reference, so a fiber on another thread sees the old pair or the new one, never half of each. + private class EncryptorCache + getter key : {String, Symbol, Bool, Bool, Int32, String} + getter encryptor : Message::Encryptor + + def initialize(@key, @encryptor) + end + end + + @encryptor_cache : EncryptorCache? = nil + + # Raises `InsecureSecretException` when `store` encrypts with the secret (a `CookieStore`, or + # any store with `encrypt_redis_data`) and the secret is missing, published or too short. + def validate_secret_for!(store : Store) : Nil + validate_secret! if store.encrypts_with_secret? end # Check if the current secret is the insecure default @@ -211,7 +277,14 @@ module Session # Validate secret security - warns or raises based on configuration def validate_secret! : Nil - return unless using_default_secret? + key = secret + if key.bytesize < MIN_SECRET_BYTES + raise InsecureSecretException.new( + "The session secret is #{key.bytesize} bytes; it must be at least #{MIN_SECRET_BYTES} bytes " \ + "(generate one with `openssl rand -hex 32`)" + ) + end + return unless key == DEFAULT_SECRET if require_secure_secret raise InsecureSecretException.new( diff --git a/src/handlers/session_handler.cr b/src/handlers/session_handler.cr index edeacad..161b61f 100644 --- a/src/handlers/session_handler.cr +++ b/src/handlers/session_handler.cr @@ -6,6 +6,7 @@ module Session end def call(context : HTTP::Server::Context) + validate_secret! load_session(context) output = commit_before_first_write(context) call_next(context) @@ -48,6 +49,12 @@ module Session Log.warn { "Failed to save the session after the response started: #{ex.message}" } end + # A store that encrypts with the secret cannot work without a usable one: fail the request + # (`InsecureSecretException`, a 500) instead of answering it with a session that is never saved. + private def validate_secret! : Nil + Session.config.validate_secret_for!(@session) + end + private def load_session(context : HTTP::Server::Context) @session.load_from(context.request.cookies) diff --git a/src/session.cr b/src/session.cr index 6973d27..8f0eaaa 100644 --- a/src/session.cr +++ b/src/session.cr @@ -118,7 +118,7 @@ module Session # Session.use_store(Session::CookieStore(UserSession)) # # Session.configure do |c| - # c.secret = ENV["SESSION_SECRET"] + # # The secret is read from SESSION_SECRET (random per process in development without it) # c.store = Session::CookieStore(UserSession).new # end # ``` @@ -132,9 +132,17 @@ module Session end end + # Applies the block to `CONFIG`, then checks it: a store that encrypts with the secret fails here, + # at boot, when the secret is unusable (`SessionHandler` checks again on each request). def self.configure(&) with CONFIG yield CONFIG CONFIG.validate_cookie_options! + config = CONFIG + if config.responds_to?(:store) && (store = config.store) + config.validate_secret_for!(store) + elsif config.encrypt_redis_data + config.validate_secret! + end end def self.config diff --git a/src/store.cr b/src/store.cr index b394884..75f6a79 100644 --- a/src/store.cr +++ b/src/store.cr @@ -22,6 +22,11 @@ module Session # Storage abstraction - must be implemented by subclasses abstract def storage : String + # Whether this store encrypts sessions with `Session.config.secret`: with `encrypt_redis_data`. + def encrypts_with_secret? : Bool + Session.config.encrypt_redis_data + end + # The session of the request being served. A store is shared by every request, so it is # kept per fiber: concurrent requests, and the next request on a keep-alive connection, # never see each other's session. `SessionHandler` calls `release` when a request ends. diff --git a/src/stores/cookie_store.cr b/src/stores/cookie_store.cr index 702d680..49142bb 100644 --- a/src/stores/cookie_store.cr +++ b/src/stores/cookie_store.cr @@ -41,6 +41,11 @@ module Session self.class.name end + # Every cookie is encrypted and signed with the secret. + def encrypts_with_secret? : Bool + true + end + def [](key : String) : T if data = cookies[data_key]? begin