[Zope-dev] [patch] More secure cookie crumbler?

[Chris Withers]

Hi Shane and zope-dev,

I think the attached patch (against CookieCrumbler 1.1) makes CookieCrumbler a 
little more secure.

It makes CookieCrumbler not store the user's password and username on the 
browser side and rotates the token stored on the browser side ever 10 seconds or 
time between request, whichever is longer. It also provides a session timeout 
controlled by the server.

I thought I'd post it here so people could have a look and let me know what you 
think.

If the patch is sound, it'd be great to see the patch make it into the next 
release of CookieCrumbler, and maybe the CMF and Plone HEADs?

cheers,

Chris

PS: To make cookie auth properly secure, you really need to be working over SSL 
only, and in addition, you should tweak CookieCrumbler further so that it sets 
the secure session bit, meaning your sessions should only get returned over a 
secure connection... mindyou, to get basic auth to be even vaguely secure, you 
also need to be working over SSL ;-) (and you should never let your users change 
their password without some other form of authentication, or at the very least 
making them enter their old password in the same atomic operation as setting 
their new password ;-)

-- 
Simplistix - Content Management, Zope & Python Consulting
            - http://www.simplistix.co.uk
-------------- next part --------------
Index: CookieCrumbler.py
===================================================================
--- CookieCrumbler.py	(revision 207)
+++ CookieCrumbler.py	(working copy)
@@ -24,17 +24,39 @@
 from AccessControl import getSecurityManager, ClassSecurityInfo, Permissions
 from ZPublisher import BeforeTraverse
 import Globals
+import threading
 from Globals import HTMLFile
 from zLOG import LOG, ERROR
 from ZPublisher.HTTPRequest import HTTPRequest
 from OFS.Folder import Folder
+from BTrees.OOBTree import OOBTree
+from time import time
 
-
 # Constants.
 ATTEMPT_NONE = 0       # No attempt at authentication
 ATTEMPT_LOGIN = 1      # Attempt to log in
 ATTEMPT_RESUME = 2     # Attempt to resume session
 
+# set this to 0 to get "old style" sessioning
+more_secure = 1
+
+# the default store of session information, for "more secure" sessioning
+lock = threading.Lock()
+session2ac = OOBTree()
+session2reset = OOBTree()
+time2session = OOBTree()
+import random
+try:
+    jn = random.randint(0,2L**160)
+    def getRandom():
+        return hex(random.randint(0,2**160))[2:-1]
+except:
+    # python < 2.3
+    jn = random.random()
+    def getRandom():
+        return str(random.random())[2:]
+random.jumpahead(jn)
+
 ModifyCookieCrumblers = 'Modify Cookie Crumblers'
 ViewManagementScreens = Permissions.view_management_screens
 
@@ -115,9 +137,74 @@
         return getattr( self.aq_inner.aq_parent, name, default )
 
     security.declarePrivate('defaultSetAuthCookie')
-    def defaultSetAuthCookie( self, resp, cookie_name, cookie_value ):
-        resp.setCookie( cookie_name, cookie_value, path=self.getCookiePath())
+    security.declarePrivate('defaultGetAuthCookie')
+    security.declarePrivate('defaultExpireOldSessions')
+    if more_secure:
+        def defaultSetAuthCookie( self, resp, cookie_name, cookie_value ):
+            session = getRandom()
+            global session2ac
+            global session2reset
+            global time2session
+            lock.acquire()
+            session2ac[session]=cookie_value
+            # tweak this for session expirey time
+            t = time()
+            expiretime = t+20*60 # 20 mins
+            if not time2session.has_key(expiretime):
+                time2session[expiretime]=[]
+            time2session[expiretime].append(session)
+            # cookie reset time (this allows rotating session values
+            # while still letting simultaneous requests such as ZMI
+            # keep working)
+            session2reset[session]=t+10 #10 secs
+            lock.release()
+            resp.setCookie( cookie_name, session, path=self.getCookiePath())
 
+        def defaultGetAuthCookie( self, resp, cookie_name, acc ):
+            lock.acquire()
+            ac = session2ac.get(acc,None)
+            if ac is None:
+                lock.release()                
+                method = self.getCookieMethod( 'expireAuthCookie'
+                                               , self.defaultExpireAuthCookie )
+                method( resp, cookie_name=self.auth_cookie )
+                return None
+            t = session2reset[acc]
+            lock.release()
+            if t<time():
+                del session2ac[acc]
+                del session2reset[acc]
+                method = self.getCookieMethod( 'setAuthCookie'
+                                             , self.defaultSetAuthCookie )
+                method( resp, cookie_name, ac )            
+            return ac
+                                      
+        def defaultExpireOldSessions(self):
+            global session2ac
+            global session2reset
+            global time2session
+            expiretime = time()
+            lock.acquire()
+            for t,sessions in time2session.items(0,expiretime):
+                for session in sessions:
+                    try:
+                        del session2ac[session]
+                        del session2reset[session]
+                    except KeyError:
+                        # it's already gone
+                        pass
+                del time2session[t]
+            lock.release()
+    else:
+        def defaultSetAuthCookie( self, resp, cookie_name, cookie_value ):
+            resp.setCookie( cookie_name, quote(cookie_value), path=self.getCookiePath())
+
+        def defaultGetAuthCookie( self, acc ):
+            return unquote(acc)
+
+        def defaultExpireOldSessions(self):
+            pass
+        
     security.declarePrivate('defaultExpireAuthCookie')
     def defaultExpireAuthCookie( self, resp, cookie_name ):
         resp.expireCookie( cookie_name, path=self.getCookiePath())
@@ -167,15 +254,22 @@
                                       path=self.getCookiePath())
                 method = self.getCookieMethod( 'setAuthCookie'
                                              , self.defaultSetAuthCookie )
-                method( resp, self.auth_cookie, quote( ac ) )
+                method( resp, self.auth_cookie, ac )
                 self.delRequestVar(req, self.name_cookie)
                 self.delRequestVar(req, self.pw_cookie)
 
             elif req.has_key(self.auth_cookie):
+                # take the opportunity to potentially delete expired sessions
+                method = self.getCookieMethod( 'expireOldSessions'
+                                               , self.defaultExpireOldSessions )
+                method()                
                 # Attempt to resume a session if the cookie is valid.
                 # Copy __ac to the auth header.
-                ac = unquote(req[self.auth_cookie])
-                if ac and ac != 'deleted':
+                acc = req[self.auth_cookie]
+                if acc and acc != 'deleted':
+                    method = self.getCookieMethod( 'getAuthCookie'
+                                                   , self.defaultGetAuthCookie )
+                    ac = method( resp, self.auth_cookie, acc)
                     try:
                         decodestring(ac)
                     except: