Skip to content

Commit

Permalink
Support encrypting binary columns
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
djmb committed Feb 2, 2024
1 parent 7c6b1f5 commit 7ed7408
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 3 deletions.
9 changes: 9 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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*

* Add an option to `ActiveRecord::Encryption::Encryptor` to disable compression

Allow compression to be disabled by setting `compress: false`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,13 @@ def decrypt(value)
with_context do
unless value.nil?
if @default && @default == value
value
decrypted_value = value
else
encryptor.decrypt(value, **decryption_options)
decrypted_value = encryptor.decrypt(value, **decryption_options)
end
end

reserialize(decrypted_value)
end
rescue ActiveRecord::Encryption::Errors::Base => error
if previous_types_without_clean_text.blank?
Expand Down Expand Up @@ -131,7 +133,7 @@ def serialize_with_current(value)

def encrypt(value)
with_context do
encryptor.encrypt(value, **encryption_options)
reserialize(encryptor.encrypt(value, **encryption_options))
end
end

Expand All @@ -150,6 +152,14 @@ def decryption_options
def clean_text_scheme
@clean_text_scheme ||= ActiveRecord::Encryption::Scheme.new(downcase: downcase?, encryptor: ActiveRecord::Encryption::NullEncryptor.new)
end

def reserialize(value)
if cast_type.binary?
cast_type.serialize(value)
else
value
end
end
end
end
end
12 changes: 12 additions & 0 deletions activerecord/test/cases/encryption/encryptable_record_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,18 @@ 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
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

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
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/book_encrypted.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,9 @@ 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
1 change: 1 addition & 0 deletions activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
t.string :format
t.column :name, :string, default: "<untitled>"
t.column :original_name, :string
t.column :logo, :binary

t.datetime :created_at
t.datetime :updated_at
Expand Down

0 comments on commit 7ed7408

Please sign in to comment.