Skip to content

gh-136547: refactor hashlib_helper for blocking and requesting digests #136762

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

Merged
merged 13 commits into from
Jul 20, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Jul 18, 2025

I made some mistakes in my previous PR and the design I introduced for requesting hashes could actually be greatly simplified. As those two are tightly coupled, I've decided to do both the fix & refactoring at the same time.

Because of all build possibilities we can have, whether at runtime or not, and because some functions can't be given usedforsecurity, tests easily fail because of the underlying configurations. Default builds where we don't have some FIPS module are easy to test, but when FIPS mode is enabled, it's hard to know what is blocked and what is not.

One really annoying thing is the treatment of BLAKE-2 which always falls back to the built-in implementations. Thus, blocking BLAKE-2 may be quite hard and quite different. Well, if we block BLAKE-2 and we don't have OpenSSL at all, then tests don't pass because hashlib.py can't even be imported. Anywsay, I'm opening a draft for now and I'll continue working on this tomorrow.

@picnixz picnixz marked this pull request as ready for review July 19, 2025 17:46
@picnixz picnixz requested review from gpshead and tiran as code owners July 19, 2025 17:46
@picnixz picnixz marked this pull request as draft July 19, 2025 17:49
@picnixz picnixz marked this pull request as ready for review July 20, 2025 09:41
@picnixz
Copy link
Member Author

picnixz commented Jul 20, 2025

Urgh, so in my previous PR, I had some bad copy pastes in the wrappers. Fortunately, they were never used until now but my bad.

@picnixz
Copy link
Member Author

picnixz commented Jul 20, 2025

!buildbot FIPS only

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 2b8d5f0 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136762%2Fmerge

The command will test the builders whose names match following regular expression: FIPS only

The builders matched are:

  • AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR
  • AMD64 CentOS9 FIPS Only Blake2 Builtin Hash PR

@picnixz
Copy link
Member Author

picnixz commented Jul 20, 2025

!buildbot FIPS only

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @picnixz for commit f4bf516 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136762%2Fmerge

The command will test the builders whose names match following regular expression: FIPS only

The builders matched are:

  • AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR
  • AMD64 CentOS9 FIPS Only Blake2 Builtin Hash PR

@picnixz picnixz merged commit c504f62 into python:main Jul 20, 2025
42 checks passed
@picnixz picnixz deleted the fix/hashlib/block-hashes-136547 branch July 20, 2025 12:32
picnixz added a commit to picnixz/cpython that referenced this pull request Jul 20, 2025
…ests (python#136762)

- Fix `hashlib_helper.block_algorithm` where the dummy functions were incorrectly defined.
- Rename `hashlib_helper.HashAPI` to `hashlib_helper.HashInfo` and add more helper methods.
- Simplify `hashlib_helper.requires_*()` functions.
- Rewrite some private helpers in `hashlib_helper`.
- Remove `find_{builtin,openssl}_hashdigest_constructor()` as they are no more needed and were
  not meant to be public in the first place.
- Fix some tests in `test_hashlib` when FIPS mode is on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants