Merge pull request rails#47325 from jonathanhefner/message_encryptor-and-message_verifier-refactor

Refactor `MessageEncryptor` and `MessageVerifier`

Author: jonathanhefner

activesupport/lib/active_support/message_encryptor.rb

@@ -3,8 +3,9 @@
 require "openssl"
 require "base64"
 require "active_support/core_ext/module/attribute_accessors"
+require "active_support/messages/codec"
+require "active_support/messages/rotator"
 require "active_support/message_verifier"
-require "active_support/messages/metadata"
 
 module ActiveSupport
   # MessageEncryptor is a simple way to encrypt values which get stored
@@ -84,8 +85,7 @@ module ActiveSupport
   # the above should be combined into:
   #
   #   crypt.rotate old_secret, cipher: "aes-256-cbc"
-  class MessageEncryptor
-    include Messages::Metadata
+  class MessageEncryptor < Messages::Codec
     prepend Messages::Rotator::Encryptor
 
     cattr_accessor :use_authenticated_message_encryption, instance_accessor: false, default: false
@@ -111,16 +111,6 @@ def self.dump(value)
       end
     end
 
-    module NullVerifier # :nodoc:
-      def self.verify(value)
-        value
-      end
-
-      def self.generate(value)
-        value
-      end
-    end
-
     class InvalidMessage < StandardError; end
     OpenSSLCipherError = OpenSSL::Cipher::CipherError
 
@@ -147,21 +137,13 @@ class InvalidMessage < StandardError; end
     # * <tt>:url_safe</tt> - Whether to encode messages using a URL-safe
     #   encoding. Default is +false+ for backward compatibility.
     def initialize(secret, sign_secret = nil, cipher: nil, digest: nil, serializer: nil, url_safe: false)
+      super(serializer: serializer || @@default_message_encryptor_serializer, url_safe: url_safe)
       @secret = secret
-      @sign_secret = sign_secret
       @cipher = cipher || self.class.default_cipher
       @aead_mode = new_cipher.authenticated?
-      @digest = digest || "SHA1" unless aead_mode?
-      @serializer = serializer ||
-        if @@default_message_encryptor_serializer.equal?(:marshal)
-          Marshal
-        elsif @@default_message_encryptor_serializer.equal?(:hybrid)
-          JsonWithMarshalFallback
-        elsif @@default_message_encryptor_serializer.equal?(:json)
-          JSON
-        end
-      @url_safe = url_safe
-      @verifier = resolve_verifier
+      @verifier = if !@aead_mode
+        MessageVerifier.new(sign_secret || secret, digest: digest || "SHA1", serializer: NullSerializer, url_safe: url_safe)
+      end
     end
 
     # Encrypt and sign a message. We need to sign the message in order to avoid
@@ -191,8 +173,8 @@ def initialize(secret, sign_secret = nil, cipher: nil, digest: nil, serializer:
     #   The purpose of the message. If specified, the same purpose must be
     #   specified when verifying the message; otherwise, verification will fail.
     #   (See #decrypt_and_verify.)
-    def encrypt_and_sign(value, expires_at: nil, expires_in: nil, purpose: nil)
-      verifier.generate(_encrypt(value, expires_at: expires_at, expires_in: expires_in, purpose: purpose))
+    def encrypt_and_sign(value, **options)
+      sign(encrypt(serialize_with_metadata(value, **options)))
     end
 
     # Decrypt and verify a message. We need to verify the message in order to
@@ -212,8 +194,10 @@ def encrypt_and_sign(value, expires_at: nil, expires_in: nil, purpose: nil)
     #     encryptor.decrypt_and_verify(message)                      # => "bye"
     #     encryptor.decrypt_and_verify(message, purpose: "greeting") # => nil
     #
-    def decrypt_and_verify(data, purpose: nil, **)
-      _decrypt(verifier.verify(data), purpose)
+    def decrypt_and_verify(message, **options)
+      deserialize_with_metadata(decrypt(verify(message)), **options)
+    rescue TypeError, ArgumentError, ::JSON::ParserError
+      raise InvalidMessage
     end
 
     # Given a cipher, returns the key length of the cipher to help generate the key of desired size
@@ -222,17 +206,15 @@ def self.key_len(cipher = default_cipher)
     end
 
     private
-      attr_reader :serializer
-
-      def encode(data)
-        @url_safe ? ::Base64.urlsafe_encode64(data, padding: false) : ::Base64.strict_encode64(data)
+      def sign(data)
+        @verifier ? @verifier.generate(data) : data
       end
 
-      def decode(data)
-        @url_safe ? ::Base64.urlsafe_decode64(data) : ::Base64.strict_decode64(data)
+      def verify(data)
+        @verifier ? @verifier.verify(data) : data
       end
 
-      def _encrypt(value, **metadata_options)
+      def encrypt(data)
         cipher = new_cipher
         cipher.encrypt
         cipher.key = @secret
@@ -241,16 +223,16 @@ def _encrypt(value, **metadata_options)
         iv = cipher.random_iv
         cipher.auth_data = "" if aead_mode?
 
-        encrypted_data = cipher.update(serialize_with_metadata(value, **metadata_options))
+        encrypted_data = cipher.update(data)
         encrypted_data << cipher.final
 
         parts = [encrypted_data, iv]
         parts << cipher.auth_tag(AUTH_TAG_LENGTH) if aead_mode?
 
-        parts.map! { |part| encode(part) }.join(SEPARATOR)
+        join_parts(parts)
       end
 
-      def _decrypt(encrypted_message, purpose)
+      def decrypt(encrypted_message)
         cipher = new_cipher
         encrypted_data, iv, auth_tag = extract_parts(encrypted_message)
 
@@ -269,9 +251,7 @@ def _decrypt(encrypted_message, purpose)
 
         decrypted_data = cipher.update(encrypted_data)
         decrypted_data << cipher.final
-
-        deserialize_with_metadata(decrypted_data, purpose: purpose)
-      rescue OpenSSLCipherError, TypeError, ArgumentError, ::JSON::ParserError
+      rescue OpenSSLCipherError
         raise InvalidMessage
       end
 
@@ -291,6 +271,10 @@ def length_of_encoded_auth_tag
         @length_of_encoded_auth_tag ||= length_after_encode(AUTH_TAG_LENGTH)
       end
 
+      def join_parts(parts)
+        parts.map! { |part| encode(part) }.join(SEPARATOR)
+      end
+
       def extract_part(encrypted_message, rindex, length)
         index = rindex - length
 
@@ -322,15 +306,7 @@ def new_cipher
         OpenSSL::Cipher.new(@cipher)
       end
 
-      attr_reader :verifier, :aead_mode
+      attr_reader :aead_mode
       alias :aead_mode? :aead_mode
-
-      def resolve_verifier
-        if aead_mode?
-          NullVerifier
-        else
-          MessageVerifier.new(@sign_secret || @secret, digest: @digest, serializer: NullSerializer, url_safe: @url_safe)
-        end
-      end
   end
 end

activesupport/lib/active_support/message_verifier.rb

@@ -4,7 +4,7 @@
 require "base64"
 require "active_support/core_ext/object/blank"
 require "active_support/security_utils"
-require "active_support/messages/metadata"
+require "active_support/messages/codec"
 require "active_support/messages/rotator"
 
 module ActiveSupport
@@ -118,8 +118,7 @@ module ActiveSupport
   #
   #   @verifier = ActiveSupport::MessageVerifier.new("secret", url_safe: true)
   #   @verifier.generate("signed message") #=> URL-safe string
-  class MessageVerifier
-    include Messages::Metadata
+  class MessageVerifier < Messages::Codec
     prepend Messages::Rotator::Verifier
 
     class InvalidSignature < StandardError; end
@@ -131,17 +130,9 @@ class InvalidSignature < StandardError; end
 
     def initialize(secret, digest: nil, serializer: nil, url_safe: false)
       raise ArgumentError, "Secret should not be nil." unless secret
+      super(serializer: serializer || @@default_message_verifier_serializer, url_safe: url_safe)
       @secret = secret
       @digest = digest&.to_s || "SHA1"
-      @serializer = serializer ||
-        if @@default_message_verifier_serializer.equal?(:marshal)
-          Marshal
-        elsif @@default_message_verifier_serializer.equal?(:hybrid)
-          JsonWithMarshalFallback
-        elsif @@default_message_verifier_serializer.equal?(:json)
-          JSON
-        end
-      @url_safe = url_safe
     end
 
     # Checks if a signed message could have been generated by signing an object
@@ -153,9 +144,8 @@ def initialize(secret, digest: nil, serializer: nil, url_safe: false)
     #
     #   tampered_message = signed_message.chop # editing the message invalidates the signature
     #   verifier.valid_message?(tampered_message) # => false
-    def valid_message?(signed_message)
-      data, digest = get_data_and_digest_from(signed_message)
-      digest_matches_data?(digest, data)
+    def valid_message?(message)
+      !!extract_encoded(message)
     end
 
     # Decodes the signed message using the +MessageVerifier+'s secret.
@@ -195,16 +185,11 @@ def valid_message?(signed_message)
     #     verifier.verified(message)                      # => "bye"
     #     verifier.verified(message, purpose: "greeting") # => nil
     #
-    def verified(signed_message, purpose: nil, **)
-      data, digest = get_data_and_digest_from(signed_message)
-      if digest_matches_data?(digest, data)
-        begin
-          deserialize_with_metadata(decode(data), purpose: purpose)
-        rescue ArgumentError => argument_error
-          return if argument_error.message.include?("invalid base64")
-          raise
-        end
-      end
+    def verified(message, **options)
+      encoded = extract_encoded(message)
+      deserialize_with_metadata(decode(encoded), **options) if encoded
+    rescue ArgumentError => error
+      raise unless error.message.include?("invalid base64")
     end
 
     # Decodes the signed message using the +MessageVerifier+'s secret.
@@ -273,21 +258,27 @@ def verify(*args, **options)
     #   The purpose of the message. If specified, the same purpose must be
     #   specified when verifying the message; otherwise, verification will fail.
     #   (See #verified and #verify.)
-    def generate(value, expires_at: nil, expires_in: nil, purpose: nil)
-      data = encode(serialize_with_metadata(value, expires_at: expires_at, expires_in: expires_in, purpose: purpose))
-      digest = generate_digest(data)
-      data << SEPARATOR << digest
+    def generate(value, **options)
+      sign_encoded(encode(serialize_with_metadata(value, **options)))
     end
 
     private
-      attr_reader :serializer
-
-      def encode(data)
-        @url_safe ? Base64.urlsafe_encode64(data, padding: false) : Base64.strict_encode64(data)
+      def sign_encoded(encoded)
+        digest = generate_digest(encoded)
+        encoded << SEPARATOR << digest
       end
 
-      def decode(data)
-        @url_safe ? Base64.urlsafe_decode64(data) : Base64.strict_decode64(data)
+      def extract_encoded(signed)
+        return if signed.nil? || !signed.valid_encoding?
+
+        if separator_index = separator_index_for(signed)
+          encoded = signed[0, separator_index]
+          digest = signed[separator_index + SEPARATOR_LENGTH, digest_length_in_hex]
+        end
+
+        return unless digest_matches_data?(digest, encoded)
+
+        encoded
       end
 
       def generate_digest(data)
@@ -308,21 +299,7 @@ def separator_at?(signed_message, index)
 
       def separator_index_for(signed_message)
         index = signed_message.length - digest_length_in_hex - SEPARATOR_LENGTH
-        return if index.negative? || !separator_at?(signed_message, index)
-
-        index
-      end
-
-      def get_data_and_digest_from(signed_message)
-        return if signed_message.nil? || !signed_message.valid_encoding? || signed_message.empty?
-
-        separator_index = separator_index_for(signed_message)
-        return if separator_index.nil?
-
-        data = signed_message[0, separator_index]
-        digest = signed_message[separator_index + SEPARATOR_LENGTH, digest_length_in_hex]
-
-        [data, digest]
+        index unless index.negative? || !separator_at?(signed_message, index)
       end
 
       def digest_matches_data?(digest, data)

activesupport/lib/active_support/messages/codec.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require "active_support/messages/metadata"
+
+module ActiveSupport
+  module Messages # :nodoc:
+    class Codec # :nodoc:
+      include Metadata
+
+      def initialize(serializer:, url_safe:)
+        @serializer =
+          case serializer
+          when :marshal
+            Marshal
+          when :hybrid
+            JsonWithMarshalFallback
+          when :json
+            JSON
+          else
+            serializer
+          end
+
+        @url_safe = url_safe
+      end
+
+      private
+        attr_reader :serializer
+
+        def encode(data, url_safe: @url_safe)
+          url_safe ? ::Base64.urlsafe_encode64(data, padding: false) : ::Base64.strict_encode64(data)
+        end
+
+        def decode(encoded, url_safe: @url_safe)
+          url_safe ? ::Base64.urlsafe_decode64(encoded) : ::Base64.strict_decode64(encoded)
+        end
+
+        def serialize(data)
+          serializer.dump(data)
+        end
+
+        def deserialize(serialized)
+          serializer.load(serialized)
+        end
+    end
+  end
+end

activesupport/lib/active_support/messages/metadata.rb

@@ -25,7 +25,7 @@ def serialize_with_metadata(data, **metadata)
             ActiveSupport::JSON.encode(envelope)
           else
             data = wrap_in_metadata_envelope({ "data" => data }, **metadata) if has_metadata
-            serializer.dump(data)
+            serialize(data)
           end
         end
 
@@ -35,7 +35,7 @@ def deserialize_with_metadata(message, **expected_metadata)
             extracted = extract_from_metadata_envelope(envelope, **expected_metadata)
             deserialize_from_json_safe_string(extracted["message"]) if extracted
           else
-            deserialized = serializer.load(message)
+            deserialized = deserialize(message)
             if metadata_envelope?(deserialized)
               extracted = extract_from_metadata_envelope(deserialized, **expected_metadata)
               extracted["data"] if extracted
@@ -90,11 +90,11 @@ def parse_expiry(expires_at)
         end
 
         def serialize_to_json_safe_string(data)
-          ::Base64.strict_encode64(serializer.dump(data))
+          encode(serialize(data), url_safe: false)
         end
 
         def deserialize_from_json_safe_string(string)
-          serializer.load(::Base64.strict_decode64(string))
+          deserialize(decode(string, url_safe: false))
         end
     end
   end