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-118224: Load default OpenSSL provider for nonsecurity algorithms #118236

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xnox
Copy link

@xnox xnox commented Apr 24, 2024

When OpenSSL is configured to only load "base+fips" providers into the Null library context, md5 might not be available at all. In such cases currently CPython fallsback to internal hashlib implementation is there is one - as there might not be if one compiles python with --with-builtin-hashlib-hashes=blake2. With this change "default" provider is attempted to be loaded to access nonsecurity hashes.

It is FedRAMP/FIPS compliance by-pass. This issue may allow using md5 without specifying "usedforsecurity=False" on systems otherwise configured to be in FIPS-mode only. And is the primary reason why documentation mentions that certain distributions of python remove md5 module altogether.

…thms

When OpenSSL is configured to only load "base+fips" providers into the
Null library context, md5 might not be available at all. In such cases
currently CPython fallsback to internal hashlib implementation is
there is one - as there might not be if one compiles python with
--with-builtin-hashlib-hashes=blake2. With this change "default"
provider is attempted to be loaded to access nonsecurity hashes.
Copy link

cpython-cla-bot bot commented Apr 24, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Apr 24, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Apr 24, 2024

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

@bedevere-app
Copy link

bedevere-app bot commented Apr 24, 2024

GH-118239 is a backport of this pull request to the 3.11 branch.

@bedevere-app
Copy link

bedevere-app bot commented Apr 24, 2024

GH-118240 is a backport of this pull request to the 3.10 branch.

@bedevere-app
Copy link

bedevere-app bot commented Apr 25, 2024

GH-118264 is a backport of this pull request to the 3.9 branch.

@nineteendo
Copy link
Contributor

Could you also configure pre-commit? https://devguide.python.org/getting-started/setup-building/#install-pre-commit

@xnox
Copy link
Author

xnox commented May 7, 2024

Could you also configure pre-commit? https://devguide.python.org/getting-started/setup-building/#install-pre-commit

@nineteendo is there something in particular that you have noticed that pre-commit should have caught?

I have now setup pre-commit, redid the commits and the checks executed and resulted in exactly the same commit, without any diff / warnings / comments.

Is my .c code style incorrect and pre-commit not catching it?

@nineteendo
Copy link
Contributor

This workflow failed: https://github.com/python/cpython/actions/runs/8823736225/job/24224829127#step:4:142
But I can't find the commit where it failed. Do you remember replacing the backticks?

@xnox
Copy link
Author

xnox commented May 7, 2024

This workflow failed: https://github.com/python/cpython/actions/runs/8823736225/job/24224829127#step:4:142 But I can't find the commit where it failed. Do you remember replacing the backticks?

yes, after the initial blurb-it there was comment about single ticks; which i replaced with double backticks and rewrote all commits in all branches/backports.

Is double-backticks appropriate formatting for the NEWS entry as currently seen on https://github.com/python/cpython/pull/118236/files ?

ps. maybe the blurb-it service needs pre-commit checking / checks for single backticks.

@xnox
Copy link
Author

xnox commented May 7, 2024

note, the up to date re-runs of actions are all passing on this pull request.

@nineteendo
Copy link
Contributor

and rewrote all commits in all branches/backports.

Did you do a rebase or delete the old branch? Because I can't find that commit.

Is double-backticks appropriate formatting

Looks like it, '``--[^\`]+``' is used 294 times, and '``--[^\`]+=[^\`]+``' 25 times.

maybe the blurb-it service needs pre-commit checking / checks for single backticks.

Yeah, I was thinking that too.

@xnox
Copy link
Author

xnox commented May 7, 2024

and rewrote all commits in all branches/backports.

Did you do a rebase or delete the old branch? Because I can't find that commit.

rebase, all commits are documented in this PR. If you use web-ui or extensive API, you can see these mentions:

@xnox xnox force-pushed the fix-nodefault-md5 branch from 7a5adff to 092ab97
2 weeks ago

@xnox force-pushed the fix-nodefault-md5 branch from 092ab97 to a47a53f
2 weeks ago

You can click on those commits to still see them dangling and not part of any branch or pull request.

7a5adff was the blurb-it service generated entry.

a47a53f is the current state of this pull request, which was fixed-up with double backticks.

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