Repository navigation
Description
Bug report
Bug description:
We have THREE different implementations for HMAC: one based on OpenSSL, one based on HACL* and a pure Python one that directly uses hash functions (HMAC can be written easily in Python if the hash function is given as input as well). Apart from that, we also a dispatching mechanism for "one-shot" HMAC that is a bit inconsistent:
- If OpenSSL is present -> we use OpenSSL's one-shot HMAC. The key and message sizes are limited to INT_MAX, otherwise an OverflowError occurs. If the algorithm doesn't exist, it falls back to HACL* if possible.
- HACL* implementations limits the key and message sizes to
$2^{32}-1$ which may be different fromINT_MAX. An OverflowError is also raised in this case. If again the algorithm isn't recognized, we fall back to the slow implementation. - There is no size restriction for the slow implementation.
Currently, the first OverflowError is actually not caught, so if we have OpenSSL + HACL*, we can't fallback to the slow path. I don't know whether it's better to actually reject large messages or not and how to fallback. I can suggest two plans with advantages and disadvantages:
Suggestion 1 (nice for the user)
- If OpenSSL is present and the key/message are too long, we fallback to HACL*.
- If it's still too large, we fallback to the slow one.
- And if it's still very large here, then that's an user issue.
My rationale here is that I want to at least give a chance to anyone using one-shot HMAC on a very large buffer even though they should do in chunks. I also plan to document this.
Suggestion 2 (nice for the CPU)
- If a message is too large to be handled by OpenSSL's HMAC, we fall back to HACL*
- If it's too large for HACL*, then we do not fall back to the slow one (currently we do!)
@gpshead suggested the following ones:
Suggestion 3
- Do the first bullet from Suggestion 2 and fall back to "the slow one" if we must.
- Document that HMAC should always be done in chunks of less that 2GiB for the best performance.
- Goal: never let OverflowError be raised.
Suggestion 4
- Always do the chunking loop ourselves if
len(msg) > LIMIT - Use memoryview in that loop to avoid huge copies.
- Goal: never let OverflowError be raised & never let performance degrade significantly.
Current behavior
- If OpenSSL is present, long keys/messages are directly rejected with no way to fall back to HACL* or slow HMAC.
- If OpenSSL is not present, long keys/messages may be rejected by one-shot HACL* HMAC but we will fall back to the slow HMAC.
This is kind of disturbing when OpenSSL is present vs when it's not.
In the end, I think suggestion 4 will be nicest while reducing performance impact.
CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
- gh-136912: fix handling of
OverflowErrorinhmac.digest#136917 - [3.14] gh-136912: fix handling of
OverflowErrorinhmac.digest(GH-136917) #137116
Note
We still need a 3.14.1 backport but we won't do it until 3.14 is stable.
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
gpshead commentedon Jul 21, 2025
I like Suggestion 4 below:
Suggestion 3
Suggestion 4
len(msg) > 0x7fff_ffff(or whatever the smallest limit among the implementations)There is no need to be optimal about the huge length here vs the given implementation available, it won't matter.
picnixz commentedon Jul 21, 2025
Mmh. Making the work on our side is also a good idea. It will really simplify the logic. Since the new HACL HMAC has been added in 3.14, I hope to be able to backport this feature, but otherwise I'll let the current code. I think going for plan 4 is the best idea for the user.
[-]HACL* HMAC one-shot digest limitations[/-][+]Improve HMAC one-shot digest limitations[/+][-]Improve HMAC one-shot digest limitations[/-][+]Solve HMAC one-shot digest limitations[/+]gpshead commentedon Jul 22, 2025
Avoiding the OverflowError could count as a bugfix, even if non-critical and it waited for 3.14.1.
picnixz commentedon Jul 26, 2025
I'm adding a deferred-blocker label so that I don't forget to put in 3.14.1.
gh-136912: fix handling of `OverflowError` in `hmac.digest` (#136917)
[3.14] pythongh-136912: fix handling of `OverflowError` in `hmac.dige…
pythongh-136912: fix handling of `OverflowError` in `hmac.digest` (py…
[3.14] gh-136912: fix handling of `OverflowError` in `hmac.digest` (G…
hugovk commentedon Oct 10, 2025
This is now merged for 3.14.1.