GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-127298: When in FIPS mode ensure builtin hashes check for usedforsecurity=False by xnox · Pull Request #127301 · python/cpython

Conversation

@xnox

@xnox xnox commented Nov 26, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

When _hashlib/OpenSSL is available, and OpenSSL is in FIPS mode, ensure that builtin (fallback) hash implementations are wrapped with a check for usedforsecurity=False. It is likely that buitin implementations are FIPS unapproved (either algorithm disallowed; or the implementation not certified by NIST).

This enables strict approved-only compliance when usedforsecurity=True on FIPS systems only.

And yet it also enables fallback access with usedforsecurity=False for any missing (historical, disallowed or missing certified implementation) algorithms (i.e. blake2, md5, shake/sha3) depending on the runtime configuration of OpenSSL.

  • Issue: Ensure builtin hashlib implementations honor usedforsecurity=True when _hashlib is in FIPS mode #127298

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@xnox
xnox requested review from gpshead and tiran as code owners November 26, 2024 17:03
@bedevere-app bedevere-app Bot mentioned this pull request Nov 26, 2024
Ensure builtin hashlib implementations honor usedforsecurity=True when _hashlib is in FIPS mode #127298
Open
@bedevere-app bedevere-app Bot added the awaiting review label Nov 26, 2024
@xnox

xnox commented Nov 26, 2024

Copy link
Copy Markdown
Author

FYI @stratakis

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xnox
xnox force-pushed the hashlib-builtin-fips-check branch 2 times, most recently from 82e72d9 to 9d6fd91 Compare November 26, 2024 17:34
This was referenced Nov 26, 2024
[3.12] gh-118224: Load default OpenSSL provider for nonsecurity algorithms (GH-118236) #118238
Closed
gh-118224: Load default OpenSSL provider for nonsecurity algorithms #118236
Closed
@xnox

This comment was marked as resolved.

Sign in to view
@xnox
xnox force-pushed the hashlib-builtin-fips-check branch 3 times, most recently from db5c976 to ad0fab8 Compare November 26, 2024 18:20
@xnox
xnox requested a review from erlend-aasland as a code owner November 26, 2024 18:20
@xnox
xnox force-pushed the hashlib-builtin-fips-check branch from ad0fab8 to 28d8035 Compare November 26, 2024 18:32
@picnixz

picnixz commented Nov 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

The built-in algorithms are based on HACL* and are therefore formally verified implementations. So I'm not entirely sure we can say that they are entirely FIPS unapproved (except that some algorithms, whatever their imementation is, might not be allowed on FIPS-only systems).

So, I would like to confirm with @msprotz whether their implementations of SHA and blake2 are FIPS-friendly or not (md5 is never FIPS-friendly I think?)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xnox

xnox commented Nov 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

The built-in algorithms are based on HACL* and are therefore formally verified implementations. So I'm not entirely sure we can say that they are entirely FIPS unapproved (except that some algorithms, whatever their imementation is, might not be allowed on FIPS-only systems).

If the binary build was not tested by a NIST accredited lab, and there is no CMVP certificate issued, it is not certified. HACL* formally verified implementation or not.

As in search on https://csrc.nist.gov/projects/cryptographic-module-validation-program/validated-modules/search should bring up an active certificate, for the binary build (not source-code) on a given supported operating environment (architecture & OS type).

So, I would like to confirm with @msprotz whether their implementations of SHA and blake2 are FIPS-friendly or not (md5 is never FIPS-friendly I think?)

It is irrelevant what is or isn't FIPS-friendly. In approved only mode, only certified implementation can be used, which has a CMVP certificate.

Meaning you cannot even use SHA2-256 HACL implementation, unless it was NIST certified. Despite it producing the same hexdigits for the same inputs =)

Hence here builtin implementations are only made available at runtime with a so called service indicator (ISO/FIPS term) when usedforsecurity=False is explicitly specified.

One cannot universally know what is or isn't allowed, as it depends on which module one is using at runtime, and what its security policy stated. There is no universal statement for FIPS as active modules last for 5 years, and historical modules continue to be in use, despite submission requirements changing.

Meaning an old 1.1.1 openssl fips module on a supported system can be missing shake/sha3, and allow md5. More recent 140-2 certificates may still allow MD5 as part of TLS MD5-SHA1 and HMAC, but not digest. Upcoming modules likely to not have MD5 at all (neither for TLS nor HMAC).

W.r.t. blake2 it is NIST disallowed for cryptographic protection of sensitive information (it lost to KECCAK in sha3 competition).

Note it is extremely useful to still have access to blake2 and md5 as "one-way non-cryptographic compression" functions. For example md5 will continue to be used to identify public cloud buckets; views in SQL databases; all US court submitted documents and so on. Thus access for non-security purposes is very relevant for users.

Example of blake2 usage for non-security purposes as a tuplehash https://github.com/aiidateam/aiida-core/blob/f74adb94cc1e8439c8076f563ec112466fdd174b/src/aiida/common/hashing.py#L78 which is not security relevant

Please also note diagrams & further commentry at https://discuss.python.org/t/python-and-openssl-fips-mode/51389/12

The goal of this patch to ensure that no distro modifications are required of Python when operating on a system with OpenSSL in FIPS mode. This pull request will ensure that one can download python.org distributed or self-built cpython builds; and they just work correctly as expected by FIPS users; and matches behavior of all other open source interpreters. By default, they only offer approved & CMVP certified implementations. It is very consent based.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Nov 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

In approved only mode, only certified implementation can be used, which has a CMVP certificate.

That's what I meant by FIPS-friendly. I wanted to confirm with the author whether this PR is needed or not. The reason is that you claimed that "It is likely that buitin implementations are FIPS unapproved" which I think should be verified by asking the relevant people first.

xnox reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xnox

xnox commented Nov 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

@gpshead I have access to some 1.x based FIPS modules (Ubuntu & Azure Linux) and I think this will work correctly as is there too at runtime.

The unit test mock is 3+ specific, but runtime implementation is 1.x compatible. As _hashlib.get_fips_mode is implemented for both 1.x & 3 ABI.

Hence news entry should mention OpenSSL, but not the providers and/or major openssl version.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead gpshead self-assigned this Nov 27, 2024
@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 27, 2024
@bedevere-bot

bedevere-bot commented Nov 27, 2024

Copy link
Copy Markdown

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

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

xnox reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 27, 2024
gpshead
gpshead previously approved these changes Nov 27, 2024
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

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.

I pushed some changes based on what I saw in my review, there was some pedantic correctness stuff like get_fips_mode raising ValueError or returning values other than 0 or 1 to be handled. I moved the repeated logic into a single execution in the global scope instead of upon every constructor registration function call.

I also updated comment and news wording (good point about this not being 3 specific, I saw that in the code after my first news revision; fixed).

I'd like it if we got rid of OpenSSL hashlib entirely in the long run, but that isn't likely for 3.14 and some vendors needing this bureaucratic oddity is a reason to keep that around anyways so long as it isn't a major burden.

I sent this to the buildbots out of curiosity to see if any have environments that trip the new hashlib_fips test up. But I think that is well enough written with the right conditionals that it'll properly skip as needed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Nov 27, 2024
@gpshead

gpshead commented Nov 27, 2024

Copy link
Copy Markdown
Member

The buildbots are turning up some interesting failures. is the setUpClass assertion I added about _hashlib and _ssl not having been loaded accurate? It is possible to execute the test suite such that each test is not getting its own new process, which might be the source of that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead
gpshead self-requested a review November 27, 2024 08:30
@xnox

xnox commented Nov 27, 2024

Copy link
Copy Markdown
Author

I see what is happening with Blake tests on a RHEL in fips mode. I think we should adjust all Blake tests to always pass in usedforsecurity=False if we want all of those Blake tests to pass when the builder host is in fips mode.

Alternatively we can wrap OpenSSL C APIs to force or lift "fips=yes" property query string.

Btw the fact that Blake is accessible when host OS is in FIPS is one of the motivators for these patch series.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

xnox
xnox commented Nov 27, 2024
View reviewed changes
Comment thread Lib/test/test_hashlib_fips.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

techalchemy added a commit to techalchemy/os that referenced this pull request Apr 29, 2025
@techalchemy
fix python fips detection and blocking of blake2
59a0f9f
- fixes chainguard-dev/internal-dev#11886
- backports from wolfi-dev#36503 (see python/cpython#127301)

Signed-off-by: Dan Ryan <[email protected]>
xnox and others added 13 commits September 8, 2025 01:16
@xnox
pythongh-127298: When in FIPS mode ensure builtin hashes check for us…
21b30a1
…edforsecurity=False

When _hashlib/OpenSSL is available, and OpenSSL is in FIPS mode,
ensure that builtin (fallback) hash implementations are wrapped with a
check for usedforsecurity=False. It is likely that buitin
implementations are FIPS unapproved (either algorithm disallowed; or
the implementation not certified by NIST).

This enables strict approved-only compliance when usedforsecurity=True
on FIPS systems only.

And yet it also enables fallback access with usedforsecurity=False for
any missing (historical, disallowed or missing certified
implementation) algorithms (i.e. blake2, md5, shake/sha3) depending on
the runtime configuration of OpenSSL.
@gpshead @xnox
use != 0 rather than == 1 and update a comment.
42b912a
@gpshead @xnox
Update the NEWS to reflect reality.
ac9dc6d
FIPS mode is an OpenSSL feature and we don't require OpenSSL. So anyone wanting to rely on this will need to ensure their build includes Modules/_hashopenssl.c as `_hashlib` linked appropriately.
@gpshead @xnox
address _some_ correctness details in the test.
4da9f77
@gpshead @xnox
avoid repeated calls, reword comment.
7a954ff
@gpshead @xnox
Update 2024-11-26-16-31-40.gh-issue-127298.jqYJvn.rst
461920d
@gpshead @xnox
avoid rerunning test in refleak hunting mode.
dabed52
@gpshead @xnox
Change to spawn _test_hashlib_fips from test_hashlib with an env.
57241db
@gpshead @xnox
Use subTest to id the algorithm.
3910723
@gpshead @xnox
lint
e8ef380
@gpshead @xnox
remove the no longer appropriate mention from ssltests.
3db1eb5
@gpshead @xnox
fix subTest context manager nesting.
4e5dddb
@xnox
Use null provider, which is guaranteed to have nothing
fd308b4
@xnox
xnox force-pushed the hashlib-builtin-fips-check branch from 905ba7f to fd308b4 Compare September 8, 2025 00:34
@xnox
xnox requested review from AA-Turner, emmatyping and picnixz as code owners September 8, 2025 00:34
@xnox

This comment was marked as resolved.

Sign in to view
@bedevere-bot

This comment was marked as resolved.

Sign in to view
@emmatyping
emmatyping removed their request for review September 8, 2025 01:09
@AA-Turner

This comment was marked as resolved.

Sign in to view
@bedevere-bot

bedevere-bot commented Sep 8, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @AA-Turner for commit fd308b4 🤖

Results will be shown at:

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

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

The builders matched are:

  • s390x RHEL9 Refleaks PR
  • s390x RHEL9 PR
  • PPC64LE RHEL8 PR
  • aarch64 RHEL8 Refleaks PR
  • PPC64LE RHEL8 LTO + PGO PR
  • AMD64 RHEL8 FIPS No Builtin Hashes PR
  • s390x RHEL9 LTO + PGO PR
  • s390x RHEL8 Refleaks PR
  • s390x RHEL9 LTO PR
  • PPC64LE RHEL8 LTO PR
  • AMD64 RHEL8 PR
  • AMD64 RHEL8 LTO + PGO PR
  • AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR
  • s390x RHEL8 PR
  • AMD64 RHEL8 LTO PR
  • s390x RHEL8 LTO PR
  • PPC64LE RHEL8 Refleaks PR
  • aarch64 RHEL8 PR
  • aarch64 RHEL8 LTO + PGO PR
  • s390x RHEL8 LTO + PGO PR
  • AMD64 RHEL8 Refleaks PR
  • aarch64 RHEL8 LTO PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xnox

xnox commented Sep 8, 2025

Copy link
Copy Markdown
Author

@AA-Turner thank you! I will work on fixing those two!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xnox

xnox commented Sep 10, 2025

Copy link
Copy Markdown
Author
  1. Ensure to update_wrapper
  2. Ensure that blake tests are usedforsecurity=False, cause even on FIPS machines we want to test blake algorithms
ERROR: test_blake2_update_over_4gb (test.test_hashlib.HashLibTestCase.test_blake2_update_over_4gb)
ValueError: blake2b is blocked when usedforsecurity=True
AttributeError: 'functools.partial' object has no attribute 'SALT_SIZE'
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 20, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Jul 23, 2026
[TRTLLMINF-191][infra] Use native pytest capture for S3 logs NVIDIA/TensorRT-LLM#16739
Merged
3 tasks done
@toabctl

toabctl commented Aug 26, 2026

Copy link
Copy Markdown

The wrapper takes an algorithm name for its message, but every multi-entry branch passes the caller's name to all of its siblings:

cache['blake2b'] = __get_wrapped_builtin(_blake2.blake2b, name)
cache['blake2s'] = __get_wrapped_builtin(_blake2.blake2s, name)

__get_builtin_constructor populates a whole family on first call and caches it, so whichever sibling is requested first labels the rest. __always_supported lists blake2b before blake2s, so blake2s can never report itself:

hashlib.blake2s(b'x') -> ValueError: blake2b is blocked when usedforsecurity=True

Same shape on the sha2, sha3 and shake branches — hashlib.new('sha224') would leave sha256 labelled sha224. Only blake2 shows up on a build configured with --with-builtin-hashlib-hashes=blake2, where it is the sole builtin.

Fix is to pass each entry its own name, e.g. __get_wrapped_builtin(_blake2.blake2s, 'blake2s'). Verified on 3.14.7: blake2s then reports blake2s, and blake2b still reports blake2b.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiran tiran Awaiting requested review from tiran
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner
@picnixz picnixz Awaiting requested review from picnixz picnixz is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

awaiting review DO-NOT-MERGE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@xnox @picnixz @bedevere-bot @gpshead @msprotz @AA-Turner @toabctl
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.