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 README.rst #60

Closed
wants to merge 1 commit into from
Closed

Conversation

paragonie-scott
Copy link

See #8

@charmander
Copy link

This doesn’t matter for bcrypt.

@paragonie-scott
Copy link
Author

This doesn’t matter for bcrypt.

Is that sufficient reason to reject it?

@alex
Copy link
Member

alex commented Feb 15, 2016

My perspective is that while it's cryptographically pointless, it might promote more conscientiousness in people writing crypto which would be a good thing. @reaperhulk ?

@charmander
Copy link

I’m usually against adding complexity to no benefit, but it’s up to the maintainers, of course.

While it’s true that adding this might prompt people to check how they’re comparing other important values, it’s still a bit misleading….

@reaperhulk
Copy link
Member

@charmander's point is well-taken (and we have rejected unneeded constant time comparisons in our x509 extension layer for cryptography in the past, although that particular example significantly increased complexity), but training lay-people to assume that constant time comparison is The Way™ to do comparisons of cryptographically sensitive data is probably a win.

I don't feel strongly about this either way, but I slightly lean towards saying we should leave it as-is since timing attacks are not a concern here.

@reaperhulk reaperhulk closed this May 23, 2016
@paragonie-scott paragonie-scott deleted the patch-1 branch May 23, 2016 19:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants