https://github.com/python/cpython/commit/304ef8440b46caa36c295bd0a47c5c24bbd1a245
commit: 304ef8440b46caa36c295bd0a47c5c24bbd1a245
branch: 3.12
author: Bénédikt Tran <10796600+picn...@users.noreply.github.com>
committer: picnixz <10796600+picn...@users.noreply.github.com>
date: 2025-03-18T10:16:26+01:00
summary:

[3.12] gh-127667: refactor and improve `_hashopenssl.c` error branches 
(#131145) (#131348)

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.

(cherry-picked from commit ac50ece6cea8745834e4ec0a9617809a51245bfc).

files:
M Modules/_hashopenssl.c

diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
index 48d7fd03feb16b..41ec16426b09b5 100644
--- a/Modules/_hashopenssl.c
+++ b/Modules/_hashopenssl.c
@@ -300,30 +300,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);
@@ -334,13 +321,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;
@@ -409,8 +432,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;
@@ -484,7 +507,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;
@@ -535,7 +558,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;
 }
@@ -575,7 +599,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]
@@ -613,7 +638,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]
@@ -685,7 +711,8 @@ EVP_get_name(EVPobject *self, void *closure)
 {
     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);
 }
@@ -800,7 +827,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]
@@ -849,7 +877,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[] = {
@@ -950,7 +979,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;
     }
@@ -966,6 +995,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;
         }
@@ -1332,19 +1362,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);
     }
@@ -1446,8 +1476,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);
@@ -1467,7 +1498,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;
 }
@@ -1515,15 +1547,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);
 }
@@ -1533,6 +1566,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
 
@@ -1580,7 +1623,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;
     }
 
@@ -1620,17 +1663,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;
 }
@@ -1651,17 +1693,21 @@ _hmac_update(HMACobject *self, PyObject *obj)
     if (self->lock != NULL) {
         Py_BEGIN_ALLOW_THREADS
         PyThread_acquire_lock(self->lock, 1);
-        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);
         PyThread_release_lock(self->lock);
         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;
@@ -1685,7 +1731,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));
@@ -1717,9 +1764,9 @@ _hmac_dealloc(HMACobject *self)
 static PyObject *
 _hmac_repr(HMACobject *self)
 {
-    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) {
@@ -1759,13 +1806,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;
@@ -1781,7 +1828,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;
     }
@@ -1806,7 +1853,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;
     }
@@ -1820,29 +1867,23 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self)
 static PyObject *
 _hashlib_hmac_get_digest_size(HMACobject *self, void *closure)
 {
-    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(HMACobject *self, void *closure)
 {
-    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(HMACobject *self, void *closure)
 {
-    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

Reply via email to