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

Deprecate use of SHA1 #147

Closed
nbraud opened this issue Mar 29, 2020 · 5 comments
Closed

Deprecate use of SHA1 #147

nbraud opened this issue Mar 29, 2020 · 5 comments

Comments

@nbraud
Copy link

nbraud commented Mar 29, 2020

Please consider deprecating the use of SHA1 as the default hashing algorithm.

While HMAC-SHA1 is not yet known to be insecure, significant weaknesses have been known in SHA1 since 2015, with collisions discovered in early 2017 (“SHAttered”), and improved attacks in early 2019 and 2020. As such, continued use of SHA1 is a risk.
TL;DR: SHA1 has been considered broken for half-a-decade, please stop using it in applications that are expected to be secure.

While I'm aware this was attempted before and rolled back (#80) because of a compatibility break, this could be done as part of the next major release, which will drop compatibility with Python 2.7 and 3.5.

Given that Python 3.6 introduced support for more-modern hash functions (BLAKE2 and SHA3/SHAKE), I would strongly recommend replacing HMAC-SHA1 with BLAKE2b for the following reasons:

  • BLAKE2 (like SHA3 and SHAKE) is strongly believed to be secure in the long-term;
  • used as a keyed hash, it is already a MAC and so doesn't need to be wrapped in HMAC (less error-prone, and an easy 2× speedup);
  • it supports personalisation, i.e. deriving a different hash function family for different applications; for instance, setting the personalisation string to palletsprojects.com/p/itsdangerous (or any other string that unambiguously designates the library) hedges against users reusing the same key for another purpose.
@davidism
Copy link
Member

Please see previous closed discussions about this. SHA-1 (and even MD5) are not vulnerable when used in HMAC.

@nbraud
Copy link
Author

nbraud commented Apr 4, 2020

@davidism As mentioned initially, I'm aware there is no known attack on HMAC-SHA1; however:

  • SHA1 itself being broken invalidates all security arguments for HMAC-SHA1 (in particular, security arguments for HMAC rely on the underlying hash being a PRF);
  • lack of publicly-known attack is not an indicator of security;
  • since SHA1 is considered to be completely broken, the only people with an incentive to find an attack on HMAC-SHA1 are the ones interested in exploiting it.

Moreover, a new major release seemed like the best time to suggest breaking changes that would improve both performance and long-term security.

However, if there is no interest in either of those, I guess I can/will simply replace itsdangerous from applications whose long-term security I rely on.

@untitaker
Copy link
Contributor

untitaker commented Apr 4, 2020

security arguments for HMAC rely on the underlying hash being a PRF

I just want to point out that this paper was written in response to an older paper that would imply that HMAC-SHA1 is not a PRF and literally says the opposite of what you're claiming. It says that SHA1 does NOT NECESSARILY need to be weakly collision resistant in order for HMAC-SHA1 to be secure (same claim for MD5), so it weakens the requirements for HMAC to be a PRF compared to the earlier paper.

@untitaker
Copy link
Contributor

btw generally speaking I agree we should eventually upgrade but the urgency is nowhere near as you make it out to be.

@davidism
Copy link
Member

davidism commented Apr 4, 2020

Hash security isn't representative of HMAC security. Discussion in SO Python chat: https://chat.stackoverflow.com/transcript/message/35836840#35836840. Discussion in cryptography mailing list: https://mail.python.org/pipermail/cryptography-dev/2017-March/000737.html. If you'd like to re-discuss it on the cryptography mailing list, and they change their conclusion, I'm willing to reconsider.

In #80 I changed it to SHA-512. That was for optics, not security, as discussed in the two links above. However, this caused compatibility issues that invalidated existing tokens, and was rolled back in #113. SHA-512 significantly increases the size of the tokens, which are often used in links or cookies, so if it's not necessary then it's not ideal to upgrade for space reasons as well.

Regardless, if a project assess that our defaults don't meet their risk requirements, it has always been possible to override the hash and digest methods. #113 also added fallback methods to upgrade old tokens. There's no need to suggest that ItsDangerous should be dropped.

@pallets pallets locked as resolved and limited conversation to collaborators Aug 22, 2020
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