diff --git a/CHANGELOG b/CHANGELOG index 04896126..c778a2de 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,18 +1,20 @@ === master -* Support hmac_secret rotation in email_base feature (jeremyevans) (#365) +* Support hmac_secret rotation in the otp feature (jeremyevans) (#365) -* Support hmac_secret rotation in webauthn feature (jeremyevans) (#365) +* Support hmac_secret rotation in the email_base feature (jeremyevans) (#365) -* Support hmac_secret rotation in jwt_refresh feature (jeremyevans) (#365) +* Support hmac_secret rotation in the webauthn feature (jeremyevans) (#365) -* Support hmac_secret rotation in single_session feature (jeremyevans) (#365) +* Support hmac_secret rotation in the jwt_refresh feature (jeremyevans) (#365) -* Support hmac_secret rotation in remember feature (jeremyevans) (#365) +* Support hmac_secret rotation in the single_session feature (jeremyevans) (#365) -* Support hmac_secret rotation via hmac_old_secret configuration method in active_sessions feature (jeremyevans) (#365) +* Support hmac_secret rotation in the remember feature (jeremyevans) (#365) -* Support argon2 secret rotation via argon2_old_secret configuration method and update_password_hash feature (jeremyevans) (#365) +* Support hmac_secret rotation via hmac_old_secret configuration method in the active_sessions feature (jeremyevans) (#365) + +* Support argon2 secret rotation via argon2_old_secret configuration method and the update_password_hash feature (jeremyevans) (#365) * Support jwt secret rotation via jwt_old_secret configuration method, if using jwt 2.4+ (jeremyevans) (#365) diff --git a/doc/otp.rdoc b/doc/otp.rdoc index 7c7eee85..27cf6640 100644 --- a/doc/otp.rdoc +++ b/doc/otp.rdoc @@ -86,5 +86,6 @@ otp_remove_auth_failures :: Removes OTP authentication failures for the current otp_setup_view :: The HTML to use for the form to setup OTP authentication. otp_tmp_key(secret) :: Set the secret to use for the temporary OTP key, during OTP setup. otp_update_last_use :: Update the last time OTP authentication was successful for the account. Return true if the authentication should be allowed, or false if it should not be allowed because the last authentication was too recent and indicates the possible reuse of a TOTP authentication code. +otp_valid_code_for_old_secret :: Called when valid OTP authentication is performed using hmac_old_secret. This indicates the OTP needs to be rotated before support for the previous hmac secret value is removed. You can use this to track users who need their OTP rotated, and take appropriate action. otp_valid_code?(auth_code) :: Whether the given code is the currently valid OTP auth code for the account. otp_valid_key?(secret) :: Whether the given secret is a valid OTP secret. diff --git a/lib/rodauth/features/otp.rb b/lib/rodauth/features/otp.rb index 095afa47..0582aeca 100644 --- a/lib/rodauth/features/otp.rb +++ b/lib/rodauth/features/otp.rb @@ -94,7 +94,8 @@ module Rodauth auth_private_methods( :otp_add_key, - :otp_tmp_key + :otp_tmp_key, + :otp_valid_code_for_old_secret ) internal_request_method :otp_setup_params @@ -248,6 +249,17 @@ def otp_exists? end def otp_valid_code?(ot_pass) + if _otp_valid_code?(ot_pass, otp) + true + elsif hmac_secret_rotation? && _otp_valid_code?(ot_pass, _otp_for_key(otp_hmac_old_secret(otp_key))) + _otp_valid_code_for_old_secret + true + else + false + end + end + + def _otp_valid_code?(ot_pass, otp) return false unless otp_exists? ot_pass = ot_pass.gsub(/\s+/, '') if drift = otp_drift @@ -368,9 +380,17 @@ def otp_hmac_secret(key) base32_encode(compute_raw_hmac(ROTP::Base32.decode(key)), key.bytesize) end + def otp_hmac_old_secret(key) + base32_encode(compute_raw_hmac_with_secret(ROTP::Base32.decode(key), hmac_old_secret), key.bytesize) + end + def otp_valid_key?(secret) return false unless secret =~ /\A([a-z2-7]{16}|[a-z2-7]{32})\z/ if otp_keys_use_hmac? + # Purposely do not allow creating new OTPs with old secrets, + # since OTP rotation is difficult. The user will get shown + # the same page with an updated secret, which they can submit + # to setup OTP. timing_safe_eql?(otp_hmac_secret(param(otp_setup_raw_param)), secret) else true @@ -400,6 +420,10 @@ def _otp_tmp_key(secret) @otp_key = secret end + # Called for valid OTP codes for old secrets + def _otp_valid_code_for_old_secret + end + def _otp_add_key(secret) # Uniqueness errors can't be handled here, as we can't be sure the secret provided # is the same as the current secret. @@ -411,8 +435,12 @@ def _otp_key otp_key_ds.get(otp_keys_column) end + def _otp_for_key(key) + otp_class.new(key, :issuer=>otp_issuer, :digits=>otp_digits, :interval=>otp_interval) + end + def _otp - otp_class.new(otp_user_key, :issuer=>otp_issuer, :digits=>otp_digits, :interval=>otp_interval) + _otp_for_key(otp_user_key) end def otp_key_ds diff --git a/spec/two_factor_spec.rb b/spec/two_factor_spec.rb index 635c5ddb..c22c056b 100644 --- a/spec/two_factor_spec.rb +++ b/spec/two_factor_spec.rb @@ -12,11 +12,21 @@ def reset_otp_last_use it "should allow two factor authentication setup, login, recovery, removal" do sms_phone = sms_message = nil hmac_secret = '123' + hmac_old_secret = nil + old_secret_used = false rodauth do enable :login, :logout, :otp, :recovery_codes, :sms_codes hmac_secret do hmac_secret end + hmac_old_secret do + hmac_old_secret + end + otp_valid_code_for_old_secret do + raise if old_secret_used + old_secret_used = true + end + sms_send do |phone, msg| proc{super(phone, msg)}.must_raise NotImplementedError sms_phone = phone @@ -115,7 +125,31 @@ def reset_otp_last_use page.html.must_include 'Invalid authentication code' reset_otp_last_use + hmac_secret = '124' + hmac_old_secret = '125' + fill_in 'Authentication Code', :with=>"#{totp.now[0..2]} #{totp.now[3..-1]}" + click_button 'Authenticate Using TOTP' + page.find('#error_flash').text.must_equal 'Error logging in via TOTP authentication' + page.html.must_include 'Invalid authentication code' + reset_otp_last_use + + otp_auth_path = page.current_path + visit otp_auth_path + old_secret_used.must_equal false + hmac_secret = '333' + hmac_old_secret = '321' + fill_in 'Authentication Code', :with=>"#{totp.now[0..2]} #{totp.now[3..-1]}" + click_button 'Authenticate Using TOTP' + page.find('#notice_flash').text.must_equal 'You have been multifactor authenticated' + page.html.must_include 'With 2FA' + old_secret_used.must_equal true + reset_otp_last_use + + logout + login + visit otp_auth_path hmac_secret = '321' + hmac_old_secret = nil fill_in 'Authentication Code', :with=>"#{totp.now[0..2]} #{totp.now[3..-1]}" click_button 'Authenticate Using TOTP' page.find('#notice_flash').text.must_equal 'You have been multifactor authenticated'