Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
On the 7.x-1.x branch in function _encrypt_decrypt()
there is a variable $key
which should contain the value stored in the secured key file.
Unfortunately, this is never used, and therefore never makes it to encrypt_encrypt_mcrypt_rij_256()
and therefore falls back on the drupal_private_key
variable. The encryption appears to work, except it is using the wrong key.
This breaks upgrades from Drupal 6 as the previous encryption key is useless and the encrypted data is unreadable.
Comment | File | Size | Author |
---|---|---|---|
encrypt_key_is_never_used.patch | 1.29 KB | mstrelan | |
Comments
Comment #1
gregglesThe 7.x-1.x tests are broken. Maybe you could help fix them?
Alternately, maybe try the 7.x-2.x branch?
I have an issue at #1867114: if the key can't be read throw an exception that might also interest you. Right now I feel like the module is a bit too graceful about dealing with errors in getting the key. My sense is it should quit with a massive error if it can't get the key. Thoughts?
Comment #2
mstrelan CreditAttribution: mstrelan commentedSorry, I haven't used this module enough to really weigh in on this. I don't know what other modules are out there that would be affected by this, but yes I would assume that an error message would be appropriate in that situation. In my case I'm using Webform Encrypted Components sandbox project and it would certainly make sense to throw an error if the component data couldn't be encrypted correctly.
Comment #3
amar.deokar CreditAttribution: amar.deokar commentedYes this is critical bug and above patch should be reflected in dev version.
Comment #4
Jons CreditAttribution: Jons commentedNoticed same issue (using 7.x-1.1). Thanks for patch. Tests all succeeded.
Comment #5
Zooney CreditAttribution: Zooney commentedI noticed this while attempting to use 7.x-1.1 for a module I was building.
The patch seems to work.
Comment #6
Cellar Door CreditAttribution: Cellar Door commentedZooney -
Have you investigated the 2.x branch yet? We're working on getting a stable release out shortly that should address many of these issues with the key and the default key.
Comment #7
Zooney CreditAttribution: Zooney commentedHi Cellar Door,
I have looked at the beta 2.x branch. I was a little concerned about the lack of guarantee for the beta to alpha branch upgrade.
Though since I'm just making calls to encrypt() and decrypt() I'm just going to keep developing using 1.x. The module I'm developing will work with 1.x or 2.x, it's just a matter of the security of the encryption.
I hope you get the new release out soon. Easy two-way encryption is something Drupal really needs. =)
Comment #8
Cellar Door CreditAttribution: Cellar Door commentedZooney - We're working to make sure the update from 1.x to 2.x is one that is a smooth transition. I'd take a look, if you're wanting a more future friendly solution this is a great way and using the existing beta branch will not prevent you from using the future alpha and RC releases (we'd actually love to make sure it DOESN'T break your site in the process).
If you're up for it, give it a shot. Using encrypt/decrypt gets much easier in 2.x since you're not having to detail many of the parameters you would in 1.x along with it.
Comment #9
Zooney CreditAttribution: Zooney commentedOkay, sounds good. I've gone ahead and switched over to the 2.x beta.
Comment #10
heddnThe patch (while it should probably use an elseif), does work. Marking as RTBC. We can refactor code later. Let's just get this committed.
Why not an elseif?
Comment #11
greggles@heddn - all of our focus is on the 7.x-2.x branch. I don't use the 7.x-1.x and from my experience watching the queue there are a variety of things broken about it. I suggest you use and help get 7.x-2.x released.
Comment #13
rlhawkThis is committed, though we are dropping support for the 1.x version of Encrypt.