From 4ebdd2aa2527d83fe0d418834d85e7c3af278a70 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 24 Oct 2017 21:21:23 +0300 Subject: [PATCH 1/3] bpo-31702: Allow to specify rounds for SHA-2 hashing in crypt.mksalt(). The log_rounds parameter for Blowfish is replaced with the rounds parameter. --- Doc/library/crypt.rst | 14 ++++++---- Doc/whatsnew/3.7.rst | 3 +++ Lib/crypt.py | 14 +++++++--- Lib/test/test_crypt.py | 26 ++++++++++++++----- .../2017-10-24-21-10-44.bpo-31702.SfwJDI.rst | 2 ++ 5 files changed, 44 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-10-24-21-10-44.bpo-31702.SfwJDI.rst diff --git a/Doc/library/crypt.rst b/Doc/library/crypt.rst index 9877b711a9af16..9ee03dacf74a0a 100644 --- a/Doc/library/crypt.rst +++ b/Doc/library/crypt.rst @@ -116,7 +116,7 @@ The :mod:`crypt` module defines the following functions: Accept ``crypt.METHOD_*`` values in addition to strings for *salt*. -.. function:: mksalt(method=None, *, log_rounds=12) +.. function:: mksalt(method=None, *, rounds=None) Return a randomly generated salt of the specified method. If no *method* is given, the strongest method available as returned by @@ -125,14 +125,18 @@ The :mod:`crypt` module defines the following functions: The return value is a string suitable for passing as the *salt* argument to :func:`crypt`. - *log_rounds* specifies the binary logarithm of the number of rounds - for ``crypt.METHOD_BLOWFISH``, and is ignored otherwise. ``8`` specifies - ``256`` rounds. + *rounds* specifies the number of rounds for ``METHOD_SHA256``, + ``METHOD_SHA512`` and ``METHOD_BLOWFISH``, and is ignored otherwise. + For ``METHOD_SHA256`` and ``METHOD_SHA512`` it must be an integer between + ``1000`` and ``999999999``. For ``METHOD_BLOWFISH`` it must be a power of + two between ``16`` and ``2147483648`` (2\ :sup:`32`), the default is + ``4096``. If it isn't a power of two, it will be rounded up to the next + power of two. .. versionadded:: 3.3 .. versionchanged:: 3.7 - Added the *log_rounds* parameter. + Added the *rounds* parameter. Examples diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 46121dcf300d64..0dce43a4eeca31 100644 --- a/Doc/whatsnew/3.7.rst +++ b/Doc/whatsnew/3.7.rst @@ -235,6 +235,9 @@ crypt Added support for the Blowfish method. (Contributed by Serhiy Storchaka in :issue:`31664`.) +The :func:`~crypt.mksalt` function now allows to specify the number of rounds +for hashing. (Contributed by Serhiy Storchaka in :issue:`31702`.) + dis --- diff --git a/Lib/crypt.py b/Lib/crypt.py index 4d73202b468796..fc993091c3eab1 100644 --- a/Lib/crypt.py +++ b/Lib/crypt.py @@ -19,7 +19,7 @@ def __repr__(self): return ''.format(self.name) -def mksalt(method=None, *, log_rounds=12): +def mksalt(method=None, *, rounds=None): """Generate a salt for the specified method. If not specified, the strongest available method will be used. @@ -30,7 +30,13 @@ def mksalt(method=None, *, log_rounds=12): if not method.ident: s = '' elif method.ident[0] == '2': + if rounds is None: + log_rounds = 12 + else: + log_rounds = (rounds-1).bit_length() s = f'${method.ident}${log_rounds:02d}$' + elif method.ident in ('5', '6') and rounds is not None: + s = f'${method.ident}$rounds={rounds}$' else: s = f'${method.ident}$' s += ''.join(_sr.choice(_saltchars) for char in range(method.salt_chars)) @@ -55,10 +61,10 @@ def crypt(word, salt=None): # available salting/crypto methods methods = [] -def _add_method(name, *args): +def _add_method(name, *args, rounds=None): method = _Method(name, *args) globals()['METHOD_' + name] = method - salt = mksalt(method, log_rounds=4) + salt = mksalt(method, rounds=rounds) result = crypt('', salt) if result and len(result) == method.total_size: methods.append(method) @@ -74,7 +80,7 @@ def _add_method(name, *args): # 'y' is the same as 'b', for compatibility # with openwall crypt_blowfish. for _v in 'b', 'y', 'a', '': - if _add_method('BLOWFISH', '2' + _v, 22, 59 + len(_v)): + if _add_method('BLOWFISH', '2' + _v, 22, 59 + len(_v), rounds=16): break _add_method('MD5', '1', 8, 34) diff --git a/Lib/test/test_crypt.py b/Lib/test/test_crypt.py index 8db1aefdf1ef27..d3efbafdb38524 100644 --- a/Lib/test/test_crypt.py +++ b/Lib/test/test_crypt.py @@ -39,12 +39,26 @@ def test_methods(self): else: self.assertEqual(crypt.methods[-1], crypt.METHOD_CRYPT) + @unittest.skipUnless(crypt.METHOD_SHA256 in crypt.methods or + crypt.METHOD_SHA512 in crypt.methods, + 'requires support of SHA-2') + def test_sha2_rounds(self): + for method in (crypt.METHOD_SHA256, crypt.METHOD_SHA512): + for rounds in 1000, 10000, 100000: + salt = crypt.mksalt(method, rounds=rounds) + self.assertIn('$rounds=%d$' % rounds, salt) + self.assertEqual(len(salt) - method.salt_chars, + 11 + len(str(rounds))) + cr = crypt.crypt('mypassword', salt) + self.assertTrue(cr) + cr2 = crypt.crypt('mypassword', cr) + self.assertEqual(cr2, cr) + @unittest.skipUnless(crypt.METHOD_BLOWFISH in crypt.methods, 'requires support of Blowfish') - def test_log_rounds(self): - self.assertEqual(len(crypt._saltchars), 64) + def test_blowfish_rounds(self): for log_rounds in range(4, 11): - salt = crypt.mksalt(crypt.METHOD_BLOWFISH, log_rounds=log_rounds) + salt = crypt.mksalt(crypt.METHOD_BLOWFISH, rounds=1< Date: Thu, 26 Oct 2017 16:06:58 +0300 Subject: [PATCH 2/3] Address review comments. --- Doc/library/crypt.rst | 8 ++++---- Lib/crypt.py | 17 ++++++++++++----- Lib/test/test_crypt.py | 24 ++++++++++++------------ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Doc/library/crypt.rst b/Doc/library/crypt.rst index 9ee03dacf74a0a..b7cf205f69f8c2 100644 --- a/Doc/library/crypt.rst +++ b/Doc/library/crypt.rst @@ -128,10 +128,10 @@ The :mod:`crypt` module defines the following functions: *rounds* specifies the number of rounds for ``METHOD_SHA256``, ``METHOD_SHA512`` and ``METHOD_BLOWFISH``, and is ignored otherwise. For ``METHOD_SHA256`` and ``METHOD_SHA512`` it must be an integer between - ``1000`` and ``999999999``. For ``METHOD_BLOWFISH`` it must be a power of - two between ``16`` and ``2147483648`` (2\ :sup:`32`), the default is - ``4096``. If it isn't a power of two, it will be rounded up to the next - power of two. + ``1000`` and ``999_999_999``, the default is ``5000``. For + ``METHOD_BLOWFISH`` it must be a power of two between ``16`` (2\ :sup:`4`) + and ``2_147_483_648`` (2\ :sup:`32`), the default is ``4096`` + (2\ :sup:`12`). .. versionadded:: 3.3 diff --git a/Lib/crypt.py b/Lib/crypt.py index fc993091c3eab1..13ca46bd618dbb 100644 --- a/Lib/crypt.py +++ b/Lib/crypt.py @@ -27,15 +27,22 @@ def mksalt(method=None, *, rounds=None): """ if method is None: method = methods[0] - if not method.ident: + if not method.ident: # traditional s = '' - elif method.ident[0] == '2': + elif method.ident[0] == '2': # Blowfish variants if rounds is None: log_rounds = 12 else: - log_rounds = (rounds-1).bit_length() + log_rounds = int.bit_length(rounds-1) + if rounds != 1 << log_rounds: + raise ValueError('rounds must be a power of 2') + if not 4 <= log_rounds <= 31: + raise ValueError('rounds out of the range 2**4 to 2**31') s = f'${method.ident}${log_rounds:02d}$' - elif method.ident in ('5', '6') and rounds is not None: + elif method.ident in ('5', '6') and rounds is not None: # SHA-2 + range(rounds) # raise a TypeError for non-integers + if not 1000 <= rounds <= 999_999_999: + raise ValueError('rounds out of the range 1000 to 999_999_999') s = f'${method.ident}$rounds={rounds}$' else: s = f'${method.ident}$' @@ -80,7 +87,7 @@ def _add_method(name, *args, rounds=None): # 'y' is the same as 'b', for compatibility # with openwall crypt_blowfish. for _v in 'b', 'y', 'a', '': - if _add_method('BLOWFISH', '2' + _v, 22, 59 + len(_v), rounds=16): + if _add_method('BLOWFISH', '2' + _v, 22, 59 + len(_v), rounds=1<<4): break _add_method('MD5', '1', 8, 34) diff --git a/Lib/test/test_crypt.py b/Lib/test/test_crypt.py index e72b50a4e75e3c..80aae898a2445e 100644 --- a/Lib/test/test_crypt.py +++ b/Lib/test/test_crypt.py @@ -44,7 +44,7 @@ def test_methods(self): 'requires support of SHA-2') def test_sha2_rounds(self): for method in (crypt.METHOD_SHA256, crypt.METHOD_SHA512): - for rounds in 1000, 10000, 100000: + for rounds in 1000, 10_000, 100_000: salt = crypt.mksalt(method, rounds=rounds) self.assertIn('$rounds=%d$' % rounds, salt) self.assertEqual(len(salt) - method.salt_chars, @@ -66,18 +66,18 @@ def test_blowfish_rounds(self): cr2 = crypt.crypt('mypassword', cr) self.assertEqual(cr2, cr) - @unittest.skipUnless(crypt.METHOD_BLOWFISH in crypt.methods, - 'requires support of Blowfish') def test_invalid_rounds(self): - for log_rounds in (0, 1, 1<<999): - salt = crypt.mksalt(crypt.METHOD_BLOWFISH, rounds=rounds) - cr = crypt.crypt('mypassword', salt) - if cr is not None: - # On failure the openwall implementation returns a magic - # string that is shorter than 13 characters and is guaranteed - # to differ from a salt. - self.assertNotEqual(cr, salt) - self.assertLess(len(cr), 13) + for method in (crypt.METHOD_SHA256, crypt.METHOD_SHA512, + crypt.METHOD_BLOWFISH): + with self.assertRaises(TypeError): + crypt.mksalt(method, rounds='4096') + with self.assertRaises(TypeError): + crypt.mksalt(method, rounds=4096.0) + for rounds in (0, 1, -1, 1<<999): + with self.assertRaises(ValueError): + crypt.mksalt(method, rounds=rounds) + with self.assertRaises(ValueError): + crypt.mksalt(crypt.METHOD_BLOWFISH, rounds=1000) if __name__ == "__main__": From 6c767153b241c42237926ca3ef64f5dc186220e5 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 9 Nov 2017 18:53:33 +0200 Subject: [PATCH 3/3] Address review comments. --- Doc/library/crypt.rst | 4 ++-- Lib/crypt.py | 25 ++++++++++++++++--------- Lib/test/test_crypt.py | 5 ++++- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/Doc/library/crypt.rst b/Doc/library/crypt.rst index b7cf205f69f8c2..dd62cb32b9d16b 100644 --- a/Doc/library/crypt.rst +++ b/Doc/library/crypt.rst @@ -126,11 +126,11 @@ The :mod:`crypt` module defines the following functions: to :func:`crypt`. *rounds* specifies the number of rounds for ``METHOD_SHA256``, - ``METHOD_SHA512`` and ``METHOD_BLOWFISH``, and is ignored otherwise. + ``METHOD_SHA512`` and ``METHOD_BLOWFISH``. For ``METHOD_SHA256`` and ``METHOD_SHA512`` it must be an integer between ``1000`` and ``999_999_999``, the default is ``5000``. For ``METHOD_BLOWFISH`` it must be a power of two between ``16`` (2\ :sup:`4`) - and ``2_147_483_648`` (2\ :sup:`32`), the default is ``4096`` + and ``2_147_483_648`` (2\ :sup:`31`), the default is ``4096`` (2\ :sup:`12`). .. versionadded:: 3.3 diff --git a/Lib/crypt.py b/Lib/crypt.py index 13ca46bd618dbb..b0e47f430c3cbc 100644 --- a/Lib/crypt.py +++ b/Lib/crypt.py @@ -27,9 +27,15 @@ def mksalt(method=None, *, rounds=None): """ if method is None: method = methods[0] + if rounds is not None and not isinstance(rounds, int): + raise TypeError(f'{rounds.__class__.__name__} object cannot be ' + f'interpreted as an integer') if not method.ident: # traditional s = '' - elif method.ident[0] == '2': # Blowfish variants + else: # modular + s = f'${method.ident}$' + + if method.ident and method.ident[0] == '2': # Blowfish variants if rounds is None: log_rounds = 12 else: @@ -38,14 +44,15 @@ def mksalt(method=None, *, rounds=None): raise ValueError('rounds must be a power of 2') if not 4 <= log_rounds <= 31: raise ValueError('rounds out of the range 2**4 to 2**31') - s = f'${method.ident}${log_rounds:02d}$' - elif method.ident in ('5', '6') and rounds is not None: # SHA-2 - range(rounds) # raise a TypeError for non-integers - if not 1000 <= rounds <= 999_999_999: - raise ValueError('rounds out of the range 1000 to 999_999_999') - s = f'${method.ident}$rounds={rounds}$' - else: - s = f'${method.ident}$' + s += f'{log_rounds:02d}$' + elif method.ident in ('5', '6'): # SHA-2 + if rounds is not None: + if not 1000 <= rounds <= 999_999_999: + raise ValueError('rounds out of the range 1000 to 999_999_999') + s += f'rounds={rounds}$' + elif rounds is not None: + raise ValueError(f"{method} doesn't support the rounds argument") + s += ''.join(_sr.choice(_saltchars) for char in range(method.salt_chars)) return s diff --git a/Lib/test/test_crypt.py b/Lib/test/test_crypt.py index 80aae898a2445e..d9189fc6f28b47 100644 --- a/Lib/test/test_crypt.py +++ b/Lib/test/test_crypt.py @@ -58,7 +58,7 @@ def test_sha2_rounds(self): 'requires support of Blowfish') def test_blowfish_rounds(self): for log_rounds in range(4, 11): - salt = crypt.mksalt(crypt.METHOD_BLOWFISH, rounds=1<