Add SHA512AuthTktAuthenticationPolicy and deprecate AuthTktAuthenticationPolicy

3 files changed, 134 insertions, 31 deletions

diff --git a/docs/api/authentication.rst b/docs/api/authentication.rst
index 5d4dbd9e3..ed96e9c98 100644
--- a/docs/api/authentication.rst
+++ b/docs/api/authentication.rst
@@ -8,6 +8,8 @@ Authentication Policies
 
 .. automodule:: pyramid.authentication
 
+  .. autoclass:: SHA512AuthTktAuthenticationPolicy
+
   .. autoclass:: AuthTktAuthenticationPolicy
 
   .. autoclass:: RepozeWho1AuthenticationPolicy
diff --git a/pyramid/authentication.py b/pyramid/authentication.py
index 83bdb13d1..730312144 100644
--- a/pyramid/authentication.py
+++ b/pyramid/authentication.py
@@ -1,10 +1,11 @@
 from codecs import utf_8_decode
 from codecs import utf_8_encode
-from hashlib import md5
+import hashlib
 import base64
 import datetime
 import re
 import time as time_mod
+import warnings
 
 from zope.interface import implementer
 
@@ -254,8 +255,7 @@ class RemoteUserAuthenticationPolicy(CallbackAuthenticationPolicy):
     def forget(self, request):
         return []
 
-@implementer(IAuthenticationPolicy)
-class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
+class BaseAuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
     """ A :app:`Pyramid` :term:`authentication policy` which
     obtains data from a Pyramid "auth ticket" cookie.
 
@@ -357,6 +357,8 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
     Objects of this class implement the interface described by
     :class:`pyramid.interfaces.IAuthenticationPolicy`.
     """
+    hashalg = ''
+
     def __init__(self,
                  secret,
                  callback=None,
@@ -382,6 +384,7 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
             http_only=http_only,
             path=path,
             wild_domain=wild_domain,
+            hashalg=self.hashalg,
             )
         self.callback = callback
         self.debug = debug
@@ -399,6 +402,32 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
     def forget(self, request):
         return self.cookie.forget(request)
 
+@implementer(IAuthenticationPolicy)
+class SHA512AuthTktAuthenticationPolicy(BaseAuthTktAuthenticationPolicy):
+    __doc__ = """
+    .. versionadded:: 1.4
+    """ + BaseAuthTktAuthenticationPolicy.__doc__
+    hashalg = 'sha512'
+
+@implementer(IAuthenticationPolicy)
+class AuthTktAuthenticationPolicy(BaseAuthTktAuthenticationPolicy):
+    __doc__ = """
+    .. warning::
+
+        Deprecated in 1.4 due to security concerns,
+        use :class:`SHA512AuthTktAuthenticationPolicy` instead.
+
+    """ + BaseAuthTktAuthenticationPolicy.__doc__
+    hashalg = 'md5'
+
+    def __init__(self, *a, **kw):
+        warnings.warn('Deprecated due to the usage of md5, '
+                      'hash function known to have collisions. '
+                      'Use SHA512AuthTktAuthenticationPolicy instead.',
+                      DeprecationWarning,
+                      stacklevel=2)
+        super(AuthTktAuthenticationPolicy, self).__init__(*a, **kw)
+
 def b64encode(v):
     return base64.b64encode(bytes_(v)).strip().replace(b'\n', b'')
 
@@ -427,7 +456,8 @@ class AuthTicket(object):
     """
 
     def __init__(self, secret, userid, ip, tokens=(), user_data='',
-                 time=None, cookie_name='auth_tkt', secure=False):
+                 time=None, cookie_name='auth_tkt', secure=False,
+                 hashalg='md5'):
         self.secret = secret
         self.userid = userid
         self.ip = ip
@@ -439,11 +469,12 @@ class AuthTicket(object):
             self.time = time
         self.cookie_name = cookie_name
         self.secure = secure
+        self.hashalg = hashalg
 
     def digest(self):
         return calculate_digest(
             self.ip, self.time, self.secret, self.userid, self.tokens,
-            self.user_data)
+            self.user_data, self.hashalg)
 
     def cookie_value(self):
         v = '%s%08x%s!' % (self.digest(), int(self.time),
@@ -465,7 +496,7 @@ class BadTicket(Exception):
         Exception.__init__(self, msg)
 
 # this function licensed under the MIT license (stolen from Paste)
-def parse_ticket(secret, ticket, ip):
+def parse_ticket(secret, ticket, ip, hashalg):
     """
     Parse the ticket, returning (timestamp, userid, tokens, user_data).
 
@@ -473,13 +504,14 @@ def parse_ticket(secret, ticket, ip):
     with an explanation.
     """
     ticket = ticket.strip('"')
-    digest = ticket[:32]
+    digest_size = hashlib.new(hashalg).digest_size * 2
+    digest = ticket[:digest_size]
     try:
-        timestamp = int(ticket[32:40], 16)
+        timestamp = int(ticket[digest_size:digest_size + 8], 16)
     except ValueError as e:
         raise BadTicket('Timestamp is not a hex integer: %s' % e)
     try:
-        userid, data = ticket[40:].split('!', 1)
+        userid, data = ticket[digest_size + 8:].split('!', 1)
     except ValueError:
         raise BadTicket('userid is not followed by !')
     userid = url_unquote(userid)
@@ -491,7 +523,7 @@ def parse_ticket(secret, ticket, ip):
         user_data = data
 
     expected = calculate_digest(ip, timestamp, secret,
-                                userid, tokens, user_data)
+                                userid, tokens, user_data, hashalg)
 
     # Avoid timing attacks (see
     # http://seb.dbzteam.org/crypto/python-oauth-timing-hmac.pdf)
@@ -504,16 +536,19 @@ def parse_ticket(secret, ticket, ip):
     return (timestamp, userid, tokens, user_data)
 
 # this function licensed under the MIT license (stolen from Paste)
-def calculate_digest(ip, timestamp, secret, userid, tokens, user_data):
+def calculate_digest(ip, timestamp, secret, userid, tokens, user_data, hashalg):
     secret = bytes_(secret, 'utf-8')
     userid = bytes_(userid, 'utf-8')
     tokens = bytes_(tokens, 'utf-8')
     user_data = bytes_(user_data, 'utf-8')
-    digest0 = md5(
+    hash_obj = hashlib.new(hashalg)
+    hash_obj.update(
         encode_ip_timestamp(ip, timestamp) + secret + userid + b'\0'
-        + tokens + b'\0' + user_data).hexdigest()
-    digest = md5(bytes_(digest0) + secret).hexdigest()
-    return digest
+        + tokens + b'\0' + user_data)
+    digest = hash_obj.hexdigest()
+    hash_obj2 = hashlib.new(hashalg)
+    hash_obj2.update(bytes_(digest) + secret)
+    return hash_obj2.hexdigest()
 
 # this function licensed under the MIT license (stolen from Paste)
 def encode_ip_timestamp(ip, timestamp):
@@ -556,7 +591,7 @@ class AuthTktCookieHelper(object):
     
     def __init__(self, secret, cookie_name='auth_tkt', secure=False,
                  include_ip=False, timeout=None, reissue_time=None,
-                 max_age=None, http_only=False, path="/", wild_domain=True):
+                 max_age=None, http_only=False, path="/", wild_domain=True, hashalg='md5'):
         self.secret = secret
         self.cookie_name = cookie_name
         self.include_ip = include_ip
@@ -567,6 +602,7 @@ class AuthTktCookieHelper(object):
         self.http_only = http_only
         self.path = path
         self.wild_domain = wild_domain
+        self.hashalg = hashalg
 
         static_flags = []
         if self.secure:
@@ -635,7 +671,7 @@ class AuthTktCookieHelper(object):
         
         try:
             timestamp, userid, tokens, user_data = self.parse_ticket(
-                self.secret, cookie, remote_addr)
+                self.secret, cookie, remote_addr, self.hashalg)
         except self.BadTicket:
             return None
 
@@ -750,7 +786,8 @@ class AuthTktCookieHelper(object):
             tokens=tokens,
             user_data=user_data,
             cookie_name=self.cookie_name,
-            secure=self.secure)
+            secure=self.secure,
+            hashalg=self.hashalg)
 
         cookie_value = ticket.cookie_value()
         return self._get_cookies(environ, cookie_value, max_age)
diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py
index e513b9a48..ae517fc40 100644
--- a/pyramid/tests/test_authentication.py
+++ b/pyramid/tests/test_authentication.py
@@ -343,13 +343,43 @@ class TestAutkTktAuthenticationPolicy(unittest.TestCase):
         inst.cookie = DummyCookieHelper(cookieidentity)
         return inst
 
-    def test_allargs(self):
-        # pass all known args
-        inst = self._getTargetClass()(
-            'secret', callback=None, cookie_name=None, secure=False,
-            include_ip=False, timeout=None, reissue_time=None,
-            )
-        self.assertEqual(inst.callback, None)
+    def test_is_subclass(self):
+        from pyramid.authentication import BaseAuthTktAuthenticationPolicy
+        inst = self._makeOne(None, None)
+        self.assertTrue(isinstance(inst, BaseAuthTktAuthenticationPolicy))
+
+    def test_md5(self):
+        inst = self._makeOne(None, None)
+        self.assertEqual(inst.hashalg, 'md5')
+
+    def test_class_implements_IAuthenticationPolicy(self):
+        from zope.interface.verify import verifyClass
+        from pyramid.interfaces import IAuthenticationPolicy
+        verifyClass(IAuthenticationPolicy, self._getTargetClass())
+
+    def test_instance_implements_IAuthenticationPolicy(self):
+        from zope.interface.verify import verifyObject
+        from pyramid.interfaces import IAuthenticationPolicy
+        verifyObject(IAuthenticationPolicy, self._makeOne(None, None))
+
+class TestSHA512AutkTktAuthenticationPolicy(unittest.TestCase):
+    def _getTargetClass(self):
+        from pyramid.authentication import SHA512AuthTktAuthenticationPolicy
+        return SHA512AuthTktAuthenticationPolicy
+
+    def _makeOne(self, callback, cookieidentity, **kw):
+        inst = self._getTargetClass()('secret', callback, **kw)
+        inst.cookie = DummyCookieHelper(cookieidentity)
+        return inst
+
+    def test_is_subclass(self):
+        from pyramid.authentication import BaseAuthTktAuthenticationPolicy
+        inst = self._makeOne(None, None)
+        self.assertTrue(isinstance(inst, BaseAuthTktAuthenticationPolicy))
+
+    def test_sha512(self):
+        inst = self._makeOne(None, None)
+        self.assertEqual(inst.hashalg, 'sha512')
 
     def test_class_implements_IAuthenticationPolicy(self):
         from zope.interface.verify import verifyClass
@@ -361,6 +391,24 @@ class TestAutkTktAuthenticationPolicy(unittest.TestCase):
         from pyramid.interfaces import IAuthenticationPolicy
         verifyObject(IAuthenticationPolicy, self._makeOne(None, None))
 
+class TestBaseAutkTktAuthenticationPolicy(unittest.TestCase):
+    def _getTargetClass(self):
+        from pyramid.authentication import BaseAuthTktAuthenticationPolicy
+        return BaseAuthTktAuthenticationPolicy
+
+    def _makeOne(self, callback, cookieidentity, **kw):
+        inst = self._getTargetClass()('secret', callback, **kw)
+        inst.cookie = DummyCookieHelper(cookieidentity)
+        return inst
+
+    def test_allargs(self):
+        # pass all known args
+        inst = self._getTargetClass()(
+            'secret', callback=None, cookie_name=None, secure=False,
+            include_ip=False, timeout=None, reissue_time=None,
+            )
+        self.assertEqual(inst.callback, None)
+
     def test_unauthenticated_userid_returns_None(self):
         request = DummyRequest({})
         policy = self._makeOne(None, None)
@@ -971,6 +1019,14 @@ class TestAuthTicket(unittest.TestCase):
         result = ticket.digest()
         self.assertEqual(result, '126fd6224912187ee9ffa80e0b81420c')
 
+    def test_digest_sha512(self):
+        ticket = self._makeOne('secret', 'userid', '0.0.0.0',
+                               time=10, hashalg='sha512')
+        result = ticket.digest()
+        self.assertEqual(result, '74770b2e0d5b1a54c2a466ec567a40f7d7823576aa49'\
+                                 '3c65fc3445e9b44097f4a80410319ef8cb256a2e60b9'\
+                                 'c2002e48a9e33a3e8ee4379352c04ef96d2cb278')
+
     def test_cookie_value(self):
         ticket = self._makeOne('secret', 'userid', '0.0.0.0', time=10,
                                tokens=('a', 'b'))
@@ -989,13 +1045,13 @@ class TestBadTicket(unittest.TestCase):
         self.assertTrue(isinstance(exc, Exception))
 
 class Test_parse_ticket(unittest.TestCase):
-    def _callFUT(self, secret, ticket, ip):
+    def _callFUT(self, secret, ticket, ip, hashalg='md5'):
         from pyramid.authentication import parse_ticket
-        return parse_ticket(secret, ticket, ip)
+        return parse_ticket(secret, ticket, ip, hashalg)
 
-    def _assertRaisesBadTicket(self, secret, ticket, ip):
+    def _assertRaisesBadTicket(self, secret, ticket, ip, hashalg='md5'):
         from pyramid.authentication import BadTicket
-        self.assertRaises(BadTicket,self._callFUT, secret, ticket, ip)
+        self.assertRaises(BadTicket,self._callFUT, secret, ticket, ip, hashalg)
 
     def test_bad_timestamp(self):
         ticket = 'x' * 64
@@ -1014,6 +1070,13 @@ class Test_parse_ticket(unittest.TestCase):
         result = self._callFUT('secret', ticket, '0.0.0.0')
         self.assertEqual(result, (10, 'userid', ['a', 'b'], ''))
 
+    def test_correct_with_user_data_sha512(self):
+        ticket = '7d947cdef99bad55f8e3382a8bd089bb9dd0547f7925b7d189adc1160cab'\
+                 '0ec0e6888faa41eba641a18522b26f19109f3ffafb769767ba8a26d02aae'\
+                 'ae56599a0000000auserid!a,b!'
+        result = self._callFUT('secret', ticket, '0.0.0.0', 'sha512')
+        self.assertEqual(result, (10, 'userid', ['a', 'b'], ''))
+
 class TestSessionAuthenticationPolicy(unittest.TestCase):
     def _getTargetClass(self):
         from pyramid.authentication import SessionAuthenticationPolicy
@@ -1150,13 +1213,14 @@ class DummyCookieHelper:
 
 class DummyAuthTktModule(object):
     def __init__(self, timestamp=0, userid='userid', tokens=(), user_data='',
-                 parse_raise=False):
+                 parse_raise=False, hashalg="md5"):
         self.timestamp = timestamp
         self.userid = userid
         self.tokens = tokens
         self.user_data = user_data
         self.parse_raise = parse_raise
-        def parse_ticket(secret, value, remote_addr):
+        self.hashalg = hashalg
+        def parse_ticket(secret, value, remote_addr, hashalg):
             self.secret = secret
             self.value = value
             self.remote_addr = remote_addr