classification
Title: hmac.secure_compare() is not time-independent for unicode strings
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: 15061 Superseder:
Assigned To: Nosy List: Jon.Oberheide, christian.heimes, hynek, ncoghlan, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2012-05-29 16:36 by Jon.Oberheide, last changed 2012-10-06 23:34 by christian.heimes. This issue is now closed.

Files
File name Uploaded Description Edit
secure-compare-fix-v1.patch Jon.Oberheide, 2012-05-29 16:36 review
secure-compare-fix-v2.patch Jon.Oberheide, 2012-06-02 17:56 review
Messages (9)
msg161898 - (view) Author: Jon Oberheide (Jon.Oberheide) Date: 2012-05-29 16:36
Hi all,

I was informed that the hmac.secure_compare() function added in 14532 is not time-independent when processing unicode values:

"The function as given is probably not timing independent if the attacker can provide unicode values. This is because (in CPython at least) all integer values in the range [-5, 256] inclusive are made singletons to avoid the performance hit of integer object creation, meaning that as long as (x ^ y) < 257, no integer object is created and the function appears constant time. When that assumption is violated, you get a timing delta that is actually fairly large compared to the delta for a single character compare."

One way to work around this issue is to perform strict 8-bit byte comparisons by converting any string type parameters to bytes via a utf-8 encode. The attached patch does this.

Regards,
Jon Oberheide
msg161966 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-05-30 20:10
I'm not sure that encoding to UTF-8 is time indenpendant. You may try UTF-32-LE or unicode-internal?
msg162159 - (view) Author: Jon Oberheide (Jon.Oberheide) Date: 2012-06-02 17:56
Thanks for the feedback, haypo. I've updated the patch to use unicode-internal. As long as the encode() of the expected non-attacker-controlled digest is not dependent on the actual contents of the digest, we should be good.
msg162737 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-06-13 22:39
The second patch looks fine.
msg162771 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-14 11:01
With PEP 393 unicode objects can have several representations, which makes it unlikely that *really* constant-timing functions can be devised.

However, a C version could provide some guarantees, by raising an error if the passed unicode strings use a different representation from each other.
msg162889 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-15 12:06
As a result of the discussions on #15061, I removed unicode comparison support altogether in f36af3766a20 (updating the warning on the hexdigest() method accordingly).

Are folks happy to close this issue on that basis? (I'll raise the question of a separate C implementation on the other issue)
msg162915 - (view) Author: Jon Oberheide (Jon.Oberheide) Date: 2012-06-15 16:12
Sounds good to me, Nick.
msg163169 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-06-19 14:28
Nick has pushed a patch in rf36af3766a20 that disables the comparison of unicode strings. See #15061
msg172264 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-06 23:34
Python 3.3 contains a secure and working implementation for bytes. unicode isn't supported unless both sides contains ASCII text only.
History
Date User Action Args
2012-10-06 23:34:12christian.heimessetstatus: open -> closed
resolution: fixed
messages: + msg172264

stage: resolved
2012-06-19 14:28:38christian.heimessetdependencies: + hmac.secure_compare() leaks information about length of strings
messages: + msg163169
2012-06-15 16:12:05Jon.Oberheidesetmessages: + msg162915
2012-06-15 12:06:34ncoghlansetnosy: + ncoghlan
messages: + msg162889
2012-06-14 11:07:45hyneksetnosy: + hynek
2012-06-14 11:01:49pitrousetnosy: + pitrou
messages: + msg162771
2012-06-13 22:39:29christian.heimessetnosy: + christian.heimes
messages: + msg162737
2012-06-02 17:56:34Jon.Oberheidesetfiles: + secure-compare-fix-v2.patch

messages: + msg162159
2012-05-30 20:10:20vstinnersetnosy: + vstinner
messages: + msg161966
2012-05-29 16:36:51Jon.Oberheidecreate