Merge pull request rails#29599 from assain/add_meta_data_to_message_encryptor

kaspth · web-flow · commit 1f7f872ac6c8 · 2017-07-19T15:17:09.000+02:00
Add purpose and expiry to messages encrypted using Message Encryptor
diff --git a/activesupport/lib/active_support/message_encryptor.rb b/activesupport/lib/active_support/message_encryptor.rb
@@ -4,6 +4,7 @@
 require "base64"
 require_relative "core_ext/array/extract_options"
 require_relative "message_verifier"
+require_relative "messages/metadata"
 
 module ActiveSupport
   # MessageEncryptor is a simple way to encrypt values which get stored
@@ -87,14 +88,15 @@ 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)
-      verifier.generate(_encrypt(value))
+    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))
     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(value)
-      _decrypt(verifier.verify(value))
+    def decrypt_and_verify(data, purpose: nil)
+      Messages::Metadata.verify(_decrypt(verifier.verify(data)), purpose)
     end
 
     # Given a cipher, returns the key length of the cipher to help generate the key of desired size
@@ -103,7 +105,6 @@ def self.key_len(cipher = default_cipher)
     end
 
     private
-
       def _encrypt(value)
         cipher = new_cipher
         cipher.encrypt
diff --git a/activesupport/lib/active_support/messages/metadata.rb b/activesupport/lib/active_support/messages/metadata.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+require "time"
+
+module ActiveSupport
+  module Messages #:nodoc:
+    class Metadata #:nodoc:
+      def initialize(expires_at, purpose)
+        @expires_at, @purpose = expires_at, 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.to_s } }
+          else
+            message
+          end
+        end
+
+        def verify(message, purpose)
+          metadata = extract_metadata(message)
+
+          if metadata.nil?
+            message if purpose.nil?
+          elsif metadata.match?(purpose.to_s) && metadata.fresh?
+            message["value"]
+          end
+        end
+
+        private
+          def pick_expiry(expires_at, expires_in)
+            if expires_at
+              expires_at.utc.iso8601(3)
+            elsif expires_in
+              expires_in.from_now.utc.iso8601(3)
+            end
+          end
+
+          def extract_metadata(message)
+            if message.is_a?(Hash) && message.key?("_rails")
+              new(message["_rails"]["exp"], message["_rails"]["pur"])
+            end
+          end
+      end
+
+      def match?(purpose)
+        @purpose == purpose
+      end
+
+      def fresh?
+        @expires_at.nil? || Time.now.utc < Time.iso8601(@expires_at)
+      end
+    end
+  end
+end
diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb
@@ -4,6 +4,7 @@
 require "openssl"
 require "active_support/time"
 require "active_support/json"
+require_relative "metadata/shared_metadata_tests"
 
 class MessageEncryptorTest < ActiveSupport::TestCase
   class JSONSerializer
@@ -106,8 +107,15 @@ def test_messing_with_aead_values_causes_failures
     assert_aead_not_decrypted(encryptor, [text, iv, auth_tag[0..-2]] * "--")
   end
 
-  private
+  def test_backwards_compatibility_decrypt_previously_encrypted_messages_without_metadata
+    secret = "\xB7\xF0\xBCW\xB1\x18`\xAB\xF0\x81\x10\xA4$\xF44\xEC\xA1\xDC\xC1\xDDD\xAF\xA9\xB8\x14\xCD\x18\x9A\x99 \x80)"
+    encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm")
+    encrypted_message = "9cVnFs2O3lL9SPvIJuxBOLS51nDiBMw=--YNI5HAfHEmZ7VDpl--ddFJ6tXA0iH+XGcCgMINYQ=="
+
+    assert_equal "Ruby on Rails", encryptor.decrypt_and_verify(encrypted_message)
+  end
 
+  private
     def assert_aead_not_decrypted(encryptor, value)
       assert_raise(ActiveSupport::MessageEncryptor::InvalidMessage) do
         encryptor.decrypt_and_verify(value)
@@ -132,3 +140,37 @@ def munge(base64_string)
       ::Base64.strict_encode64(bits)
     end
 end
+
+class MessageEncryptorMetadataTest < ActiveSupport::TestCase
+  include SharedMessageMetadataTests
+
+  setup do
+    @secret    = SecureRandom.random_bytes(32)
+    @encryptor = ActiveSupport::MessageEncryptor.new(@secret, encryptor_options)
+  end
+
+  private
+    def generate(message, **options)
+      @encryptor.encrypt_and_sign(message, options)
+    end
+
+    def parse(data, **options)
+      @encryptor.decrypt_and_verify(data, options)
+    end
+
+    def encryptor_options; end
+end
+
+class MessageEncryptorMetadataMarshalTest < MessageEncryptorMetadataTest
+  private
+    def encryptor_options
+      { serializer: Marshal }
+    end
+end
+
+class MessageEncryptorMetadataJSONTest < MessageEncryptorMetadataTest
+  private
+    def encryptor_options
+      { serializer: MessageEncryptorTest::JSONSerializer.new }
+    end
+end
diff --git a/activesupport/test/metadata/shared_metadata_tests.rb b/activesupport/test/metadata/shared_metadata_tests.rb
@@ -0,0 +1,88 @@
+# 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 test_encryption_and_decryption_with_same_purpose
+    assert_equal @message, parse(generate(@message, purpose: "checkout"), purpose: "checkout")
+    assert_equal @message, parse(generate(@message))
+
+    string_message = "address: #23, main street"
+    assert_equal string_message, parse(generate(string_message, purpose: "shipping"), purpose: "shipping")
+
+    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")
+  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: "")
+  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)
+  end
+
+  def test_passing_expires_at_sets_expiration_date
+    encrypted_message = generate(@message, expires_at: 1.hour.from_now)
+
+    travel 59.minutes
+    assert_equal @message, 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)
+
+    travel 1.hour
+    assert_equal @message, parse(encrypted_message)
+
+    travel 1.hour + 1.second
+    assert_nil parse(encrypted_message)
+  end
+
+  def test_passing_expires_in_less_than_a_second_is_not_expired
+    freeze_time do
+      encrypted_message = generate(@message, expires_in: 1.second)
+
+      travel 0.5.seconds
+      assert_equal @message, parse(encrypted_message)
+
+      travel 1.second
+      assert_nil parse(encrypted_message)
+    end
+  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)
+
+    travel 1.year
+    assert_equal @message, 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")
+
+    travel 100.years
+    assert_equal @message, parse(payment_related_message, purpose: "payment")
+  end
+end
