Merge pull request #45473 from jonathanhefner/message_encryptor-urlsafe-option

Support `:urlsafe` option for `MessageEncryptor`

activesupport/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Support `:urlsafe` option for `MessageEncryptor`.
+
+    The `MessageEncryptor` constructor now accepts a `:urlsafe` option, similar
+    to the `MessageVerifier` constructor.  When enabled, this option ensures
+    that messages use a URL-safe encoding.
+
+    *Jonathan Hefner*
+
 *   Add `urlsafe` option to `ActiveSupport::MessageVerifier` initializer
 
     `ActiveSupport::MessageVerifier.new` now takes optional `urlsafe` argument.

activesupport/lib/active_support/message_encryptor.rb

@@ -124,9 +124,7 @@ module ActiveSupport
     OpenSSLCipherError = OpenSSL::Cipher::CipherError
 
     AUTH_TAG_LENGTH = 16 # :nodoc:
-    AUTH_TAG_LENGTH_IN_BASE64 = ((4 * AUTH_TAG_LENGTH / 3) + 3) & ~3 # :nodoc:
     SEPARATOR = "--" # :nodoc:
-    SEPARATOR_LENGTH = SEPARATOR.length # :nodoc:
 
     # Initialize a new MessageEncryptor. +secret+ must be at least as long as
     # the cipher key size. For the default 'aes-256-gcm' cipher, this is 256
@@ -145,12 +143,14 @@ module ActiveSupport
     # * <tt>:digest</tt> - String of digest to use for signing. Default is
     #   +SHA1+. Ignored when using an AEAD cipher like 'aes-256-gcm'.
     # * <tt>:serializer</tt> - Object serializer to use. Default is +JSON+.
-    def initialize(secret, sign_secret = nil, cipher: nil, digest: nil, serializer: nil)
+    # * <tt>:urlsafe</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, urlsafe: false)
       @secret = secret
       @sign_secret = sign_secret
       @cipher = cipher || self.class.default_cipher
+      @aead_mode = new_cipher.authenticated?
       @digest = digest || "SHA1" unless aead_mode?
-      @verifier = resolve_verifier
       @serializer = serializer ||
         if @@default_message_encryptor_serializer.equal?(:marshal)
           Marshal
@@ -159,6 +159,8 @@ module ActiveSupport
         elsif @@default_message_encryptor_serializer.equal?(:json)
           JSON
         end
+      @urlsafe = urlsafe
+      @verifier = resolve_verifier
     end
 
     # Encrypt and sign a message. We need to sign the message in order to avoid
@@ -187,6 +189,14 @@ module ActiveSupport
         @serializer.load(value)
       end
 
+      def encode(data)
+        @urlsafe ? ::Base64.urlsafe_encode64(data, padding: false) : ::Base64.strict_encode64(data)
+      end
+
+      def decode(data)
+        @urlsafe ? ::Base64.urlsafe_decode64(data) : ::Base64.strict_decode64(data)
+      end
+
       def _encrypt(value, **metadata_options)
         cipher = new_cipher
         cipher.encrypt
@@ -199,25 +209,20 @@ module ActiveSupport
         encrypted_data = cipher.update(Messages::Metadata.wrap(serialize(value), **metadata_options))
         encrypted_data << cipher.final
 
-        encoded_encrypted_data = ::Base64.strict_encode64(encrypted_data)
-        encoded_iv = ::Base64.strict_encode64(iv)
+        parts = [encrypted_data, iv]
+        parts << cipher.auth_tag(AUTH_TAG_LENGTH) if aead_mode?
 
-        if aead_mode?
-          encoded_auth_tag = ::Base64.strict_encode64(cipher.auth_tag(AUTH_TAG_LENGTH))
-          "#{encoded_encrypted_data}#{SEPARATOR}#{encoded_iv}#{SEPARATOR}#{encoded_auth_tag}"
-        else
-          "#{encoded_encrypted_data}#{SEPARATOR}#{encoded_iv}"
-        end
+        parts.map! { |part| encode(part) }.join(SEPARATOR)
       end
 
       def _decrypt(encrypted_message, purpose)
         cipher = new_cipher
-        encrypted_data, iv, auth_tag = get_encrypted_data_and_iv_and_auth_tag_from(encrypted_message)
+        encrypted_data, iv, auth_tag = extract_parts(encrypted_message)
 
         # Currently the OpenSSL bindings do not raise an error if auth_tag is
         # truncated, which would allow an attacker to easily forge it. See
         # https://github.com/ruby/openssl/issues/63
-        raise InvalidMessage if aead_mode? && (auth_tag.nil? || auth_tag.bytes.length != AUTH_TAG_LENGTH)
+        raise InvalidMessage if aead_mode? && auth_tag.bytesize != AUTH_TAG_LENGTH
 
         cipher.decrypt
         cipher.key = @secret
@@ -236,52 +241,61 @@ module ActiveSupport
         raise InvalidMessage
       end
 
-      def iv_length_in_base64
-        @iv_length_in_base64 ||= ((4 * new_cipher.iv_len / 3) + 3) & ~3
+      def length_after_encode(length_before_encode)
+        if @urlsafe
+          (4 * length_before_encode / 3.0).ceil # length without padding
+        else
+          4 * (length_before_encode / 3.0).ceil # length with padding
+        end
       end
 
-      def separator_at?(encrypted_message, index)
-        encrypted_message[index, SEPARATOR_LENGTH] == SEPARATOR
+      def length_of_encoded_iv
+        @length_of_encoded_iv ||= length_after_encode(new_cipher.iv_len)
       end
 
-      def auth_tag_and_iv_separators_indexes_for(encrypted_message)
+      def length_of_encoded_auth_tag
+        @length_of_encoded_auth_tag ||= length_after_encode(AUTH_TAG_LENGTH)
+      end
+
+      def extract_part(encrypted_message, rindex, length)
+        index = rindex - length
+
+        if encrypted_message[index - SEPARATOR.length, SEPARATOR.length] == SEPARATOR
+          encrypted_message[index, length]
+        else
+          raise InvalidMessage
+        end
+      end
+
+      def extract_parts(encrypted_message)
+        parts = []
+        rindex = encrypted_message.length
+
         if aead_mode?
-          auth_tag_separator_index = encrypted_message.length - AUTH_TAG_LENGTH_IN_BASE64 - SEPARATOR_LENGTH
-          return if auth_tag_separator_index < SEPARATOR_LENGTH || !separator_at?(encrypted_message, auth_tag_separator_index)
+          parts << extract_part(encrypted_message, rindex, length_of_encoded_auth_tag)
+          rindex -= SEPARATOR.length + length_of_encoded_auth_tag
         end
 
-        iv_separator_index = (auth_tag_separator_index || encrypted_message.length) - iv_length_in_base64 - SEPARATOR_LENGTH
-        return if iv_separator_index.negative? || !separator_at?(encrypted_message, iv_separator_index)
+        parts << extract_part(encrypted_message, rindex, length_of_encoded_iv)
+        rindex -= SEPARATOR.length + length_of_encoded_iv
 
-        [auth_tag_separator_index, iv_separator_index]
-      end
+        parts << encrypted_message[0, rindex]
 
-      def get_encrypted_data_and_iv_and_auth_tag_from(encrypted_message)
-        auth_tag_separator_index, iv_separator_index = auth_tag_and_iv_separators_indexes_for(encrypted_message)
-        return if iv_separator_index.nil? || (aead_mode? && auth_tag_separator_index.nil?)
-
-        encrypted_data = encrypted_message[0, iv_separator_index]
-        iv = encrypted_message[iv_separator_index + SEPARATOR_LENGTH, iv_length_in_base64]
-        auth_tag = encrypted_message[auth_tag_separator_index + SEPARATOR_LENGTH, AUTH_TAG_LENGTH_IN_BASE64] if aead_mode?
-
-        [encrypted_data, iv, auth_tag].map! { |v| ::Base64.strict_decode64(v) if v.present? }
+        parts.reverse!.map! { |part| decode(part) }
       end
 
       def new_cipher
         OpenSSL::Cipher.new(@cipher)
       end
 
-      attr_reader :verifier
-
-      def aead_mode?
-        @aead_mode ||= new_cipher.authenticated?
-      end
+      attr_reader :verifier, :aead_mode
+      alias :aead_mode? :aead_mode
 
       def resolve_verifier
         if aead_mode?
           NullVerifier
         else
-          MessageVerifier.new(@sign_secret || @secret, digest: @digest, serializer: NullSerializer)
+          MessageVerifier.new(@sign_secret || @secret, digest: @digest, serializer: NullSerializer, urlsafe: @urlsafe)
         end
       end
   end

activesupport/test/message_encryptor_test.rb

@@ -83,6 +83,41 @@ class MessageEncryptorTest < ActiveSupport::TestCase
     assert_not_verified([iv,  message] * bad_encoding_characters)
   end
 
+  test "supports URL-safe encoding when using authenticated encryption" do
+    encryptor = ActiveSupport::MessageEncryptor.new(@secret, urlsafe: true, cipher: "aes-256-gcm")
+
+    # Because encrypted data appears random, we cannot control whether it will
+    # contain bytes that _would_ be encoded as non-URL-safe characters (i.e. "+"
+    # or "/") if `urlsafe: true` were broken.  Therefore, to make our test
+    # falsifiable, we use a large string so that the encrypted data will almost
+    # certainly contain such bytes.
+    data = "x" * 10001
+    message = encryptor.encrypt_and_sign(data)
+
+    assert_equal data, encryptor.decrypt_and_verify(message)
+    assert_equal message, URI.encode_www_form_component(message)
+  end
+
+  test "supports URL-safe encoding when using unauthenticated encryption" do
+    encryptor = ActiveSupport::MessageEncryptor.new(@secret, urlsafe: true, cipher: "aes-256-cbc")
+
+    # When using unauthenticated encryption, messages are double encoded: once
+    # when encrypting and once again when signing with a MessageVerifier.  The
+    # 1st encode eliminates the possibility of a 6-bit aligned occurrence of
+    # `0b111110` or `0b111111`, which the 2nd encode _would_ map to a
+    # non-URL-safe character (i.e. "+" or "/") if `urlsafe: true` were broken.
+    # Therefore, to ensure our test is falsifiable, we also assert that the
+    # message payload _would_ have padding characters (i.e. "=") if
+    # `urlsafe: true` were broken.
+    data = 1
+    message = encryptor.encrypt_and_sign(data)
+
+    assert_equal data, encryptor.decrypt_and_verify(message)
+    assert_equal message, URI.encode_www_form_component(message)
+    assert_not_equal 0, message.rpartition("--").first.length % 4,
+      "Unable to assert that the message payload is unpadded, because it does not require padding"
+  end
+
   def test_aead_mode_encryption
     encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm")
     message = encryptor.encrypt_and_sign(@data)