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

gh-117233: Detect libcrypto BLAKE2, Shake, SHA3, and Truncated-SHA512 support at hashlib build time #117234

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

WillChilds-Klein
Copy link
Contributor

@WillChilds-Klein WillChilds-Klein commented Mar 25, 2024

Notes

While OpenSSL supports both "b" and "s" variants of the BLAKE2 hash
function, other cryptographic libraries may lack support for one or both
of the variants. This commit modifies hashlib's C code to detect
whether or not the linked libcrypto supports each BLAKE2 variant, and
elides references to each variant's NID accordingly. In cases where the
underlying libcrypto doesn't fully support BLAKE2, CPython's
./configure script can be given the following flag to use CPython's
interned BLAKE2 implementation: --with-builtin-hashlib-hashes=blake2.

This pull request implements Issue #117233.

@encukou
Copy link
Member

encukou commented Mar 28, 2024

SSL experts @jackjansen, @tiran, @dstufft, @alex: Please chime in if you have any comments.

I plan to start reviewing this next week.

@gpshead gpshead self-assigned this Mar 28, 2024
Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to reference the OPENSSL_NO_BLAKE2 define, which OpenSSL (and BoringSSL) already expose?

Lib/test/test_ssl.py Outdated Show resolved Hide resolved
@gpshead
Copy link
Member

gpshead commented Mar 29, 2024

Would it make more sense to reference the OPENSSL_NO_BLAKE2 define, which OpenSSL (and BoringSSL) already expose?

possibly? though the NID_ constants being #define'd in all the libraries I'm aware of (unknown unknowns territory) makes them very convenient at the lower level API level. do we also have "_NO_" style defines for each of the other sets of things not always supported?

@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 29, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @gpshead for commit bb38e0e 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 29, 2024
@gpshead gpshead changed the title gh-117233: Detect crypto library BLAKE2 support gh-117233: Detect libcrypto BLAKE2, Shake, SHA3, and Truncated-SHA512 support at hashlib build time Mar 29, 2024
WillChilds-Klein and others added 4 commits March 29, 2024 11:08
While OpenSSL supports both "b" and "s" variants of the BLAKE2 hash
function, other cryptographic libraries may lack support for one or both
of the variants. This commit modifies `hashlib`'s C code to detect
whether or not the linked libcrypto supports each BLAKE2 variant, and
elides references to each variant's NID accordingly. In cases where the
underlying libcrypto doesn't fully support BLAKE2, CPython's
`./configure` script can be given the following flag to use CPython's
interned BLAKE2 implementation: `--with-builtin-hashlib-hashes=blake2`.
Detect BLAKE2, SHA3, Shake, & truncated SHA512 support in the
OpenSSL-ish libcrypto library at build time.  This helps allow hashlib's
\_hashopenssl to be used with libraries that do not to support every
algorithm that upstream OpenSSL does.  Such as AWS-LC & BoringSSL.
@gpshead
Copy link
Member

gpshead commented Mar 29, 2024

FYI - please try to avoid force-pushing to PR branches in CPython. https://devguide.python.org/getting-started/pull-request-lifecycle/#quick-guide

Modules/_hashopenssl.c Outdated Show resolved Hide resolved
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now :)

@encukou
Copy link
Member

encukou commented Apr 8, 2024

@gpshead, please shout if you want to review more. Otherwise I'll merge in a few days

@encukou encukou merged commit b8eaad3 into python:main Apr 11, 2024
38 checks passed
@WillChilds-Klein WillChilds-Klein deleted the guard-blake2 branch April 11, 2024 15:17
@WillChilds-Klein
Copy link
Contributor Author

thank you @encukou and @gpshead! would it be possible to backport this to 3.12?

WillChilds-Klein added a commit to WillChilds-Klein/aws-lc that referenced this pull request Apr 11, 2024
This is in light of upstream [PR 117234#][1] being merged.

[1]: python/cpython#117234
samuel40791765 pushed a commit to WillChilds-Klein/aws-lc that referenced this pull request Apr 11, 2024
This is in light of upstream [PR 117234#][1] being merged.

[1]: python/cpython#117234
@gpshead gpshead added the needs backport to 3.12 bug and security fixes label Apr 11, 2024
@miss-islington-app
Copy link

Thanks @WillChilds-Klein for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 11, 2024
…ime (pythonGH-117234)

Detect libcrypto BLAKE2, Shake, SHA3, and Truncated-SHA512 support at hashlib build time

GH-GH- BLAKE2

While OpenSSL supports both "b" and "s" variants of the BLAKE2 hash
function, other cryptographic libraries may lack support for one or both
of the variants. This commit modifies `hashlib`'s C code to detect
whether or not the linked libcrypto supports each BLAKE2 variant, and
elides references to each variant's NID accordingly. In cases where the
underlying libcrypto doesn't fully support BLAKE2, CPython's
`./configure` script can be given the following flag to use CPython's
interned BLAKE2 implementation: `--with-builtin-hashlib-hashes=blake2`.

GH-GH- SHA3, Shake, & truncated SHA512.

Detect BLAKE2, SHA3, Shake, & truncated SHA512 support in the
OpenSSL-ish libcrypto library at build time.  This helps allow hashlib's
`_hashopenssl` to be used with libraries that do not to support every
algorithm that upstream OpenSSL does.  Such as AWS-LC & BoringSSL.

(cherry picked from commit b8eaad3)

Co-authored-by: Will Childs-Klein <willck93@gmail.com>
Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>
@bedevere-app
Copy link

bedevere-app bot commented Apr 11, 2024

GH-117767 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Apr 11, 2024
gpshead added a commit that referenced this pull request Apr 11, 2024
…time (GH-117234) (#117767)

gh-117233: Detect support for several hashes at hashlib build time (GH-117234)

Detect libcrypto BLAKE2, Shake, SHA3, and Truncated-SHA512 support at hashlib build time

GH-GH- BLAKE2

While OpenSSL supports both "b" and "s" variants of the BLAKE2 hash
function, other cryptographic libraries may lack support for one or both
of the variants. This commit modifies `hashlib`'s C code to detect
whether or not the linked libcrypto supports each BLAKE2 variant, and
elides references to each variant's NID accordingly. In cases where the
underlying libcrypto doesn't fully support BLAKE2, CPython's
`./configure` script can be given the following flag to use CPython's
interned BLAKE2 implementation: `--with-builtin-hashlib-hashes=blake2`.

GH-GH- SHA3, Shake, & truncated SHA512.

Detect BLAKE2, SHA3, Shake, & truncated SHA512 support in the
OpenSSL-ish libcrypto library at build time.  This helps allow hashlib's
`_hashopenssl` to be used with libraries that do not to support every
algorithm that upstream OpenSSL does.  Such as AWS-LC & BoringSSL.

(cherry picked from commit b8eaad3)

Co-authored-by: Will Childs-Klein <willck93@gmail.com>
Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>
WillChilds-Klein added a commit to WillChilds-Klein/aws-lc that referenced this pull request Apr 11, 2024
This is in light of upstream [PR 117234#][1] being merged.

[1]: python/cpython#117234
WillChilds-Klein added a commit to WillChilds-Klein/aws-lc that referenced this pull request Apr 11, 2024
This is in light of upstream [PR 117234#][1] being merged.

[1]: python/cpython#117234
@hugovk hugovk mentioned this pull request Apr 11, 2024
27 tasks
WillChilds-Klein added a commit to aws/aws-lc that referenced this pull request Apr 11, 2024
This is in light of upstream [PR 117234#][1] being merged.

[1]: python/cpython#117234
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
…ime (pythonGH-117234)

Detect libcrypto BLAKE2, Shake, SHA3, and Truncated-SHA512 support at hashlib build time

## BLAKE2

While OpenSSL supports both "b" and "s" variants of the BLAKE2 hash
function, other cryptographic libraries may lack support for one or both
of the variants. This commit modifies `hashlib`'s C code to detect
whether or not the linked libcrypto supports each BLAKE2 variant, and
elides references to each variant's NID accordingly. In cases where the
underlying libcrypto doesn't fully support BLAKE2, CPython's
`./configure` script can be given the following flag to use CPython's
interned BLAKE2 implementation: `--with-builtin-hashlib-hashes=blake2`.

## SHA3, Shake, & truncated SHA512.

Detect BLAKE2, SHA3, Shake, & truncated SHA512 support in the
OpenSSL-ish libcrypto library at build time.  This helps allow hashlib's
`_hashopenssl` to be used with libraries that do not to support every
algorithm that upstream OpenSSL does.  Such as AWS-LC & BoringSSL.

Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>
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.

None yet

5 participants