From a68ddea3bf7e9bb77d096c613bce2ec1e67a28f4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 14 Jul 2025 12:49:34 +0200 Subject: [PATCH] gh-90733: improve `hashlib.scrypt` interface (#136100) * add `scrypt` to `hashlib.__all__` * improve `hashlib.scrypt` exception messages --- Lib/hashlib.py | 3 +- Lib/test/test_hashlib.py | 117 +++++++++++------- ...5-06-29-15-22-13.gh-issue-90733.NiquaA.rst | 2 + Modules/_hashopenssl.c | 76 +++++++----- Modules/clinic/_hashopenssl.c.h | 10 +- 5 files changed, 120 insertions(+), 88 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-06-29-15-22-13.gh-issue-90733.NiquaA.rst diff --git a/Lib/hashlib.py b/Lib/hashlib.py index 6d4f84a5a6e9..6c72fba03bf6 100644 --- a/Lib/hashlib.py +++ b/Lib/hashlib.py @@ -187,7 +187,8 @@ except ImportError: try: # OpenSSL's scrypt requires OpenSSL 1.1+ - from _hashlib import scrypt # noqa: F401 + from _hashlib import scrypt + __all__ += ('scrypt',) except ImportError: pass diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 5bad483ae9da..65e18639f82b 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1184,12 +1184,6 @@ class KDFTests(unittest.TestCase): (b'pass\0word', b'sa\0lt', 4096, 16), ] - scrypt_test_vectors = [ - (b'', b'', 16, 1, 1, unhexlify('77d6576238657b203b19ca42c18a0497f16b4844e3074ae8dfdffa3fede21442fcd0069ded0948f8326a753a0fc81f17e8d3e0fb2e0d3628cf35e20c38d18906')), - (b'password', b'NaCl', 1024, 8, 16, unhexlify('fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640')), - (b'pleaseletmein', b'SodiumChloride', 16384, 8, 1, unhexlify('7023bdcb3afd7348461c06cd81fd38ebfda8fbba904f8e3ea9b543f6545da1f2d5432955613f0fcf62d49705242a9af9e61e85dc0d651e40dfcf017b45575887')), - ] - pbkdf2_results = { "sha1": [ # official test vectors from RFC 6070 @@ -1281,46 +1275,6 @@ class KDFTests(unittest.TestCase): def test_pbkdf2_hmac_c(self): self._test_pbkdf2_hmac(openssl_hashlib.pbkdf2_hmac, openssl_md_meth_names) - @unittest.skipUnless(hasattr(hashlib, 'scrypt'), - ' test requires OpenSSL > 1.1') - @unittest.skipIf(get_fips_mode(), reason="scrypt is blocked in FIPS mode") - def test_scrypt(self): - for password, salt, n, r, p, expected in self.scrypt_test_vectors: - result = hashlib.scrypt(password, salt=salt, n=n, r=r, p=p) - self.assertEqual(result, expected) - - # this values should work - hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1) - # password and salt must be bytes-like - with self.assertRaises(TypeError): - hashlib.scrypt('password', salt=b'salt', n=2, r=8, p=1) - with self.assertRaises(TypeError): - hashlib.scrypt(b'password', salt='salt', n=2, r=8, p=1) - # require keyword args - with self.assertRaises(TypeError): - hashlib.scrypt(b'password') - with self.assertRaises(TypeError): - hashlib.scrypt(b'password', b'salt') - with self.assertRaises(TypeError): - hashlib.scrypt(b'password', 2, 8, 1, salt=b'salt') - for n in [-1, 0, 1, None]: - with self.assertRaises((ValueError, OverflowError, TypeError)): - hashlib.scrypt(b'password', salt=b'salt', n=n, r=8, p=1) - for r in [-1, 0, None]: - with self.assertRaises((ValueError, OverflowError, TypeError)): - hashlib.scrypt(b'password', salt=b'salt', n=2, r=r, p=1) - for p in [-1, 0, None]: - with self.assertRaises((ValueError, OverflowError, TypeError)): - hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=p) - for maxmem in [-1, None]: - with self.assertRaises((ValueError, OverflowError, TypeError)): - hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1, - maxmem=maxmem) - for dklen in [-1, None]: - with self.assertRaises((ValueError, OverflowError, TypeError)): - hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1, - dklen=dklen) - def test_normalized_name(self): self.assertNotIn("blake2b512", hashlib.algorithms_available) self.assertNotIn("sha3-512", hashlib.algorithms_available) @@ -1362,5 +1316,76 @@ class KDFTests(unittest.TestCase): hashlib.file_digest(NonBlocking(), hashlib.sha256) +@unittest.skipUnless(hasattr(hashlib, 'scrypt'), 'requires OpenSSL 1.1+') +@unittest.skipIf(get_fips_mode(), reason="scrypt is blocked in FIPS mode") +class TestScrypt(unittest.TestCase): + + scrypt_test_vectors = [ + (b'', b'', 16, 1, 1, unhexlify('77d6576238657b203b19ca42c18a0497f16b4844e3074ae8dfdffa3fede21442fcd0069ded0948f8326a753a0fc81f17e8d3e0fb2e0d3628cf35e20c38d18906')), + (b'password', b'NaCl', 1024, 8, 16, unhexlify('fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640')), + (b'pleaseletmein', b'SodiumChloride', 16384, 8, 1, unhexlify('7023bdcb3afd7348461c06cd81fd38ebfda8fbba904f8e3ea9b543f6545da1f2d5432955613f0fcf62d49705242a9af9e61e85dc0d651e40dfcf017b45575887')), + ] + + def test_scrypt(self): + for password, salt, n, r, p, expected in self.scrypt_test_vectors: + result = hashlib.scrypt(password, salt=salt, n=n, r=r, p=p) + self.assertEqual(result, expected) + + # these parameters must be valid + hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1) + hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1, maxmem=0) + hashlib.scrypt(b'password', salt=b'salt', n=2, r=8, p=1, dklen=1) + + def test_scrypt_types(self): + # password and salt must be bytes-like + with self.assertRaises(TypeError): + hashlib.scrypt('password', salt=b'salt', n=2, r=8, p=1) + with self.assertRaises(TypeError): + hashlib.scrypt(b'password', salt='salt', n=2, r=8, p=1) + # require keyword args + with self.assertRaises(TypeError): + hashlib.scrypt(b'password') + with self.assertRaises(TypeError): + hashlib.scrypt(b'password', b'salt') + with self.assertRaises(TypeError): + hashlib.scrypt(b'password', 2, 8, 1, salt=b'salt') + + def test_scrypt_validate(self): + def scrypt(password=b"password", /, **kwargs): + # overwrite well-defined parameters with bad ones + kwargs = dict(salt=b'salt', n=2, r=8, p=1) | kwargs + return hashlib.scrypt(password, **kwargs) + + for param_name in ('n', 'r', 'p', 'maxmem', 'dklen'): + param = {param_name: None} + with self.subTest(**param): + self.assertRaises(TypeError, scrypt, **param) + + self.assertRaises(ValueError, scrypt, n=0) + self.assertRaises(ValueError, scrypt, n=-1) + self.assertRaises(ValueError, scrypt, n=1) + + self.assertRaises(ValueError, scrypt, r=0) + self.assertRaises(ValueError, scrypt, r=-1) + + self.assertRaises(ValueError, scrypt, p=-1) + self.assertRaises(ValueError, scrypt, p=0) + + self.assertRaises(ValueError, scrypt, maxmem=-1) + # OpenSSL hard limit for 'maxmem' is an 'uint64_t' but for now, + # we do not use the 'uint64' Clinic converter but the 'long' one. + self.assertRaises(OverflowError, scrypt, maxmem=(1 << 64)) + # Historically, Python allowed 'maxmem' to be at most INT_MAX, + # which is at most 2**32-1 (on Windows, sizeof(long) == 4, so + # an OverflowError will be raised instead of a ValueError). + numeric_exc_types = (OverflowError, ValueError) + self.assertRaises(numeric_exc_types, scrypt, maxmem=(1 << 32)) + + self.assertRaises(ValueError, scrypt, dklen=-1) + self.assertRaises(ValueError, scrypt, dklen=0) + MAX_DKLEN = ((1 << 32) - 1) * 32 # see RFC 7914 + self.assertRaises(numeric_exc_types, scrypt, dklen=MAX_DKLEN + 1) + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Library/2025-06-29-15-22-13.gh-issue-90733.NiquaA.rst b/Misc/NEWS.d/next/Library/2025-06-29-15-22-13.gh-issue-90733.NiquaA.rst new file mode 100644 index 000000000000..cba930c58605 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-06-29-15-22-13.gh-issue-90733.NiquaA.rst @@ -0,0 +1,2 @@ +Improve error messages when reporting invalid parameters in +:func:`hashlib.scrypt`. Patch by Bénédikt Tran. diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 1a6c831e4837..6f04d9ec0c47 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -48,7 +48,6 @@ #define MUNCH_SIZE INT_MAX -#define PY_OPENSSL_HAS_SCRYPT 1 #if defined(NID_sha3_224) && defined(NID_sha3_256) && defined(NID_sha3_384) && defined(NID_sha3_512) #define PY_OPENSSL_HAS_SHA3 1 #endif @@ -1557,7 +1556,25 @@ end: return key_obj; } -#ifdef PY_OPENSSL_HAS_SCRYPT +// --- PBKDF: scrypt (RFC 7914) ----------------------------------------------- + +/* + * By default, OpenSSL 1.1.0 restricts 'maxmem' in EVP_PBE_scrypt() + * to 32 MiB (1024 * 1024 * 32) but only if 'maxmem = 0' and allows + * for an arbitrary large limit fitting on an uint64_t otherwise. + * + * For legacy reasons, we limited 'maxmem' to be at most INTMAX, + * but if users need a more relaxed value, we will revisit this + * limit in the future. + */ +#define HASHLIB_SCRYPT_MAX_MAXMEM INT_MAX + +/* + * Limit 'dklen' to INT_MAX even if it can be at most (32 * UINT32_MAX). + * + * See https://datatracker.ietf.org/doc/html/rfc7914.html for details. + */ +#define HASHLIB_SCRYPT_MAX_DKLEN INT_MAX /*[clinic input] _hashlib.scrypt @@ -1571,7 +1588,6 @@ _hashlib.scrypt maxmem: long = 0 dklen: long = 64 - scrypt password-based key derivation function. [clinic start generated code]*/ @@ -1579,77 +1595,73 @@ static PyObject * _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt, unsigned long n, unsigned long r, unsigned long p, long maxmem, long dklen) -/*[clinic end generated code: output=d424bc3e8c6b9654 input=0c9a84230238fd79]*/ +/*[clinic end generated code: output=d424bc3e8c6b9654 input=bdeac9628d07f7a1]*/ { - PyObject *key_obj = NULL; - char *key; + PyObject *key = NULL; int retval; if (password->len > INT_MAX) { - PyErr_SetString(PyExc_OverflowError, - "password is too long."); + PyErr_SetString(PyExc_OverflowError, "password is too long"); return NULL; } if (salt->len > INT_MAX) { - PyErr_SetString(PyExc_OverflowError, - "salt is too long."); + PyErr_SetString(PyExc_OverflowError, "salt is too long"); return NULL; } if (n < 2 || n & (n - 1)) { - PyErr_SetString(PyExc_ValueError, - "n must be a power of 2."); + PyErr_SetString(PyExc_ValueError, "n must be a power of 2"); return NULL; } - if (maxmem < 0 || maxmem > INT_MAX) { - /* OpenSSL 1.1.0 restricts maxmem to 32 MiB. It may change in the - future. The maxmem constant is private to OpenSSL. */ + if (maxmem < 0 || maxmem > HASHLIB_SCRYPT_MAX_MAXMEM) { PyErr_Format(PyExc_ValueError, - "maxmem must be positive and smaller than %d", - INT_MAX); + "maxmem must be positive and at most %d", + HASHLIB_SCRYPT_MAX_MAXMEM); return NULL; } - if (dklen < 1 || dklen > INT_MAX) { + if (dklen < 1 || dklen > HASHLIB_SCRYPT_MAX_DKLEN) { PyErr_Format(PyExc_ValueError, - "dklen must be greater than 0 and smaller than %d", - INT_MAX); + "dklen must be at least 1 and at most %d", + HASHLIB_SCRYPT_MAX_DKLEN); return NULL; } /* let OpenSSL validate the rest */ - retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0); + retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, + (uint64_t)maxmem, NULL, 0); if (!retval) { - notify_ssl_error_occurred( - "Invalid parameter combination for n, r, p, maxmem."); + notify_ssl_error_occurred("invalid parameter combination for " + "n, r, p, and maxmem"); return NULL; } - key_obj = PyBytes_FromStringAndSize(NULL, dklen); - if (key_obj == NULL) { + key = PyBytes_FromStringAndSize(NULL, dklen); + if (key == NULL) { return NULL; } - key = PyBytes_AS_STRING(key_obj); Py_BEGIN_ALLOW_THREADS retval = EVP_PBE_scrypt( - (const char*)password->buf, (size_t)password->len, + (const char *)password->buf, (size_t)password->len, (const unsigned char *)salt->buf, (size_t)salt->len, - n, r, p, maxmem, - (unsigned char *)key, (size_t)dklen + (uint64_t)n, (uint64_t)r, (uint64_t)p, (uint64_t)maxmem, + (unsigned char *)PyBytes_AS_STRING(key), (size_t)dklen ); Py_END_ALLOW_THREADS if (!retval) { - Py_CLEAR(key_obj); + Py_DECREF(key); notify_ssl_error_occurred_in(Py_STRINGIFY(EVP_PBE_scrypt)); return NULL; } - return key_obj; + return key; } -#endif /* PY_OPENSSL_HAS_SCRYPT */ + +#undef HASHLIB_SCRYPT_MAX_DKLEN +#undef HASHLIB_SCRYPT_MAX_MAXMEM /* Fast HMAC for hmac.digest() */ diff --git a/Modules/clinic/_hashopenssl.c.h b/Modules/clinic/_hashopenssl.c.h index 61ea10e2a482..3d81a6dcce14 100644 --- a/Modules/clinic/_hashopenssl.c.h +++ b/Modules/clinic/_hashopenssl.c.h @@ -1492,8 +1492,6 @@ exit: return return_value; } -#if defined(PY_OPENSSL_HAS_SCRYPT) - PyDoc_STRVAR(_hashlib_scrypt__doc__, "scrypt($module, /, password, *, salt, n, r, p, maxmem=0, dklen=64)\n" "--\n" @@ -1601,8 +1599,6 @@ exit: return return_value; } -#endif /* defined(PY_OPENSSL_HAS_SCRYPT) */ - PyDoc_STRVAR(_hashlib_hmac_singleshot__doc__, "hmac_digest($module, /, key, msg, digest)\n" "--\n" @@ -1980,8 +1976,4 @@ exit: #ifndef _HASHLIB_OPENSSL_SHAKE_256_METHODDEF #define _HASHLIB_OPENSSL_SHAKE_256_METHODDEF #endif /* !defined(_HASHLIB_OPENSSL_SHAKE_256_METHODDEF) */ - -#ifndef _HASHLIB_SCRYPT_METHODDEF - #define _HASHLIB_SCRYPT_METHODDEF -#endif /* !defined(_HASHLIB_SCRYPT_METHODDEF) */ -/*[clinic end generated code: output=29f4aaf01714778e input=a9049054013a1b77]*/ +/*[clinic end generated code: output=cd5ff436f6dc2938 input=a9049054013a1b77]*/ -- 2.47.2