From aa41a74125f93f42dd8bf7de1ac0ca4c9a8bc878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 22 Jul 2025 11:40:59 +0200 Subject: [PATCH 1/5] fortify macro usage in cryptographic modules --- Modules/_hashopenssl.c | 16 ++++----- Modules/hashlib.h | 69 ++++++++++++++++++++++-------------- Modules/hmacmodule.c | 80 ++++++++++++++++++++++++++---------------- 3 files changed, 100 insertions(+), 65 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index d79e4b360e95c5..412efdc1fb0814 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -1252,14 +1252,7 @@ _hashlib_HASH(_hashlibstate *state, const char *digestname, PyObject *data_obj, } #define CALL_HASHLIB_NEW(MODULE, NAME, DATA, STRING, USEDFORSECURITY) \ - do { \ - PyObject *data_obj; \ - if (_Py_hashlib_data_argument(&data_obj, DATA, STRING) < 0) { \ - return NULL; \ - } \ - _hashlibstate *state = get_hashlib_state(MODULE); \ - return _hashlib_HASH(state, NAME, data_obj, USEDFORSECURITY); \ - } while (0) + return _hashlib_HASH_new_impl(MODULE, NAME, DATA, USEDFORSECURITY, STRING) /* The module-level function: new() */ @@ -1285,7 +1278,12 @@ _hashlib_HASH_new_impl(PyObject *module, const char *name, PyObject *data, int usedforsecurity, PyObject *string) /*[clinic end generated code: output=b905aaf9840c1bbd input=c34af6c6e696d44e]*/ { - CALL_HASHLIB_NEW(module, name, data, string, usedforsecurity); + PyObject *data_obj; + if (_Py_hashlib_data_argument(&data_obj, data, string) < 0) { + return NULL; + } + _hashlibstate *state = get_hashlib_state(module); + return _hashlib_HASH(state, name, data_obj, usedforsecurity); } diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 5de5922c345047..5ada4ef4b863ec 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -12,34 +12,51 @@ #define HASHLIB_UNSUPPORTED_STR_ALGORITHM "unsupported hash algorithm %s" /* - * Given a PyObject* obj, fill in the Py_buffer* viewp with the result - * of PyObject_GetBuffer. Sets an exception and issues the erraction - * on any errors, e.g. 'return NULL' or 'goto error'. + * Obtain a buffer view from a buffer-like object 'obj'. + * + * On success, store the result in 'view' and return 0. + * On error, set an exception and return -1. */ -#define GET_BUFFER_VIEW_OR_ERROR(obj, viewp, erraction) do { \ - if (PyUnicode_Check((obj))) { \ - PyErr_SetString(PyExc_TypeError, \ - "Strings must be encoded before hashing");\ - erraction; \ - } \ - if (!PyObject_CheckBuffer((obj))) { \ - PyErr_SetString(PyExc_TypeError, \ - "object supporting the buffer API required"); \ - erraction; \ - } \ - if (PyObject_GetBuffer((obj), (viewp), PyBUF_SIMPLE) == -1) { \ - erraction; \ - } \ - if ((viewp)->ndim > 1) { \ - PyErr_SetString(PyExc_BufferError, \ - "Buffer must be single dimension"); \ - PyBuffer_Release((viewp)); \ - erraction; \ - } \ - } while(0) +static inline int +_Py_hashlib_get_buffer_view(PyObject *obj, Py_buffer *view) +{ + if (PyUnicode_Check(obj)) { + PyErr_SetString(PyExc_TypeError, + "Strings must be encoded before hashing"); + return -1; + } + if (!PyObject_CheckBuffer(obj)) { + PyErr_SetString(PyExc_TypeError, + "object supporting the buffer API required"); + return -1; + } + if (PyObject_GetBuffer(obj, view, PyBUF_SIMPLE) == -1) { + return -1; + } + if (view->ndim > 1) { + PyErr_SetString(PyExc_BufferError, + "Buffer must be single dimension"); + PyBuffer_Release(view); + return -1; + } + return 0; +} + +/* + * Call _Py_hashlib_get_buffer_view() and check if it succeeded. + * + * On error, set an exception and execute the ERRACTION statements. + */ +#define GET_BUFFER_VIEW_OR_ERROR(OBJ, VIEW, ERRACTION) \ + do { \ + if (_Py_hashlib_get_buffer_view(OBJ, VIEW) < 0) { \ + assert(PyErr_Occurred()); \ + ERRACTION; \ + } \ + } while (0) -#define GET_BUFFER_VIEW_OR_ERROUT(obj, viewp) \ - GET_BUFFER_VIEW_OR_ERROR(obj, viewp, return NULL) +#define GET_BUFFER_VIEW_OR_ERROUT(OBJ, VIEW) \ + GET_BUFFER_VIEW_OR_ERROR(OBJ, VIEW, return NULL) /* * Helper code to synchronize access to the hash object when the GIL is diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index b5405c99f1f8ce..10bfb5e2fa7064 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -1098,6 +1098,36 @@ _hmac_compute_digest_impl(PyObject *module, PyObject *key, PyObject *msg, return info->api.compute_py(module, key, msg); } +/* + * Obtain a view from 'key' and 'msg', storing it in 'keyview' and 'msgview'. + * + * Return 0 on success; otherwise set an exception and return -1. + */ +static int +hmac_get_buffer_views(PyObject *key, Py_buffer *keyview, + PyObject *msg, Py_buffer *msgview) +{ + if (_Py_hashlib_get_buffer_view(key, keyview) < 0) { + return -1; + } + if (!has_uint32_t_buffer_length(keyview)) { + PyBuffer_Release(keyview); + set_invalid_key_length_error(); + return -1; + } + if (_Py_hashlib_get_buffer_view(msg, msgview) < 0) { + PyBuffer_Release(keyview); + return -1; + } + if (!has_uint32_t_buffer_length(msgview)) { + PyBuffer_Release(msgview); + PyBuffer_Release(keyview); + set_invalid_msg_length_error(); + return -1; + } + return 0; +} + /* * One-shot HMAC-HASH using the given HACL_HID. * @@ -1105,36 +1135,24 @@ _hmac_compute_digest_impl(PyObject *module, PyObject *key, PyObject *msg, * lest an OverflowError is raised. The Python implementation takes care * of dispatching to the OpenSSL implementation in this case. */ -#define Py_HMAC_HACL_ONESHOT(HACL_HID, KEY, MSG) \ - do { \ - Py_buffer keyview, msgview; \ - GET_BUFFER_VIEW_OR_ERROUT((KEY), &keyview); \ - if (!has_uint32_t_buffer_length(&keyview)) { \ - PyBuffer_Release(&keyview); \ - set_invalid_key_length_error(); \ - return NULL; \ - } \ - GET_BUFFER_VIEW_OR_ERROR((MSG), &msgview, \ - PyBuffer_Release(&keyview); \ - return NULL); \ - if (!has_uint32_t_buffer_length(&msgview)) { \ - PyBuffer_Release(&msgview); \ - PyBuffer_Release(&keyview); \ - set_invalid_msg_length_error(); \ - return NULL; \ - } \ - uint8_t out[Py_hmac_## HACL_HID ##_digest_size]; \ - Py_hmac_## HACL_HID ##_compute_func( \ - out, \ - (uint8_t *)keyview.buf, (uint32_t)keyview.len, \ - (uint8_t *)msgview.buf, (uint32_t)msgview.len \ - ); \ - PyBuffer_Release(&msgview); \ - PyBuffer_Release(&keyview); \ - return PyBytes_FromStringAndSize( \ - (const char *)out, \ - Py_hmac_## HACL_HID ##_digest_size \ - ); \ +#define Py_HMAC_HACL_ONESHOT(HACL_HID, KEY, MSG) \ + do { \ + Py_buffer keyview, msgview; \ + if (hmac_get_buffer_views(key, &keyview, msg, &msgview) < 0) { \ + return NULL; \ + } \ + uint8_t out[Py_hmac_## HACL_HID ##_digest_size]; \ + Py_hmac_## HACL_HID ##_compute_func( \ + out, \ + (uint8_t *)keyview.buf, (uint32_t)keyview.len, \ + (uint8_t *)msgview.buf, (uint32_t)msgview.len \ + ); \ + PyBuffer_Release(&msgview); \ + PyBuffer_Release(&keyview); \ + return PyBytes_FromStringAndSize( \ + (const char *)out, \ + Py_hmac_## HACL_HID ##_digest_size \ + ); \ } while (0) /*[clinic input] @@ -1329,6 +1347,8 @@ _hmac_compute_blake2b_32_impl(PyObject *module, PyObject *key, PyObject *msg) Py_HMAC_HACL_ONESHOT(blake2b_32, key, msg); } +#undef Py_HMAC_HACL_ONESHOT + // --- HMAC module methods ---------------------------------------------------- static PyMethodDef hmacmodule_methods[] = { From dbe1a3235e93fe86e5e6448bbcf27eb32bbc385c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 26 Jul 2025 09:31:56 +0200 Subject: [PATCH 2/5] move comment --- Modules/hmacmodule.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 10bfb5e2fa7064..348a017c4b27c2 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -1102,6 +1102,10 @@ _hmac_compute_digest_impl(PyObject *module, PyObject *key, PyObject *msg, * Obtain a view from 'key' and 'msg', storing it in 'keyview' and 'msgview'. * * Return 0 on success; otherwise set an exception and return -1. + * + * The length of the key and message buffers must not exceed UINT32_MAX, + * lest an OverflowError is raised. The Python implementation takes care + * of dispatching to the OpenSSL implementation in this case. */ static int hmac_get_buffer_views(PyObject *key, Py_buffer *keyview, @@ -1130,10 +1134,6 @@ hmac_get_buffer_views(PyObject *key, Py_buffer *keyview, /* * One-shot HMAC-HASH using the given HACL_HID. - * - * The length of the key and message buffers must not exceed UINT32_MAX, - * lest an OverflowError is raised. The Python implementation takes care - * of dispatching to the OpenSSL implementation in this case. */ #define Py_HMAC_HACL_ONESHOT(HACL_HID, KEY, MSG) \ do { \ From 2ab8f83d934faa3f953900d5d53b82ee9e88735a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 26 Jul 2025 09:40:25 +0200 Subject: [PATCH 3/5] rename macro --- Modules/hmacmodule.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 348a017c4b27c2..a888705f2118f0 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -1135,7 +1135,7 @@ hmac_get_buffer_views(PyObject *key, Py_buffer *keyview, /* * One-shot HMAC-HASH using the given HACL_HID. */ -#define Py_HMAC_HACL_ONESHOT(HACL_HID, KEY, MSG) \ +#define HACL_HMAC_COMPUTE_NAMED_DIGEST(HACL_HID, KEY, MSG) \ do { \ Py_buffer keyview, msgview; \ if (hmac_get_buffer_views(key, &keyview, msg, &msgview) < 0) { \ @@ -1168,7 +1168,7 @@ static PyObject * _hmac_compute_md5_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=7837a4ceccbbf636 input=77a4b774c7d61218]*/ { - Py_HMAC_HACL_ONESHOT(md5, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(md5, key, msg); } /*[clinic input] @@ -1184,7 +1184,7 @@ static PyObject * _hmac_compute_sha1_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=79fd7689c83691d8 input=3b64dccc6bdbe4ba]*/ { - Py_HMAC_HACL_ONESHOT(sha1, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha1, key, msg); } /*[clinic input] @@ -1200,7 +1200,7 @@ static PyObject * _hmac_compute_sha2_224_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=7f21f1613e53979e input=a1a75f25f23449af]*/ { - Py_HMAC_HACL_ONESHOT(sha2_224, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha2_224, key, msg); } /*[clinic input] @@ -1216,7 +1216,7 @@ static PyObject * _hmac_compute_sha2_256_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=d4a291f7d9a82459 input=5c9ccf2df048ace3]*/ { - Py_HMAC_HACL_ONESHOT(sha2_256, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha2_256, key, msg); } /*[clinic input] @@ -1232,7 +1232,7 @@ static PyObject * _hmac_compute_sha2_384_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=f211fa26e3700c27 input=2fee2c14766af231]*/ { - Py_HMAC_HACL_ONESHOT(sha2_384, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha2_384, key, msg); } /*[clinic input] @@ -1248,7 +1248,7 @@ static PyObject * _hmac_compute_sha2_512_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=d5c20373762cecca input=3371eaac315c7864]*/ { - Py_HMAC_HACL_ONESHOT(sha2_512, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha2_512, key, msg); } /*[clinic input] @@ -1264,7 +1264,7 @@ static PyObject * _hmac_compute_sha3_224_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=a242ccac9ad9c22b input=d0ab0c7d189c3d87]*/ { - Py_HMAC_HACL_ONESHOT(sha3_224, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha3_224, key, msg); } /*[clinic input] @@ -1280,7 +1280,7 @@ static PyObject * _hmac_compute_sha3_256_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=b539dbb61af2fe0b input=f05d7b6364b35d02]*/ { - Py_HMAC_HACL_ONESHOT(sha3_256, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha3_256, key, msg); } /*[clinic input] @@ -1296,7 +1296,7 @@ static PyObject * _hmac_compute_sha3_384_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=5eb372fb5c4ffd3a input=d842d393e7aa05ae]*/ { - Py_HMAC_HACL_ONESHOT(sha3_384, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha3_384, key, msg); } /*[clinic input] @@ -1312,7 +1312,7 @@ static PyObject * _hmac_compute_sha3_512_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=154bcbf8c2eacac1 input=166fe5baaeaabfde]*/ { - Py_HMAC_HACL_ONESHOT(sha3_512, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(sha3_512, key, msg); } /*[clinic input] @@ -1328,7 +1328,7 @@ static PyObject * _hmac_compute_blake2s_32_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=cfc730791bc62361 input=d22c36e7fe31a985]*/ { - Py_HMAC_HACL_ONESHOT(blake2s_32, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(blake2s_32, key, msg); } /*[clinic input] @@ -1344,10 +1344,10 @@ static PyObject * _hmac_compute_blake2b_32_impl(PyObject *module, PyObject *key, PyObject *msg) /*[clinic end generated code: output=765c5c4fb9124636 input=4a35ee058d172f4b]*/ { - Py_HMAC_HACL_ONESHOT(blake2b_32, key, msg); + HACL_HMAC_COMPUTE_NAMED_DIGEST(blake2b_32, key, msg); } -#undef Py_HMAC_HACL_ONESHOT +#undef HACL_HMAC_COMPUTE_NAMED_DIGEST // --- HMAC module methods ---------------------------------------------------- From a9dae22ad03241973a14a6bb30aec2addb97c525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 26 Jul 2025 09:42:50 +0200 Subject: [PATCH 4/5] better comment --- Modules/hmacmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index a888705f2118f0..694e2a095744ff 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -1099,7 +1099,7 @@ _hmac_compute_digest_impl(PyObject *module, PyObject *key, PyObject *msg, } /* - * Obtain a view from 'key' and 'msg', storing it in 'keyview' and 'msgview'. + * Obtain a view for 'key' and 'msg', storing it in 'keyview' and 'msgview'. * * Return 0 on success; otherwise set an exception and return -1. * From 71ea85a0978364f6779f43534cc38920a4e287d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 28 Jul 2025 09:09:04 +0200 Subject: [PATCH 5/5] add maintenance comment --- Modules/_hashopenssl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 412efdc1fb0814..00f98c090b3952 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -1251,6 +1251,9 @@ _hashlib_HASH(_hashlibstate *state, const char *digestname, PyObject *data_obj, return (PyObject *)self; } +// In Python 3.19, we can remove the "STRING" argument and would also be able +// to remove the macro (or keep it as an alias for better naming) since calls +// to _hashlib_HASH_new_impl() would fit on 80 characters. #define CALL_HASHLIB_NEW(MODULE, NAME, DATA, STRING, USEDFORSECURITY) \ return _hashlib_HASH_new_impl(MODULE, NAME, DATA, USEDFORSECURITY, STRING)