all verify() methods now use "constant time" comparison function (see CHANGELOG for details)

2 files changed, 72 insertions, 7 deletions

diff --git a/passlib/utils/__init__.py b/passlib/utils/__init__.py
index 6c897bb..1315174 100644
--- a/passlib/utils/__init__.py
+++ b/passlib/utils/__init__.py
@@ -453,6 +453,70 @@ def is_ascii_safe(source):
 #=================================================================================
 #string helpers
 #=================================================================================
+
+def consteq(left, right):
+    """check two strings/bytes for equality, taking constant time relative
+    to the size of the righthand input.
+
+    The purpose of this function is to aid in preventing timing attacks
+    during digest comparisons (see the 1.6 changelog
+    :ref:`entry <consteq-issue>` for more details).
+    """
+    # NOTE:
+    # This function attempts to take an amount of time proportional
+    # to ``THETA(len(right))``. The main loop is designed so that timing attacks
+    # against this function should reveal nothing about how much (or which
+    # parts) of the two inputs match.
+    #
+    # Why ``THETA(len(right))``?
+    # Assuming the attacker controls one of the two inputs, padding to
+    # the largest input or trimming to the smallest input both allow
+    # a timing attack to reveal the length of the controlled input.
+    # However, by fixing the runtime to be proportional to the right input:
+    # * If the right value is attacker controlled, the runtime is proportional
+    #   to their input, giving nothing away about the left value's size.
+    # * If the left value is attacker controlled, the runtime is constant
+    #   relative to their input, giving nothing away about the right value's size.
+
+    # validate types
+    if isinstance(left, unicode):
+        if not isinstance(right, unicode):
+            raise TypeError("inputs must be both unicode or bytes")
+        # Py3k #
+        #isbytes = False
+        # end Py3k #
+    elif isinstance(left, bytes):
+        if not isinstance(right, bytes):
+            raise TypeError("inputs must be both unicode or bytes")
+        # Py3k #
+        #isbytes = True
+        # end Py3k #
+    else:
+        raise TypeError("inputs must be both unicode or bytes")
+
+    # do size comparison.
+    if len(left) == len(right):
+        # if sizes are the same, setup loop to perform actual check of contents.
+        tmp = left
+        result = 0
+    else:
+        # if sizes aren't the same, set 'result' so equality will fail regardless
+        # of contents. then, to ensure we do exactly 'len(right)' iterations
+        # of the loop, just compare 'right' against itself.
+        tmp = right
+        result = 1
+
+    # run constant-time string comparision
+    # Py3k #
+    #if isbytes:
+    #    for l,r in zip(tmp, right):
+    #        result |= l ^ r
+    #    return result == 0
+    # end Py3k #
+    for l,r in zip(tmp, right):
+        result |= ord(l) ^ ord(r)
+    return result == 0
+
 def splitcomma(source, sep=","):
     "split comma-separated string into list of elements, stripping whitespace and discarding empty elements"
     return [
diff --git a/passlib/utils/handlers.py b/passlib/utils/handlers.py
index 877b4da..e555421 100644
--- a/passlib/utils/handlers.py
+++ b/passlib/utils/handlers.py
@@ -14,7 +14,7 @@ from warnings import warn
 #site
 #libs
 from passlib.registry import get_crypt_handler
-from passlib.utils import to_hash_str, bytes, b, \
+from passlib.utils import to_hash_str, bytes, b, consteq, \
         classproperty, h64, getrandstr, getrandbytes, \
         rng, is_crypt_handler, ALL_BYTE_VALUES, MissingBackendError
 #pkg
@@ -224,7 +224,7 @@ class StaticHandler(object):
             raise ValueError("no hash specified")
         hash = cls._norm_hash(hash)
         result = cls.genhash(secret, hash, *cargs, **context)
-        return cls._norm_hash(result) == hash
+        return consteq(cls._norm_hash(result), hash)
 
     @classmethod
     def _norm_hash(cls, hash):
@@ -462,7 +462,7 @@ class GenericHandler(object):
         # may wish to either override this, or override norm_checksum
         # to normalize any checksums provided by from_string()
         self = cls.from_string(hash)
-        return self.checksum == self.calc_checksum(secret)
+        return consteq(self.calc_checksum(secret), self.checksum)
 
     #=========================================================
     #eoc
@@ -778,6 +778,7 @@ class HasSalt(GenericHandler):
         if salt is None:
             if strict:
                 raise ValueError("no salt specified")
+            #XXX: should we run generated salts through norm_salt? probably.
             return cls.generate_salt(salt_size=salt_size, strict=strict)
 
         #validate input charset
@@ -994,11 +995,11 @@ class HasManyBackends(GenericHandler):
     which is using :class:`HasManyBackends` as a mixin:
 
     .. attribute:: backends
-    
+
         This attribute should be a tuple containing the names of the backends
         which are supported. Two common names are ``"os_crypt"`` (if backend
         uses :mod:`crypt`), and ``"builtin"`` (if the backend is a pure-python
-        fallback). 
+        fallback).
 
     .. attribute:: _has_backend_{name}
 
@@ -1006,9 +1007,9 @@ class HasManyBackends(GenericHandler):
         specific backend is available, it should be either ``True``
         or ``False``. One of these should be provided by
         the subclass for each backend listed in :attr:`backends`.
-        
+
     .. classmethod:: _calc_checksum_{name}
-    
+
         private class method that should implement :meth:`calc_checksum`
         for a given backend. it will only be called if the backend has
         been selected by :meth:`set_backend`. One of these should be provided