Suggested Changes to kay

11 views
Skip to first unread message

Prateek Malhotra

unread,
Nov 23, 2011, 3:10:33 PM11/23/11
to kay-...@googlegroups.com
Hey,

I was going through the DatastoreUser model (I want to extend this and implement my own features) and wanted to suggest the attached patches to KAY. It basically makes the crypto package unbiased towards the hash function used. By default, it uses the sha1 (so as to not break any code). I also randomized the length of the salt (I saw this recommended somewhere but don't know enough theory to say how useful it really is).

This sort of breaks the kay.auth.models package. I was going through this file to "update" it as needed as saw a lot of issues:

The create_inactive_user() bit of the DatastoreUserDBOperationMixin class conflicts with how kay.auth.create_user() works. It seems a lot of effort and time was put into making this "activation key" method version of registering a user but the entire process isn't documented very well (from what I found) and parts of it are broken. I've included a patch to update the models.py file though I may have inadvertently assumed things and made changes that breaks other parts of the framework. For my need, my modifications are working fine, but that's not to say I've done a fully inclusive test of all part of KAY. I never got to fully test the "registration" flow of KAY so not sure if anything else may have broken.

These are suggested patches. I've started to rewrite parts of KAY and include mirrors of them as packages in my own "app" so KAY can update and I won't have to worry about breaking any modifications I made. If these make it to KAY please let know, I'd like to minimize any duplicate code.

Thanks,
Prateek

P.S. I'm not too familiar with submitting patches in this fashion so sorry if my formatting is incorrect.

kay\utils\crypto.py:
--- crypto2.py 2011-11-23 14:43:33.781625000 -0500
+++ crypto.py 2011-11-23 14:47:19.461572100 -0500
@@ -13,9 +13,8 @@
 This file originally derives from Zine project.
 """
 
-import string
+import string, hashlib
 from random import choice, randrange
-from hashlib import sha1, md5
 
 KEY_CHARS = string.ascii_letters + string.digits
 IDENTIFIER_START = string.ascii_letters + '_'
@@ -91,15 +90,17 @@
   return pw
 
 
-def gen_pwhash(password):
+def gen_pwhash(password, alg='sha1'):
   """Return a the password encrypted in sha format with a random salt."""
+  if not alg in hashlib.algorithms:
+      raise ValueError("incorrect hash algorithm specified: %s" % hash)
   if isinstance(password, unicode):
     password = password.encode('utf-8')
-  salt = gen_salt(6)
-  h = sha1()
+  salt = gen_salt(choice(range(4,12)))
+  h = hashlib.new(alg)
   h.update(salt)
   h.update(password)
-  return 'sha$%s$%s' % (salt, h.hexdigest())
+  return '%s$%s$%s' % (alg, salt, h.hexdigest())
 
 
 def check_pwhash(pwhash, password):
@@ -148,13 +149,10 @@
   method, salt, hashval = pwhash.split('$', 2)
   if method == 'plain':
     return hashval == password
-  elif method == 'md5':
-    h = md5()
-  elif method == 'sha':
-    h = sha1()
+  elif method in hashlib.algorithms:
+    h = hashlib.new(method)
   else:
     return False
   h.update(salt)
   h.update(password)
   return h.hexdigest() == hashval
-  
\ No newline at end of file

kay\auth\models.py:
--- models2.py 2011-11-23 14:57:51.790813000 -0500
+++ models.py 2011-11-23 14:55:41.403572100 -0500
@@ -3,7 +3,7 @@
 """
 Kay authentication models.
 
-:Copyright: (c) 2009 Accense Technology, Inc.
+:Copyright: (c) 2009 Accense Technology, Inc. 
                      Takashi Matsuo <tma...@candit.jp>,
                      Ian Lewis <IanM...@gmail.com>
                      All rights reserved.
@@ -67,8 +67,7 @@
         raise DuplicateKeyError(_(u"This user name is already taken."
                                   " Please choose another user name."))
       if do_registration:
-        salt = crypto.gen_salt()
-        activation_key = crypto.sha1(salt+user_name).hexdigest()
+        activation_key = crypto.gen_activation_key(40)
         profile_key = db.Key.from_path(cls.kind(), key_name,
                                        RegistrationProfile.kind(),
                                        activation_key)
@@ -82,13 +81,13 @@
                                   registration_key=str(profile_key)),
                       transactional=True)
         user = cls(key_name=key_name, activated=False, user_name=user_name,
-                   password=crypto.gen_pwhash(password), email=email)
+                   password=cls.hash_password(password), email=email)
         profile = RegistrationProfile(user=user, parent=user,
                                       key_name=activation_key)
         db.put([profile, user])
       else:
         user = cls(key_name=key_name, activated=False, user_name=user_name,
-                   password=crypto.gen_pwhash(password), email=email)
+                   password=cls.hash_password(password), email=email)
         db.put(user)
       return user
     user = db.run_in_transaction(txn)
@@ -231,4 +230,3 @@
       id = crypto.new_iid()
       session = db.run_in_transaction(txn, id)
     return session
-

Paolo Casciello

unread,
Nov 23, 2011, 4:05:12 PM11/23/11
to kay-...@googlegroups.com
I don't use custom logins in GAE (I love the automation of Federated Login.. :P ) but I've done this sort of things a lot in other context.
Generally speaking, randomizing the length of the salt is a good practice. For SHA1, if there's something better is worth using it. Just for paranoia. :D
And, again generally speaking, if the crypto library you use has the hmac function, it's the best way to generate signatures/pwhash.
I don't know your code and I don't know that part of Kay so these were only advices :)

Bye,
 Paolo

Ian Lewis

unread,
Nov 24, 2011, 10:59:59 AM11/24/11
to kay-...@googlegroups.com

Hi

If you could, please create an issue for this and attach your patch as a file.That would be very helpful.

Thanks,
Ian

2011/11/24 5:10 "Prateek Malhotra" <some...@gmail.com>:
--
You received this message because you are subscribed to the Google Groups "kay-users" group.
To view this discussion on the web visit https://groups.google.com/d/msg/kay-users/-/-YsJUx7AV0kJ.
To post to this group, send email to kay-...@googlegroups.com.
To unsubscribe from this group, send email to kay-users+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/kay-users?hl=en.
Reply all
Reply to author
Forward
0 new messages