> Therefore the expected digest changes each time.
Timing attacks (which aren't really new :-) depend on a constant digest to successively determine the characters composing the digest. Here, that won't work, because the digest changes every time.
I don't see the point of obfuscating the code to avoid a vulnerability
to which the code is not even vulnerable, just so that it can be used
There are *thousands* of ways to introduce security flaws, and the
Python code base if not a security handbook ;-)
> You call it obfuscating, I call it security correctness and developer education. Tomayto, tomahto. ;-)
Well, I'd be prompt to changing to a more robust digest check
algorithm if the current one had a flaw, but AFAICT, it's not the case
(but I'm no security expert).
> Anywho, your call of course, feel free to close.
Being a core Python developer doesn't mean I'm right ;-)
I just don't think that "set an example for other hmac module users"
is a good reason on its own to complicate the code, which is currently
readable and - AFICT - safe (complexity usually introduces bugs).
Furthermore, I somehow doubt that hmac users will go and have a look
at the multiprocessing connection challenge code when looking for an
One thing that could definitely be interesting is to look through the
code base and example to see if a similar - but vulnerable - pattern
is used, and fix such occurrences.
It would also be reasonable to add a comment to the code mentioning why this particular (security) comparison is *not* vulnerable to a timing attack, which would serve the education purpose if someone does look at the code.
> One thing that could definitely be interesting is to look through the
> code base and example to see if a similar - but vulnerable - pattern
> is used, and fix such occurrences.
Based on some quick greps, I didn't see many internal users of hmac and the current users don't seem to use it in a scenario that would be at risk (eg. attacker supplied digest compared against an expected digest).
Given that this issue has affected a lot of security-sensitive third-party code (keyczar, openid providers, almost every python web service that implements "secure cookies"  or other HMAC-based REST API signatures), I do like the idea of adding a warning in the relevant documentation as sbt proposed.
The only reason I'd recommend _not_ putting a time_independent_comparison() function in the hmac module is that it's not really HMAC-specific. In practice, any fixed-length secrets should be compared in a time-independent manner. It just happens that HMAC verification is a pretty common case for this vulnerable construct. :-)
> Given that this issue has affected a lot of security-sensitive third-party code (keyczar, openid providers, almost every python web service that implements "secure cookies"  or other HMAC-based REST API signatures), I do like the idea of adding a warning in the relevant documentation as sbt proposed.
This does sound reasonable, along with the addition of a comparison
function immune to timing attacks to the hmac module (as noted, it's
not specific to hmac, but it looks like a resonable place to add it).
Would you like to submit a patch (new comparison function with
documentation and test)?
You're correct, the length check does leak the length of the expected digest as a performance enhancement (otherwise, your comparison runtime is bounded by the length of the attackers input).
Generally, exposing the length and thereby potentially the underlying cryptographic hash function (eg. 20 bytes -> hmac-sha1) is not considered a security risk for this type of scenario, whereas leaking key material certainly is. I considered including this nuance in the documentation and probably should.
> It's better to write isinstance(a, bytes). You should raise a
> TypeError if a is not a bytes or str.