From 5debcecdf106051453300c7002cdd5cef3c27cf0 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Thu, 19 Mar 2020 04:03:00 +0900 Subject: [PATCH] Support rolling deploys for cookie serialization/encryption changes (#37628) In a distributed configuration like rolling update, users may observe both old and new instances during deployment. Users may be served by a new instance and then by an old instance. That means when the server changes `cookies_serializer` from `:marshal` to `:hybrid` or the server changes `use_authenticated_cookie_encryption` from `false` to `true`, users may lose their sessions if they access the server during deployment. We added fallbacks to downgrade the cookie format when necessary during deployment, ensuring compatibility on both old and new instances. --- actionpack/CHANGELOG.md | 16 +++++ .../lib/action_dispatch/middleware/cookies.rb | 25 ++++++- actionpack/test/dispatch/cookies_test.rb | 71 +++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 44bf862ccf..6d68c1ad07 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,19 @@ +* Support rolling deploys for cookie serialization/encryption changes. + + In a distributed configuration like rolling update, users may observe + both old and new instances during deployment. Users may be served by a + new instance and then by an old instance. + + That means when the server changes `cookies_serializer` from `:marshal` + to `:hybrid` or the server changes `use_authenticated_cookie_encryption` + from `false` to `true`, users may lose their sessions if they access the + server during deployment. + + We added fallbacks to downgrade the cookie format when necessary during + deployment, ensuring compatibility on both old and new instances. + + *Masaki Hara* + * `ActionDispatch::Request.remote_ip` has ip address even when all sites are trusted. Before, if all `X-Forwarded-For` sites were trusted, the `remote_ip` would default to `127.0.0.1`. diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index b5f1158e8c..7e2246534e 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -266,6 +266,12 @@ def upgrade_legacy_hmac_aes_cbc_cookies? request.use_authenticated_cookie_encryption end + def prepare_upgrade_legacy_hmac_aes_cbc_cookies? + request.secret_key_base.present? && + request.authenticated_encrypted_cookie_salt.present? && + !request.use_authenticated_cookie_encryption + end + def encrypted_cookie_cipher request.encrypted_cookie_cipher || "aes-256-gcm" end @@ -510,6 +516,18 @@ def commit(name, options) end end + class MarshalWithJsonFallback # :nodoc: + def self.load(value) + Marshal.load(value) + rescue TypeError => e + ActiveSupport::JSON.decode(value) rescue raise e + end + + def self.dump(value) + Marshal.dump(value) + end + end + class JsonSerializer # :nodoc: def self.load(value) ActiveSupport::JSON.decode(value) @@ -557,7 +575,7 @@ def serializer serializer = request.cookies_serializer || :marshal case serializer when :marshal - Marshal + MarshalWithJsonFallback when :json, :hybrid JsonSerializer else @@ -627,6 +645,11 @@ def initialize(parent_jar) sign_secret = request.key_generator.generate_key(request.encrypted_signed_cookie_salt) @encryptor.rotate(secret, sign_secret, cipher: legacy_cipher, digest: digest, serializer: SERIALIZER) + elsif prepare_upgrade_legacy_hmac_aes_cbc_cookies? + future_cipher = encrypted_cookie_cipher + secret = request.key_generator.generate_key(request.authenticated_encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len(future_cipher)) + + @encryptor.rotate(secret, nil, cipher: future_cipher, serializer: SERIALIZER) end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 41697b1eab..2411e2467c 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -606,6 +606,24 @@ def test_signed_cookie_using_custom_serializer assert_equal "45 was dumped and loaded", cookies.signed[:user_id] end + def test_signed_cookie_using_marshal_serializer_can_read_from_json_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :marshal + + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) + + json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) + @request.headers["Cookie"] = "user_id=#{json_value}" + + get :get_signed_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + + assert_nil @response.cookies["user_id"] + end + def test_signed_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json @request.env["action_dispatch.cookies_serializer"] = :hybrid @@ -828,6 +846,59 @@ def test_rotating_signed_cookies_digest assert_equal 45, verifier.verify(@response.cookies["user_id"]) end + def test_legacy_hmac_aes_cbc_marshal_mode_falls_back_to_authenticated_encrypted_cookie + @request.env["action_dispatch.use_authenticated_cookie_encryption"] = nil + + key_generator = @request.env["action_dispatch.key_generator"] + aead_salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] + aead_secret = key_generator.generate_key(aead_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")) + aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: "aes-256-gcm", serializer: Marshal) + marshal_value = aead_encryptor.encrypt_and_sign("bar") + + @request.headers["Cookie"] = "foo=#{::Rack::Utils.escape marshal_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] + + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-cbc")) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + hmac_cbc_encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", serializer: Marshal) + + assert_equal "bar", hmac_cbc_encryptor.decrypt_and_verify(@response.cookies["foo"]) + end + + def test_legacy_hmac_aes_cbc_json_mode_falls_back_to_authenticated_encrypted_cookie + @request.env["action_dispatch.use_authenticated_cookie_encryption"] = nil + @request.env["action_dispatch.cookies_serializer"] = :json + + key_generator = @request.env["action_dispatch.key_generator"] + aead_salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] + aead_secret = key_generator.generate_key(aead_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")) + aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: "aes-256-gcm", serializer: JSON) + marshal_value = aead_encryptor.encrypt_and_sign("bar") + + @request.headers["Cookie"] = "foo=#{::Rack::Utils.escape marshal_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] + + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-cbc")) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + hmac_cbc_encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", serializer: JSON) + + assert_equal "bar", hmac_cbc_encryptor.decrypt_and_verify(@response.cookies["foo"]) + end + def test_legacy_hmac_aes_cbc_encrypted_marshal_cookie_is_upgraded_to_authenticated_encrypted_cookie key_generator = @request.env["action_dispatch.key_generator"] encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] -- GitLab