diff options
15 files changed, 95 insertions, 41 deletions
diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index a50f0ca8c1..265ce5d6f3 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -121,11 +121,11 @@ module ActionController #:nodoc: class NullCookieJar < ActionDispatch::Cookies::CookieJar #:nodoc: def self.build(request) - secret = request.env[ActionDispatch::Cookies::TOKEN_KEY] - host = request.host - secure = request.ssl? + key_generator = request.env[ActionDispatch::Cookies::GENERATOR_KEY] + host = request.host + secure = request.ssl? - new(secret, host, secure) + new(key_generator, host, secure) end def write(*) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index eaf922595a..0ec7f24a14 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -27,7 +27,7 @@ module ActionDispatch # cookies[:login] = { value: "XJ-122", expires: 1.hour.from_now } # # # Sets a signed cookie, which prevents users from tampering with its value. - # # The cookie is signed by your app's <tt>config.secret_token</tt> value. + # # The cookie is signed by your app's <tt>config.secret_token_key</tt> value. # # It can be read using the signed method <tt>cookies.signed[:key]</tt> # cookies.signed[:user_id] = current_user.id # @@ -79,8 +79,8 @@ module ActionDispatch # * <tt>:httponly</tt> - Whether this cookie is accessible via scripting or # only HTTP. Defaults to +false+. class Cookies - HTTP_HEADER = "Set-Cookie".freeze - TOKEN_KEY = "action_dispatch.secret_token".freeze + HTTP_HEADER = "Set-Cookie".freeze + GENERATOR_KEY = "action_dispatch.key_generator".freeze # Raised when storing more than 4K of session data. CookieOverflow = Class.new StandardError @@ -103,17 +103,19 @@ module ActionDispatch DOMAIN_REGEXP = /[^.]*\.([^.]*|..\...|...\...)$/ def self.build(request) - secret = request.env[TOKEN_KEY] + env = request.env + key_generator = env[GENERATOR_KEY] + host = request.host secure = request.ssl? - new(secret, host, secure).tap do |hash| + new(key_generator, host, secure).tap do |hash| hash.update(request.cookies) end end - def initialize(secret = nil, host = nil, secure = false) - @secret = secret + def initialize(key_generator, host = nil, secure = false) + @key_generator = key_generator @set_cookies = {} @delete_cookies = {} @host = host @@ -220,7 +222,7 @@ module ActionDispatch # cookies.permanent.signed[:remember_me] = current_user.id # # => Set-Cookie: remember_me=BAhU--848956038e692d7046deab32b7131856ab20e14e; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT def permanent - @permanent ||= PermanentCookieJar.new(self, @secret) + @permanent ||= PermanentCookieJar.new(self, @key_generator) end # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from @@ -228,7 +230,7 @@ module ActionDispatch # cookie was tampered with by the user (or a 3rd party), an ActiveSupport::MessageVerifier::InvalidSignature exception will # be raised. # - # This jar requires that you set a suitable secret for the verification on your app's +config.secret_token+. + # This jar requires that you set a suitable secret for the verification on your app's +config.secret_token_key+. # # Example: # @@ -237,7 +239,7 @@ module ActionDispatch # # cookies.signed[:discount] # => 45 def signed - @signed ||= SignedCookieJar.new(self, @secret) + @signed ||= SignedCookieJar.new(self, @key_generator) end def write(headers) @@ -261,8 +263,9 @@ module ActionDispatch end class PermanentCookieJar < CookieJar #:nodoc: - def initialize(parent_jar, secret) - @parent_jar, @secret = parent_jar, secret + def initialize(parent_jar, key_generator) + @parent_jar = parent_jar + @key_generator = key_generator end def []=(key, options) @@ -285,9 +288,10 @@ module ActionDispatch MAX_COOKIE_SIZE = 4096 # Cookies can typically store 4096 bytes. SECRET_MIN_LENGTH = 30 # Characters - def initialize(parent_jar, secret) - ensure_secret_secure(secret) + def initialize(parent_jar, key_generator) @parent_jar = parent_jar + secret = key_generator.generate_key('signed cookie') + ensure_secret_secure(secret) @verifier = ActiveSupport::MessageVerifier.new(secret) end @@ -323,7 +327,7 @@ module ActionDispatch if secret.blank? raise ArgumentError, "A secret is required to generate an " + "integrity hash for cookie session data. Use " + - "config.secret_token = \"some secret phrase of at " + + "config.secret_token_key = \"some secret phrase of at " + "least #{SECRET_MIN_LENGTH} characters\"" + "in config/initializers/secret_token.rb" end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 8340aab4d2..d8c78db8c0 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -1,4 +1,6 @@ require 'abstract_unit' +# FIXME remove DummyKeyGenerator and this require in 4.1 +require 'active_support/key_generator' class FlashTest < ActionController::TestCase class TestController < ActionController::Base @@ -217,7 +219,7 @@ end class FlashIntegrationTest < ActionDispatch::IntegrationTest SessionKey = '_myapp_session' - SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' + Generator = ActiveSupport::DummyKeyGenerator.new('b3c631c314c0bbca50c1b2843150fe33') class TestController < ActionController::Base add_flash_types :bar @@ -291,7 +293,7 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest # Overwrite get to send SessionSecret in env hash def get(path, parameters = nil, env = {}) - env["action_dispatch.secret_token"] ||= SessionSecret + env["action_dispatch.key_generator"] ||= Generator super end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 347b3b3b5a..fc0db8b91d 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -1,4 +1,6 @@ require 'abstract_unit' +# FIXME remove DummyKeyGenerator and this require in 4.1 +require 'active_support/key_generator' class CookiesTest < ActionController::TestCase class TestController < ActionController::Base @@ -146,7 +148,7 @@ class CookiesTest < ActionController::TestCase def setup super - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") @request.host = "www.nextangle.com" end @@ -329,29 +331,29 @@ class CookiesTest < ActionController::TestCase def test_raises_argument_error_if_missing_secret assert_raise(ArgumentError, nil.inspect) { - @request.env["action_dispatch.secret_token"] = nil + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new(nil) get :set_signed_cookie } assert_raise(ArgumentError, ''.inspect) { - @request.env["action_dispatch.secret_token"] = "" + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new("") get :set_signed_cookie } end def test_raises_argument_error_if_secret_is_probably_insecure assert_raise(ArgumentError, "password".inspect) { - @request.env["action_dispatch.secret_token"] = "password" + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new("password") get :set_signed_cookie } assert_raise(ArgumentError, "secret".inspect) { - @request.env["action_dispatch.secret_token"] = "secret" + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new("secret") get :set_signed_cookie } assert_raise(ArgumentError, "12345678901234567890123456789".inspect) { - @request.env["action_dispatch.secret_token"] = "12345678901234567890123456789" + @request.env["action_dispatch.key_generator"] = ActiveSupport::DummyKeyGenerator.new("12345678901234567890123456789") get :set_signed_cookie } end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 41fa036a92..1677dee524 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -1,9 +1,12 @@ require 'abstract_unit' require 'stringio' +# FIXME remove DummyKeyGenerator and this require in 4.1 +require 'active_support/key_generator' class CookieStoreTest < ActionDispatch::IntegrationTest SessionKey = '_myapp_session' SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' + Generator = ActiveSupport::DummyKeyGenerator.new(SessionSecret) Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, :digest => 'SHA1') SignedBar = Verifier.generate(:foo => "bar", :session_id => SecureRandom.hex(16)) @@ -330,7 +333,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest # Overwrite get to send SessionSecret in env hash def get(path, parameters = nil, env = {}) - env["action_dispatch.secret_token"] ||= SessionSecret + env["action_dispatch.key_generator"] ||= Generator super end diff --git a/activesupport/lib/active_support/key_generator.rb b/activesupport/lib/active_support/key_generator.rb index 04d170f801..8b49ad8414 100644 --- a/activesupport/lib/active_support/key_generator.rb +++ b/activesupport/lib/active_support/key_generator.rb @@ -20,4 +20,14 @@ module ActiveSupport OpenSSL::PKCS5.pbkdf2_hmac_sha1(@secret, salt, @iterations, key_size) end end + + class DummyKeyGenerator + def initialize(secret) + @secret = secret + end + + def generate_key(salt) + @secret + end + end end diff --git a/guides/code/getting_started/config/initializers/secret_token.rb b/guides/code/getting_started/config/initializers/secret_token.rb index f36ebdda18..8d52c11b69 100644 --- a/guides/code/getting_started/config/initializers/secret_token.rb +++ b/guides/code/getting_started/config/initializers/secret_token.rb @@ -6,4 +6,4 @@ # no regular words or you'll be exposed to dictionary attacks. # Make sure your secret key is kept private # if you're sharing your code publicly. -Blog::Application.config.secret_token = '685a9bf865b728c6549a191c90851c1b5ec41ecb60b9e94ad79dd3f824749798aa7b5e94431901960bee57809db0947b481570f7f13376b7ca190fa28099c459' +Blog::Application.config.secret_token_key = '685a9bf865b728c6549a191c90851c1b5ec41ecb60b9e94ad79dd3f824749798aa7b5e94431901960bee57809db0947b481570f7f13376b7ca190fa28099c459' diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index dd8d229e6a..d5d74ca3f5 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -219,7 +219,7 @@ Rails sets up (for the CookieStore) a secret key used for signing the session da # If you change this key, all old signed cookies will become invalid! # Make sure the secret is at least 30 characters and all random, # no regular words or you'll be exposed to dictionary attacks. -YourApp::Application.config.secret_token = '49d3f3de9ed86c74b94ad6bd0...' +YourApp::Application.config.secret_token_key = '49d3f3de9ed86c74b94ad6bd0...' ``` NOTE: Changing the secret when using the `CookieStore` will invalidate all existing sessions. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 0b4f183d61..cadc09ae33 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -113,7 +113,7 @@ These configuration methods are to be called on a `Rails::Railtie` object, such * `config.reload_classes_only_on_change` enables or disables reloading of classes only when tracked files change. By default tracks everything on autoload paths and is set to true. If `config.cache_classes` is true, this option is ignored. -* `config.secret_token` used for specifying a key which allows sessions for the application to be verified against a known secure key to prevent tampering. Applications get `config.secret_token` initialized to a random key in `config/initializers/secret_token.rb`. +* `config.secret_token_key` used for specifying a key which allows sessions for the application to be verified against a known secure key to prevent tampering. Applications get `config.secret_token_key` initialized to a random key in `config/initializers/secret_token.rb`. * `config.serve_static_assets` configures Rails itself to serve static assets. Defaults to true, but in the production environment is turned off as the server software (e.g. Nginx or Apache) used to run the application should serve static assets instead. Unlike the default setting set this to true when running (absolutely not recommended!) or testing your app in production mode using WEBrick. Otherwise you won´t be able use page caching and requests for files that exist regularly under the public directory will anyway hit your Rails app. diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 9ef001c7d0..f22025d35e 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -1,5 +1,7 @@ require 'fileutils' require 'active_support/queueing' +# FIXME remove DummyKeyGenerator and this require in 4.1 +require 'active_support/key_generator' require 'rails/engine' module Rails @@ -106,7 +108,11 @@ module Rails def key_generator # number of iterations selected based on consultation with the google security # team. Details at https://github.com/rails/rails/pull/6952#issuecomment-7661220 - @key_generator ||= ActiveSupport::KeyGenerator.new(config.secret_token, iterations: 1000) + @key_generator ||= if config.secret_token_key + ActiveSupport::KeyGenerator.new(config.secret_token_key, iterations: 1000) + else + ActiveSupport::DummyKeyGenerator.new(config.secret_token) + end end # Stores some of the Rails initial environment parameters which @@ -119,6 +125,7 @@ module Rails # * "action_dispatch.show_detailed_exceptions" => config.consider_all_requests_local, # * "action_dispatch.logger" => Rails.logger, # * "action_dispatch.backtrace_cleaner" => Rails.backtrace_cleaner + # * "action_dispatch.key_generator" => key_generator # # These parameters will be used by middlewares and engines to configure themselves # diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index cc21213f1c..b01b97aa67 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -10,12 +10,12 @@ module Rails :cache_classes, :cache_store, :consider_all_requests_local, :console, :eager_load, :exceptions_app, :file_watcher, :filter_parameters, :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, - :railties_order, :relative_url_root, :secret_token, + :railties_order, :relative_url_root, :secret_token_key, :serve_static_assets, :ssl_options, :static_cache_control, :session_options, :time_zone, :reload_classes_only_on_change, :queue, :queue_consumer, :beginning_of_week - attr_writer :log_level + attr_writer :secret_token, :log_level attr_reader :encoding def initialize(*) @@ -46,6 +46,8 @@ module Rails @queue = ActiveSupport::SynchronousQueue.new @queue_consumer = nil @eager_load = nil + @secret_token = nil + @secret_token_key = nil @assets = ActiveSupport::OrderedOptions.new @assets.enabled = false @@ -144,6 +146,10 @@ module Rails def whiny_nils=(*) ActiveSupport::Deprecation.warn "config.whiny_nils option is deprecated and no longer works" end + + def secret_token + @secret_token_key || @secret_token + end end end end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/secret_token.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/secret_token.rb.tt index 3c5611ca59..d96185ae2a 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/secret_token.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/secret_token.rb.tt @@ -7,6 +7,6 @@ # no regular words or you'll be exposed to dictionary attacks. # You can use `rake secret` to generate a secure secret key. -# Make sure your secret_token is kept private +# Make sure your secret_token_key is kept private # if you're sharing your code publicly. -<%= app_const %>.config.secret_token = '<%= app_secret %>' +<%= app_const %>.config.secret_token_key = '<%= app_secret %>' diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index c4c1100f19..daf9dd3505 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -225,9 +225,9 @@ module ApplicationTests assert_equal Pathname.new(app_path).join("somewhere"), Rails.public_path end - test "config.secret_token is sent in env" do + test "config.secret_token_key is sent in env" do make_basic_app do |app| - app.config.secret_token = 'b3c631c314c0bbca50c1b2843150fe33' + app.config.secret_token_key = 'b3c631c314c0bbca50c1b2843150fe33' app.config.session_store :disabled end @@ -242,6 +242,26 @@ module ApplicationTests assert_equal 'b3c631c314c0bbca50c1b2843150fe33', last_response.body end + test "Use key_generator when secret_token_key is set" do + make_basic_app do |app| + app.config.secret_token_key = 'b3c631c314c0bbca50c1b2843150fe33' + app.config.session_store :disabled + end + + class ::OmgController < ActionController::Base + def index + cookies.signed[:some_key] = "some_value" + render text: cookies[:some_key] + end + end + + get "/" + + secret = app.key_generator.generate_key('signed cookie') + verifier = ActiveSupport::MessageVerifier.new(secret) + assert_equal 'some_value', verifier.verify(last_response.body) + end + test "protect from forgery is the default in a new app" do make_basic_app diff --git a/railties/test/application/url_generation_test.rb b/railties/test/application/url_generation_test.rb index 2a48adae5c..fb83659b0c 100644 --- a/railties/test/application/url_generation_test.rb +++ b/railties/test/application/url_generation_test.rb @@ -14,7 +14,7 @@ module ApplicationTests require "action_controller/railtie" class MyApp < Rails::Application - config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" + config.secret_token_key = "3b7cd727ee24e8444053437c36cc66c4" config.session_store :cookie_store, key: "_myapp_session" config.active_support.deprecation = :log config.eager_load = false diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index e59488f97d..2c92f2ded5 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -119,7 +119,7 @@ module TestHelpers add_to_config <<-RUBY config.eager_load = false - config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" + config.secret_token_key = "3b7cd727ee24e8444053437c36cc66c4" config.session_store :cookie_store, key: "_myapp_session" config.active_support.deprecation = :log config.action_controller.allow_forgery_protection = false @@ -138,7 +138,7 @@ module TestHelpers app = Class.new(Rails::Application) app.config.eager_load = false - app.config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" + app.config.secret_token_key = "3b7cd727ee24e8444053437c36cc66c4" app.config.session_store :cookie_store, key: "_myapp_session" app.config.active_support.deprecation = :log |