Deprecated ActiveSupport::MessageEncryptor#encrypt and decrypt.

activesupport/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Deprecated ActiveSupport::MessageEncryptor#encrypt and decrypt. *José Valim*
+
 *   ActiveSupport::Notifications.subscribed provides subscriptions to events while a block runs. *fxn*
 
 *   Module#qualified_const_(defined?|get|set) are analogous to the corresponding methods

activesupport/lib/active_support/message_encryptor.rb

@@ -25,6 +25,32 @@ module ActiveSupport
     end
 
     def encrypt(value)
+      ActiveSupport::Deprecation.warn "MessageEncryptor#encrypt is deprecated as it is not safe without a signature. " \
+        "Please use MessageEncryptor#encrypt_and_sign instead."
+      _encrypt(value)
+    end
+
+    def decrypt(value)
+      ActiveSupport::Deprecation.warn "MessageEncryptor#decrypt is deprecated as it is not safe without a signature. " \
+        "Please use MessageEncryptor#decrypt_and_verify instead."
+      _decrypt(value)
+    end
+
+    # 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))
+    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))
+    end
+
+    private
+
+    def _encrypt(value)
       cipher = new_cipher
       # Rely on OpenSSL for the initialization vector
       iv = cipher.random_iv
@@ -39,7 +65,7 @@ module ActiveSupport
       [encrypted_data, iv].map {|v| ActiveSupport::Base64.encode64s(v)}.join("--")
     end
 
-    def decrypt(encrypted_message)
+    def _decrypt(encrypted_message)
       cipher = new_cipher
       encrypted_data, iv = encrypted_message.split("--").map {|v| ActiveSupport::Base64.decode64(v)}
 
@@ -55,23 +81,12 @@ module ActiveSupport
       raise InvalidMessage
     end
 
-    def encrypt_and_sign(value)
-      verifier.generate(encrypt(value))
+    def new_cipher
+      OpenSSL::Cipher::Cipher.new(@cipher)
     end
 
-    def decrypt_and_verify(value)
-      decrypt(verifier.verify(value))
+    def verifier
+      MessageVerifier.new(@secret)
     end
-
-
-
-    private
-      def new_cipher
-        OpenSSL::Cipher::Cipher.new(@cipher)
-      end
-
-      def verifier
-        MessageVerifier.new(@secret)
-      end
   end
 end

activesupport/test/message_encryptor_test.rb

@@ -23,29 +23,34 @@ class MessageEncryptorTest < ActiveSupport::TestCase
   end
 
   def setup
-    @encryptor = ActiveSupport::MessageEncryptor.new(SecureRandom.hex(64))
+    @secret    = SecureRandom.hex(64)
+    @verifier  = ActiveSupport::MessageVerifier.new(@secret)
+    @encryptor = ActiveSupport::MessageEncryptor.new(@secret)
     @data = { :some => "data", :now => Time.local(2010) }
   end
 
-  def test_simple_round_tripping
-    message = @encryptor.encrypt(@data)
-    assert_equal @data, @encryptor.decrypt(message)
-  end
-
   def test_encrypting_twice_yields_differing_cipher_text
-    first_messqage = @encryptor.encrypt(@data)
-    second_message = @encryptor.encrypt(@data)
+    first_messqage = @encryptor.encrypt_and_sign(@data).split("--").first
+    second_message = @encryptor.encrypt_and_sign(@data).split("--").first
     assert_not_equal first_messqage, second_message
   end
 
-  def test_messing_with_either_value_causes_failure
-    text, iv = @encryptor.encrypt(@data).split("--")
+  def test_messing_with_either_encrypted_values_causes_failure
+    text, iv = @verifier.verify(@encryptor.encrypt_and_sign(@data)).split("--")
     assert_not_decrypted([iv, text] * "--")
     assert_not_decrypted([text, munge(iv)] * "--")
     assert_not_decrypted([munge(text), iv] * "--")
     assert_not_decrypted([munge(text), munge(iv)] * "--")
   end
 
+  def test_messing_with_verified_values_causes_failures
+    text, iv = @encryptor.encrypt_and_sign(@data).split("--")
+    assert_not_verified([iv, text] * "--")
+    assert_not_verified([text, munge(iv)] * "--")
+    assert_not_verified([munge(text), iv] * "--")
+    assert_not_verified([munge(text), munge(iv)] * "--")
+  end
+
   def test_signed_round_tripping
     message = @encryptor.encrypt_and_sign(@data)
     assert_equal @data, @encryptor.decrypt_and_verify(message)
@@ -64,17 +69,24 @@ class MessageEncryptorTest < ActiveSupport::TestCase
   end
 
   private
-    def assert_not_decrypted(value)
-      assert_raise(ActiveSupport::MessageEncryptor::InvalidMessage) do
-        @encryptor.decrypt(value)
-      end
-    end
 
-    def munge(base64_string)
-      bits = ActiveSupport::Base64.decode64(base64_string)
-      bits.reverse!
-      ActiveSupport::Base64.encode64s(bits)
+  def assert_not_decrypted(value)
+    assert_raise(ActiveSupport::MessageEncryptor::InvalidMessage) do
+      @encryptor.decrypt_and_verify(@verifier.generate(value))
     end
+  end
+
+  def assert_not_verified(value)
+    assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do
+      @encryptor.decrypt_and_verify(value)
+    end
+  end
+
+  def munge(base64_string)
+    bits = ActiveSupport::Base64.decode64(base64_string)
+    bits.reverse!
+    ActiveSupport::Base64.encode64s(bits)
+  end
 end
 
 end