https://github.com/python/cpython/commit/ac50ece6cea8745834e4ec0a9617809a51245bfc commit: ac50ece6cea8745834e4ec0a9617809a51245bfc branch: main author: Bénédikt Tran <10796600+picn...@users.noreply.github.com> committer: picnixz <10796600+picn...@users.noreply.github.com> date: 2025-03-17T11:12:55+01:00 summary:
gh-127667: refactor and improve `_hashopenssl.c` error branches (#131145) Refactor `_setException()` into different helpers that can be used separately: - set_ssl_exception_from_errcode(): set an exception from an explicit SSL error code. - raise_ssl_error(): set an exception from the last SSL error code or use a user-defined message. - notify_ssl_error_occurred(): same as raise_ssl_error() but with a generic default message. files: M Modules/_hashopenssl.c diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 360f057ed80952..08f5c0ece0a18c 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -304,30 +304,17 @@ class _hashlib.HMAC "HMACobject *" "((_hashlibstate *)PyModule_GetState(module)) /* LCOV_EXCL_START */ -static PyObject * -_setException(PyObject *exc, const char* altmsg, ...) -{ - unsigned long errcode = ERR_peek_last_error(); - const char *lib, *func, *reason; - va_list vargs; - va_start(vargs, altmsg); - if (!errcode) { - if (altmsg == NULL) { - PyErr_SetString(exc, "no reason supplied"); - } else { - PyErr_FormatV(exc, altmsg, vargs); - } - va_end(vargs); - return NULL; - } - va_end(vargs); - ERR_clear_error(); +/* Set an exception of given type using the given OpenSSL error code. */ +static void +set_ssl_exception_from_errcode(PyObject *exc, unsigned long errcode) +{ + assert(errcode != 0); /* ERR_ERROR_STRING(3) ensures that the messages below are ASCII */ - lib = ERR_lib_error_string(errcode); - func = ERR_func_error_string(errcode); - reason = ERR_reason_error_string(errcode); + const char *lib = ERR_lib_error_string(errcode); + const char *func = ERR_func_error_string(errcode); + const char *reason = ERR_reason_error_string(errcode); if (lib && func) { PyErr_Format(exc, "[%s: %s] %s", lib, func, reason); @@ -338,13 +325,49 @@ _setException(PyObject *exc, const char* altmsg, ...) else { PyErr_SetString(exc, reason); } - return NULL; +} + +/* + * Set an exception of given type. + * + * By default, the exception's message is constructed by using the last SSL + * error that occurred. If no error occurred, the 'fallback_format' is used + * to create a C-style formatted fallback message. + */ +static void +raise_ssl_error(PyObject *exc, const char *fallback_format, ...) +{ + assert(fallback_format != NULL); + unsigned long errcode = ERR_peek_last_error(); + if (errcode) { + ERR_clear_error(); + set_ssl_exception_from_errcode(exc, errcode); + } + else { + va_list vargs; + va_start(vargs, fallback_format); + PyErr_FormatV(exc, fallback_format, vargs); + va_end(vargs); + } +} + +/* + * Set an exception with a generic default message after an error occurred. + * + * It can also be used without previous calls to SSL built-in functions, + * in which case a generic error message is provided. + */ +static inline void +notify_ssl_error_occurred(void) +{ + raise_ssl_error(PyExc_ValueError, "no reason supplied"); } /* LCOV_EXCL_STOP */ static PyObject* py_digest_name(const EVP_MD *md) { + assert(md != NULL); int nid = EVP_MD_nid(md); const char *name = NULL; const py_hashentry_t *h; @@ -430,8 +453,8 @@ py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht) } } if (digest == NULL) { - (void)_setException(state->unsupported_digestmod_error, - "unsupported hash type %s", name); + raise_ssl_error(state->unsupported_digestmod_error, + "unsupported hash type %s", name); return NULL; } return digest; @@ -504,7 +527,7 @@ EVP_hash(EVPobject *self, const void *vp, Py_ssize_t len) else process = Py_SAFE_DOWNCAST(len, Py_ssize_t, unsigned int); if (!EVP_DigestUpdate(self->ctx, (const void*)cp, process)) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); return -1; } len -= process; @@ -554,7 +577,8 @@ EVP_copy_impl(EVPobject *self) if (!locked_EVP_MD_CTX_copy(newobj->ctx, self)) { Py_DECREF(newobj); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } return (PyObject *)newobj; } @@ -594,7 +618,8 @@ EVP_digest_impl(EVPobject *self) error: EVP_MD_CTX_free(temp_ctx); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } /*[clinic input] @@ -632,7 +657,8 @@ EVP_hexdigest_impl(EVPobject *self) error: EVP_MD_CTX_free(temp_ctx); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } /*[clinic input] @@ -703,7 +729,8 @@ EVP_get_name(PyObject *op, void *Py_UNUSED(closure)) EVPobject *self = EVPobject_CAST(op); const EVP_MD *md = EVP_MD_CTX_md(self->ctx); if (md == NULL) { - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } return py_digest_name(md); } @@ -808,7 +835,8 @@ EVPXOF_digest_impl(EVPobject *self, Py_ssize_t length) error: Py_DECREF(retval); EVP_MD_CTX_free(temp_ctx); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } /*[clinic input] @@ -857,7 +885,8 @@ EVPXOF_hexdigest_impl(EVPobject *self, Py_ssize_t length) error: PyMem_Free(digest); EVP_MD_CTX_free(temp_ctx); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } static PyMethodDef EVPXOF_methods[] = { @@ -955,7 +984,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj, int result = EVP_DigestInit_ex(self->ctx, digest, NULL); if (!result) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); Py_CLEAR(self); goto exit; } @@ -971,6 +1000,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj, result = EVP_hash(self, view.buf, view.len); } if (result == -1) { + assert(PyErr_Occurred()); Py_CLEAR(self); goto exit; } @@ -1337,19 +1367,19 @@ pbkdf2_hmac_impl(PyObject *module, const char *hash_name, key = PyBytes_AS_STRING(key_obj); Py_BEGIN_ALLOW_THREADS - retval = PKCS5_PBKDF2_HMAC((char*)password->buf, (int)password->len, - (unsigned char *)salt->buf, (int)salt->len, + retval = PKCS5_PBKDF2_HMAC((const char *)password->buf, (int)password->len, + (const unsigned char *)salt->buf, (int)salt->len, iterations, digest, dklen, (unsigned char *)key); Py_END_ALLOW_THREADS if (!retval) { Py_CLEAR(key_obj); - _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); goto end; } - end: +end: if (digest != NULL) { PY_EVP_MD_free(digest); } @@ -1451,8 +1481,9 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt, /* let OpenSSL validate the rest */ retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0); if (!retval) { - return _setException(PyExc_ValueError, - "Invalid parameter combination for n, r, p, maxmem."); + raise_ssl_error(PyExc_ValueError, + "Invalid parameter combination for n, r, p, maxmem."); + return NULL; } key_obj = PyBytes_FromStringAndSize(NULL, dklen); @@ -1472,7 +1503,8 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt, if (!retval) { Py_CLEAR(key_obj); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } return key_obj; } @@ -1520,15 +1552,16 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, Py_BEGIN_ALLOW_THREADS result = HMAC( evp, - (const void*)key->buf, (int)key->len, - (const unsigned char*)msg->buf, (int)msg->len, + (const void *)key->buf, (int)key->len, + (const unsigned char *)msg->buf, (size_t)msg->len, md, &md_len ); Py_END_ALLOW_THREADS PY_EVP_MD_free(evp); if (result == NULL) { - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } return PyBytes_FromStringAndSize((const char*)md, md_len); } @@ -1538,6 +1571,16 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, static int _hmac_update(HMACobject*, PyObject*); +static const EVP_MD * +_hashlib_hmac_get_md(HMACobject *self) +{ + const EVP_MD *md = HMAC_CTX_get_md(self->ctx); + if (md == NULL) { + raise_ssl_error(PyExc_ValueError, "missing EVP_MD for HMAC context"); + } + return md; +} + /*[clinic input] _hashlib.hmac_new @@ -1585,7 +1628,7 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, r = HMAC_Init_ex(ctx, key->buf, (int)key->len, digest, NULL /* impl */); PY_EVP_MD_free(digest); if (r == 0) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); goto error; } @@ -1625,17 +1668,16 @@ locked_HMAC_CTX_copy(HMAC_CTX *new_ctx_p, HMACobject *self) /* returning 0 means that an error occurred and an exception is set */ static unsigned int -_hmac_digest_size(HMACobject *self) +_hashlib_hmac_digest_size(HMACobject *self) { - const EVP_MD *md = HMAC_CTX_get_md(self->ctx); + const EVP_MD *md = _hashlib_hmac_get_md(self); if (md == NULL) { - (void)_setException(PyExc_ValueError, NULL); return 0; } unsigned int digest_size = EVP_MD_size(md); assert(digest_size <= EVP_MAX_MD_SIZE); if (digest_size == 0) { - (void)_setException(PyExc_ValueError, NULL); + raise_ssl_error(PyExc_ValueError, "invalid digest size"); } return digest_size; } @@ -1654,17 +1696,21 @@ _hmac_update(HMACobject *self, PyObject *obj) if (self->use_mutex) { Py_BEGIN_ALLOW_THREADS PyMutex_Lock(&self->mutex); - r = HMAC_Update(self->ctx, (const unsigned char*)view.buf, view.len); + r = HMAC_Update(self->ctx, + (const unsigned char *)view.buf, + (size_t)view.len); PyMutex_Unlock(&self->mutex); Py_END_ALLOW_THREADS } else { - r = HMAC_Update(self->ctx, (const unsigned char*)view.buf, view.len); + r = HMAC_Update(self->ctx, + (const unsigned char *)view.buf, + (size_t)view.len); } PyBuffer_Release(&view); if (r == 0) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); return 0; } return 1; @@ -1688,7 +1734,8 @@ _hashlib_HMAC_copy_impl(HMACobject *self) } if (!locked_HMAC_CTX_copy(ctx, self)) { HMAC_CTX_free(ctx); - return _setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); + return NULL; } retval = PyObject_New(HMACobject, Py_TYPE(self)); @@ -1719,9 +1766,9 @@ static PyObject * _hmac_repr(PyObject *op) { HMACobject *self = HMACobject_CAST(op); - const EVP_MD *md = HMAC_CTX_get_md(self->ctx); + const EVP_MD *md = _hashlib_hmac_get_md(self); if (md == NULL) { - return _setException(PyExc_ValueError, NULL); + return NULL; } PyObject *digest_name = py_digest_name(md); if (digest_name == NULL) { @@ -1761,13 +1808,13 @@ _hmac_digest(HMACobject *self, unsigned char *buf, unsigned int len) } if (!locked_HMAC_CTX_copy(temp_ctx, self)) { HMAC_CTX_free(temp_ctx); - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); return 0; } int r = HMAC_Final(temp_ctx, buf, &len); HMAC_CTX_free(temp_ctx); if (r == 0) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); return 0; } return 1; @@ -1783,7 +1830,7 @@ _hashlib_HMAC_digest_impl(HMACobject *self) /*[clinic end generated code: output=1b1424355af7a41e input=bff07f74da318fb4]*/ { unsigned char digest[EVP_MAX_MD_SIZE]; - unsigned int digest_size = _hmac_digest_size(self); + unsigned int digest_size = _hashlib_hmac_digest_size(self); if (digest_size == 0) { return NULL; } @@ -1808,7 +1855,7 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self) /*[clinic end generated code: output=80d825be1eaae6a7 input=5abc42702874ddcf]*/ { unsigned char digest[EVP_MAX_MD_SIZE]; - unsigned int digest_size = _hmac_digest_size(self); + unsigned int digest_size = _hashlib_hmac_digest_size(self); if (digest_size == 0) { return NULL; } @@ -1823,31 +1870,25 @@ static PyObject * _hashlib_hmac_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) { HMACobject *self = HMACobject_CAST(op); - unsigned int digest_size = _hmac_digest_size(self); - if (digest_size == 0) { - return NULL; - } - return PyLong_FromLong(digest_size); + unsigned int digest_size = _hashlib_hmac_digest_size(self); + return digest_size == 0 ? NULL : PyLong_FromLong(digest_size); } static PyObject * _hashlib_hmac_get_block_size(PyObject *op, void *Py_UNUSED(closure)) { HMACobject *self = HMACobject_CAST(op); - const EVP_MD *md = HMAC_CTX_get_md(self->ctx); - if (md == NULL) { - return _setException(PyExc_ValueError, NULL); - } - return PyLong_FromLong(EVP_MD_block_size(md)); + const EVP_MD *md = _hashlib_hmac_get_md(self); + return md == NULL ? NULL : PyLong_FromLong(EVP_MD_block_size(md)); } static PyObject * _hashlib_hmac_get_name(PyObject *op, void *Py_UNUSED(closure)) { HMACobject *self = HMACobject_CAST(op); - const EVP_MD *md = HMAC_CTX_get_md(self->ctx); + const EVP_MD *md = _hashlib_hmac_get_md(self); if (md == NULL) { - return _setException(PyExc_ValueError, NULL); + return NULL; } PyObject *digest_name = py_digest_name(md); if (digest_name == NULL) { @@ -2000,7 +2041,7 @@ _hashlib_get_fips_mode_impl(PyObject *module) // But 0 is also a valid result value. unsigned long errcode = ERR_peek_last_error(); if (errcode) { - (void)_setException(PyExc_ValueError, NULL); + notify_ssl_error_occurred(); return -1; } } _______________________________________________ Python-checkins mailing list -- python-checkins@python.org To unsubscribe send an email to python-checkins-le...@python.org https://mail.python.org/mailman3/lists/python-checkins.python.org/ Member address: arch...@mail-archive.com