Skip to content

Commit c29e91a

Browse files
committed
issue2551251 - migrate pbkdf2 passwords ... test fixes and doc update
Fixed a couple of tests where calls to needs_migration() was missing its config parameter. Documented need to update config.ini's password_pbkdf2_default_rounds.
1 parent a1f35b3 commit c29e91a

File tree

2 files changed

+56
-5
lines changed

2 files changed

+56
-5
lines changed

doc/upgrading.txt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,42 @@ ephemeral, there is no plan to migrate this data to the new
179179
SQLite databases. If you want to keep using the data set the
180180
``sessiondb`` ``backend`` option as described above.
181181

182+
Update ``config.ini``'s ``password_pbkdf2_default_rounds`` (required)
183+
---------------------------------------------------------------------
184+
185+
Roundup hashes passwords using PBKDF2 with SHA1. PBKDF2 has a
186+
parameter that makes hashing a password more difficult to do.
187+
The original 10000 value was set years ago. It has not been
188+
updated for advancements in computing power.
189+
190+
This release of Roundup changes the value to 2000000 (2
191+
million). This exceeds the current `recommended setting of
192+
1,300,000`_ for PBKDF2 when used with SHA1.
193+
194+
After the change users will still be able to log in using the
195+
older 10000 round hashed passwords. If ``migrate_passwords`` is
196+
set to ``yes``, passwords will be automatically re-hashed using
197+
the new higher value when the user logs in.
198+
199+
This re-hashing might result in a slight delay (under 1
200+
second). If you see a large slowdown, check to see if you can
201+
execute::
202+
203+
python3 -c 'from hashlib import pbkdf2_hmac'
204+
205+
without an error.
206+
207+
If you get an ImportError, you are using Roundup's fallback
208+
PBKDF2 implementation. It is written in Python and is much slower
209+
than the library version. As a result re-encrypting the password
210+
(and logging in which requires calculating the encrypted
211+
password) will be very slow.
212+
213+
You should find out how to make this succeed. You may need to
214+
install an OS vendor package or some other library.
215+
216+
.. _recommended setting of 1,300,000: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2
217+
182218
Session/OTK data storage using Redis (optional)
183219
-----------------------------------------------
184220

test/test_cgi.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ def testPasswordMigration(self):
560560
# assume that the "best" algorithm is the first one and doesn't
561561
# need migration, all others should be migrated.
562562
cl.db.config.WEB_LOGIN_ATTEMPTS_MIN = 200
563-
563+
cl.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 10000
564564
# The third item always fails. Regardless of what is there.
565565
# ['plaintext', 'SHA', 'crypt', 'MD5']:
566566
print(password.Password.deprecated_schemes)
@@ -571,23 +571,38 @@ def testPasswordMigration(self):
571571
continue # crypt is not available on Windows
572572
pw1 = password.Password('foo', scheme=scheme)
573573
print(pw1)
574-
self.assertEqual(pw1.needs_migration(), True)
574+
self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
575575
self.db.user.set(chef, password=pw1)
576576
self.db.commit()
577577
actions.LoginAction(cl).handle()
578578
pw = cl.db.user.get(chef, 'password')
579579
print(pw)
580580
self.assertEqual(pw, 'foo')
581-
self.assertEqual(pw.needs_migration(), False)
581+
self.assertEqual(pw.needs_migration(config=cl.db.config), False)
582582
cl.db.Otk = self.db.Otk
583583
pw1 = pw
584-
self.assertEqual(pw1.needs_migration(), False)
584+
self.assertEqual(pw1.needs_migration(config=cl.db.config), False)
585585
scheme = password.Password.known_schemes[0]
586586
self.assertEqual(scheme, pw1.scheme)
587587
actions.LoginAction(cl).handle()
588588
pw = cl.db.user.get(chef, 'password')
589589
self.assertEqual(pw, 'foo')
590590
self.assertEqual(pw, pw1)
591+
592+
# migrate if rounds has increased above rounds was 10000
593+
# below will be 100000
594+
cl.db.Otk = self.db.Otk
595+
pw1 = pw
596+
cl.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 100000
597+
self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
598+
scheme = password.Password.known_schemes[0]
599+
self.assertEqual(scheme, pw1.scheme)
600+
actions.LoginAction(cl).handle()
601+
pw = cl.db.user.get(chef, 'password')
602+
self.assertEqual(pw, 'foo')
603+
# do not assert self.assertEqual(pw, pw1) as pw is a 100,000
604+
# cycle while pw1 is only 10,000. They won't compare equally.
605+
591606
cl.db.close()
592607

593608
def testPasswordConfigOption(self):
@@ -596,7 +611,7 @@ def testPasswordConfigOption(self):
596611
cl = self._make_client(form)
597612
self.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 1000
598613
pw1 = password.Password('foo', scheme='MD5')
599-
self.assertEqual(pw1.needs_migration(), True)
614+
self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
600615
self.db.user.set(chef, password=pw1)
601616
self.db.commit()
602617
actions.LoginAction(cl).handle()

0 commit comments

Comments
 (0)