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

Timing attack fix #16

Merged
merged 4 commits into from
May 10, 2020
Merged

Timing attack fix #16

merged 4 commits into from
May 10, 2020

Conversation

Vlix
Copy link
Collaborator

@Vlix Vlix commented May 7, 2020

Added in the constEq anywhere hashes are being compared.

Also cleaned up tests, since with low rounds we can just run 100 on the property tests.

password/ChangeLog.md Outdated Show resolved Hide resolved
@cdepillabout cdepillabout linked an issue May 8, 2020 that may be closed by this pull request
@maralorn
Copy link

maralorn commented May 8, 2020

I am very much not a security auditor. But yes this looks fine to me.

Co-authored-by: Dennis Gosnell <cdep.illabout@gmail.com>
@Vlix
Copy link
Collaborator Author

Vlix commented May 8, 2020

@maralorn Of course, but just checking that I've adjusted it at the correct location where you found the issue.

@Vlix
Copy link
Collaborator Author

Vlix commented May 10, 2020

So it builds and addresses the issue, shall I just merge and upload later today?

@cdepillabout
Copy link
Owner

@Vlix Yes, please do!

Also, you may want to deprecate version 2.0.0.0 and 2.0.0.1 using the Hackage UI so that people know not to use it.

@Vlix
Copy link
Collaborator Author

Vlix commented May 10, 2020

@cdepillabout because it doesn't have the constEq? By that logic all of the previous versions would have to be deprecated.

@Vlix Vlix merged commit 86a6785 into cdepillabout:master May 10, 2020
@cdepillabout
Copy link
Owner

@Vlix

By that logic all of the previous versions would have to be deprecated.

I'd be fine with that as well.


Although thinking more critically about this, I guess it depends on how big of a problem this is.

Are people likely to have accounts compromised because of not using a constant-time equals? If so, then we should deprecate. If not, then it is probably not a problem, and we don't have to deprecate old versions.

@Vlix Vlix deleted the timing-attack-fix branch September 6, 2020 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

potential timing attack in checkPassword
3 participants