Skip to content

Conversation

@gpshead
Copy link
Member

@gpsheadgpshead commented Dec 1, 2024

@gpsheadgpshead added tests Tests in the Lib/test dir skip news labels Dec 1, 2024
@gpshead
Copy link
MemberAuthor

!buildbot FIPS

@bedevere-bot
Copy link

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

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

The builders matched are:

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

@gpshead
Copy link
MemberAuthor

!buildbot FIPS

@bedevere-bot
Copy link

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

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

The builders matched are:

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

@gpsheadgpshead changed the title Refactor test_hashlib for better usedforsecurity & openssl fips mode env support.gh-127298: Refactor test_hashlib for better usedforsecurity & openssl fips mode env support.Dec 2, 2024
@gpsheadgpshead self-assigned this Dec 2, 2024
@gpsheadgpshead added stdlib Standard Library Python modules in the Lib/ directory and removed skip news labels Dec 2, 2024
@gpshead
Copy link
MemberAuthor

#127467 is follow-on work to this that combined gets the FIPS mode buildbots passing in main.

@gpsheadgpshead marked this pull request as ready for review December 2, 2024 04:38
@gpsheadgpshead requested a review from tiran as a code ownerDecember 2, 2024 04:38
@gpsheadgpshead requested review from vstinner and removed request for tiranDecember 2, 2024 04:38
Co-authored-by: Nice Zombies <[email protected]>
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

@stratakis
Copy link
Contributor

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.

@vstinner
Copy link
Member

I tested manually the change on RHEL8. 3 MD5 tests of test_hashlib are failing:

====================================================================== ERROR: test_case_md5_0 (test.test_hashlib.HashLibTestCase.test_case_md5_0) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 547, in test_case_md5_0 self.check( ~~~~~~~~~~^ 'md5', b'', 'd41d8cd98f00b204e9800998ecf8427e', ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ usedforsecurity=False ^^^^^^^^^^^^^^^^^^^^^ ) ^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 427, in check self.check_file_digest(name, data, hexdigest) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 447, in check_file_digest hashlib.file_digest(buf, digest).hexdigest(), hexdigest ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/hashlib.py", line 211, in file_digest digestobj = digest() ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS ====================================================================== ERROR: test_case_md5_1 (test.test_hashlib.HashLibTestCase.test_case_md5_1) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 553, in test_case_md5_1 self.check( ~~~~~~~~~~^ 'md5', b'abc', '900150983cd24fb0d6963f7d28e17f72', ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ usedforsecurity=False ^^^^^^^^^^^^^^^^^^^^^ ) ^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 427, in check self.check_file_digest(name, data, hexdigest) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 447, in check_file_digest hashlib.file_digest(buf, digest).hexdigest(), hexdigest ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/hashlib.py", line 211, in file_digest digestobj = digest() ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS ====================================================================== ERROR: test_case_md5_2 (test.test_hashlib.HashLibTestCase.test_case_md5_2) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 559, in test_case_md5_2 self.check( ~~~~~~~~~~^ 'md5', ^^^^^^ ...<2 lines>... usedforsecurity=False ^^^^^^^^^^^^^^^^^^^^^ ) ^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 427, in check self.check_file_digest(name, data, hexdigest) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/test/test_hashlib.py", line 447, in check_file_digest hashlib.file_digest(buf, digest).hexdigest(), hexdigest ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^ File "/home/vstinner/cpython/Lib/hashlib.py", line 211, in file_digest digestobj = digest() ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS ---------------------------------------------------------------------- 

@vstinner
Copy link
Member

I built Python with ./configure --with-pydebug && make. MD5 is available:

[vstinner@python-builder-rhel8-fips cpython]$ ./python Python 3.14.0a2+ (heads/pr/127492:3f9ec82db, Dec 5 2024, 06:10:34) [GCC 8.5.0 20210514 (Red Hat 8.5.0-22)] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import hashlib >>> hashlib.md5(usedforsecurity=False).hexdigest() 'd41d8cd98f00b204e9800998ecf8427e' 

@vstinner
Copy link
Member

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)

@xnox
Copy link

xnox commented Dec 11, 2024

@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?

@xnox
Copy link

xnox commented Apr 28, 2025

@gpshead hi, my PRs had comments that you are taking over with these changes instead, but they now seem to have stalled. Are you still actively working on this and related PRs?

@picnixz
Copy link
Member

picnixz commented Apr 28, 2025

Note that changing how we materialize a digest object should done carefully (see PEP-452). For new(), it should be fine as we're using our own hsahlib.new but for digest(), we should be careful here because not all digest constructors necessarily need to accept usedforsecurity (see the examples in https://docs.python.org/3/library/hashlib.html#hashlib.file_digest):

buf=io.BytesIO(b"somedata") mac1=hmac.HMAC(b"key", digestmod=hashlib.sha512) digest=hashlib.file_digest(buf, lambda: mac1)

@xnox
Copy link

xnox commented Sep 8, 2025

@gpshead Any updates on landing this?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core reviewstdlibStandard Library Python modules in the Lib/ directorytestsTests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@gpshead@bedevere-bot@stratakis@vstinner@xnox@picnixz@nineteendo