Support encrypting binary columns #50920
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djmb
commented
Jan 30, 2024
activerecord/lib/active_record/encryption/encrypted_attribute_type.rb
Outdated
Show resolved
Hide resolved
jorgemanrubia
approved these changes
Feb 2, 2024
There was a problem hiding this comment.
Great one @djmb. Left a minor suggestion for your consideration.
I'd also link this PR with the PostgreSQL issue. Will this fix it out of the box?
activerecord/lib/active_record/encryption/encrypted_attribute_type.rb
Outdated
Show resolved
Hide resolved
djmb
force-pushed
the
encrypt-binary-columns
branch
2 times, most recently
from
1b296d6
to
16330f1
Compare
|
Thanks @jorgemanrubia! There isn't a Rails issue for this - one was raised on the SolidCache repo though (rails/solid_cache#102) |
jeremy
approved these changes
Feb 14, 2024
| @@ -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 reserialize(value) | |||
| if cast_type.binary? | |||
There was a problem hiding this comment.
Is this the lone case needing reserialization?
There was a problem hiding this comment.
Text and binary columns are the only ones able to store the encrypted JSON blobs.
I checked whether we could always serialize here instead, in the hope that it would be a no-op for String columns, but type serialization is much more complicated than that - it might be converting serialized or store values for instance.
I've changed this in 38b7df7, now we just wrap strings as Binary::Data, as that's actually reversing what the encryptor does. I've also renamed the method as "serialize" is overloaded with meanings here.
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.
djmb
force-pushed
the
encrypt-binary-columns
branch
from
684d86f
to
085af11
Compare
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.
djmb
force-pushed
the
encrypt-binary-columns
branch
from
085af11
to
38b7df7
Compare
Ridhwana
pushed a commit
to Ridhwana/rails
that referenced
this pull request
Mar 7, 2024
* 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.
viralpraxis
pushed a commit
to viralpraxis/rails
that referenced
this pull request
Mar 24, 2024
* 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Background
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.
Detail
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.