From 5cb5319bbbdee50f9246bcb6ab59b777c1cf4380 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Mar 2021 18:13:29 +0100 Subject: [PATCH 1/7] bpo-40645: use C implementation of HMAC Signed-off-by: Christian Heimes --- Lib/hashlib.py | 28 ++++++++++++++++ Lib/hmac.py | 90 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 82 insertions(+), 36 deletions(-) diff --git a/Lib/hashlib.py b/Lib/hashlib.py index 58c340d56e3ba5..f063a92a5ccae8 100644 --- a/Lib/hashlib.py +++ b/Lib/hashlib.py @@ -173,6 +173,7 @@ def __hash_new(name, data=b'', **kwargs): algorithms_available = algorithms_available.union( _hashlib.openssl_md_meth_names) except ImportError: + _hashlib = None new = __py_new __get_hash = __get_builtin_constructor @@ -257,6 +258,33 @@ def prf(msg, inner=inner, outer=outer): logging.exception('code for hash %s was not found.', __func_name) +def _digestmod_to_name(digestmod, from_instance=False): + """Convert a string, callable or digest module to a digest name + """ + if isinstance(digestmod, str): + # assumes name is a valid digestmod name + return digestmod + elif callable(digestmod): + # it's a callable object, check if it's an _hashopenssl.c object + dundername = getattr(digestmod, "__name__", None) + if ( + dundername and dundername.startswith("openssl_") and + getattr(_hashlib, dundername, None) is digestmod + ): + # it's a hash constructor from _hashopenssl.c, chop of prefix + return dundername[8:] + elif from_instance: + # it's some other callable, get PEP 452 name + return digestmod().name + else: + return None + else: + # it has to be a module with "new" function + if from_instance: + return digestmod.new().name + return None + + # Cleanup locals() del __always_supported, __func_name, __get_hash del __py_new, __hash_new, __get_openssl_constructor diff --git a/Lib/hmac.py b/Lib/hmac.py index 180bc378b52d62..56d696e033da0f 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -8,10 +8,8 @@ import _hashlib as _hashopenssl except ImportError: _hashopenssl = None - _openssl_md_meths = None from _operator import _compare_digest as compare_digest else: - _openssl_md_meths = frozenset(_hashopenssl.openssl_md_meth_names) compare_digest = _hashopenssl.compare_digest import hashlib as _hashlib @@ -23,7 +21,6 @@ digest_size = None - class HMAC: """RFC 2104 HMAC class. Also complies with RFC 4231. @@ -32,7 +29,7 @@ class HMAC: blocksize = 64 # 512-bit HMAC; can be changed in subclasses. __slots__ = ( - "_digest_cons", "_inner", "_outer", "block_size", "digest_size" + "_hmac", "_inner", "_outer", "_name", "block_size", "digest_size" ) def __init__(self, key, msg=None, digestmod=''): @@ -55,15 +52,32 @@ def __init__(self, key, msg=None, digestmod=''): if not digestmod: raise TypeError("Missing required parameter 'digestmod'.") + if ( + _hashopenssl is not None and + (digestname := _hashlib._digestmod_to_name(digestmod)) + ): + self._init_hmac(key, msg, digestname) + self._inner = self._outer = None + else: + self._init_old(key, msg, digestmod) + self._hmac = None + + def _init_hmac(self, key, msg, digestname): + self._hmac = _hashopenssl.hmac_new(key, msg, digestmod=digestname) + self._name = digestname + self.digest_size = self._hmac.digest_size + self.block_size = self._hmac.block_size + + def _init_old(self, key, msg, digestmod): if callable(digestmod): - self._digest_cons = digestmod + digest_cons = digestmod elif isinstance(digestmod, str): - self._digest_cons = lambda d=b'': _hashlib.new(digestmod, d) + digest_cons = lambda d=b'': _hashlib.new(digestmod, d) else: - self._digest_cons = lambda d=b'': digestmod.new(d) + digest_cons = lambda d=b'': digestmod.new(d) - self._outer = self._digest_cons() - self._inner = self._digest_cons() + self._outer = digest_cons() + self._inner = digest_cons() self.digest_size = self._inner.digest_size if hasattr(self._inner, 'block_size'): @@ -79,13 +93,13 @@ def __init__(self, key, msg=None, digestmod=''): RuntimeWarning, 2) blocksize = self.blocksize + if len(key) > blocksize: + key = digest_cons(key).digest() + # self.blocksize is the default blocksize. self.block_size is # effective block size as well as the public API attribute. self.block_size = blocksize - if len(key) > blocksize: - key = self._digest_cons(key).digest() - key = key.ljust(blocksize, b'\0') self._outer.update(key.translate(trans_5C)) self._inner.update(key.translate(trans_36)) @@ -94,23 +108,14 @@ def __init__(self, key, msg=None, digestmod=''): @property def name(self): - return "hmac-" + self._inner.name - - @property - def digest_cons(self): - return self._digest_cons - - @property - def inner(self): - return self._inner - - @property - def outer(self): - return self._outer + return "hmac-" + self._name def update(self, msg): """Feed data from msg into this hashing object.""" - self._inner.update(msg) + obj = self._hmac + if obj is None: + obj = self._inner + obj.update(msg) def copy(self): """Return a separate copy of this hashing object. @@ -119,10 +124,15 @@ def copy(self): """ # Call __new__ directly to avoid the expensive __init__. other = self.__class__.__new__(self.__class__) - other._digest_cons = self._digest_cons other.digest_size = self.digest_size - other._inner = self._inner.copy() - other._outer = self._outer.copy() + other._name = self._name + if self._hmac is not None: + other._hmac = self._hmac.copy() + other._inner = other._outer = None + else: + other._hmac = None + other._inner = self._inner.copy() + other._outer = self._outer.copy() return other def _current(self): @@ -141,14 +151,20 @@ def digest(self): not altered in any way by this function; you can continue updating the object after calling this function. """ - h = self._current() - return h.digest() + if self._hmac is not None: + return self._hmac.digest() + else: + h = self._current() + return h.digest() def hexdigest(self): """Like digest(), but returns a string of hexadecimal digits instead. """ - h = self._current() - return h.hexdigest() + if self._hmac is not None: + return self._hmac.hexdigest() + else: + h = self._current() + return h.hexdigest() def new(key, msg=None, digestmod=''): """Create a new hashing object and return it. @@ -179,9 +195,11 @@ def digest(key, msg, digest): A hashlib constructor returning a new hash object. *OR* A module supporting PEP 247. """ - if (_hashopenssl is not None and - isinstance(digest, str) and digest in _openssl_md_meths): - return _hashopenssl.hmac_digest(key, msg, digest) + if ( + _hashopenssl is not None and + (digestname := _hashlib._digestmod_to_name(digest)) + ): + return _hashopenssl.hmac_digest(key, msg, digestname) if callable(digest): digest_cons = digest From 6dd9fa9b436b44f127213e3378202035d98c95fe Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Mar 2021 21:01:23 +0100 Subject: [PATCH 2/7] Better approach to convert digestmod to EVP Recognize _hashlib.openssl_sha256 function object as "sha256". --- Lib/hashlib.py | 27 -------- Lib/hmac.py | 33 ++++----- Lib/test/test_hmac.py | 72 +++++++++++-------- Modules/_hashopenssl.c | 119 ++++++++++++++++++++++++++++---- Modules/clinic/_hashopenssl.c.h | 38 ++-------- 5 files changed, 171 insertions(+), 118 deletions(-) diff --git a/Lib/hashlib.py b/Lib/hashlib.py index f063a92a5ccae8..ffa3be049a4f35 100644 --- a/Lib/hashlib.py +++ b/Lib/hashlib.py @@ -258,33 +258,6 @@ def prf(msg, inner=inner, outer=outer): logging.exception('code for hash %s was not found.', __func_name) -def _digestmod_to_name(digestmod, from_instance=False): - """Convert a string, callable or digest module to a digest name - """ - if isinstance(digestmod, str): - # assumes name is a valid digestmod name - return digestmod - elif callable(digestmod): - # it's a callable object, check if it's an _hashopenssl.c object - dundername = getattr(digestmod, "__name__", None) - if ( - dundername and dundername.startswith("openssl_") and - getattr(_hashlib, dundername, None) is digestmod - ): - # it's a hash constructor from _hashopenssl.c, chop of prefix - return dundername[8:] - elif from_instance: - # it's some other callable, get PEP 452 name - return digestmod().name - else: - return None - else: - # it has to be a module with "new" function - if from_instance: - return digestmod.new().name - return None - - # Cleanup locals() del __always_supported, __func_name, __get_hash del __py_new, __hash_new, __get_openssl_constructor diff --git a/Lib/hmac.py b/Lib/hmac.py index 56d696e033da0f..cc4c17de9ac4a7 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -8,9 +8,12 @@ import _hashlib as _hashopenssl except ImportError: _hashopenssl = None + _functype = None from _operator import _compare_digest as compare_digest else: compare_digest = _hashopenssl.compare_digest + _functype = type(_hashopenssl.openssl_sha256) # builtin type + import hashlib as _hashlib trans_5C = bytes((x ^ 0x5C) for x in range(256)) @@ -29,7 +32,7 @@ class HMAC: blocksize = 64 # 512-bit HMAC; can be changed in subclasses. __slots__ = ( - "_hmac", "_inner", "_outer", "_name", "block_size", "digest_size" + "_hmac", "_inner", "_outer", "block_size", "digest_size" ) def __init__(self, key, msg=None, digestmod=''): @@ -52,19 +55,13 @@ def __init__(self, key, msg=None, digestmod=''): if not digestmod: raise TypeError("Missing required parameter 'digestmod'.") - if ( - _hashopenssl is not None and - (digestname := _hashlib._digestmod_to_name(digestmod)) - ): - self._init_hmac(key, msg, digestname) - self._inner = self._outer = None + if _hashopenssl is not None and isinstance(digestmod, (str, _functype)): + self._init_hmac(key, msg, digestmod) else: self._init_old(key, msg, digestmod) - self._hmac = None - def _init_hmac(self, key, msg, digestname): - self._hmac = _hashopenssl.hmac_new(key, msg, digestmod=digestname) - self._name = digestname + def _init_hmac(self, key, msg, digestmod): + self._hmac = _hashopenssl.hmac_new(key, msg, digestmod=digestmod) self.digest_size = self._hmac.digest_size self.block_size = self._hmac.block_size @@ -76,6 +73,7 @@ def _init_old(self, key, msg, digestmod): else: digest_cons = lambda d=b'': digestmod.new(d) + self._hmac = None self._outer = digest_cons() self._inner = digest_cons() self.digest_size = self._inner.digest_size @@ -108,7 +106,10 @@ def _init_old(self, key, msg, digestmod): @property def name(self): - return "hmac-" + self._name + if self._hmac: + return self._hmac.name + else: + return f"hmac-{self._inner.name}" def update(self, msg): """Feed data from msg into this hashing object.""" @@ -125,7 +126,6 @@ def copy(self): # Call __new__ directly to avoid the expensive __init__. other = self.__class__.__new__(self.__class__) other.digest_size = self.digest_size - other._name = self._name if self._hmac is not None: other._hmac = self._hmac.copy() other._inner = other._outer = None @@ -195,11 +195,8 @@ def digest(key, msg, digest): A hashlib constructor returning a new hash object. *OR* A module supporting PEP 247. """ - if ( - _hashopenssl is not None and - (digestname := _hashlib._digestmod_to_name(digest)) - ): - return _hashopenssl.hmac_digest(key, msg, digestname) + if _hashopenssl is not None and isinstance(digest, (str, _functype)): + return _hashopenssl.hmac_digest(key, msg, digest) if callable(digest): digest_cons = digest diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index 6daf22ca06fb80..032ea025aaebe4 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -11,10 +11,12 @@ from _operator import _compare_digest as operator_compare_digest try: + import _hashlib as _hashopenssl from _hashlib import HMAC as C_HMAC from _hashlib import hmac_new as c_hmac_new from _hashlib import compare_digest as openssl_compare_digest except ImportError: + _hashopenssl = None C_HMAC = None c_hmac_new = None openssl_compare_digest = None @@ -32,22 +34,27 @@ def wrapper(*args, **kwargs): class TestVectorsTestCase(unittest.TestCase): - def asssert_hmac( - self, key, data, digest, hashfunc, hashname, digest_size, block_size + def assert_hmac_internals( + self, h, digest, hashname, digest_size, block_size ): - h = hmac.HMAC(key, data, digestmod=hashfunc) self.assertEqual(h.hexdigest().upper(), digest.upper()) self.assertEqual(h.digest(), binascii.unhexlify(digest)) self.assertEqual(h.name, f"hmac-{hashname}") self.assertEqual(h.digest_size, digest_size) self.assertEqual(h.block_size, block_size) + def assert_hmac( + self, key, data, digest, hashfunc, hashname, digest_size, block_size + ): + h = hmac.HMAC(key, data, digestmod=hashfunc) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) + h = hmac.HMAC(key, data, digestmod=hashname) - self.assertEqual(h.hexdigest().upper(), digest.upper()) - self.assertEqual(h.digest(), binascii.unhexlify(digest)) - self.assertEqual(h.name, f"hmac-{hashname}") - self.assertEqual(h.digest_size, digest_size) - self.assertEqual(h.block_size, block_size) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) h = hmac.HMAC(key, digestmod=hashname) h2 = h.copy() @@ -56,11 +63,9 @@ def asssert_hmac( self.assertEqual(h.hexdigest().upper(), digest.upper()) h = hmac.new(key, data, digestmod=hashname) - self.assertEqual(h.hexdigest().upper(), digest.upper()) - self.assertEqual(h.digest(), binascii.unhexlify(digest)) - self.assertEqual(h.name, f"hmac-{hashname}") - self.assertEqual(h.digest_size, digest_size) - self.assertEqual(h.block_size, block_size) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) h = hmac.new(key, None, digestmod=hashname) h.update(data) @@ -81,23 +86,18 @@ def asssert_hmac( hmac.digest(key, data, digest=hashfunc), binascii.unhexlify(digest) ) - with unittest.mock.patch('hmac._openssl_md_meths', {}): - self.assertEqual( - hmac.digest(key, data, digest=hashname), - binascii.unhexlify(digest) - ) - self.assertEqual( - hmac.digest(key, data, digest=hashfunc), - binascii.unhexlify(digest) - ) + + h = hmac.HMAC.__new__(hmac.HMAC) + h._init_old(key, data, digestmod=hashname) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) if c_hmac_new is not None: h = c_hmac_new(key, data, digestmod=hashname) - self.assertEqual(h.hexdigest().upper(), digest.upper()) - self.assertEqual(h.digest(), binascii.unhexlify(digest)) - self.assertEqual(h.name, f"hmac-{hashname}") - self.assertEqual(h.digest_size, digest_size) - self.assertEqual(h.block_size, block_size) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) h = c_hmac_new(key, digestmod=hashname) h2 = h.copy() @@ -105,12 +105,24 @@ def asssert_hmac( h.update(data) self.assertEqual(h.hexdigest().upper(), digest.upper()) + func = getattr(_hashopenssl, f"openssl_{hashname}") + h = c_hmac_new(key, data, digestmod=func) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) + + h = hmac.HMAC.__new__(hmac.HMAC) + h._init_hmac(key, data, digestmod=hashname) + self.assert_hmac_internals( + h, digest, hashname, digest_size, block_size + ) + @hashlib_helper.requires_hashdigest('md5', openssl=True) def test_md5_vectors(self): # Test the HMAC module against test vectors from the RFC. def md5test(key, data, digest): - self.asssert_hmac( + self.assert_hmac( key, data, digest, hashfunc=hashlib.md5, hashname="md5", @@ -150,7 +162,7 @@ def md5test(key, data, digest): @hashlib_helper.requires_hashdigest('sha1', openssl=True) def test_sha_vectors(self): def shatest(key, data, digest): - self.asssert_hmac( + self.assert_hmac( key, data, digest, hashfunc=hashlib.sha1, hashname="sha1", @@ -191,7 +203,7 @@ def _rfc4231_test_cases(self, hashfunc, hash_name, digest_size, block_size): def hmactest(key, data, hexdigests): digest = hexdigests[hashfunc] - self.asssert_hmac( + self.assert_hmac( key, data, digest, hashfunc=hashfunc, hashname=hash_name, diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index d4295d7c3638d6..5cacfcab3ee3eb 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -86,6 +86,7 @@ typedef struct { #ifdef PY_OPENSSL_HAS_SHAKE PyTypeObject *EVPXOFtype; #endif + PyObject *constructs; } _hashlibstate; static inline _hashlibstate* @@ -292,6 +293,46 @@ py_digest_by_name(const char *name) return digest; } +/* Get digest EVP from object + * + * * string + * * _hashopenssl builtin function + * + * on error returns NULL with exception set. + */ +static const EVP_MD* +py_digest_by_digestmod(PyObject *module, PyObject *digestmod) { + const EVP_MD* evp; + PyObject *name_obj = NULL; + const char *name; + + if (PyUnicode_Check(digestmod)) { + name_obj = digestmod; + } else { + _hashlibstate *state = get_hashlib_state(module); + // borrowed ref + name_obj = PyDict_GetItem(state->constructs, digestmod); + } + if (name_obj == NULL) { + PyErr_Clear(); + PyErr_Format(PyExc_ValueError, "Unsupported digestmod %R", digestmod); + return NULL; + } + + name = PyUnicode_AsUTF8(name_obj); + if (name == NULL) { + return NULL; + } + + evp = py_digest_by_name(name); + if (evp == NULL) { + PyErr_Format(PyExc_ValueError, "unsupported hash type %s", name); + return NULL; + } + + return evp; +} + static EVPobject * newEVPobject(PyTypeObject *type) { @@ -1328,26 +1369,26 @@ _hashlib.hmac_digest as _hashlib_hmac_singleshot key: Py_buffer msg: Py_buffer - digest: str + digest: object Single-shot HMAC. [clinic start generated code]*/ static PyObject * _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, - Py_buffer *msg, const char *digest) -/*[clinic end generated code: output=15658ede5ab98185 input=019dffc571909a46]*/ + Py_buffer *msg, PyObject *digest) +/*[clinic end generated code: output=82f19965d12706ac input=0a0790cc3db45c2e]*/ { unsigned char md[EVP_MAX_MD_SIZE] = {0}; unsigned int md_len = 0; unsigned char *result; const EVP_MD *evp; - evp = py_digest_by_name(digest); + evp = py_digest_by_digestmod(module, digest); if (evp == NULL) { - PyErr_SetString(PyExc_ValueError, "unsupported hash type"); return NULL; } + if (key->len > INT_MAX) { PyErr_SetString(PyExc_OverflowError, "key is too long."); @@ -1385,15 +1426,15 @@ _hashlib.hmac_new key: Py_buffer msg as msg_obj: object(c_default="NULL") = b'' - digestmod: str(c_default="NULL") = None + digestmod: object(c_default="NULL") = None Return a new hmac object. [clinic start generated code]*/ static PyObject * _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, - const char *digestmod) -/*[clinic end generated code: output=9a35673be0cbea1b input=a0878868eb190134]*/ + PyObject *digestmod) +/*[clinic end generated code: output=c20d9e4d9ed6d219 input=5f4071dcc7f34362]*/ { PyTypeObject *type = get_hashlib_state(module)->HMACtype; const EVP_MD *digest; @@ -1407,15 +1448,14 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, return NULL; } - if ((digestmod == NULL) || !strlen(digestmod)) { + if (digestmod == NULL) { PyErr_SetString( PyExc_TypeError, "Missing required parameter 'digestmod'."); return NULL; } - digest = py_digest_by_name(digestmod); - if (!digest) { - PyErr_SetString(PyExc_ValueError, "unknown hash function"); + digest = py_digest_by_digestmod(module, digestmod); + if (digest == NULL) { return NULL; } @@ -1985,6 +2025,7 @@ hashlib_traverse(PyObject *m, visitproc visit, void *arg) #ifdef PY_OPENSSL_HAS_SHAKE Py_VISIT(state->EVPXOFtype); #endif + Py_VISIT(state->constructs); return 0; } @@ -1997,6 +2038,7 @@ hashlib_clear(PyObject *m) #ifdef PY_OPENSSL_HAS_SHAKE Py_CLEAR(state->EVPXOFtype); #endif + Py_CLEAR(state->constructs); return 0; } @@ -2071,6 +2113,58 @@ hashlib_init_hmactype(PyObject *module) return 0; } +static int +hashlib_init_constructors(PyObject *module) +{ + /* Create dict from builtin openssl_hash functions to name + * {_hashlib.openssl_sha256: "sha256", ...} + */ + PyModuleDef *mdef; + PyMethodDef *fdef; + PyObject *proxy; + PyObject *func, *name_obj; + _hashlibstate *state = get_hashlib_state(module); + + mdef = PyModule_GetDef(module); + if (mdef == NULL) { + return -1; + } + + state->constructs = PyDict_New(); + if (state->constructs == NULL) { + return -1; + } + + for (fdef = mdef->m_methods; fdef->ml_name != NULL; fdef++) { + if (strncmp(fdef->ml_name, "openssl_", 8)) { + continue; + } + name_obj = PyUnicode_FromString(fdef->ml_name + 8); + if (name_obj == NULL) { + return -1; + } + func = PyObject_GetAttrString(module, fdef->ml_name); + if (func == NULL) { + return -1; + } + if (PyDict_SetItem(state->constructs, func, name_obj) < 0) { + return -1; + } + Py_DECREF(func); + Py_DECREF(name_obj); + } + + proxy = PyDictProxy_New(state->constructs); + if (proxy == NULL) { + return -1; + } + if (PyModule_AddObjectRef(module, "constructors", proxy) < 0) { + return -1; + } + return 0; +} + + static PyModuleDef_Slot hashlib_slots[] = { /* OpenSSL 1.0.2 and LibreSSL */ {Py_mod_exec, hashlib_openssl_legacy_init}, @@ -2078,6 +2172,7 @@ static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_init_evpxoftype}, {Py_mod_exec, hashlib_init_hmactype}, {Py_mod_exec, hashlib_md_meth_names}, + {Py_mod_exec, hashlib_init_constructors}, {0, NULL} }; diff --git a/Modules/clinic/_hashopenssl.c.h b/Modules/clinic/_hashopenssl.c.h index e72b55885fe1cb..fbdff26026f6cc 100644 --- a/Modules/clinic/_hashopenssl.c.h +++ b/Modules/clinic/_hashopenssl.c.h @@ -1081,7 +1081,7 @@ PyDoc_STRVAR(_hashlib_hmac_singleshot__doc__, static PyObject * _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, - Py_buffer *msg, const char *digest); + Py_buffer *msg, PyObject *digest); static PyObject * _hashlib_hmac_singleshot(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) @@ -1092,7 +1092,7 @@ _hashlib_hmac_singleshot(PyObject *module, PyObject *const *args, Py_ssize_t nar PyObject *argsbuf[3]; Py_buffer key = {NULL, NULL}; Py_buffer msg = {NULL, NULL}; - const char *digest; + PyObject *digest; args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 3, 3, 0, argsbuf); if (!args) { @@ -1112,19 +1112,7 @@ _hashlib_hmac_singleshot(PyObject *module, PyObject *const *args, Py_ssize_t nar _PyArg_BadArgument("hmac_digest", "argument 'msg'", "contiguous buffer", args[1]); goto exit; } - if (!PyUnicode_Check(args[2])) { - _PyArg_BadArgument("hmac_digest", "argument 'digest'", "str", args[2]); - goto exit; - } - Py_ssize_t digest_length; - digest = PyUnicode_AsUTF8AndSize(args[2], &digest_length); - if (digest == NULL) { - goto exit; - } - if (strlen(digest) != (size_t)digest_length) { - PyErr_SetString(PyExc_ValueError, "embedded null character"); - goto exit; - } + digest = args[2]; return_value = _hashlib_hmac_singleshot_impl(module, &key, &msg, digest); exit: @@ -1151,7 +1139,7 @@ PyDoc_STRVAR(_hashlib_hmac_new__doc__, static PyObject * _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, - const char *digestmod); + PyObject *digestmod); static PyObject * _hashlib_hmac_new(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) @@ -1163,7 +1151,7 @@ _hashlib_hmac_new(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1; Py_buffer key = {NULL, NULL}; PyObject *msg_obj = NULL; - const char *digestmod = NULL; + PyObject *digestmod = NULL; args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 3, 0, argsbuf); if (!args) { @@ -1185,19 +1173,7 @@ _hashlib_hmac_new(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO goto skip_optional_pos; } } - if (!PyUnicode_Check(args[2])) { - _PyArg_BadArgument("hmac_new", "argument 'digestmod'", "str", args[2]); - goto exit; - } - Py_ssize_t digestmod_length; - digestmod = PyUnicode_AsUTF8AndSize(args[2], &digestmod_length); - if (digestmod == NULL) { - goto exit; - } - if (strlen(digestmod) != (size_t)digestmod_length) { - PyErr_SetString(PyExc_ValueError, "embedded null character"); - goto exit; - } + digestmod = args[2]; skip_optional_pos: return_value = _hashlib_hmac_new_impl(module, &key, msg_obj, digestmod); @@ -1417,4 +1393,4 @@ _hashlib_compare_digest(PyObject *module, PyObject *const *args, Py_ssize_t narg #ifndef _HASHLIB_GET_FIPS_MODE_METHODDEF #define _HASHLIB_GET_FIPS_MODE_METHODDEF #endif /* !defined(_HASHLIB_GET_FIPS_MODE_METHODDEF) */ -/*[clinic end generated code: output=2bbd6159493f44ea input=a9049054013a1b77]*/ +/*[clinic end generated code: output=980087de1b03ad42 input=a9049054013a1b77]*/ From 37825534f3089af253595d6bf8d6b40c0f1d9d9d Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 18 Mar 2021 22:54:46 +0100 Subject: [PATCH 3/7] Address review comments --- Lib/hmac.py | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/Lib/hmac.py b/Lib/hmac.py index cc4c17de9ac4a7..69badc8bdd7c9e 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -55,8 +55,11 @@ def __init__(self, key, msg=None, digestmod=''): if not digestmod: raise TypeError("Missing required parameter 'digestmod'.") - if _hashopenssl is not None and isinstance(digestmod, (str, _functype)): - self._init_hmac(key, msg, digestmod) + if _hashopenssl and isinstance(digestmod, (str, _functype)): + try: + self._init_hmac(key, msg, digestmod) + except ValueError: + self._init_old(key, msg, digestmod) else: self._init_old(key, msg, digestmod) @@ -113,10 +116,8 @@ def name(self): def update(self, msg): """Feed data from msg into this hashing object.""" - obj = self._hmac - if obj is None: - obj = self._inner - obj.update(msg) + inst = self._hmac or self._inner + inst.update(msg) def copy(self): """Return a separate copy of this hashing object. @@ -126,7 +127,7 @@ def copy(self): # Call __new__ directly to avoid the expensive __init__. other = self.__class__.__new__(self.__class__) other.digest_size = self.digest_size - if self._hmac is not None: + if self._hmac: other._hmac = self._hmac.copy() other._inner = other._outer = None else: @@ -140,9 +141,12 @@ def _current(self): To be used only internally with digest() and hexdigest(). """ - h = self._outer.copy() - h.update(self._inner.digest()) - return h + if self._hmac: + return self._hmac + else: + h = self._outer.copy() + h.update(self._inner.digest()) + return h def digest(self): """Return the hash value of this hashing object. @@ -151,20 +155,14 @@ def digest(self): not altered in any way by this function; you can continue updating the object after calling this function. """ - if self._hmac is not None: - return self._hmac.digest() - else: - h = self._current() - return h.digest() + h = self._current() + return h.digest() def hexdigest(self): """Like digest(), but returns a string of hexadecimal digits instead. """ - if self._hmac is not None: - return self._hmac.hexdigest() - else: - h = self._current() - return h.hexdigest() + h = self._current() + return h.hexdigest() def new(key, msg=None, digestmod=''): """Create a new hashing object and return it. @@ -196,7 +194,10 @@ def digest(key, msg, digest): A module supporting PEP 247. """ if _hashopenssl is not None and isinstance(digest, (str, _functype)): - return _hashopenssl.hmac_digest(key, msg, digest) + try: + return _hashopenssl.hmac_digest(key, msg, digest) + except ValueError: + pass if callable(digest): digest_cons = digest From f76bf5d8a8d1597c74583078b3a577c79221a6df Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Mar 2021 09:22:37 +0100 Subject: [PATCH 4/7] Fix test cases --- Lib/test/test_hmac.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index 032ea025aaebe4..d1013145526408 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -459,21 +459,21 @@ def test_exercise_all_methods(self): class CopyTestCase(unittest.TestCase): @hashlib_helper.requires_hashdigest('sha256') - def test_attributes(self): + def test_attributes_old(self): # Testing if attributes are of same type. - h1 = hmac.HMAC(b"key", digestmod="sha256") + h1 = hmac.HMAC.__new__(hmac.HMAC) + h1._init_old(b"key", b"msg", digestmod="sha256") h2 = h1.copy() - self.assertTrue(h1._digest_cons == h2._digest_cons, - "digest constructors don't match.") self.assertEqual(type(h1._inner), type(h2._inner), "Types of inner don't match.") self.assertEqual(type(h1._outer), type(h2._outer), "Types of outer don't match.") @hashlib_helper.requires_hashdigest('sha256') - def test_realcopy(self): + def test_realcopy_old(self): # Testing if the copy method created a real copy. - h1 = hmac.HMAC(b"key", digestmod="sha256") + h1 = hmac.HMAC.__new__(hmac.HMAC) + h1._init_old(b"key", b"msg", digestmod="sha256") h2 = h1.copy() # Using id() in case somebody has overridden __eq__/__ne__. self.assertTrue(id(h1) != id(h2), "No real copy of the HMAC instance.") @@ -481,17 +481,15 @@ def test_realcopy(self): "No real copy of the attribute 'inner'.") self.assertTrue(id(h1._outer) != id(h2._outer), "No real copy of the attribute 'outer'.") - self.assertEqual(h1._inner, h1.inner) - self.assertEqual(h1._outer, h1.outer) - self.assertEqual(h1._digest_cons, h1.digest_cons) + self.assertIs(h1._hmac, None) + @unittest.skipIf(_hashopenssl is None, "test requires _hashopenssl") @hashlib_helper.requires_hashdigest('sha256') - def test_properties(self): - # deprecated properties - h1 = hmac.HMAC(b"key", digestmod="sha256") - self.assertEqual(h1._inner, h1.inner) - self.assertEqual(h1._outer, h1.outer) - self.assertEqual(h1._digest_cons, h1.digest_cons) + def test_realcopy_hmac(self): + h1 = hmac.HMAC.__new__(hmac.HMAC) + h1._init_hmac(b"key", b"msg", digestmod="sha256") + h2 = h1.copy() + self.assertTrue(id(h1._hmac) != id(h2._hmac)) @hashlib_helper.requires_hashdigest('sha256') def test_equality(self): From 504e08e84bf98a09980dbdd1f240770c25903a33 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Mar 2021 09:35:49 +0100 Subject: [PATCH 5/7] Test with non-openssl digest module --- Lib/test/test_hmac.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index d1013145526408..adf52adbf22afa 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -21,6 +21,11 @@ c_hmac_new = None openssl_compare_digest = None +try: + import _sha256 as sha256_module +except ImportError: + sha256_module = None + def ignore_warning(func): @functools.wraps(func) @@ -439,6 +444,15 @@ def test_internal_types(self): ): C_HMAC() + @unittest.skipUnless(sha256_module is not None, 'need _sha256') + def test_with_sha256_module(self): + h = hmac.HMAC(b"key", b"hash this!", digestmod=sha256_module.sha256) + self.assertEqual(h.hexdigest(), self.expected) + self.assertEqual(h.name, "hmac-sha256") + + digest = hmac.digest(b"key", b"hash this!", sha256_module.sha256) + self.assertEqual(digest, binascii.unhexlify(self.expected)) + class SanityTestCase(unittest.TestCase): From 83ae2d38612f915fe27b21f4fd633a4e68295bc5 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Mar 2021 09:36:28 +0100 Subject: [PATCH 6/7] Add new internal exception to indicate unsupported constructor --- Lib/hmac.py | 4 ++-- Modules/_hashopenssl.c | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Lib/hmac.py b/Lib/hmac.py index 69badc8bdd7c9e..8b4f920db954ca 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -58,7 +58,7 @@ def __init__(self, key, msg=None, digestmod=''): if _hashopenssl and isinstance(digestmod, (str, _functype)): try: self._init_hmac(key, msg, digestmod) - except ValueError: + except _hashopenssl.UnsupportedDigestmodError: self._init_old(key, msg, digestmod) else: self._init_old(key, msg, digestmod) @@ -196,7 +196,7 @@ def digest(key, msg, digest): if _hashopenssl is not None and isinstance(digest, (str, _functype)): try: return _hashopenssl.hmac_digest(key, msg, digest) - except ValueError: + except _hashopenssl.UnsupportedDigestmodError: pass if callable(digest): diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 5cacfcab3ee3eb..6c83b9e4a41fa2 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -87,6 +87,7 @@ typedef struct { PyTypeObject *EVPXOFtype; #endif PyObject *constructs; + PyObject *unsupported_digestmod_error; } _hashlibstate; static inline _hashlibstate* @@ -290,6 +291,11 @@ py_digest_by_name(const char *name) #endif } + if (digest == NULL) { + PyErr_Format(PyExc_ValueError, "unsupported hash type %s", name); + return NULL; + } + return digest; } @@ -314,8 +320,11 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod) { name_obj = PyDict_GetItem(state->constructs, digestmod); } if (name_obj == NULL) { + _hashlibstate *state = get_hashlib_state(module); PyErr_Clear(); - PyErr_Format(PyExc_ValueError, "Unsupported digestmod %R", digestmod); + PyErr_Format( + state->unsupported_digestmod_error, + "Unsupported digestmod %R", digestmod); return NULL; } @@ -326,7 +335,6 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod) { evp = py_digest_by_name(name); if (evp == NULL) { - PyErr_Format(PyExc_ValueError, "unsupported hash type %s", name); return NULL; } @@ -870,6 +878,9 @@ EVP_new_impl(PyObject *module, PyObject *name_obj, PyObject *data_obj, GET_BUFFER_VIEW_OR_ERROUT(data_obj, &view); digest = py_digest_by_name(name); + if (digest == NULL) { + return NULL; + } ret_obj = EVPnew(module, digest, (unsigned char*)view.buf, view.len, @@ -1165,7 +1176,6 @@ pbkdf2_hmac_impl(PyObject *module, const char *hash_name, digest = py_digest_by_name(hash_name); if (digest == NULL) { - PyErr_SetString(PyExc_ValueError, "unsupported hash type"); goto end; } @@ -2026,6 +2036,7 @@ hashlib_traverse(PyObject *m, visitproc visit, void *arg) Py_VISIT(state->EVPXOFtype); #endif Py_VISIT(state->constructs); + Py_VISIT(state->unsupported_digestmod_error); return 0; } @@ -2039,6 +2050,7 @@ hashlib_clear(PyObject *m) Py_CLEAR(state->EVPXOFtype); #endif Py_CLEAR(state->constructs); + Py_CLEAR(state->unsupported_digestmod_error); return 0; } @@ -2158,7 +2170,23 @@ hashlib_init_constructors(PyObject *module) if (proxy == NULL) { return -1; } - if (PyModule_AddObjectRef(module, "constructors", proxy) < 0) { + if (PyModule_AddObjectRef(module, "_constructors", proxy) < 0) { + return -1; + } + return 0; +} + +static int +hashlib_exception(PyObject *module) +{ + _hashlibstate *state = get_hashlib_state(module); + state->unsupported_digestmod_error = PyErr_NewException( + "_hashlib.UnsupportedDigestmodError", PyExc_ValueError, NULL); + if (state->unsupported_digestmod_error == NULL) { + return -1; + } + if (PyModule_AddObjectRef(module, "UnsupportedDigestmodError", + state->unsupported_digestmod_error) < 0) { return -1; } return 0; @@ -2173,6 +2201,7 @@ static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_init_hmactype}, {Py_mod_exec, hashlib_md_meth_names}, {Py_mod_exec, hashlib_init_constructors}, + {Py_mod_exec, hashlib_exception}, {0, NULL} }; From 9ff2d864b723ada99a0e3912a58236bcd324280d Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 19 Mar 2021 10:23:09 +0100 Subject: [PATCH 7/7] blurb --- .../next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst diff --git a/Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst b/Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst new file mode 100644 index 00000000000000..a9ab1c09150bb0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst @@ -0,0 +1,2 @@ +The :mod:`hmac` module now uses OpenSSL's HMAC implementation when digestmod +argument is a hash name or builtin hash function.