Skip to content

gh-136968: fortify macro usage in cryptographic modules #136973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 28, 2025
Merged

gh-136968: fortify macro usage in cryptographic modules #136973

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa41a74
fortify macro usage in cryptographic modules
picnixz Jul 22, 2025
dbe1a32
move comment
picnixz Jul 26, 2025
2ab8f83
rename macro
picnixz Jul 26, 2025
a9dae22
better comment
picnixz Jul 26, 2025
71ea85a
add maintenance comment
picnixz Jul 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions Modules/_hashopenssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() */

Expand All @@ -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);
}


Expand Down
69 changes: 43 additions & 26 deletions Modules/hashlib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 51 additions & 31 deletions Modules/hmacmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,42 +1099,60 @@ _hmac_compute_digest_impl(PyObject *module, PyObject *key, PyObject *msg,
}

/*
* One-shot HMAC-HASH using the given HACL_HID.
* 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.
*/
#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 \
); \
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.
*/
#define Py_HMAC_HACL_ONESHOT(HACL_HID, KEY, MSG) \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have rewritten this macro as well, but I don't really want to trade safety for speed here. By using a function I would need:

  • Over-allocate the digest buffer or still use a macro to pre-allocate the buffer with its exact size or pass it to that function. None of them is satisfying to me because of it would make the function non-reusable due to its signature or the fact that it expects the buffer to be pre-allocated.
  • Use pointer to functions for the dedicated computation functions. This is not optimal. Alternatively, I could use a dispatch mechanism to recover both the function and the digest size dynamically but it's again an overkill when I can directly access them.

At least, the part that made me suffer has been turned into a regular function (namely hmac_get_buffer_views) so I'm happy for now.

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]
Expand Down Expand Up @@ -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[] = {
Expand Down
Loading