attr_encrypted is incompatible with the upcoming Rails 7 ActiveRecord::Encryption API

[dark-panda]

Both the upcoming Rails 7 ActiveRecord::Encryption module and attr_encrypted use a class attribute named encrypted_attributes which leads to all sorts of issues when you use attr_encrypted on any Rails 7 model.

I have a proof-of-concept which changes devise-two-factor to use the new Rails 7 API and drops attr_encrypted entirely. For the POC, I've changed the devise-two-factor interface a little to use things like the Rails 7 encryption options sort of configuration for consistency, but as a result the POC is incompatible with the current configuration options interface, but perhaps it might be a useful starting point for a more fleshed out solution.

[mattrasband]

It looks like this bit will work since the methods will be defined, but the gemspec is defensive with < 6.2. I’ll try removing the restriction tomorrow in my fork and try to comment back if that’s enough.

[dark-panda]

Here's what we did to make this work. We've back ported the Rails encryption infrastructure into Rails 6.1 as we were doing some similar work on encrypted structures and this being merged for Rails 7 was a happy coincidence that we decided to take advantage of. We migrated devise-two-factor to the new encryption mechanism without issue using this patch:

master...cybersecuricy:securicy-activerecord-encryption-rails-6-1

We've only tested this with our back port and Rails 6.1, but it has been working without issue so far.

[shaneshort]

Just checking if there's any movement on this one, it looks like carbidesecure@e409e20 might be suitable to drop in place?

[mxdavis]

👀

[jason-hobbs]

I tried using the securicy-fixed-rails-7 and it won't save the otp_secret. User.generate_otp_secret works but it won't save the user attribute. No error but when I check the value of user.otp_secret it is nil

[dark-panda]

@jason-hobbs hm, it's working for us, and we've been using our securicy-fixes-rails-6-1 branch in production using an extraction of the Rails 7 encryption infrastructure for Rails 6.1 for months now. I just checked on our Rails 7 branch and our User#otp_secret values are decrypting as expected. Here's a session from our console:

>> puts Rails.version
7.0.1
>> user = User.first
>> user.otp_secret = User.generate_otp_secret
>> user.save!
>> user = User.first
>> puts user.otp_secret
KVIMFHNIS5Z5WEOUJU2US6YX
>> puts user.attributes_before_type_cast['otp_secret']
{"p":"kf2Ja9hAUEAXx4udzoMnzlSB98SABuj9","h":{"iv":"E4FB4eUSx3eDFKje","at":"MAQzYmGAzx74LDNroFMuYA==","e":"QVNDSUktOEJJVA=="}}

The specs also all run in our securicy-fixes-rails-7 branch.

% bundle exec rspec

Randomized with seed 48138
.......................................

Finished in 16.43 seconds (files took 2 seconds to load)
39 examples, 0 failures

[jason-hobbs]

It is really strange, rails 6 works fine but here are my steps:
gemfile:
gem 'devise-two-factor', git: 'https://github.com/cybersecuricy/devise-two-factor.git', branch: 'securicy-fixes-rails-7'
Then I ran:
bin/rails db:encryption:init
I added those keys to my credentials.

It looks like the rails6 version is using encrypted_otp_secret:
<User id: 1, email: "admin@example.com", created_at: "2021-10-19 22:36:04.281187000 -0500", updated_at: "2022-01-26 12:25:44.483882000 -0600", admin: true, username: "Admin User", slug: "admin-user", assignee: true, avatar_file_name: nil, avatar_content_type: nil, avatar_file_size: nil, avatar_updated_at: nil, disabled: false, avatar: nil, dark: false, authentication_token: nil, readnews: "2022-01-26 11:42:44.570260000 -0600", encrypted_otp_secret: [FILTERED], encrypted_otp_secret_iv: [FILTERED], encrypted_otp_secret_salt: [FILTERED], consumed_timestep: nil, otp_required_for_login: [FILTERED], otp_backup_codes: [FILTERED], phonenum: nil, noemail: false, otp_secret: nil>

the rails7 version seems to be ignoring all encrypted* fields

[jason-hobbs]

Could it be because of old bcrypt dependency? here is my devise.rb file:

Devise.setup do |config|
  config.warden do |manager|
    manager.default_strategies(:scope => :user).unshift :two_factor_authenticatable
    manager.default_strategies(:scope => :user).unshift :two_factor_backupable
  end

  require 'devise/orm/active_record'

  config.authentication_keys = [:login]

  config.case_insensitive_keys = [:email]

  config.strip_whitespace_keys = [:email]

  config.skip_session_storage = [:http_auth]

  config.stretches = Rails.env.test? ? 1 : 10

  config.confirm_within = 3.days

  config.reconfirmable = true

  config.confirmation_keys = [:email]

  config.remember_for = 1.year

  config.expire_all_remember_me_on_sign_out = true

  config.password_length = 10..72

  config.email_regexp = /\A[^@]+@[^@]+\z/

  config.lock_strategy = :failed_attempts

  config.unlock_keys = [:login]

  config.unlock_strategy = :email

  config.maximum_attempts = 15

  config.unlock_in = 1.hour

  config.reset_password_keys = [:email]

  config.reset_password_within = 2.hours

  config.scoped_views = true

  config.sign_out_via = :delete

end
 

Here is the top of my user.rb model:

devise :two_factor_authenticatable, :two_factor_backupable,
         :otp_secret_encryption_key => Rails.application.credentials.OTP_KEY
  devise :registerable, :async,
         :recoverable, :rememberable, :trackable, :validatable,
         :confirmable, :lockable, :masqueradable

[dark-panda]

@jason-hobbs okay, so for existing projects, there is some legwork you'll have to do. First off, you'd need to migrate the existing encrypted_otp_secret values to the new internal Rails encrypted formats, which basically involves decrypting the encrypted_otp_* fields and putting that into an otp_secret field. You'll need to create a new column in your users table for that, because the built-in Rails 7 encryption infrastructure just decrypts the values in place -- there is no separate encrypted_ATTRIBUTE_NAME column and then a plaintext ATTRIBUTE_NAME attribute. You would then drop the old encrypted_otp_secret column once it's no longer needed. You also don't need multiple fields for the IV and salt and that sort of thing, because the new Rails 7 encryption scheme puts it all into a single JSON structure within the encrypted field.

Something like the following would work:

class MigrateOtpCodes < ActiveRecord::Migration[7.0]
  def up
    add_column :users, :otp_secret, :text

    User.reset_column_information

    User.where.not(encrypted_otp_secret: nil).find_each do |user|
      user.update!(
        otp_secret: Encryptor.decrypt(
          Base64.decode64(user.encrypted_otp_secret),
          key: <the encryption key you used for devise-two-factor>,
          iv: user.encrypted_otp_secret_iv.unpack1('m'),
          salt: user.encrypted_otp_secret_salt.slice(1..-1).unpack1('m')
        )
      )
    end
  end

  def down
    remove_column :users, :otp_secret
  end
end

That will migrate the existing data into the new Rails 7 encryption format. At that point you can drop the old encrypted columns and you're done. Note that you need to include the encryptor gem for Encryptor, which is what the encrypted attributes use in devise-two-factor.

This worked for us, and has been working in both Rails 6.1 using the aforementioned extraction gem I posted above and appears to be working in Rails 7, which we are in the process of evaluating for eventual use.

[jason-hobbs]

That worked perfectly! Thank you so much! I will use your branch for now and hopefully they merge it soon!

[dwilkie]

@dark-panda this is perfect. Do you mind opening a PR for this?

[dark-panda]

I could write up a PR for this, but I think some documentation would have to be added to include the details on migrating your existing data from Rails pre-7.0, as per the migration code above. It would perhaps help to have a generator for a migration that would help guide a migration. There would also likely have to be some kind of major version upgrade -- as it exists now, my branch does not care about backwards compatibility at all, and works only with Rails 7+. Some good documentation would have to be in place to make sure people realize this, so if I were to go a head with a PR we should discuss some of these issues so we can work out how to move forward with Rails 7+.

[Jonakemon]

@dark-panda are you in touch with the guys from Tinfoil about this or working on a PR? If not, I can see if I can give it a shot. If you are working on it, let me know if I can help out!

[elsurudo]

I tried using the securicy-fixed-rails-7 and it won't save the otp_secret. User.generate_otp_secret works but it won't save the user attribute. No error but when I check the value of user.otp_secret it is nil

I'm having the same issue as you with the @jason-hobbs – I'm wondering if you can help.

Mine is a brand new implementation of 2FA so I don't need to handle any legacy token migration. However, the encrypted attr otp_secret is not saving to my DB (no errors or anything else).

I am wondering about this:

devise :two_factor_authenticatable, :two_factor_backupable,
         :otp_secret_encryption_key => Rails.application.credentials.OTP_KEY

Where does Rails.application.credentials.OTP_KEY come from, and why is it needed? I though that since we are using Rails' encrypted attributes support, it's enough to add the results of rails db:encryption:init to yours credentials file, and that's it? Or does Rails.application.credentials.OTP_KEY actually refer to one of those values?

Currently I'm just doing this:

devise :two_factor_authenticatable,
         :otp_secret_encryption_key => ENV['TWO_FACTOR_AUTH_ENCRYPTION_KEY']

where ENV['TWO_FACTOR_AUTH_ENCRYPTION_KEY'] is a totally unrelated key I generated.

[elsurudo]

Ah, so I indeed needed to modify it to this to get it working:

devise :two_factor_authenticatable,
       :otp_secret_encryption_key => Rails.application.credentials.active_record_encryption.primary_key

[dark-panda]

@Jonakemon i haven’t been in touch with anyone besides what’s here in this thread. Any and all help and discussion would be appreciated, as there would need to be decisions made about proceed, backwards compatibility, migrations, etc. These sorts of things are perhaps outside of what I’m capable of deciding without some discussion and direction. What I can say is that the patches provided have been running in production for us for over 8 months or so now without issue, but ours is but a single use-case and some care needs to be given to roll it all out to a wider audience.

[elsurudo]

IMO it's more important that this gem works with the latest version of rails than that it has documentation about migration, etc. The major version can be bumped according to semver, and the migration procedure docs can be added later, for example (with a quick note and link to this thread in the interim).

But I'm not in any way associated with the project 🤷

FWIW I'm using https://github.com/cybersecuricy/devise-two-factor/tree/securicy-fixes-rails-7 (rebased onto the latest master), although not yet in prod.

[agvidas]

Hi there, I ran into various issues migrating from rails 6.1 to 7.0 as well. I've tried the migration mentioned here #192 (comment) and it worked with this branch: https://github.com/cybersecuricy/devise-two-factor/tree/securicy-fixes-rails-7

However, I was not able to replicate the success across multiple other environments, here's what seemed to work for me in the rails console:

# Given small amounts of users of 250

UserStruct = Struct.new(:id, :otp_secret, keyword_init: true)

users_secret = User.where.not(encrypted_otp_secret: nil).map do |user|
  UserStruct.new(id: user.id, otp_secret: user.otp_secret)
end

# Save data just in case
# data = Marshal.dump(users_secret)
# users_secret = Marshal.load(data)

# Give you have a broken migration that currently fails
# Migrate up 20220304142415
migration_version = "20220304142415"
ActiveRecord::Migration.add_column :users, :otp_secret, :text
ActiveRecord::SchemaMigration.create!(version: migration_version)

# Migrate down
# ActiveRecord::Migration.remove_column :users, :otp_secret
# ActiveRecord::SchemaMigration.find_by(version: migration_version).destroy

User.reset_column_information

users_secret.each do |user_secret|
  user = User.find_by_id(user_secret.id)
  user.update!(otp_secret: user_secret.otp_secret)
rescue 
  pp "User update failed: #{user_secret.id}, secret: #{user_secret.otp_secret}"
end

That being done, am I compromising anything doing this, or is it fine? Excluding the fact it's not a good idea to run schema/ data migration from console 🙈

[r7kamura]

What about making changes to attr_encrypted gem to make it compatible with activerecord 7?

• Rename encrypted_attributes to attr_encrypted_attributes attr-encrypted/attr_encrypted#399

[andyjeffries]

There's a branch by Cybersecuricy that works with Rails 7, but it doesn't include the recent CVE fix. I've commented on the recent PR for Rails 7 support, but I don't know if that will get seen. #204

[kzkn]

@r7kamura it works for me

[elsurudo]

What about making changes to attr_encrypted gem to make it compatible with activerecord 7?

On the other hand, if rails/AC supports encrypted attrs natively now, perhaps it's simply worth using that functionality and removing the dependency on attr-encrypted altogether?

[mtmail]

Anybody using the https://github.com/cybersecuricy/devise-two-factor/tree/securicy-fixes-rails-7 fork who want to switch to mainline devise-two-factor version 5.0 again (which merged a similar/improved PR for the same functionality)

In your model you probably call
devise :two_factor_authenticatable, otp_secret_encryption_key: 'some_secret_value'
Change that to
devise :two_factor_authenticatable
and instead add this to your config/initializers/devise.rb:
config.otp_encrypted_attribute_options = { key: 'some_secret_value' }