Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #818 (closed enhancement: wontfix)

Opened 11 years ago

Last modified 10 years ago

implement site_secret in passwords

Reported by: jvanasco@… Owned by: anonymous
Priority: normal Milestone: 1.5
Component: Identity Version: 0.9a5
Severity: trivial Keywords:
Cc:

Description

passwords are fine as md5/sha1, but they could be a little stronger if a sitesecret was introduced

doing this just makes a md5('a') different between sites, which is kind of nice as a application developer as it insulates you even more from the chance that someone who md5'd the dictionary ( and then some ) can reverse your account passwords

suggestion:

app.cfg

+ identity.sapprovider.site_secret = "aaaaaaa"

identity/saprovider.py

-        algorithm= get( "identity.saprovider.encryption_algorithm", None )
-        if "md5"==algorithm:
-            self.encrypt_password= lambda pw: md5.new(pw).hexdigest()
-        elif "sha1"==algorithm:
-            self.encrypt_password= lambda pw: sha.new(pw).hexdigest()
-        else:
-            self.encrypt_password= lambda pw: pw

+        algorithm= get( "identity.saprovider.encryption_algorithm", None )
+            self.encrypt_password= lambda pw: md5.new("%s%s"%(pw, get( "identity.saprovider.site_secret", None )).hexdigest()
+        elif "sha1"==algorithm:
+            self.encrypt_password= lambda pw: sha.new("%s%s"%(pw, get( "identity.saprovider.site_secret", None )).hexdigest()
+        else:
+            self.encrypt_password= lambda pw: pw

Change History

comment:1 Changed 11 years ago by godoy

I'm -1 on this. Specially because it makes one more point that need to be checked if your applications share the same database (different schemas or even the same schema) and userbase. Besides, this will just make it one more step to reproduce what you said: MD5 a dictionary as site secret then use it together with the dictionary again to get MD5 for passwords.

It is better to enforce high quality passwords using other means --- PAM, for example, can use cracklib to check for strong or weak passwords and only the superuser can set a weak password --- that are easy to find.

This is very specific and providing this with the same default for all applications won't enhance security; providing this with different "sitesecret" will make integrating applications harder.

Did I say that I'm -1 on this? ;-)

comment:2 Changed 11 years ago by jvanasco@…

well i use it on the databases accessed by different apps and different languages - it all works the same as long as they share the salt. md5 and concatenation aren't implemented differently between programs.

right now there are a bunch of sites that have the dictionary md5'd, along with a sampling of permutations and word concatanations. type in an md5, and it looks for a reverse mapping. using a site secret as a salt offsets this

the way i suggest it be implemented makes this optional -- None will be returned if the value is unset -- so you won't be forced to use it. but if you do use it, the public dictionaries will be pretty much useless at reverse mapping your digests.

comment:3 Changed 11 years ago by godoy

This isn't backwards compatible, though:

>>> print "%s" % None
None
>>> print "-%s-" % None
-None-
>>> print "%s%s" % ('string', None)
stringNone
>>> 

If one applies this, then all passwords that are hashed will have to be reset because of the concatenation with the string "None".

To make it backwards compatible, replace None by '' transforming

get( "identity.saprovider.site_secret", None )

into

get("identity.saprovider.site_secret", "")
>>> print "%s%s" % ('string', '')
string
>>>

comment:4 Changed 11 years ago by godoy

Forgot to mention, but an "soprovider" is missing in this patch...

comment:5 Changed 11 years ago by jorge.vargas

-1 this is just one more layer and a very weak one since all the attacker needs to find out is if the site is written in TG

comment:6 Changed 11 years ago by jorge.vargas

  • Status changed from new to closed
  • Resolution set to wontfix

if you provide a patch that doesn't affects any existing code this may be added.

Note: See TracTickets for help on using tickets.