-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-103256: Fix hmac algorithm to support fallback implementation #103286
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
Conversation
Would you like to take a look at this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! somewhat surprised we hadn't run into this at work, but I guess there isn't much demand for hmac-sha3 yet. the other folks who might hit this probably have openssl builds without md5 or sha1?
It looks so, and the latter looks minor case in this era :) |
Thanks @gpshead for reviewing and supplementing this PR! |
Thanks @corona10 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
GH-103328 is a backport of this pull request to the 3.11 branch. |
pythongh-103286) (cherry picked from commit efb0a2c) Co-authored-by: Dong-hee Na <donghee.na@python.org> Co-authored-by: Gregory P. Smith <greg@krypto.org>
SHA3 is fine since OpenSSL (at least in Python 3.11.2) provides it;
The only hash algorithms that were a problem are ones that OpenSSL doesn't provide via named lookup. So none of the built-in algorithms would trigger it as OpenSSL provides all of them (even whirlpool, mdc2, and the composite md5-sha1 work as expected) - that's why it took me (ab)using hashlib to find it. But patching hashlib (directly in the file or on the fly) to include a new algorithm does trigger it (as that's what I was doing). But you're right that if md5 or sha1 were removed from OpenSSL while still being kept in hashlib (e.g. with a stand-alone CPython implementation), then the problem would get triggered. |
Not everyone's OpenSSL provides it. 1.1.1 and BoringSSL do not. |
pythongh-103286) Co-authored-by: Gregory P. Smith <greg@krypto.org>
Uh oh!
There was an error while loading. Please reload this page.