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.
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
PEP 466: backport hmac.compare_digest #65505
Comments
Tracker issue for the hmac.compare_digest backport to 2.7 described in PEP-466. |
Design question here: compare_digest on Python 3 supports comparing str (text) objects, if they're both ascii-only. This feature is provided, primarily, so you can compare hexdigests or similar. Should the Python 2 version support comparing unicodes? Arguments in favor: some amount of consistency. Against: it's not necessary because hexdigest is still a str (binary), further it's not actually posisble to replicate the ascii only semantic. |
try: ? |
8-bit str only makes more sense to me. The wishy-washiness of some APIs in |
encode("ascii") has data dependent branches, so it's to be avoided. |
Thanks Nick. I'll get a patch up for str (bytes) only this afternoon. |
I'm not sure that the timing leakage in an encode is actually something to be worried about. I'm not sure what secret information would be getting leaked in a way that you could determine it by examining the timing. However I think the bigger thing is if I'm an app developer and I attempt to pass a unicode to hmac.compare_digest() and it tells me it only accepts bytes, the first thing I'm going to do is is .encode() it myself before I pass it in. IOW hmac.compare_digest could avoid the encode, but it's just pushing that back up to the user of hmac.compare_digest, who might possibly have a byte string laying around that they won't have to do the encode/decode dance on (although if they have a unicode they have already done it at least once), or they only have a unicode available to them then they'll be forced to do the encode themselves. |
Attached patch implements compare_digest. Code is mostly a 1-1 from 3.x, except the Unicode paths are changed, and the tests are a tiny bit different.
If the patch looks good to folks I'll add the docs as well. |
Attached patch now includes documentation and should be complete. |
The attached patch looks good to me. |
New changeset b40f1a00b134 by Benjamin Peterson in branch '2.7': |
Currently (Debian's 2.7.7-rc1 package) hmac.compare_digest accepts two bytestring arguments, or two Unicode stings, but not one bytestring and one unicode. I don't think that's a good idea. |
That restriction is deliberate (and documented). As a 3.x backport, this |
That's also a security sensitive thing, you don't want to compare two different encoding and have it accidentally fail. Strictly speaking you can only do a constant time comparison on bytes, the fact it accepts unicode at all (even on Python 3.x) is a convenience feature. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: