Unverified Commit 165b8f4f authored by Benjamin Neff's avatar Benjamin Neff
Browse files

Don't encrypt the OTP secret 

It doesn't add any security to have this encrypted, but it adds
complexity for podmins, because they need to backup the key.

closes #8014
parent 2d23a260

.gitignore

+0 −1
Original line number Diff line number Diff line
@@ -11,7 +11,6 @@ app/assets/images/custom/ 
# Configuration files

config/diaspora.yml

config/initializers/secret_token.rb

config/initializers/twofa_encryption_key.rb 

.bundle

vendor/bundle/

vendor/cache/

app/models/user.rb

+1 −2
Original line number Diff line number Diff line
@@ -19,11 +19,10 @@ class User < ApplicationRecord 
  scope :halfyear_actives, ->(time = Time.now) { logged_in_since(time - 6.month) }

  scope :active, -> { joins(:person).where(people: {closed_account: false}) }



  attribute :otp_secret

  attr_encrypted :otp_secret, if: false, prefix: "plain_"



  devise :two_factor_authenticatable,

         :two_factor_backupable,

         otp_secret_encryption_key:  AppConfig.twofa_encryption_key,

         otp_backup_code_length:     16,

         otp_number_of_backup_codes: 10

db/migrate/20171229175654_add_devise_two_factor_to_users.rb

+7 −5
Original line number Diff line number Diff line
@@ -2,10 +2,12 @@ 


class AddDeviseTwoFactorToUsers < ActiveRecord::Migration[5.1]

  def change

    add_column :users, :encrypted_otp_secret, :string

    add_column :users, :encrypted_otp_secret_iv, :string

    add_column :users, :encrypted_otp_secret_salt, :string

    add_column :users, :consumed_timestep, :integer

    add_column :users, :otp_required_for_login, :boolean

    change_table :users, bulk: true do |t|

      t.string :encrypted_otp_secret

      t.string :encrypted_otp_secret_iv

      t.string :encrypted_otp_secret_salt

      t.integer :consumed_timestep

      t.boolean :otp_required_for_login

    end

  end

end

db/migrate/20190511150503_decrypt_two_factor_secret.rb

0 → 100644
+52 −0
Original line number Diff line number Diff line 
# frozen_string_literal: true



class DecryptTwoFactorSecret < ActiveRecord::Migration[5.1]

  class User < ApplicationRecord

  end



  def up

    add_column :users, :plain_otp_secret, :string



    key = twofa_encryption_key

    decrypt_existing_secrets(key) if key



    change_table :users, bulk: true do |t|

      t.remove :encrypted_otp_secret

      t.remove :encrypted_otp_secret_iv

      t.remove :encrypted_otp_secret_salt

    end

  end



  def down

    raise ActiveRecord::IrreversibleMigration

  end



  private



  def twofa_encryption_key

    if AppConfig.heroku?

      ENV["TWOFA_ENCRYPTION_KEY"]

    else

      key_file = File.expand_path("../../config/initializers/twofa_encryption_key.rb", File.dirname(__FILE__))



      if File.exist? key_file

        require key_file

        File.delete(key_file)



        return Diaspora::Application.config.twofa_encryption_key

      end

    end

  end



  def decrypt_existing_secrets(key)

    User.where.not(encrypted_otp_secret: nil).each do |user|

      user.plain_otp_secret = Encryptor.decrypt(

        value: user.encrypted_otp_secret.unpack("m").first,

        key:   key,

        iv:    user.encrypted_otp_secret_iv.unpack("m").first,

        salt:  user.encrypted_otp_secret_salt.slice(1..-1).unpack("m").first

      )

      user.save!

    end

  end

end

lib/configuration_methods.rb

+0 −18
Original line number Diff line number Diff line
@@ -68,24 +68,6 @@ module Configuration 
      end

    end



    def twofa_encryption_key

      if heroku?

        return ENV["TWOFA_ENCRYPTION_KEY"] if ENV["TWOFA_ENCRYPTION_KEY"]



        warn "FATAL: Running on Heroku with TWOFA_ENCRYPTION_KEY unset"

        warn "       Run heroku config:add TWOFA_ENCRYPTION_KEY=#{SecureRandom.hex(32)}"

        abort

      else

        key_file = File.expand_path(

          "../config/initializers/twofa_encryption_key.rb",

          File.dirname(__FILE__)

        )

        system "DISABLE_SPRING=1 bin/rake generate:twofa_key" unless File.exist? key_file

        require key_file

        Diaspora::Application.config.twofa_encryption_key

      end

    end



    def version_string

      return @version_string unless @version_string.nil?