From 13906d6a2eb8c852d32752dca9e06e7d203d86d8 Mon Sep 17 00:00:00 2001 From: Ryan Kelly Date: Sat, 15 Oct 2011 11:14:19 +1100 Subject: [PATCH] Avoid timing attacks in AuthTktAutenticationPolicy This factors out the timing-invariant string comparison code from session.py and re-uses it for signature checking in AuthTkt code. --- pyramid/authentication.py | 6 +++++- pyramid/session.py | 12 +++--------- pyramid/util.py | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 955e870b4b..3909ce2d8f 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -22,6 +22,8 @@ from pyramid.security import Authenticated from pyramid.security import Everyone +from pyramid.util import strings_differ + VALID_TOKEN = re.compile(r"^[A-Za-z][A-Za-z0-9+_-]*$") class CallbackAuthenticationPolicy(object): @@ -485,7 +487,9 @@ def parse_ticket(secret, ticket, ip): expected = calculate_digest(ip, timestamp, secret, userid, tokens, user_data) - if expected != digest: + # Avoid timing attacks (see + # http://seb.dbzteam.org/crypto/python-oauth-timing-hmac.pdf) + if strings_differ(expected, digest): raise BadTicket('Digest signature is not correct', expected=(expected, digest)) diff --git a/pyramid/session.py b/pyramid/session.py index a59f9c6289..8d0c6c4238 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -13,6 +13,7 @@ from pyramid.compat import bytes_ from pyramid.compat import native_ from pyramid.interfaces import ISession +from pyramid.util import strings_differ def manage_accessed(wrapped): """ Decorator which causes a cookie to be set when a wrapped @@ -262,17 +263,10 @@ def signed_deserialize(serialized, secret, hmac=hmac): sig = hmac.new(bytes_(secret), pickled, sha1).hexdigest() - if len(sig) != len(input_sig): - raise ValueError('Wrong signature length') - # Avoid timing attacks (see # http://seb.dbzteam.org/crypto/python-oauth-timing-hmac.pdf) - invalid_bits = 0 - for a, b in zip(sig, input_sig): - invalid_bits += a != b - - if invalid_bits: - raise ValueError('Invalid bits in signature') + if strings_differ(sig, input_sig): + raise ValueError('Invalid signature') return pickle.loads(pickled) diff --git a/pyramid/util.py b/pyramid/util.py index a43b50aef3..3eb4b3fede 100644 --- a/pyramid/util.py +++ b/pyramid/util.py @@ -208,3 +208,23 @@ def last(self): oid = self._order[-1] return self._items[oid]() +def strings_differ(string1, string2): + """Check whether two strings differ while avoiding timing attacks. + + This function returns True if the given strings differ and False + if they are equal. It's careful not to leak information about *where* + they differ as a result of its running time, which can be very important + to avoid certain timing-related crypto attacks: + + http://seb.dbzteam.org/crypto/python-oauth-timing-hmac.pdf + + """ + if len(string1) != len(string2): + return True + + invalid_bits = 0 + for a, b in zip(string1, string2): + invalid_bits += a != b + + return invalid_bits != 0 +