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

Change the default from SHA1 #77

Closed
devnul3 opened this issue Feb 24, 2017 · 10 comments
Closed

Change the default from SHA1 #77

devnul3 opened this issue Feb 24, 2017 · 10 comments

Comments

@devnul3
Copy link

devnul3 commented Feb 24, 2017

SHA1 has been demonstrated to have collisions in the wild (https://security.googleblog.com/2017/02/announcing-first-sha1-collision.html), the default should be changed to e.g. SHA256

@untitaker
Copy link
Contributor

in the wild

That particular aspect is incorrect, but I agree we should change the default.

@davidism
Copy link
Member

davidism commented Feb 24, 2017

Are we okay with SHA-256? Should be fine, just want to make sure. Could also just go all the way to SHA-512.

@devnul3
Copy link
Author

devnul3 commented Feb 24, 2017

SHA-512 performs 80 rounds with a blocksize of 1024 bits, whilst SHA-256 performs 64 rounds with a blocksize of 512 bits. This leads to SHA-512 being faster on 64-bit machines for message lengths of more than a few hundred bits.

There are potential legacy client support issues with SHA-512 vs SHA-256, but performance at least should not be a reason to only use SHA-256 (and in fact may be a reason to use SHA-512).

@davidism
Copy link
Member

Hmm, looks like this already came up before in #66, which I closed because hash security isn't representative of HMAC security. Is there actually any issue with SHA-1 as it's used in this library?

@devnul3
Copy link
Author

devnul3 commented Feb 24, 2017

Looks like you're right about that, closing.

@devnul3 devnul3 closed this as completed Feb 24, 2017
@davidism
Copy link
Member

See this chat transcript for more discussion: http://chat.stackoverflow.com/transcript/message/35836840#35836840

@untitaker
Copy link
Contributor

I would vote to still upgrade to sha256 for added security.

@davidism
Copy link
Member

Unfortunately, I'm out of my element here. Is it actually adding any security as opposed to just taking longer to generate longer hashes? I emailed the cryptography-dev mailinglist to see if some devs with more experience in this area could weigh in. (Hopefully that was the right place to ask.)

@untitaker
Copy link
Contributor

With current knowledge it doesn't change anything, but it just seems generally safer should that knowledge change.

@davidism
Copy link
Member

See #80, I'm upgrading this to SHA-512 after more discussion.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants