From 4d9945dfdc1630579b99ee992c7887600ddb7f3e Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Fri, 15 Apr 2011 08:09:59 +0000 Subject: [PATCH] Add new config-option 'password_pbkdf2_default_rounds' in 'main' section to configure the default parameter for new password generation. Set this to a higher value on faster systems which want more security. Thanks to Eli Collins for implementing this (see issue2550688). This now passes a config object (default None in which case we fall back to hard-coded parameters) into the password generation routine. This way we can add further parameters for password generation in the future. Also added a small regression test for this new feature. git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4595 57a73879-2fb5-44c3-a270-3262357dd7e2 --- CHANGES.txt | 5 +++++ roundup/admin.py | 2 +- roundup/cgi/actions.py | 7 ++++--- roundup/cgi/form_parser.py | 2 +- roundup/configuration.py | 4 ++++ roundup/mailgw.py | 2 +- roundup/password.py | 23 ++++++++++++----------- test/test_cgi.py | 14 ++++++++++++++ 8 files changed, 42 insertions(+), 17 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 4fe82cf..cc011fc 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -84,6 +84,11 @@ Fixed: is "yes" so if you don't want that passwords are auto-migrated to a more secure password scheme on user login, set this to "no" before running your tracker(s) after the upgrade. +- Add new config-option 'password_pbkdf2_default_rounds' in 'main' + section to configure the default parameter for new password + generation. Set this to a higher value on faster systems which want + more security. Thanks to Eli Collins for implementing this (see + issue2550688). 2010-10-08 1.4.16 (r4541) diff --git a/roundup/admin.py b/roundup/admin.py index 511046d..1a4e241 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -511,7 +511,7 @@ Erase it? Y/N: """)) init.write_select_db(tracker_home, backend, tracker.config.DATABASE) # GO - tracker.init(password.Password(adminpw)) + tracker.init(password.Password(adminpw, config=tracker.config)) return 0 diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py index 41f5979..d48fb59 100755 --- a/roundup/cgi/actions.py +++ b/roundup/cgi/actions.py @@ -353,7 +353,7 @@ class EditCSVAction(Action): if isinstance(prop, hyperdb.Multilink): value = value.split(':') elif isinstance(prop, hyperdb.Password): - value = password.Password(value) + value = password.Password(value, config=self.db.config) elif isinstance(prop, hyperdb.Interval): value = date.Interval(value) elif isinstance(prop, hyperdb.Date): @@ -711,7 +711,7 @@ class PassResetAction(Action): # XXX we need to make the "default" page be able to display errors! try: # set the password - cl.set(uid, password=password.Password(newpw)) + cl.set(uid, password=password.Password(newpw, config=self.db.config)) # clear the props from the otk database otks.destroy(otk) self.db.commit() @@ -1013,7 +1013,8 @@ class LoginAction(Action): stored = db.user.get(userid, 'password') if givenpw == stored: if db.config.WEB_MIGRATE_PASSWORDS and stored.needs_migration(): - db.user.set(userid, password=password.Password(givenpw)) + newpw = password.Password(givenpw, config=db.config) + db.user.set(userid, password=newpw) db.commit() return 1 if not givenpw and not stored: diff --git a/roundup/cgi/form_parser.py b/roundup/cgi/form_parser.py index ce6b630..b5c5d49 100755 --- a/roundup/cgi/form_parser.py +++ b/roundup/cgi/form_parser.py @@ -383,7 +383,7 @@ class FormParser: raise FormError, self._('Password and confirmation text ' 'do not match') try: - value = password.Password(value) + value = password.Password(value, config=self.db.config) except hyperdb.HyperdbValueError, msg: raise FormError, msg diff --git a/roundup/configuration.py b/roundup/configuration.py index 9a8d165..aa5ab85 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -537,6 +537,10 @@ SETTINGS = ( "starting with python 2.5. Set this to a higher value if you\n" "get the error 'Error: field larger than field limit' during\n" "import."), + (IntegerNumberOption, 'password_pbkdf2_default_rounds', '10000', + "Sets the default number of rounds used when encoding passwords\n" + "using the PBKDF2 scheme. Set this to a higher value on faster\n" + "systems which want more security."), )), ("tracker", ( (Option, "name", "Roundup issue tracker", diff --git a/roundup/mailgw.py b/roundup/mailgw.py index 922dc12..1664484 100644 --- a/roundup/mailgw.py +++ b/roundup/mailgw.py @@ -1688,7 +1688,7 @@ def uidFromAddress(db, address, create=1, **user_props): try: return db.user.create(username=trying, address=address, realname=realname, roles=db.config.NEW_EMAIL_USER_ROLES, - password=password.Password(password.generatePassword()), + password=password.Password(password.generatePassword(), config=db.config), **user_props) except exceptions.Reject: return 0 diff --git a/roundup/password.py b/roundup/password.py index adb2cc4..eab3007 100644 --- a/roundup/password.py +++ b/roundup/password.py @@ -135,7 +135,7 @@ def pbkdf2_unpack(pbkdf2): raw_salt = h64decode(salt) return rounds, salt, raw_salt, digest -def encodePassword(plaintext, scheme, other=None): +def encodePassword(plaintext, scheme, other=None, config=None): """Encrypt the plaintext password. """ if plaintext is None: @@ -146,9 +146,10 @@ def encodePassword(plaintext, scheme, other=None): else: raw_salt = getrandbytes(20) salt = h64encode(raw_salt) - #FIXME: find way to access config, so default rounds - # can be altered for faster/slower hosts via config.ini - rounds = 10000 + if config: + rounds = config.PASSWORD_PBKDF2_DEFAULT_ROUNDS + else: + rounds = 10000 if rounds < 1000: raise PasswordValueError, "invalid PBKDF2 hash (rounds too low)" raw_digest = pbkdf2(plaintext, raw_salt, rounds, 20) @@ -243,14 +244,14 @@ class Password(JournalPassword): deprecated_schemes = ["SHA", "MD5", "crypt", "plaintext"] known_schemes = ["PBKDF2"] + deprecated_schemes - def __init__(self, plaintext=None, scheme=None, encrypted=None, strict=False): + def __init__(self, plaintext=None, scheme=None, encrypted=None, strict=False, config=None): """Call setPassword if plaintext is not None.""" if scheme is None: scheme = self.default_scheme if plaintext is not None: - self.setPassword (plaintext, scheme) + self.setPassword (plaintext, scheme, config=config) elif encrypted is not None: - self.unpack(encrypted, scheme, strict=strict) + self.unpack(encrypted, scheme, strict=strict, config=config) else: self.scheme = self.default_scheme self.password = None @@ -267,7 +268,7 @@ class Password(JournalPassword): return True return False - def unpack(self, encrypted, scheme=None, strict=False): + def unpack(self, encrypted, scheme=None, strict=False, config=None): """Set the password info from the scheme: string (the inverse of __str__) """ @@ -278,16 +279,16 @@ class Password(JournalPassword): self.plaintext = None else: # currently plaintext - encrypt - self.setPassword(encrypted, scheme) + self.setPassword(encrypted, scheme, config=config) if strict and self.scheme not in self.known_schemes: raise PasswordValueError, "unknown encryption scheme: %r" % (self.scheme,) - def setPassword(self, plaintext, scheme=None): + def setPassword(self, plaintext, scheme=None, config=None): """Sets encrypts plaintext.""" if scheme is None: scheme = self.default_scheme self.scheme = scheme - self.password = encodePassword(plaintext, scheme) + self.password = encodePassword(plaintext, scheme, config=config) self.plaintext = plaintext def __str__(self): diff --git a/test/test_cgi.py b/test/test_cgi.py index 2d63ed4..7fbe89c 100644 --- a/test/test_cgi.py +++ b/test/test_cgi.py @@ -449,6 +449,20 @@ class FormTestCase(unittest.TestCase): self.assertEqual(pw, 'foo') self.assertEqual(pw, pw1) + def testPasswordConfigOption(self): + chef = self.db.user.lookup('Chef') + form = dict(__login_name='Chef', __login_password='foo') + cl = self._make_client(form) + self.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 1000 + pw1 = password.Password('foo', scheme='crypt') + self.assertEqual(pw1.needs_migration(), True) + self.db.user.set(chef, password=pw1) + self.db.commit() + actions.LoginAction(cl).handle() + pw = self.db.user.get(chef, 'password') + self.assertEqual('PBKDF2', pw.scheme) + self.assertEqual(1000, password.pbkdf2_unpack(pw.password)[0]) + # # Boolean # -- 2.30.2