Security String

I think this whole page can be condensed to:

If a user logs in but forgets to log out on a public terminal, he cannot revoke that login later, and people could steal the cookie there. My recent cookie changes (see commits 1933:19e3af7cabcd and a few before that) in 1.6 would allow revoking other logins trivially by simply invalidating all stored cookie secrets other than the current one. This just needs some UI.

This patch achieves this by introducing a "security string" feature that is hashed into the cookie signature so you can change the security string and thereby log out all your possible sessions.

By simply removing all other cookie secrets, the same is achieved and the user is even kept logged on at the current machine. -- JohannesBerg 2007-04-03 16:50:52


TODO for this page: polish english, remove redundancy, refactor

This patch in moin-1.5 and for security string.

/!\ If find any bug. Please let me <frankie AT FREESPAM openworkshop DOT org> to know.

:-)) = Happy New Year. =

(!) I am sorry, I need to alter MoinMoin code for security's reason.

A Story

One day a company boss sat in a coffee shop. Suddenly he had a very good idea. Then he wanted to enter it in his company's MoinMoin wiki.

The Boss said: "Yes, It is a very great idea."

But he didn't have a private computer, he was using Coffee Shop's computer to do it. He entered the idea. He was very happy.

His phone rang. He left the coffee shop, but he forgot to logout. ...


What time MoinMoin Change or Create the user's security_string

  1. when the user create.
  2. when the user profile hasn't security_string.
    • . and When the user done password_auth.
  3. when the user request account_sendmail.
  4. when the user logout.
  5. when the user after moin_url auth.

Why security_string is good for security

Why change the user in logout

  1. First every member need a way to change his security_string.
  2. I think logout is a better session then login for change it.
    1. Member think logout is a global logout.
      • If member login in a remote computer like above story. he can logout in another computer, then the remote computer cann't do cookie_auth again.
        • What does happen with the open connection on the system were he has forgotten to log out? How does it protect for someone using the account or changing the password on the running system? Is probably an automatic vigilance device needed (dead-man switch)? -- ReimarBauer 2006-02-01 22:03:20

          • All problem is in the cookie, because the moin_id contents is fix, cann't easy to change it. -- FrankieChow 2006-03-22 16:10:50

          • Of Course, you can add another contents in Cookie, like Security_String and don't modify the moin_id. ( But Modify is simple. )
          • Now MoinMoin's Security Policy is very bad, Not Administrator like to Change it. ( Include me ) so .. dead-man switch .. -- FrankieChow 2006-03-22 16:10:50

      • If member logout mean that: He will do password_auth in next time.
        • That's difficult too, because some people do only logout on a foreigner computer and probably not on theirs. -- ReimarBauer 2006-02-01 22:34:02

          • I know it. but this is simple way to modify the cookie contents for MoinMoin Users. I don't like modify in userform, maybe not people like modify in there. -- FrankieChow 2006-03-22 16:10:50

Why change the user in account_sendmail

  1. I think another way to change security_string on account_sendmail. because maybe the member lost info, so he want to MoinMoin to send it for mail. Of course he lose include security_string and cookie info.

Please see MoinMoinBugs/Cookie is not secure enough, the cracker know user.id is not a security problem, because the cookie have another part: security_string. it is keep security for cracker.

Send the user.id in cookie, it will keep the code simple.


About the Patch

Applies to
base core code.

Modify

Add

Maybe

You don't need to change multiconfig.py, Maybe you can just modify in wikiconfig.py ( Modify multiconfig.py for moin_url auth module ) .

Add a line to wikiconfig.py
Like it.
  •    auth = [authmodule.moin_cookie, authmodule.moin_url]
Purpose

Protect the Cookie for MoinMoin.auth.moin_cookie .

Description

see MoinMoinBugs/Cookie is not secure enough

Patch Job

I want to record my patch job in here.

TODO

  1. freeze code.

Done

  1. Care the cache, when the security_string change. then need to update it.
  2. When member input the utf-8 string in the securty_string field in UserPreferences

  3. Redesign cal_security_userid module. ( now have bug. )
  4. Don't change anything in wiki/config/farmconfig.py
  5. Move Active the url_auth to another patch.
    1. ReActive in here.

  6. Change the user's security_string, When the user require mail account
  7. Change the userform, Keep setCookie, when user's security_string isn't same of user datafile. ( not all time. )
  8. Don't modify the MoinMoin.request.setCookie Module.

  9. When the user logout, then change the user.security_string.
  10. Disable the user security_string handle in User's UserPreferences.

  11. Group luck string into securitystring handle.
  12. Add the make_cache module to securitystring to create the caching obj.
  13. Add the update_cache module to securitystring to handle the cache update
    1. update_cache module will use securitystring.cal_* to update the cache.
  14. Change user.save() to do securitystring.update_cache when change user.security_string.
  15. ?Need do it:: Change the securitystring to a class not modules.
  16. Check the cache content before update it.
  17. active the auth_url and disable the {SHA} string auth.
  18. change the cal_security_userid module, keep it return the User, not just uid.
  19. represent securitystring.py use patch format.
  20. Do the Document.
  21. see mod_env

    • :D <!> I think make the security_string
      os.environ['MOIN_SECURITY_STRING'] + user.security_string
      is better then just user.security_string.

    •    1     #!python
         2     def make_security_key(request, securitystring, userid):
         3         """
         4         Make the hmac value for
         5          Key: securitystring
         6          msg: userid
         7      
         8         HOWTO set env['MOIN_SECURITY_STRING'] in cgi mode or mod_python mode.
         9             Please see: http://httpd.apache.org/docs/1.3/mod/mod_env.html
        10         """
        11         if request.env.has_key('MOIN_SECURITY_STRING'):
        12             return hmac.new( str(request.env['MOIN_SECURITY_STRING']) + securitystring, userid).hexdigest()
        13         return hmac.new(securitystring, userid).hexdigest()
      
  22. If you use the patch in moin-1.5.0 then you can change above code to
    •     +        if check_hmac_string:
          +            if not user_data.has_key('security_string'):
          +                return    

Patch

   1 * looking for arch@arch.thinkmo.de--2003-archives/moin--main--1.5--patch-361 to compare with
   2 * comparing to arch@arch.thinkmo.de--2003-archives/moin--main--1.5--patch-361
   3 M  MoinMoin/auth.py
   4 M  MoinMoin/multiconfig.py
   5 M  MoinMoin/request.py
   6 M  MoinMoin/user.py
   7 M  MoinMoin/userform.py
   8 A  MoinMoin/util/securitystring.py
   9 
  10 * modified files
  11 
  12 --- orig/MoinMoin/auth.py
  13 +++ mod/MoinMoin/auth.py
  14 @@ -45,6 +45,7 @@
  15  
  16  import Cookie
  17  from MoinMoin import user
  18 +from MoinMoin.util import securitystring
  19  
  20  def moin_cookie(request, **kw):
  21      """ authenticate via the MOIN_ID cookie """
  22 @@ -54,17 +55,17 @@
  23          u = user.User(request, name=name, password=password,
  24                        auth_method='login_userpassword')
  25          if u.valid:
  26 +            # Update the User security_string,
  27 +            #  If he don't have security_string.
  28 +            try:
  29 +                u.security_string
  30 +            except AttributeError:
  31 +                u.security_string = securitystring.gen(30)
  32 +                u.save()
  33              request.user = u # needed by setCookie
  34              request.setCookie()
  35              return u, False
  36          return None, True
  37 -
  38 -    if kw.get('logout'):
  39 -        # clear the cookie in the browser and locally. Does not
  40 -        # check if we have a valid user logged, just make sure we
  41 -        # don't have one after this call.
  42 -        request.deleteCookie()
  43 -        return None, True
  44      
  45      try:
  46          cookie = Cookie.SimpleCookie(request.saved_cookie)
  47 @@ -72,23 +73,59 @@
  48          # ignore invalid cookies, else user can't relogin
  49          cookie = None
  50      if cookie and cookie.has_key('MOIN_ID'):
  51 -        u = user.User(request, id=cookie['MOIN_ID'].value,
  52 +        # Use security_string to handle the Cookie.
  53 +        u = user.User(request, hmac_uid_string=cookie['MOIN_ID'].value,
  54                        auth_method='moin_cookie', auth_attribs=())
  55          if u.valid:
  56 -            return u, False
  57 +            if kw.get('logout'):
  58 +                # FrankieChow: Why Does not check it?
  59 +                # I think need to check it, or pass it to another auth handle.
  60 +                #
  61 +                ### clear the cookie in the browser and locally. Does not
  62 +                ### check if we have a valid user logged, just make sure we
  63 +                ### don't have one after this call.
  64 +                request.deleteCookie()
  65 +                # When the user do global logout then change the
  66 +                # security_string. ( in here. All logout is global logout. )
  67 +                u.security_string = securitystring.gen(30)
  68 +                u.save()
  69 +                return None, True
  70 +            else:
  71 +                return u, False
  72 +
  73 +        # If the brower don't have MOIN_ID cookie, just delete the cookie.
  74 +        if kw.get('logout'):
  75 +            request.deleteCookie()
  76 +            return None, True
  77 +            
  78      return None, True
  79  
  80  
  81 +def moin_url(request, **kw):
  82 +    # The url syntax is like this: action=userform&uid=
  83 +    action = request.form.get('action',[None])[0]
  84 +    uid = request.form.get('uid',[None])[0]
  85 +    user_obj = user.User(request)
  86 +    if action == 'userform' :
  87 +        u = user.User(request, auth_method='moin_url', hmac_uid_string=uid )
  88 +        if u.valid:
  89 +            u.security_string = securitystring.gen(30)
  90 +            u.save()
  91 +            request.user = u
  92 +            request.setCookie()
  93 +            return u, False
  94 +    return None, True
  95 +
  96  #
  97  #   idea: maybe we should call back to the request object like:
  98  #         username, password, authenticated, authtype = request.getUserPassAuth()
  99 -#	 WhoEver   geheim    false          basic      (twisted, doityourself pw check)
 100 -#	 WhoEver   None      true           basic/...  (apache)
 101 -#	 
 102 +#         WhoEver   geheim    false          basic      (twisted, doityourself pw check)
 103 +#         WhoEver   None      true           basic/...  (apache)
 104 +#         
 105  #        thus, the server specific code would stay in request object implementation.
 106  #
 107  #     THIS IS NOT A WIKI PAGE ;-)
 108 -	 
 109 +         
 110  def http(request, **kw):
 111      """ authenticate via http basic/digest/ntlm auth """
 112      from MoinMoin.request import RequestTwisted, RequestCLI
 113 
 114 
 115 --- orig/MoinMoin/multiconfig.py
 116 +++ mod/MoinMoin/multiconfig.py
 117 @@ -172,7 +172,7 @@
 118      actions_excluded = [] # ['DeletePage', 'AttachFile', 'RenamePage']
 119      allow_xslt = 0
 120      attachments = None # {'dir': path, 'url': url-prefix}
 121 -    auth = [authmodule.moin_cookie]
 122 +    auth = [authmodule.moin_cookie, authmodule.moin_url]
 123      
 124      backup_compression = 'gz'
 125      backup_users = []
 126 
 127 
 128 --- orig/MoinMoin/request.py
 129 +++ mod/MoinMoin/request.py
 130 @@ -9,7 +9,7 @@
 131  
 132  import os, time, sys, cgi, StringIO
 133  from MoinMoin import config, wikiutil, user
 134 -from MoinMoin.util import MoinMoinNoFooter, IsWin9x
 135 +from MoinMoin.util import MoinMoinNoFooter, IsWin9x, securitystring
 136  
 137  # Timing ---------------------------------------------------------------
 138  
 139 @@ -1216,7 +1216,16 @@
 140          # Set the cookie
 141          from Cookie import SimpleCookie
 142          c = SimpleCookie()
 143 -        c['MOIN_ID'] = self.user.id
 144 +        # Modify the Cookie String Syntax.
 145 +        # Keep the self.user.id in Cookie.
 146 +        #   1. easy for auth.
 147 +        #   2. and don't need to care the 
 148 +        #       securitystring.make_security_key(self, security_string, self.user.id)
 149 +        #      is unique.
 150 +        c['MOIN_ID'] = '%s%s%s' %(
 151 +           securitystring.make_security_key(self, self.user.security_string, self.user.id),
 152 +           securitystring.luck(self),
 153 +           self.user.id )
 154          c['MOIN_ID']['max-age'] = maxage
 155          if self.cfg.cookie_domain:
 156              c['MOIN_ID']['domain'] = self.cfg.cookie_domain
 157 
 158 
 159 --- orig/MoinMoin/user.py
 160 +++ mod/MoinMoin/user.py
 161 @@ -17,7 +17,7 @@
 162  PICKLE_PROTOCOL = pickle.HIGHEST_PROTOCOL
 163  
 164  from MoinMoin import config, caching, wikiutil
 165 -from MoinMoin.util import datetime, filesys
 166 +from MoinMoin.util import datetime, filesys, securitystring
 167  
 168  
 169  def getUserList(request):
 170 @@ -191,7 +191,9 @@
 171  class User:
 172      """A MoinMoin User"""
 173  
 174 -    def __init__(self, request, id=None, name="", password=None, auth_username="", **kw):
 175 +    # FrankieChow: I think keep the hmac_uid_string auth in User,
 176 +    #  It is better then other, just alter more core code.
 177 +    def __init__(self, request, id=None, name="", password=None, auth_username="", hmac_uid_string=None, **kw):
 178          """ Initialize User object
 179          
 180          @param request: the request object
 181 @@ -255,6 +257,7 @@
 182          # attrs not saved to profile
 183          self._request = request
 184          self._trail = []
 185 +        self._hmac_uid_string = hmac_uid_string
 186  
 187          # we got an already authenticated username:
 188          check_pass = 0
 189 @@ -272,6 +275,11 @@
 190                  self.load_from_id(1)
 191              else:
 192                  self.id = self.make_id()
 193 +        elif self._hmac_uid_string:
 194 +            try:
 195 +                self._hmac_string, self.id = self._hmac_uid_string.split( securitystring.luck(self._request) )[:2]
 196 +            except: pass
 197 +            self.load_from_id(check_pass=0, check_hmac_string=1)
 198          else:
 199              self.id = self.make_id()
 200  
 201 @@ -331,7 +339,9 @@
 202          """
 203          return os.path.exists(self.__filename())
 204  
 205 -    def load_from_id(self, check_pass=0):
 206 +    # In User Class, here is handle User Auth.
 207 +    #   So change it to require the check_hmac_string.
 208 +    def load_from_id(self, check_pass=0, check_hmac_string=0):
 209          """ Load user account data from disk.
 210  
 211          Can only load user data if the id number is already known.
 212 @@ -375,6 +385,17 @@
 213                  return
 214              else:
 215                  self.trusted = 1
 216 +        
 217 +        # If User haven't security_string, Don't handle in here,
 218 +        #    Just handle in auth.py in moin_cookie after the user done passd auth.
 219 +        if check_hmac_string:
 220 +            if not user_data.has_key('security_string'):
 221 +                return
 222 +            valid, changed = self._validateHmacString(user_data)
 223 +            if not valid: 
 224 +                return
 225 +            else:
 226 +                self.trusted = 1
 227  
 228          # Remove ignored checkbox values from user data
 229          for key, label in self._cfg.user_checkbox_fields:
 230 @@ -488,6 +509,11 @@
 231          # No encoded password match, this must be wrong password
 232          return False, False
 233  
 234 +    def _validateHmacString(self, data):
 235 +        if self._hmac_string == securitystring.make_security_key(self._request, data['security_string'], self.id):
 236 +            return True, False
 237 +        return False, False
 238 +
 239      def save(self):
 240          """ Save user account data to user account file on disk.
 241  
 242 @@ -935,14 +961,18 @@
 243          from MoinMoin.util import mail
 244          _ = self._request.getText
 245  
 246 +        # If MoinMoin use security_string logic to do url_auth. 
 247 +        #     When use SSHA to disable the Login Password.
 248          text = '\n' + _("""\
 249  Login Name: %s
 250  
 251 -Login Password: %s
 252 -
 253 -Login URL: %s/?action=userform&uid=%s
 254 +Login URL: %s/?action=userform&uid=%s%s%s
 255  """, formatted=False) % (
 256 -                        self.name, self.enc_password, self._request.getBaseURL(), self.id)
 257 +                        self.name, self._request.getBaseURL(), 
 258 +                        securitystring.make_security_key(self._request, self.security_string, self.id),
 259 +                        securitystring.luck(self._request),
 260 +                        self.id
 261 +                        )
 262  
 263          text = _("""\
 264  Somebody has requested to submit your account data to this email address.
 265 
 266 
 267 --- orig/MoinMoin/userform.py
 268 +++ mod/MoinMoin/userform.py
 269 @@ -8,7 +8,7 @@
 270  
 271  import string, time, re
 272  from MoinMoin import user, util, wikiutil
 273 -from MoinMoin.util import web, mail, datetime
 274 +from MoinMoin.util import web, mail, datetime, securitystring
 275  from MoinMoin.widget import html
 276  
 277  _debug = 0
 278 @@ -77,6 +77,10 @@
 279              for uid in users:
 280                  theuser = user.User(self.request, uid)
 281                  if theuser.valid and theuser.email.lower() == email:
 282 +                    # Change the security_string
 283 +                    #    When the user request the account_sendmail.
 284 +                    theuser.security_string = securitystring.gen(30)
 285 +                    theuser.save()
 286                      msg = theuser.mailAccountData()
 287                      return wikiutil.escape(msg)
 288  
 289 @@ -148,6 +152,8 @@
 290                      if thisuser.email == theuser.email and not thisuser.disabled:
 291                          return _("This email already belongs to somebody else.")
 292  
 293 +            # Before create the user's profile, create the user's security_string.
 294 +            theuser.security_string = securitystring.gen(30)
 295              # save data
 296              theuser.save()
 297              if form.has_key('create_and_mail'):
 298 
 299 
 300 --- orig/MoinMoin/util/securitystring.py
 301 +++ mod/MoinMoin/util/securitystring.py
 302 @@ -0,0 +1,36 @@
 303 +"""
 304 +    MoinMoin - Handle the Security String
 305 +    @copyright: (c) Bastian Blank, Florian Festi, Thomas Waldmann
 306 +    @copyright: MoinMoin:FrankieChow
 307 +    @license: GNU GPL, see COPYING for details.
 308 +"""
 309 +import os, hmac, string, random
 310 +
 311 +# Follow http://moinmoin.wikiwikiweb.de/MoinMoinBugs/Cookie_is_not_secure_enough
 312 +# This code is write by Nir Soffer
 313 +
 314 +def gen(number):
 315 +    # Make the number is more security.
 316 +    random_number = random.randint(number/2,number)
 317 +    safe = string.ascii_letters + string.digits + '_-'
 318 +    return "%s" % (''.join([random.choice(safe) for i in range(random_number)]))
 319 +
 320 +###
 321 +
 322 +def luck(request):
 323 +    # ':=:' is FrankieChow luck string. maybe you can change this to
 324 +    #   self.cfg.site_luck_string
 325 +    return ':=:'
 326 +    
 327 +def make_security_key(request, securitystring, userid):
 328 +    """
 329 +    Make the hmac value for
 330 +      Key: securitystring
 331 +      msg: userid
 332 +
 333 +    HOWTO set env['MOIN_SECURITY_STRING'] in cgi mode or mod_python mode.
 334 +        Please see: http://httpd.apache.org/docs/1.3/mod/mod_env.html
 335 +    """
 336 +    if request.env.has_key('MOIN_SECURITY_STRING'):
 337 +        return hmac.new( str(request.env['MOIN_SECURITY_STRING']) + securitystring, userid).hexdigest()
 338 +    return hmac.new(securitystring, userid).hexdigest()
 339 

security_string17.patch

Discussion

  1. In this patch. I don't disable the {SHA} auth.
    • Please wait my {SSHA} patch.

Should we still wait for something or begin with what's here???

Did you see: http://www.xml.com/pub/a/2003/12/17/dive.html

Plan


CategoryMoinMoinPatch

MoinMoin: MoinMoinPatch/SecurityString (last edited 2007-10-29 19:08:55 by localhost)