Fix binary decryption on Postgres

When encrypting attributes we need to do it just before inserting the
data into the database, so after any other serialization steps, e.g. for
serialized types or normalization. And we need things to happen in
reverse order when decrypting.

With attribute decoration we end up with types nested in other
types. To ensure that the encryption happens in the right place, the
EncryptedAttributeType first serializes the value with the type it is
wrapping and then encrypts it. And in reverse it decrypts then
deserializes with the wrapped type.

There's an assumption here, which is that the wrapped type doesn't need
to do anything in between the database and the encryption layer - so
any database specific casting is skipped.

This works fine for String columns as there's nothing for them to do. It
also works for binary columns for MySQL and SQLite. But is doesn't for
PostgreSQL which needs to receive the data as Binary::Data and has to
call `PG::Connection.unescape_bytea` when deserializing the data.

The serialization part was fixed in https://github.com/rails/rails/pull/50920,
where the encryption output is wrapped in Binary::Data, which let's the
PostgreSQL adapter know to convert the value
[here](5a0b2fa5a3/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb (L83)).

That PR however didn't fix deserializing the data when it comes back out
of the database (it wasn't round-tripping the data properly in the
tests).

We need to deserialize binary types before decrypting them - and we'll
have to just assume that the wrapped type can do that for us.

This won't work for serialized types as they'll also attempt to convert
the data with the coder which needs to happen after decryption, so we
need to special case them and extract the subtype instead.

This isn't ideal but it should work ok for all built in types.

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Deserialize binary data before decrypting
+
+    This ensures that we call `PG::Connection.unescape_bytea` on PostgreSQL before decryption.
+
+    *Donal McBreen*
+
 *   Ensure `ActiveRecord::Encryption.config` is always ready before access.
 
     Previously, `ActiveRecord::Encryption` configuration was deferred until `ActiveRecord::Base`

activerecord/lib/active_record/encryption/encrypted_attribute_type.rb

@@ -100,7 +100,7 @@ module ActiveRecord
         end
 
         def decrypt(value)
-          text_to_database_type decrypt_as_text(value)
+          text_to_database_type decrypt_as_text(database_type_to_text(value))
         end
 
         def try_to_deserialize_with_previous_encrypted_types(value)
@@ -170,6 +170,15 @@ module ActiveRecord
             value
           end
         end
+
+        def database_type_to_text(value)
+          if value && cast_type.binary?
+            binary_cast_type = cast_type.serialized? ? cast_type.subtype : cast_type
+            binary_cast_type.deserialize(value)
+          else
+            value
+          end
+        end
     end
   end
 end

activerecord/test/cases/encryption/encryptable_record_message_pack_serialized_test.rb

@@ -5,19 +5,22 @@ require "models/author_encrypted"
 require "models/book_encrypted"
 require "active_record/encryption/message_pack_message_serializer"
 
-class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::EncryptionTestCase
+class ActiveRecord::Encryption::EncryptableRecordMessagePackSerializedTest < ActiveRecord::EncryptionTestCase
   fixtures :encrypted_books
 
   test "binary data can be serialized with message pack" do
     all_bytes = (0..255).map(&:chr).join
-    assert_equal all_bytes, EncryptedBookWithBinaryMessagePackSerialized.create!(logo: all_bytes).logo
+    book = EncryptedBookWithBinaryMessagePackSerialized.create!(logo: all_bytes)
+    assert_encrypted_attribute(book, :logo, all_bytes)
   end
 
   test "binary data can be encrypted uncompressed and serialized with message pack" do
+    # Strings below 140 bytes are not compressed
     low_bytes = (0..127).map(&:chr).join
     high_bytes = (128..255).map(&:chr).join
-    assert_equal low_bytes, EncryptedBookWithBinaryMessagePackSerialized.create!(logo: low_bytes).logo
-    assert_equal high_bytes, EncryptedBookWithBinaryMessagePackSerialized.create!(logo: high_bytes).logo
+
+    assert_encrypted_attribute(EncryptedBookWithBinaryMessagePackSerialized.create!(logo: low_bytes), :logo, low_bytes)
+    assert_encrypted_attribute(EncryptedBookWithBinaryMessagePackSerialized.create!(logo: high_bytes), :logo, high_bytes)
   end
 
   test "text columns cannot be serialized with message pack" do

activerecord/test/cases/encryption/encryptable_record_test.rb

@@ -92,6 +92,12 @@ class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::Encryption
     assert_encrypted_attribute(traffic_light, :state, states)
   end
 
+  test "encrypts serialized attributes where encrypts is declared first" do
+    states = ["green", "red"]
+    traffic_light = EncryptedFirstTrafficLight.create!(state: states, long_state: states)
+    assert_encrypted_attribute(traffic_light, :state, states)
+  end
+
   test "encrypts store attributes with accessors" do
     traffic_light = EncryptedTrafficLightWithStoreState.create!(color: "red", long_state: ["green", "red"])
     assert_equal "red", traffic_light.color
@@ -404,13 +410,14 @@ class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::Encryption
   test "binary data can be encrypted uncompressed" do
     low_bytes = (0..127).map(&:chr).join
     high_bytes = (128..255).map(&:chr).join
-    assert_equal low_bytes, EncryptedBookWithBinary.create!(logo: low_bytes).logo
-    assert_equal high_bytes, EncryptedBookWithBinary.create!(logo: high_bytes).logo
+    assert_encrypted_attribute EncryptedBookWithBinary.create!(logo: low_bytes), :logo, low_bytes
+    assert_encrypted_attribute EncryptedBookWithBinary.create!(logo: high_bytes), :logo, high_bytes
   end
 
   test "serialized binary data can be encrypted" do
     json_bytes = (32..127).map(&:chr)
-    assert_equal json_bytes, EncryptedBookWithSerializedBinary.create!(logo: json_bytes).logo
+    assert_encrypted_attribute EncryptedBookWithSerializedFirstBinary.create!(logo: json_bytes), :logo, json_bytes
+    assert_encrypted_attribute EncryptedBookWithSerializedSecondBinary.create!(logo: json_bytes), :logo, json_bytes
   end
 
   test "can compress data with custom compressor" do
@@ -423,6 +430,17 @@ class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::Encryption
     assert_equal :text, EncryptedPost.type_for_attribute(:body).type
   end
 
+  test "encrypts normalized data" do
+    assert_encrypted_attribute EncryptedBookNormalizedFirst.create!(name: "Book"), :name, "book"
+    assert_encrypted_attribute EncryptedBookNormalizedSecond.create!(name: "Book"), :name, "book"
+    assert_encrypted_attribute EncryptedBookNormalizedFirst.create!(logo: "Book"), :logo, "book"
+    assert_encrypted_attribute EncryptedBookNormalizedSecond.create!(logo: "Book"), :logo, "book"
+  end
+
+  test "encrypts attribute data" do
+    assert_encrypted_attribute EncryptedBookAttribute.create!(name: "2024-01-01"), :name, Date.new(2024, 1, 1)
+  end
+
   private
     def build_derived_key_provider_with(hash_digest_class)
       ActiveRecord::Encryption.with_encryption_context(key_generator: ActiveRecord::Encryption::KeyGenerator.new(hash_digest_class: hash_digest_class)) do

activerecord/test/models/book_encrypted.rb

@@ -24,6 +24,31 @@ class EncryptedBookWithDowncaseName < ActiveRecord::Base
   encrypts :name, deterministic: true, downcase: true
 end
 
+class EncryptedBookNormalizedFirst < ActiveRecord::Base
+  self.table_name = "encrypted_books"
+
+  normalizes :name, with: ->(value) { value.to_s.downcase }
+  encrypts :name
+  normalizes :logo, with: ->(value) { value.to_s.downcase }
+  encrypts :logo
+end
+
+class EncryptedBookNormalizedSecond < ActiveRecord::Base
+  self.table_name = "encrypted_books"
+
+  encrypts :name
+  normalizes :name, with: ->(value) { value.to_s.downcase }
+  encrypts :logo
+  normalizes :logo, with: ->(value) { value.to_s.downcase }
+end
+
+class EncryptedBookAttribute < ActiveRecord::Base
+  self.table_name = "encrypted_books"
+
+  attribute :name, :date
+  encrypts :name
+end
+
 class EncryptedBookThatIgnoresCase < ActiveRecord::Base
   self.table_name = "encrypted_books"
 
@@ -50,13 +75,20 @@ class EncryptedBookWithBinary < ActiveRecord::Base
   encrypts :logo
 end
 
-class EncryptedBookWithSerializedBinary < ActiveRecord::Base
+class EncryptedBookWithSerializedFirstBinary < ActiveRecord::Base
   self.table_name = "encrypted_books"
 
   serialize :logo, coder: JSON
   encrypts :logo
 end
 
+class EncryptedBookWithSerializedSecondBinary < ActiveRecord::Base
+  self.table_name = "encrypted_books"
+
+  encrypts :logo
+  serialize :logo, coder: JSON
+end
+
 class EncryptedBookWithCustomCompressor < ActiveRecord::Base
   module CustomCompressor
     def self.deflate(value)

activerecord/test/models/traffic_light_encrypted.rb

@@ -7,6 +7,14 @@ class EncryptedTrafficLight < TrafficLight
   encrypts :state
 end
 
+class EncryptedFirstTrafficLight < ActiveRecord::Base
+  self.table_name = "traffic_lights"
+
+  encrypts :state
+  serialize :state, type: Array
+  serialize :long_state, type: Array
+end
+
 class EncryptedTrafficLightWithStoreState < TrafficLight
   store :state, accessors: %i[ color ], coder: ActiveRecord::Coders::JSON
   encrypts :state