Permalink
Browse files

Don't marshal dump twice when using encryptor.

  • Loading branch information...
1 parent 71e84a3 commit a625523e755421b0f96fe5b0a885462ef51582b1 @josevalim josevalim committed Nov 9, 2011
@@ -10,6 +10,16 @@ module ActiveSupport
# This can be used in situations similar to the <tt>MessageVerifier</tt>, but where you don't
# want users to be able to determine the value of the payload.
class MessageEncryptor
+ module NullSerializer #:nodoc:
+ def self.load(value)
+ value
+ end
+
+ def self.dump(value)
+ value
+ end
+ end
+
class InvalidMessage < StandardError; end
OpenSSLCipherError = OpenSSL::Cipher.const_defined?(:CipherError) ? OpenSSL::Cipher::CipherError : OpenSSL::CipherError
@@ -21,6 +31,7 @@ def initialize(secret, options = {})
@secret = secret
@cipher = options[:cipher] || 'aes-256-cbc'
+ @verifier = MessageVerifier.new(@secret, :serializer => NullSerializer)
@serializer = options[:serializer] || Marshal
end
@@ -86,7 +97,7 @@ def new_cipher
end
def verifier
- MessageVerifier.new(@secret)
+ @verifier
end
end
end
@@ -11,7 +11,6 @@
require 'active_support/json'
class MessageEncryptorTest < ActiveSupport::TestCase
-
class JSONSerializer
def dump(value)
ActiveSupport::JSON.encode(value)
@@ -24,7 +23,7 @@ def load(value)
def setup
@secret = SecureRandom.hex(64)
- @verifier = ActiveSupport::MessageVerifier.new(@secret)
+ @verifier = ActiveSupport::MessageVerifier.new(@secret, :serializer => ActiveSupport::MessageEncryptor::NullSerializer)
@encryptor = ActiveSupport::MessageEncryptor.new(@secret)
@data = { :some => "data", :now => Time.local(2010) }
end

2 comments on commit a625523

Contributor

bjeanes replied Mar 9, 2014

This commit broke the ability to decrypt persisted values that were encrypted using Rails 3.1. The :serializer option here is essentially hard-coded to use NullSerializer while the fall-back in MessageVerifier is and was Marshal.

Attempting to decrypt a value saved with Rails 3.1 from Rails 3.2 results in an ActiveSupport::MessageEncryptor::InvalidMessage exception. Judging by the code even in the latest 4.1.0 release candidate, this is still the case. Prior to this commit, all encrypted values would have the message verifier marshaling the value. If those values are persisted, any attempt to decrypt would fail.

Because the verifier's serializer is not configurable, in order to start upgrading my old encrypted values I had to (unfortunately) do the following:

legacy_encryptor = ::ActiveSupport::MessageEncryptor.new(default_encryption_key)
legacy_verifier = ::ActiveSupport::MessageVerifier.new(default_encryption_key, serializer: Marshal)
legacy_encryptor.instance_variable_set(:@verifier, legacy_verifier)
Owner

jeremy replied Mar 9, 2014

Nice catch @bjeanes. That's no good—nasty compatibility break.

Would you be interested in doing a PR that 1. makes the serializer configurable and 2. provides a graceful-compatible serializer that attempts unmarshaling for <3.1 values and then presume null serialization?

Please sign in to comment.