Merge pull request #30171 from kaspth/verifier-encryptor-null-serializer-metadata

Perform self-serialization once metadata is involved.

activesupport/lib/active_support/message_encryptor.rb

@@ -121,14 +121,13 @@ def initialize(secret, *signature_key_or_options)
     # Encrypt and sign a message. We need to sign the message in order to avoid
     # padding attacks. Reference: http://www.limited-entropy.com/padding-oracle-attacks.
     def encrypt_and_sign(value, expires_at: nil, expires_in: nil, purpose: nil)
-      data = Messages::Metadata.wrap(value, expires_at: expires_at, expires_in: expires_in, purpose: purpose)
-      verifier.generate(_encrypt(data))
+      verifier.generate(_encrypt(value, expires_at: expires_at, expires_in: expires_in, purpose: purpose))
     end
 
     # Decrypt and verify a message. We need to verify the message in order to
     # avoid padding attacks. Reference: http://www.limited-entropy.com/padding-oracle-attacks.
     def decrypt_and_verify(data, purpose: nil)
-      Messages::Metadata.verify(_decrypt(verifier.verify(data)), purpose)
+      _decrypt(verifier.verify(data), purpose)
     end
 
     # Given a cipher, returns the key length of the cipher to help generate the key of desired size
@@ -137,7 +136,7 @@ def self.key_len(cipher = default_cipher)
     end
 
     private
-      def _encrypt(value)
+      def _encrypt(value, **metadata_options)
         cipher = new_cipher
         cipher.encrypt
         cipher.key = @secret
@@ -146,7 +145,7 @@ def _encrypt(value)
         iv = cipher.random_iv
         cipher.auth_data = "" if aead_mode?
 
-        encrypted_data = cipher.update(@serializer.dump(value))
+        encrypted_data = cipher.update(Messages::Metadata.wrap(@serializer.dump(value), metadata_options))
         encrypted_data << cipher.final
 
         blob = "#{::Base64.strict_encode64 encrypted_data}--#{::Base64.strict_encode64 iv}"
@@ -154,7 +153,7 @@ def _encrypt(value)
         blob
       end
 
-      def _decrypt(encrypted_message)
+      def _decrypt(encrypted_message, purpose)
         cipher = new_cipher
         encrypted_data, iv, auth_tag = encrypted_message.split("--".freeze).map { |v| ::Base64.strict_decode64(v) }
 
@@ -174,7 +173,8 @@ def _decrypt(encrypted_message)
         decrypted_data = cipher.update(encrypted_data)
         decrypted_data << cipher.final
 
-        @serializer.load(decrypted_data)
+        message = Messages::Metadata.verify(decrypted_data, purpose)
+        @serializer.load(message) if message
       rescue OpenSSLCipherError, TypeError, ArgumentError
         raise InvalidMessage
       end

activesupport/lib/active_support/message_verifier.rb

@@ -124,7 +124,8 @@ def verified(signed_message, purpose: nil)
       if valid_message?(signed_message)
         begin
           data = signed_message.split("--".freeze)[0]
-          Messages::Metadata.verify(@serializer.load(decode(data)), purpose)
+          message = Messages::Metadata.verify(decode(data), purpose)
+          @serializer.load(message) if message
         rescue ArgumentError => argument_error
           return if argument_error.message.include?("invalid base64")
           raise
@@ -156,7 +157,7 @@ def verify(signed_message, purpose: nil)
     #   verifier = ActiveSupport::MessageVerifier.new 's3Krit'
     #   verifier.generate 'a private message' # => "BAhJIhRwcml2YXRlLW1lc3NhZ2UGOgZFVA==--e2d724331ebdee96a10fb99b089508d1c72bd772"
     def generate(value, expires_at: nil, expires_in: nil, purpose: nil)
-      data = encode(@serializer.dump(Messages::Metadata.wrap(value, expires_at: expires_at, expires_in: expires_in, purpose: purpose)))
+      data = encode(Messages::Metadata.wrap(@serializer.dump(value), expires_at: expires_at, expires_in: expires_in, purpose: purpose))
       "#{data}--#{generate_digest(data)}"
     end
 

activesupport/lib/active_support/messages/metadata.rb

@@ -5,27 +5,25 @@
 module ActiveSupport
   module Messages #:nodoc:
     class Metadata #:nodoc:
-      def initialize(expires_at, purpose)
-        @expires_at, @purpose = expires_at, purpose.to_s
+      def initialize(message, expires_at = nil, purpose = nil)
+        @message, @expires_at, @purpose = message, expires_at, purpose
+      end
+
+      def as_json(options = {})
+        { _rails: { message: @message, exp: @expires_at, pur: @purpose } }
       end
 
       class << self
         def wrap(message, expires_at: nil, expires_in: nil, purpose: nil)
           if expires_at || expires_in || purpose
-            { "value" => message, "_rails" => { "exp" => pick_expiry(expires_at, expires_in), "pur" => purpose } }
+            JSON.encode new(encode(message), pick_expiry(expires_at, expires_in), purpose)
           else
             message
           end
         end
 
         def verify(message, purpose)
-          metadata = extract_metadata(message)
-
-          if metadata.nil?
-            message if purpose.nil?
-          elsif metadata.match?(purpose) && metadata.fresh?
-            message["value"]
-          end
+          extract_metadata(message).verify(purpose)
         end
 
         private
@@ -38,19 +36,36 @@ def pick_expiry(expires_at, expires_in)
           end
 
           def extract_metadata(message)
-            if message.is_a?(Hash) && message.key?("_rails")
-              new(message["_rails"]["exp"], message["_rails"]["pur"])
+            data = JSON.decode(message) rescue nil
+
+            if data.is_a?(Hash) && data.key?("_rails")
+              new(decode(data["_rails"]["message"]), data["_rails"]["exp"], data["_rails"]["pur"])
+            else
+              new(message)
             end
           end
-      end
 
-      def match?(purpose)
-        @purpose == purpose.to_s
+          def encode(message)
+            ::Base64.strict_encode64(message)
+          end
+
+          def decode(message)
+            ::Base64.strict_decode64(message)
+          end
       end
 
-      def fresh?
-        @expires_at.nil? || Time.now.utc < Time.iso8601(@expires_at)
+      def verify(purpose)
+        @message if match?(purpose) && fresh?
       end
+
+      private
+        def match?(purpose)
+          @purpose.to_s == purpose.to_s
+        end
+
+        def fresh?
+          @expires_at.nil? || Time.now.utc < Time.iso8601(@expires_at)
+        end
     end
   end
 end

activesupport/test/message_verifier_test.rb

@@ -101,12 +101,12 @@ class MessageVerifierMetadataTest < ActiveSupport::TestCase
 
   def test_verify_raises_when_purpose_differs
     assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do
-      @verifier.verify(@verifier.generate(@message, purpose: "payment"), purpose: "shipping")
+      @verifier.verify(generate(data, purpose: "payment"), purpose: "shipping")
     end
   end
 
   def test_verify_raises_when_expired
-    signed_message = @verifier.generate(@message, expires_in: 1.month)
+    signed_message = generate(data, expires_in: 1.month)
 
     travel 2.months
     assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do
@@ -141,3 +141,18 @@ def verifier_options
       { serializer: MessageVerifierTest::JSONSerializer.new }
     end
 end
+
+class MessageEncryptorMetadataNullSerializerTest < MessageVerifierMetadataTest
+  private
+    def data
+      "string message"
+    end
+
+    def null_serializing?
+      true
+    end
+
+    def verifier_options
+      { serializer: ActiveSupport::MessageEncryptor::NullSerializer }
+    end
+end

activesupport/test/metadata/shared_metadata_tests.rb

 # frozen_string_literal: true
 
 module SharedMessageMetadataTests
-  def setup
-    @message = { "credit_card_no" => "5012-6784-9087-5678", "card_holder" => { "name" => "Donald" } }
-
-    super
-  end
-
   def teardown
     travel_back
-
     super
   end
 
+  def null_serializing?
+    false
+  end
+
   def test_encryption_and_decryption_with_same_purpose
-    assert_equal @message, parse(generate(@message, purpose: "checkout"), purpose: "checkout")
-    assert_equal @message, parse(generate(@message))
+    assert_equal data, parse(generate(data, purpose: "checkout"), purpose: "checkout")
+    assert_equal data, parse(generate(data))
 
     string_message = "address: #23, main street"
     assert_equal string_message, parse(generate(string_message, purpose: "shipping"), purpose: "shipping")
+  end
 
-    array_message = ["credit_card_no: 5012-6748-9087-5678", { "card_holder" => "Donald", "issued_on" => Time.local(2017) }, 12345]
-    assert_equal array_message, parse(generate(array_message, purpose: "registration"), purpose: "registration")
+  def test_verifies_array_when_purpose_matches
+    unless null_serializing?
+      data = [ "credit_card_no: 5012-6748-9087-5678", { "card_holder" => "Donald", "issued_on" => Time.local(2017) }, 12345 ]
+      assert_equal data, parse(generate(data, purpose: :registration), purpose: :registration)
+    end
   end
 
   def test_encryption_and_decryption_with_different_purposes_returns_nil
-    assert_nil parse(generate(@message, purpose: "payment"), purpose: "sign up")
-    assert_nil parse(generate(@message, purpose: "payment"))
-    assert_nil parse(generate(@message), purpose: "sign up")
-    assert_nil parse(generate(@message), purpose: "")
+    assert_nil parse(generate(data, purpose: "payment"), purpose: "sign up")
+    assert_nil parse(generate(data, purpose: "payment"))
+    assert_nil parse(generate(data), purpose: "sign up")
   end
 
   def test_purpose_using_symbols
-    assert_equal @message, parse(generate(@message, purpose: :checkout), purpose: :checkout)
-    assert_equal @message, parse(generate(@message, purpose: :checkout), purpose: "checkout")
-    assert_equal @message, parse(generate(@message, purpose: "checkout"), purpose: :checkout)
+    assert_equal data, parse(generate(data, purpose: :checkout), purpose: :checkout)
+    assert_equal data, parse(generate(data, purpose: :checkout), purpose: "checkout")
+    assert_equal data, parse(generate(data, purpose: "checkout"), purpose: :checkout)
   end
 
   def test_passing_expires_at_sets_expiration_date
-    encrypted_message = generate(@message, expires_at: 1.hour.from_now)
+    encrypted_message = generate(data, expires_at: 1.hour.from_now)
 
     travel 59.minutes
-    assert_equal @message, parse(encrypted_message)
+    assert_equal data, parse(encrypted_message)
 
     travel 2.minutes
     assert_nil parse(encrypted_message)
   end
 
   def test_set_relative_expiration_date_by_passing_expires_in
-    encrypted_message = generate(@message, expires_in: 2.hours)
+    encrypted_message = generate(data, expires_in: 2.hours)
 
     travel 1.hour
-    assert_equal @message, parse(encrypted_message)
+    assert_equal data, parse(encrypted_message)
 
     travel 1.hour + 1.second
     assert_nil parse(encrypted_message)
@@ -59,10 +59,10 @@ def test_set_relative_expiration_date_by_passing_expires_in
 
   def test_passing_expires_in_less_than_a_second_is_not_expired
     freeze_time do
-      encrypted_message = generate(@message, expires_in: 1.second)
+      encrypted_message = generate(data, expires_in: 1.second)
 
       travel 0.5.seconds
-      assert_equal @message, parse(encrypted_message)
+      assert_equal data, parse(encrypted_message)
 
       travel 1.second
       assert_nil parse(encrypted_message)
@@ -70,19 +70,24 @@ def test_passing_expires_in_less_than_a_second_is_not_expired
   end
 
   def test_favor_expires_at_over_expires_in
-    payment_related_message = generate(@message, purpose: "payment", expires_at: 2.year.from_now, expires_in: 1.second)
+    payment_related_message = generate(data, purpose: "payment", expires_at: 2.year.from_now, expires_in: 1.second)
 
     travel 1.year
-    assert_equal @message, parse(payment_related_message, purpose: :payment)
+    assert_equal data, parse(payment_related_message, purpose: :payment)
 
     travel 1.year + 1.day
     assert_nil parse(payment_related_message, purpose: "payment")
   end
 
   def test_skip_expires_at_and_expires_in_to_disable_expiration_check
-    payment_related_message = generate(@message, purpose: "payment")
+    payment_related_message = generate(data, purpose: "payment")
 
     travel 100.years
-    assert_equal @message, parse(payment_related_message, purpose: "payment")
+    assert_equal data, parse(payment_related_message, purpose: "payment")
   end
+
+  private
+    def data
+      { "credit_card_no" => "5012-6784-9087-5678", "card_holder" => { "name" => "Donald" } }
+    end
 end