Skip to content

Commit 01d7d7a

Browse files
author
Ralf Schlatterbeck
committed
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.
1 parent 80ea798 commit 01d7d7a

File tree

8 files changed

+42
-17
lines changed

8 files changed

+42
-17
lines changed

CHANGES.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ Fixed:
8484
is "yes" so if you don't want that passwords are auto-migrated to a
8585
more secure password scheme on user login, set this to "no" before
8686
running your tracker(s) after the upgrade.
87+
- Add new config-option 'password_pbkdf2_default_rounds' in 'main'
88+
section to configure the default parameter for new password
89+
generation. Set this to a higher value on faster systems which want
90+
more security. Thanks to Eli Collins for implementing this (see
91+
issue2550688).
8792

8893
2010-10-08 1.4.16 (r4541)
8994

roundup/admin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ def do_initialise(self, tracker_home, args):
511511
init.write_select_db(tracker_home, backend, tracker.config.DATABASE)
512512

513513
# GO
514-
tracker.init(password.Password(adminpw))
514+
tracker.init(password.Password(adminpw, config=tracker.config))
515515

516516
return 0
517517

roundup/cgi/actions.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ def handle(self):
353353
if isinstance(prop, hyperdb.Multilink):
354354
value = value.split(':')
355355
elif isinstance(prop, hyperdb.Password):
356-
value = password.Password(value)
356+
value = password.Password(value, config=self.db.config)
357357
elif isinstance(prop, hyperdb.Interval):
358358
value = date.Interval(value)
359359
elif isinstance(prop, hyperdb.Date):
@@ -711,7 +711,7 @@ def handle(self):
711711
# XXX we need to make the "default" page be able to display errors!
712712
try:
713713
# set the password
714-
cl.set(uid, password=password.Password(newpw))
714+
cl.set(uid, password=password.Password(newpw, config=self.db.config))
715715
# clear the props from the otk database
716716
otks.destroy(otk)
717717
self.db.commit()
@@ -1013,7 +1013,8 @@ def verifyPassword(self, userid, givenpw):
10131013
stored = db.user.get(userid, 'password')
10141014
if givenpw == stored:
10151015
if db.config.WEB_MIGRATE_PASSWORDS and stored.needs_migration():
1016-
db.user.set(userid, password=password.Password(givenpw))
1016+
newpw = password.Password(givenpw, config=db.config)
1017+
db.user.set(userid, password=newpw)
10171018
db.commit()
10181019
return 1
10191020
if not givenpw and not stored:

roundup/cgi/form_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ def parse(self, create=0, num_re=re.compile('^\d+$')):
383383
raise FormError, self._('Password and confirmation text '
384384
'do not match')
385385
try:
386-
value = password.Password(value)
386+
value = password.Password(value, config=self.db.config)
387387
except hyperdb.HyperdbValueError, msg:
388388
raise FormError, msg
389389

roundup/configuration.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,10 @@ def str2value(self, value):
537537
"starting with python 2.5. Set this to a higher value if you\n"
538538
"get the error 'Error: field larger than field limit' during\n"
539539
"import."),
540+
(IntegerNumberOption, 'password_pbkdf2_default_rounds', '10000',
541+
"Sets the default number of rounds used when encoding passwords\n"
542+
"using the PBKDF2 scheme. Set this to a higher value on faster\n"
543+
"systems which want more security."),
540544
)),
541545
("tracker", (
542546
(Option, "name", "Roundup issue tracker",

roundup/mailgw.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1688,7 +1688,7 @@ def uidFromAddress(db, address, create=1, **user_props):
16881688
try:
16891689
return db.user.create(username=trying, address=address,
16901690
realname=realname, roles=db.config.NEW_EMAIL_USER_ROLES,
1691-
password=password.Password(password.generatePassword()),
1691+
password=password.Password(password.generatePassword(), config=db.config),
16921692
**user_props)
16931693
except exceptions.Reject:
16941694
return 0

roundup/password.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def pbkdf2_unpack(pbkdf2):
135135
raw_salt = h64decode(salt)
136136
return rounds, salt, raw_salt, digest
137137

138-
def encodePassword(plaintext, scheme, other=None):
138+
def encodePassword(plaintext, scheme, other=None, config=None):
139139
"""Encrypt the plaintext password.
140140
"""
141141
if plaintext is None:
@@ -146,9 +146,10 @@ def encodePassword(plaintext, scheme, other=None):
146146
else:
147147
raw_salt = getrandbytes(20)
148148
salt = h64encode(raw_salt)
149-
#FIXME: find way to access config, so default rounds
150-
# can be altered for faster/slower hosts via config.ini
151-
rounds = 10000
149+
if config:
150+
rounds = config.PASSWORD_PBKDF2_DEFAULT_ROUNDS
151+
else:
152+
rounds = 10000
152153
if rounds < 1000:
153154
raise PasswordValueError, "invalid PBKDF2 hash (rounds too low)"
154155
raw_digest = pbkdf2(plaintext, raw_salt, rounds, 20)
@@ -243,14 +244,14 @@ class Password(JournalPassword):
243244
deprecated_schemes = ["SHA", "MD5", "crypt", "plaintext"]
244245
known_schemes = ["PBKDF2"] + deprecated_schemes
245246

246-
def __init__(self, plaintext=None, scheme=None, encrypted=None, strict=False):
247+
def __init__(self, plaintext=None, scheme=None, encrypted=None, strict=False, config=None):
247248
"""Call setPassword if plaintext is not None."""
248249
if scheme is None:
249250
scheme = self.default_scheme
250251
if plaintext is not None:
251-
self.setPassword (plaintext, scheme)
252+
self.setPassword (plaintext, scheme, config=config)
252253
elif encrypted is not None:
253-
self.unpack(encrypted, scheme, strict=strict)
254+
self.unpack(encrypted, scheme, strict=strict, config=config)
254255
else:
255256
self.scheme = self.default_scheme
256257
self.password = None
@@ -267,7 +268,7 @@ def needs_migration(self):
267268
return True
268269
return False
269270

270-
def unpack(self, encrypted, scheme=None, strict=False):
271+
def unpack(self, encrypted, scheme=None, strict=False, config=None):
271272
"""Set the password info from the scheme:<encryted info> string
272273
(the inverse of __str__)
273274
"""
@@ -278,16 +279,16 @@ def unpack(self, encrypted, scheme=None, strict=False):
278279
self.plaintext = None
279280
else:
280281
# currently plaintext - encrypt
281-
self.setPassword(encrypted, scheme)
282+
self.setPassword(encrypted, scheme, config=config)
282283
if strict and self.scheme not in self.known_schemes:
283284
raise PasswordValueError, "unknown encryption scheme: %r" % (self.scheme,)
284285

285-
def setPassword(self, plaintext, scheme=None):
286+
def setPassword(self, plaintext, scheme=None, config=None):
286287
"""Sets encrypts plaintext."""
287288
if scheme is None:
288289
scheme = self.default_scheme
289290
self.scheme = scheme
290-
self.password = encodePassword(plaintext, scheme)
291+
self.password = encodePassword(plaintext, scheme, config=config)
291292
self.plaintext = plaintext
292293

293294
def __str__(self):

test/test_cgi.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,20 @@ def testPasswordMigration(self):
449449
self.assertEqual(pw, 'foo')
450450
self.assertEqual(pw, pw1)
451451

452+
def testPasswordConfigOption(self):
453+
chef = self.db.user.lookup('Chef')
454+
form = dict(__login_name='Chef', __login_password='foo')
455+
cl = self._make_client(form)
456+
self.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 1000
457+
pw1 = password.Password('foo', scheme='crypt')
458+
self.assertEqual(pw1.needs_migration(), True)
459+
self.db.user.set(chef, password=pw1)
460+
self.db.commit()
461+
actions.LoginAction(cl).handle()
462+
pw = self.db.user.get(chef, 'password')
463+
self.assertEqual('PBKDF2', pw.scheme)
464+
self.assertEqual(1000, password.pbkdf2_unpack(pw.password)[0])
465+
452466
#
453467
# Boolean
454468
#

0 commit comments

Comments
 (0)