Skip to content

Commit

Permalink
Ensure we deserialize database values before decrypting
Browse files Browse the repository at this point in the history
Adds the missing step of deserializing the database value before
decryption.

For string columns, this is a no-op, For binary columns this is a no-op
for SQLite and MySQL, but not for [PostgreSQL](https://github.com/rails/rails/blob/b60cf01e9d076d235fdb02db1072ea2a31c94dbe/activerecord/lib/active_record/connection_adapters/postgresql/oid/bytea.rb#L11).

The deserialization is messy, because of the need to handle serialized
types (those declared with `serializes :foo, JSON`). The docs
[recommend](https://guides.rubyonrails.org/active_record_encryption.html#supported-types)
that you should have that declaration before the `encrypts :foo`.

If we recommended that they were called the other way round this could
be greatly simplified and we could also remove the special case handling
of binary types in ActiveRecord::Encryption::EncryptedAttributeType#text_to_database_type.
  • Loading branch information
djmb committed Aug 19, 2024
1 parent b60cf01 commit 475cfa2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def decrypt_as_text(value)
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)
Expand Down Expand Up @@ -170,6 +170,18 @@ def text_to_database_type(value)
value
end
end

def database_type_to_text(value)
if value && cast_type.binary?
if cast_type.is_a?(ActiveRecord::Type::Serialized)
cast_type.subtype.deserialize(value)
else
cast_type.deserialize(value)
end
else
value
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,22 @@
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
Expand Down
6 changes: 3 additions & 3 deletions activerecord/test/cases/encryption/encryptable_record_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,13 @@ def name
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 EncryptedBookWithSerializedBinary.create!(logo: json_bytes), :logo, json_bytes
end

test "can compress data with custom compressor" do
Expand Down

0 comments on commit 475cfa2

Please sign in to comment.