Skip to content

GH-161739: Don't test PBKDF2 HMAC on FIPS with len(salt) < 16 - #161748

Open
hroncok wants to merge 3 commits into
python:mainfrom
hroncok:gh161739
Open

hroncok wants to merge 3 commits into
python:mainfrom
hroncok:gh161739

Conversation

@hroncok

@hroncok hroncok commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

To compensate the skipped test coverage,
add some tests with longer salt.

To compensate the skipped test coverage,
add some tests with longer salt.
Comment thread Lib/test/test_hashlib.py Resolved

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it only fail on OpenSSL 4.x? if it's valid for any OpenSSL version, then it's fine to amend the NEWS entry a bit.

Comment thread Lib/test/test_hashlib.py Resolved
@picnixz picnixz added awaiting review needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes and removed awaiting merge needs backport to 3.13 only security fixes labels Nov 10, 2026
@hroncok

hroncok commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Does it only fail on OpenSSL 4.x

Actually, I only reproduced it on one specific version of 4.x, but that is my understanding, yes.

@picnixz

picnixz commented Nov 10, 2026

Copy link
Copy Markdown
Member

Mmh. That's weird that it's only in OpenSSL 4.x and not in others, because this looks like it means that the salt validation was not done previously.

@hroncok

hroncok commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

I'm double checking.

@hroncok

hroncok commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

My testing is heavily based on using OPENSSL_FORCE_FIPS_MODE=1. There, it only happens with OpenSSL 4+ but that does not necessarily mean that a real FIPS environment would not failed even on older SSL. Let's just remove the version information from news then.

Comment thread Lib/test/test_hashlib.py Resolved
Co-authored-by: Victor Stinner <vstinner@python.org>
Comment thread Lib/test/test_hashlib.py
(b'passwordPASSWORDpassword', b'saltSALTsaltSALTsaltSALTsaltSALTsalt',
4096, -1),
(b'pass\0word', b'sa\0lt', 4096, 16),
(b'password', b'salt'*4, 1, None),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
(b'password', b'salt'*4, 1, None),
# FIPS needs salt of at least 128 bits (b'salt'*4 is 128-bit)
(b'password', b'salt'*4, 1, None),

@picnixz picnixz Nov 10, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add the FIPS guidelines ref here (it's NIST SP 800-132, §5.1), but since the guidelines will be ervisied (well they are under revs since 2023), you can drop the paragraph actually.

Comment thread Lib/test/test_hashlib.py
@@ -1236,6 +1236,9 @@ class KDFTests(unittest.TestCase):
(b'passwordPASSWORDpassword', b'saltSALTsaltSALTsaltSALTsaltSALTsalt',
4096, -1),
(b'pass\0word', b'sa\0lt', 4096, 16),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might be interesting to test also this case (null bytes) using b'sa\0lt'*4.

@vstinner

Copy link
Copy Markdown
Member

On Fedora 45 with OpenSSL 4.0.3 29 Sep 2026, OPENSSL_FORCE_FIPS_MODE=1 ./python -m test -v test_hashlib -m test_pbkdf2_hmac_c fails with:

ERROR: test_pbkdf2_hmac_c (test.test_hashlib.KDFTests.test_pbkdf2_hmac_c)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/test/test_hashlib.py", line 1362, in test_pbkdf2_hmac_c
    self._test_pbkdf2_hmac(openssl_hashlib.pbkdf2_hmac, openssl_md_meth_names)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vstinner/python/main/Lib/test/test_hashlib.py", line 1316, in _test_pbkdf2_hmac
    out = pbkdf2(digest_name, password, salt, rounds, dklen)
ValueError: [Provider routines] invalid iteration count

The test pass with sha1 b'passwordPASSWORDpassword' b'saltSALTsaltSALTsaltSALTsaltSALTsalt' 4096 25, but fails with sha1 b'password' b'saltsaltsaltsalt' 1 None. It seems like dklen=None is rejected in FIPS mode.

@picnixz

picnixz commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

NIST SP 800-132 actually specifies 1000 as iteration counts as well I think and kLen should be 112 (i think dklen is klen in the NIST guidelines), see https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf

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

Labels

awaiting review needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants