Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependency devise-two-factor to v6 [SECURITY] #31957

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Sep 18, 2024

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
devise-two-factor 5.1.0 -> 6.0.0 age adoption passing confidence

GitHub Vulnerability Alerts

CVE-2024-8796

Summary

Under the default configuration, Devise-Two-Factor versions >= 2.2.0 & < 6.0.0 generate TOTP shared secrets that are 120 bits instead of the 128-bit minimum defined by RFC 4226. Using a shared secret shorter than the minimum to generate a multi-factor authentication code could make it easier for an attacker to guess the shared secret and generate valid TOTP codes.

Remediation

Devise-Two-Factor should be upgraded to version v6.0.0 as soon as possible. After upgrading, the length of shared secrets and TOTP URLs generated by the library will increase since the new shared secrets will be longer.

If upgrading is not possible, you can override the default otp_secret_length attribute in the model when configuring two_factor_authenticable and set it to a value of at least 26 to ensure newly generated shared secrets are at least 128-bits long.

After upgrading or implementing the workaround, applications using Devise-Two-Factor may wish to migrate users to the new OTP length to provide increased protection for those accounts. Turning off OTP for users by setting otp_required_for_login to false is not recommended since it would leave accounts unprotected. However, you may wish to implement application logic that checks the length of a user's shared secret and prompts users to re-enroll in OTP.

Background

Devise-Two-Factor uses ROTP to generate shared secrets for TOTP. In ROTP < 5.0.0, the first argument to the ROTP::Base32#random_base32 function represented the number of bytes to read from SecureRandom which were then returned as a base32-encoded string. In ROTP 5.1.0, this function was changed so that the first argument now represents the length of the base32-encoded string returned by the function instead of the number of bytes to read from SecureRandom resulting in a shorter key being generated for the same input value. (mdp/rotp@c6c24ab).


Release Notes

tinfoil/devise-two-factor (devise-two-factor)

v6.0.0

Compare Source


Configuration

📅 Schedule: Branch creation - "" (UTC), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR was generated by Mend Renovate. View the repository job log.

@renovate renovate bot added dependencies Pull requests that update a dependency file ruby Pull requests that update Ruby code labels Sep 18, 2024
@ClearlyClaire
Copy link
Contributor

Running the two_factor_backupable shared example provided by devise-two-factor now fails with ActiveRecord::NotNullViolation, but I'm not entirely sure why. This might be because of devise-two-factor/devise-two-factor@d63af07

@oneiros
Copy link
Contributor

oneiros commented Sep 19, 2024

Running the two_factor_backupable shared example provided by devise-two-factor now fails with ActiveRecord::NotNullViolation, but I'm not entirely sure why. This might be because of devise-two-factor/devise-two-factor@d63af07

Yes, that is exactly why. Since we did not supply an explicit subject the shared examples used an implicit one. And that is just an "empty" User instance that cannot be saved.

I just pushed what I consider to be a minimally viable fix.

Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me. Though I wonder if the switch to save! could have ill effects.

We will also have to consider how to handle the underlying issue (existing OTP secrets have slightly less entropy than they should have), and how to handle the stable branches.

Copy link
Contributor Author

renovate bot commented Sep 19, 2024

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@Gargron Gargron added this pull request to the merge queue Sep 19, 2024
Merged via the queue into main with commit 6801afa Sep 19, 2024
31 checks passed
@Gargron Gargron deleted the renovate/rubygems-devise-two-factor-vulnerability branch September 19, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants