-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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-127298: Refactor test_hashlib for better usedforsecurity & openssl fips mode env support. #127492
base: main
Are you sure you want to change the base?
Conversation
gpshead
commented
Dec 1, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: Ensure builtin hashlib implementations honor usedforsecurity=True when _hashlib is in FIPS mode #127298
…environment support.
!buildbot FIPS |
🤖 New build scheduled with the buildbot fleet by @gpshead for commit 59d9a85 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot FIPS |
🤖 New build scheduled with the buildbot fleet by @gpshead for commit bd46651 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
7a5ab6c
to
bd46651
Compare
#127467 is follow-on work to this that combined gets the FIPS mode buildbots passing in main. |
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.
Misc/NEWS.d/next/Library/2024-12-02-04-08-22.gh-issue-127298.8cpkfk.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Victor Stinner <vstinner@python.org>
Thanks for the PR! I'll have a look at it and I can also provide ssh access to the FIPS buildbots if that would make things easier to debug. |
I tested manually the change on RHEL8. 3 MD5 tests of test_hashlib are failing:
|
I built Python with
|
Suggested fix for file digest: diff --git a/Lib/hashlib.py b/Lib/hashlib.py
index 44656c33a..93d602571 100644
--- a/Lib/hashlib.py
+++ b/Lib/hashlib.py
@@ -192,7 +192,7 @@ def __hash_new(name, data=b'', **kwargs):
pass
-def file_digest(fileobj, digest, /, *, _bufsize=2**18):
+def file_digest(fileobj, digest, /, *, usedforsecurity=True, _bufsize=2**18):
"""Hash the contents of a file-like object. Returns a digest object.
*fileobj* must be a file-like object opened for reading in binary mode.
@@ -206,9 +206,9 @@ def file_digest(fileobj, digest, /, *, _bufsize=2**18):
# On Linux we could use AF_ALG sockets and sendfile() to archive zero-copy
# hashing with hardware acceleration.
if isinstance(digest, str):
- digestobj = new(digest)
+ digestobj = new(digest, usedforsecurity=usedforsecurity)
else:
- digestobj = digest()
+ digestobj = digest(usedforsecurity=usedforsecurity)
if hasattr(fileobj, "getbuffer"):
# io.BytesIO object, use zero-copy buffer
diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py
index 1c1a0396c..a7cca0ba5 100644
--- a/Lib/test/test_hashlib.py
+++ b/Lib/test/test_hashlib.py
@@ -443,11 +443,13 @@ def check_file_digest(self, name, data, hexdigest):
for digest in digests:
buf = io.BytesIO(data)
buf.seek(0)
- self.assertEqual(
- hashlib.file_digest(buf, digest).hexdigest(), hexdigest
- )
+ digestobj = hashlib.file_digest(buf, digest,
+ usedforsecurity=False)
+ self.assertEqual(digestobj.hexdigest(), hexdigest)
+
with open(os_helper.TESTFN, "rb") as f:
- digestobj = hashlib.file_digest(f, digest)
+ digestobj = hashlib.file_digest(f, digest,
+ usedforsecurity=False)
self.assertEqual(digestobj.hexdigest(), hexdigest)
finally:
os.unlink(os_helper.TESTFN) |
@gpshead the suggestion from @vstinner looks reasonable in #127492 (comment) will you push that to this pr? Or should that change be done separately, with a separate blurb entry? |