Merge pull request #29716 from mikeycgto/active-support-key-rotator

Add Key Rotation to MessageEncryptor and MessageVerifier and simplify the Cookies middleware

actionpack/CHANGELOG.md

@@ -1,3 +1,12 @@
+*   Simplify cookies middleware with key rotation support
+
+    Use the `rotate` method for both `MessageEncryptor` and
+    `MessageVerifier` to add key rotation support for encrypted and
+    signed cookies. This also helps simplify support for legacy cookie
+    security.
+
+    *Michael J Coyne*
+
 *  Use Capybara registered `:puma` server config.
 
     The Capybara registered `:puma` server ensures the puma server is run in process so

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -49,6 +49,18 @@ def authenticated_encrypted_cookie_salt
       get_header Cookies::AUTHENTICATED_ENCRYPTED_COOKIE_SALT
     end
 
+    def use_authenticated_cookie_encryption
+      get_header Cookies::USE_AUTHENTICATED_COOKIE_ENCRYPTION
+    end
+
+    def encrypted_cookie_cipher
+      get_header Cookies::ENCRYPTED_COOKIE_CIPHER
+    end
+
+    def signed_cookie_digest
+      get_header Cookies::SIGNED_COOKIE_DIGEST
+    end
+
     def secret_token
       get_header Cookies::SECRET_TOKEN
     end
@@ -64,6 +76,11 @@ def cookies_serializer
     def cookies_digest
       get_header Cookies::COOKIES_DIGEST
     end
+
+    def cookies_rotations
+      get_header Cookies::COOKIES_ROTATIONS
+    end
+
     # :startdoc:
   end
 
@@ -157,10 +174,14 @@ class Cookies
     ENCRYPTED_COOKIE_SALT = "action_dispatch.encrypted_cookie_salt".freeze
     ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt".freeze
     AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "action_dispatch.authenticated_encrypted_cookie_salt".freeze
+    USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption".freeze
+    ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher".freeze
+    SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest".freeze
     SECRET_TOKEN = "action_dispatch.secret_token".freeze
     SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze
     COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze
     COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze
+    COOKIES_ROTATIONS = "action_dispatch.cookies_rotations".freeze
 
     # Cookies can typically store 4096 bytes.
     MAX_COOKIE_SIZE = 4096
@@ -201,12 +222,7 @@ def permanent
       #
       #   cookies.signed[:discount] # => 45
       def signed
-        @signed ||=
-          if upgrade_legacy_signed_cookies?
-            UpgradeLegacySignedCookieJar.new(self)
-          else
-            SignedCookieJar.new(self)
-          end
+        @signed ||= SignedKeyRotatingCookieJar.new(self)
       end
 
       # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read.
@@ -223,18 +239,11 @@ def signed
       # Example:
       #
       #   cookies.encrypted[:discount] = 45
-      #   # => Set-Cookie: discount=ZS9ZZ1R4cG1pcUJ1bm80anhQang3dz09LS1mbDZDSU5scGdOT3ltQ2dTdlhSdWpRPT0%3D--ab54663c9f4e3bc340c790d6d2b71e92f5b60315; path=/
+      #   # => Set-Cookie: discount=DIQ7fw==--K3n//8vvnSbGq9dA--7Xh91HfLpwzbj1czhBiwOg==; path=/
       #
       #   cookies.encrypted[:discount] # => 45
       def encrypted
-        @encrypted ||=
-          if upgrade_legacy_signed_cookies?
-            UpgradeLegacyEncryptedCookieJar.new(self)
-          elsif upgrade_legacy_hmac_aes_cbc_cookies?
-            UpgradeLegacyHmacAesCbcCookieJar.new(self)
-          else
-            EncryptedCookieJar.new(self)
-          end
+        @encrypted ||= EncryptedKeyRotatingCookieJar.new(self)
       end
 
       # Returns the +signed+ or +encrypted+ jar, preferring +encrypted+ if +secret_key_base+ is set.
@@ -256,33 +265,17 @@ def upgrade_legacy_signed_cookies?
 
         def upgrade_legacy_hmac_aes_cbc_cookies?
           request.secret_key_base.present?                       &&
-            request.authenticated_encrypted_cookie_salt.present? &&
             request.encrypted_signed_cookie_salt.present?        &&
-            request.encrypted_cookie_salt.present?
+            request.encrypted_cookie_salt.present?               &&
+            request.use_authenticated_cookie_encryption
         end
-    end
-
-    # Passing the ActiveSupport::MessageEncryptor::NullSerializer downstream
-    # to the Message{Encryptor,Verifier} allows us to handle the
-    # (de)serialization step within the cookie jar, which gives us the
-    # opportunity to detect and migrate legacy cookies.
-    module VerifyAndUpgradeLegacySignedMessage # :nodoc:
-      def initialize(*args)
-        super
-        @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, serializer: ActiveSupport::MessageEncryptor::NullSerializer)
-      end
 
-      def verify_and_upgrade_legacy_signed_message(name, signed_message)
-        deserialize(name, @legacy_verifier.verify(signed_message)).tap do |value|
-          self[name] = { value: value }
+        def encrypted_cookie_cipher
+          request.encrypted_cookie_cipher || "aes-256-gcm"
         end
-      rescue ActiveSupport::MessageVerifier::InvalidSignature
-        nil
-      end
 
-      private
-        def parse(name, signed_message)
-          super || verify_and_upgrade_legacy_signed_message(name, signed_message)
+        def signed_cookie_digest
+          request.signed_cookie_digest || "SHA1"
         end
     end
 
@@ -524,6 +517,7 @@ def self.dump(value)
 
     module SerializedCookieJars # :nodoc:
       MARSHAL_SIGNATURE = "\x04\x08".freeze
+      SERIALIZER = ActiveSupport::MessageEncryptor::NullSerializer
 
       protected
         def needs_migration?(value)
@@ -534,12 +528,16 @@ def serialize(value)
           serializer.dump(value)
         end
 
-        def deserialize(name, value)
+        def deserialize(name)
+          rotate = false
+          value  = yield -> { rotate = true }
+
           if value
-            if needs_migration?(value)
-              Marshal.load(value).tap do |v|
-                self[name] = { value: v }
-              end
+            case
+            when needs_migration?(value)
+              self[name] = Marshal.load(value)
+            when rotate
+              self[name] = serializer.load(value)
             else
               serializer.load(value)
             end
@@ -561,24 +559,31 @@ def serializer
         def digest
           request.cookies_digest || "SHA1"
         end
-
-        def key_generator
-          request.key_generator
-        end
     end
 
-    class SignedCookieJar < AbstractCookieJar # :nodoc:
+    class SignedKeyRotatingCookieJar < AbstractCookieJar # :nodoc:
       include SerializedCookieJars
 
       def initialize(parent_jar)
         super
-        secret = key_generator.generate_key(request.signed_cookie_salt)
-        @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer)
+
+        secret = request.key_generator.generate_key(request.signed_cookie_salt)
+        @verifier = ActiveSupport::MessageVerifier.new(secret, digest: signed_cookie_digest, serializer: SERIALIZER)
+
+        request.cookies_rotations.signed.each do |rotation_options|
+          @verifier.rotate serializer: SERIALIZER, **rotation_options
+        end
+
+        if upgrade_legacy_signed_cookies?
+          @verifier.rotate raw_key: request.secret_token, serializer: SERIALIZER
+        end
       end
 
       private
         def parse(name, signed_message)
-          deserialize name, @verifier.verified(signed_message)
+          deserialize(name) do |rotate|
+            @verifier.verified(signed_message, on_rotation: rotate)
+          end
         end
 
         def commit(options)
@@ -588,77 +593,54 @@ def commit(options)
         end
     end
 
-    # UpgradeLegacySignedCookieJar is used instead of SignedCookieJar if
-    # secrets.secret_token and secret_key_base are both set. It reads
-    # legacy cookies signed with the old dummy key generator and signs and
-    # re-saves them using the new key generator to provide a smooth upgrade path.
-    class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc:
-      include VerifyAndUpgradeLegacySignedMessage
-    end
-
-    class EncryptedCookieJar < AbstractCookieJar # :nodoc:
+    class EncryptedKeyRotatingCookieJar < AbstractCookieJar # :nodoc:
       include SerializedCookieJars
 
       def initialize(parent_jar)
         super
 
-        if ActiveSupport::LegacyKeyGenerator === key_generator
-          raise "You didn't set secret_key_base, which is required for this cookie jar. " \
-            "Read the upgrade documentation to learn more about this new config option."
+        key_len = ActiveSupport::MessageEncryptor.key_len(encrypted_cookie_cipher)
+        secret = request.key_generator.generate_key(request.authenticated_encrypted_cookie_salt, key_len)
+        @encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: encrypted_cookie_cipher, serializer: SERIALIZER)
+
+        request.cookies_rotations.encrypted.each do |rotation_options|
+          @encryptor.rotate serializer: SERIALIZER, **rotation_options
         end
 
-        cipher = "aes-256-gcm"
-        key_len = ActiveSupport::MessageEncryptor.key_len(cipher)
-        secret = key_generator.generate_key(request.authenticated_encrypted_cookie_salt || "")[0, key_len]
+        if upgrade_legacy_hmac_aes_cbc_cookies?
+          @encryptor.rotate \
+            key_generator: request.key_generator, salt: request.encrypted_cookie_salt, signed_salt: request.encrypted_signed_cookie_salt,
+            cipher: "aes-256-cbc", digest: digest, serializer: SERIALIZER
+        end
 
-        @encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: ActiveSupport::MessageEncryptor::NullSerializer)
+        if upgrade_legacy_signed_cookies?
+          @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, digest: digest, serializer: SERIALIZER)
+        end
       end
 
       private
         def parse(name, encrypted_message)
-          deserialize name, @encryptor.decrypt_and_verify(encrypted_message)
-        rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage
-          nil
+          deserialize(name) do |rotate|
+            @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate)
+          end
+        rescue ActiveSupport::MessageEncryptor::InvalidMessage, ActiveSupport::MessageVerifier::InvalidSignature
+          parse_legacy_signed_message(name, encrypted_message)
         end
 
         def commit(options)
           options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), expiry_options(options))
 
           raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE
         end
-    end
 
-    # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore
-    # instead of EncryptedCookieJar if secrets.secret_token and secret_key_base
-    # are both set. It reads legacy cookies signed with the old dummy key generator and
-    # encrypts and re-saves them using the new key generator to provide a smooth upgrade path.
-    class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc:
-      include VerifyAndUpgradeLegacySignedMessage
-    end
+        def parse_legacy_signed_message(name, legacy_signed_message)
+          if defined?(@legacy_verifier)
+            deserialize(name) do |rotate|
+              rotate.call
 
-    # UpgradeLegacyHmacAesCbcCookieJar is used by ActionDispatch::Session::CookieStore
-    # to upgrade cookies encrypted with AES-256-CBC with HMAC to AES-256-GCM
-    class UpgradeLegacyHmacAesCbcCookieJar < EncryptedCookieJar
-      def initialize(parent_jar)
-        super
-
-        secret = key_generator.generate_key(request.encrypted_cookie_salt || "")[0, ActiveSupport::MessageEncryptor.key_len]
-        sign_secret = key_generator.generate_key(request.encrypted_signed_cookie_salt || "")
-
-        @legacy_encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer)
-      end
-
-      def decrypt_and_verify_legacy_encrypted_message(name, signed_message)
-        deserialize(name, @legacy_encryptor.decrypt_and_verify(signed_message)).tap do |value|
-          self[name] = { value: value }
-        end
-      rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage
-        nil
-      end
-
-      private
-        def parse(name, signed_message)
-          super || decrypt_and_verify_legacy_encrypted_message(name, signed_message)
+              @legacy_verifier.verified(legacy_signed_message)
+            end
+          end
         end
     end
 

actionpack/lib/action_dispatch/railtie.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "action_dispatch"
+require "active_support/messages/rotation_configuration"
 
 module ActionDispatch
   class Railtie < Rails::Railtie # :nodoc:
@@ -18,6 +19,7 @@ class Railtie < Rails::Railtie # :nodoc:
     config.action_dispatch.signed_cookie_salt = "signed cookie"
     config.action_dispatch.encrypted_cookie_salt = "encrypted cookie"
     config.action_dispatch.encrypted_signed_cookie_salt = "signed encrypted cookie"
+    config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie"
     config.action_dispatch.use_authenticated_cookie_encryption = false
     config.action_dispatch.perform_deep_munge = true
 
@@ -27,6 +29,8 @@ class Railtie < Rails::Railtie # :nodoc:
       "X-Content-Type-Options" => "nosniff"
     }
 
+    config.action_dispatch.cookies_rotations = ActiveSupport::Messages::RotationConfiguration.new
+
     config.eager_load_namespaces << ActionDispatch
 
     initializer "action_dispatch.configure" do |app|
@@ -39,8 +43,6 @@ class Railtie < Rails::Railtie # :nodoc:
       ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses)
       ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates)
 
-      config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie" if config.action_dispatch.use_authenticated_cookie_encryption
-
       config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil?
       ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie
 

actionpack/test/controller/flash_test.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "abstract_unit"
-require "active_support/key_generator"
+require "active_support/messages/rotation_configuration"
 
 class FlashTest < ActionController::TestCase
   class TestController < ActionController::Base
@@ -243,6 +243,7 @@ def test_does_not_add_flash_type_to_parent_class
 class FlashIntegrationTest < ActionDispatch::IntegrationTest
   SessionKey = "_myapp_session"
   Generator  = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33")
+  Rotations  = ActiveSupport::Messages::RotationConfiguration.new
 
   class TestController < ActionController::Base
     add_flash_types :bar
@@ -348,6 +349,7 @@ def get(path, *args)
       args[0] ||= {}
       args[0][:env] ||= {}
       args[0][:env]["action_dispatch.key_generator"] ||= Generator
+      args[0][:env]["action_dispatch.cookies_rotations"] = Rotations
       super(path, *args)
     end
 

actionpack/test/controller/request_forgery_protection_test.rb

@@ -2,6 +2,7 @@
 
 require "abstract_unit"
 require "active_support/log_subscriber/test_helper"
+require "active_support/messages/rotation_configuration"
 
 # common controller actions
 module RequestForgeryProtectionActions
@@ -630,13 +631,14 @@ class RequestForgeryProtectionControllerUsingResetSessionTest < ActionController
 
 class RequestForgeryProtectionControllerUsingNullSessionTest < ActionController::TestCase
   class NullSessionDummyKeyGenerator
-    def generate_key(secret)
+    def generate_key(secret, length = nil)
       "03312270731a2ed0d11ed091c2338a06"
     end
   end
 
   def setup
     @request.env[ActionDispatch::Cookies::GENERATOR_KEY] = NullSessionDummyKeyGenerator.new
+    @request.env[ActionDispatch::Cookies::COOKIES_ROTATIONS] = ActiveSupport::Messages::RotationConfiguration.new
   end
 
   test "should allow to set signed cookies" do