Skip to content

gh-90733: improve hashlib.scrypt interface #136100

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Lib/hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ def __hash_new(name, *args, **kwargs):

try:
# OpenSSL's scrypt requires OpenSSL 1.1+
from _hashlib import scrypt # noqa: F401
from _hashlib import scrypt
__all__ += ('scrypt',)
except ImportError:
pass

Expand Down
117 changes: 71 additions & 46 deletions Lib/test/test_hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1281,46 +1275,6 @@ def _test_pbkdf2_hmac(self, pbkdf2, supported):
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)
Expand Down Expand Up @@ -1362,5 +1316,76 @@ def readable(self):
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()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Improve error messages when reporting invalid parameters in
:func:`hashlib.scrypt`. Patch by Bénédikt Tran.
76 changes: 44 additions & 32 deletions Modules/_hashopenssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1540,7 +1539,25 @@ pbkdf2_hmac_impl(PyObject *module, const char *hash_name,
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
Expand All @@ -1554,85 +1571,80 @@ _hashlib.scrypt
maxmem: long = 0
dklen: long = 64


scrypt password-based key derivation function.
[clinic start generated code]*/

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()
*/
Expand Down
10 changes: 1 addition & 9 deletions Modules/clinic/_hashopenssl.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading