From 3275f68bf19139cbefd3c384f2761a3f4ee29dc5 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Thu, 15 Feb 2024 16:45:56 +0000 Subject: [PATCH] Support encrypting binary columns (#50920) * Support encrypting binary columns ActiveRecord Encryption doesn't prevent you from encrypting binary columns but it doesn't have proper support for it either. When the data is fed through encrypt/decrypt it is converted to a String. This means that the the encryption layer is not transparent to binary data - which should be passed as Type::Binary::Data. As a result the data is not properly escaped in the SQL queries or deserialized correctly after decryption. However it just happens to work fine for MySQL and SQLite because the MessageSerializer doesn't use any characters that need to be encoded. However if you try to use a custom serializer that does then it breaks. PostgreSQL on the other hand does not work - because the Bytea type is passed a String rather than a Type::Binary::Data to deserialize, it attempts to unescape the data and either mangles it or raises an error if it contains null bytes. The commit fixes the issue, by reserializing the data after encryption and decryption. For text data that's a no-op, but for binary data we'll convert it back to a Type::Binary::Data. * Extract decrypt_as_text/encrypt_as_text * Handle serialized binary data in encrypted columns Calling `serialize` is not always possible, because the column type might not expect to be serializing a String, for example when declared as serialzed or store attribute. With binary data the encryptor was passed an `ActiveModel::Type::Binary::Data`` and returned a `String``. In order to remain transparent we need to turn the data back into a `ActiveModel::Type::Binary::Data` before passing it on. We'll also rename `serialize`` to `text_to_database_type` to be a bit more descriptive. --- activerecord/CHANGELOG.md | 9 +++++++++ .../encryption/encrypted_attribute_type.rb | 20 +++++++++++++++++-- .../encryption/encryptable_record_test.rb | 19 ++++++++++++++++++ activerecord/test/models/book_encrypted.rb | 13 ++++++++++++ activerecord/test/schema/schema.rb | 1 + 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 8b37f79f077b7..f4efdf4aae2f0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Add support for encrypting binary columns + + Ensure encryption and decryption pass `Type::Binary::Data` around for binary data. + + Previously encrypting binary columns with the `ActiveRecord::Encryption::MessageSerializer` + incidentally worked for MySQL and SQLite, but not PostgreSQL. + + *Donal McBreen* + * Deprecated `ENV["SCHEMA_CACHE"]` in favor of `schema_cache_path` in the database configuration. *Rafael Mendonça França* diff --git a/activerecord/lib/active_record/encryption/encrypted_attribute_type.rb b/activerecord/lib/active_record/encryption/encrypted_attribute_type.rb index a44d78dbdaccc..8da9ac31dfc9b 100644 --- a/activerecord/lib/active_record/encryption/encrypted_attribute_type.rb +++ b/activerecord/lib/active_record/encryption/encrypted_attribute_type.rb @@ -81,7 +81,7 @@ def previous_type? @previous_type end - def decrypt(value) + def decrypt_as_text(value) with_context do unless value.nil? if @default && @default == value @@ -99,6 +99,10 @@ def decrypt(value) end end + def decrypt(value) + text_to_database_type decrypt_as_text(value) + end + def try_to_deserialize_with_previous_encrypted_types(value) previous_types.each.with_index do |type, index| break type.deserialize(value) @@ -129,12 +133,16 @@ def serialize_with_current(value) encrypt(casted_value.to_s) unless casted_value.nil? end - def encrypt(value) + def encrypt_as_text(value) with_context do encryptor.encrypt(value, **encryption_options) end end + def encrypt(value) + text_to_database_type encrypt_as_text(value) + end + def encryptor ActiveRecord::Encryption.encryptor end @@ -150,6 +158,14 @@ def decryption_options def clean_text_scheme @clean_text_scheme ||= ActiveRecord::Encryption::Scheme.new(downcase: downcase?, encryptor: ActiveRecord::Encryption::NullEncryptor.new) end + + def text_to_database_type(value) + if value && cast_type.binary? + ActiveModel::Type::Binary::Data.new(value) + else + value + end + end end end end diff --git a/activerecord/test/cases/encryption/encryptable_record_test.rb b/activerecord/test/cases/encryption/encryptable_record_test.rb index 14180f6b47e17..18affa72110fd 100644 --- a/activerecord/test/cases/encryption/encryptable_record_test.rb +++ b/activerecord/test/cases/encryption/encryptable_record_test.rb @@ -394,6 +394,25 @@ def name assert_predicate OtherEncryptedPost.type_for_attribute(:title).scheme.previous_schemes, :one? end + test "binary data can be encrypted" do + all_bytes = (0..255).map(&:chr).join + assert_equal all_bytes, EncryptedBookWithBinary.create!(logo: all_bytes).logo + assert_nil EncryptedBookWithBinary.create!(logo: nil).logo + assert_equal "", EncryptedBookWithBinary.create!(logo: "").logo + end + + 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 + 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 + 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 diff --git a/activerecord/test/models/book_encrypted.rb b/activerecord/test/models/book_encrypted.rb index 0c4971379551e..942e26b210d39 100644 --- a/activerecord/test/models/book_encrypted.rb +++ b/activerecord/test/models/book_encrypted.rb @@ -43,3 +43,16 @@ class EncryptedBookWithUnencryptedDataOptedIn < ActiveRecord::Base validates :name, uniqueness: true encrypts :name, deterministic: true, support_unencrypted_data: true end + +class EncryptedBookWithBinary < ActiveRecord::Base + self.table_name = "encrypted_books" + + encrypts :logo +end + +class EncryptedBookWithSerializedBinary < ActiveRecord::Base + self.table_name = "encrypted_books" + + serialize :logo, coder: JSON + encrypts :logo +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7d9c2f831f032..8c44978076299 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -163,6 +163,7 @@ t.string :format t.column :name, :string, default: "" t.column :original_name, :string + t.column :logo, :binary t.datetime :created_at t.datetime :updated_at