Houdini / two_factor_authentication

Two factor authentication extension for Devise
MIT License
400 stars 270 forks source link

encryptor 3.0.0 release with security fix #75

Open jetheredge opened 8 years ago

jetheredge commented 8 years ago

Right now the gemspec does not lock encryptor down to 2.0.0 and the encryptor gem recently had a release to 3.0.0 because of a security issue. The fix in 3.0.0 seems to break this gem. Here is the text that the encryptor gem has up on their README:

A bug was discovered in Encryptor 2.0.0 wherein the IV was not being used when using an AES--GCM algorithm. Unfornately fixing this major security issue results in the inability to decrypt records encrypted using an AES--GCM algorithm from Encryptor v2.0.0. While the behavior change is minimal between v2.0.0 and v3.0.0, the change has a significant impact on users that used v2.0.0 and encrypted data using an AES--GCM algorithm, which is the default algorithm for v2.0.0. Consequently, we decided to increment the version with a major bump to help people avoid a confusing situation where some of their data will not decrypt. A new option is available in Encryptor 3.0.0 that allows decryption of data encrypted using an AES--GCM algorithm from Encryptor v2.0.0.

It appears this gem is affected by this issue, because even though it specifies 'aes-256-cbc' when getting an iv, it does not specify an algorithm when calling encrypt or decrypt.

Any ideas on the best way around this at this point?

jetheredge commented 8 years ago

After a bit more research, I found this:

The :v2_gcm_iv option is available to allow Encryptor to set the IV as it was set in Encryptor v2.0.0. This is provided to assist with migrating data that unsafely encrypted using an AES-*-GCM algorithm from Encryptor v2.0.0.

So it looks like a task could be created which goes through and decrypts passing the v2_gcm_iv: true and then encrypts it passing v2_gcm_iv: false (or leaving it off). If there was some way to affect the options at the user level, it seems like it would be as easy as getting the value of otp_secret_key, turning the option off and then assigning the value back.

jetheredge commented 8 years ago

I wanted to ping you on this again, I'm happy to try to put together a pull request for this, let me know if you're willing to accept a pull request for this issue so that we don't duplicate effort.

Houdini commented 8 years ago

I will be happy if you can help with pull requests

nikolalsvk commented 7 years ago

Can you help me with migrating data from Encryptor v2 to v3?

I've managed to decrypt encrypted_otp_secret_key like this:

decrypted_secret_otp_key = Encryptor.decrypt({ 
  :value => user.encrypted_otp_secret_key.unpack('m').first, 
  :key => Devise.otp_secret_encryption_key, 
  :iv => user.encrypted_otp_secret_key_iv.unpack('m').first, 
  :salt => user.encrypted_otp_secret_key_salt.unpack('m').first, 
  :v2_gcm_iv => true 
})

I then overwrote encrypted_otp_secret_key in the User model with:

user.update_attributes(
  :encrypted_otp_secret_key => Encryptor.encrypt({ 
    :value => decrypted_secret_otp_key, 
    :key => Devise.otp_secret_encryption_key, 
    :iv => user.encrypted_otp_secret_key_iv.unpack('m').first, 
    :salt => user.encrypted_otp_secret_key_salt.unpack('m').first 
  })
)

But it doesn't work πŸ€” Can you help me out with this one πŸ™ @Houdini @jetheredge?

Houdini commented 7 years ago

@nikolalsvk can I check your full commit, please? Could you give me link to your branch?

nikolalsvk commented 7 years ago

@Houdini The project I'm trying to get this to work is not open source unfortunately 😞

I was trying this in the bundle exec rails console with the User model.

What happens is:

  1. old encrypted_otp_secret_key gets decrypted (I don't get exceptions and a I get a value).
  2. I try to encrypt it, and succeed with writing the ecrypted value to the database
  3. I try to log in with 2FA previously set with v2 Encryptor I get some strange behaviour in my application (I get asked to write 2FA again)

I want to know if this is the legit way of doing decrpytion and encrpytion when migrating data from one version to another?