changeset 7166:1549c7e74ef8

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.
author John Rouillard <rouilj@ieee.org>
date Fri, 24 Feb 2023 23:47:28 -0500
parents 970cd6d2b8ea
children f6b24a8524cd
files doc/upgrading.txt test/test_cgi.py
diffstat 2 files changed, 56 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/doc/upgrading.txt	Thu Feb 23 19:34:39 2023 -0500
+++ b/doc/upgrading.txt	Fri Feb 24 23:47:28 2023 -0500
@@ -179,6 +179,42 @@
 SQLite databases. If you want to keep using the data set the
 ``sessiondb`` ``backend`` option as described above.
 
+Update ``config.ini``'s ``password_pbkdf2_default_rounds`` (required)
+---------------------------------------------------------------------
+
+Roundup hashes passwords using PBKDF2 with SHA1. PBKDF2 has a
+parameter that makes hashing a password more difficult to do.
+The original 10000 value was set years ago. It has not been
+updated for advancements in computing power.
+
+This release of Roundup changes the value to 2000000 (2
+million). This exceeds the current `recommended setting of
+1,300,000`_ for PBKDF2 when used with SHA1.
+
+After the change users will still be able to log in using the
+older 10000 round hashed passwords. If ``migrate_passwords`` is
+set to ``yes``, passwords will be automatically re-hashed using
+the new higher value when the user logs in.
+
+This re-hashing might result in a slight delay (under 1
+second). If you see a large slowdown, check to see if you can
+execute::
+
+  python3 -c 'from hashlib import pbkdf2_hmac'
+
+without an error.
+
+If you get an ImportError, you are using Roundup's fallback
+PBKDF2 implementation. It is written in Python and is much slower
+than the library version.  As a result re-encrypting the password
+(and logging in which requires calculating the encrypted
+password) will be very slow.
+
+You should find out how to make this succeed. You may need to
+install an OS vendor package or some other library.
+
+.. _recommended setting of 1,300,000: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2
+
 Session/OTK data storage using Redis (optional)
 -----------------------------------------------
 
--- a/test/test_cgi.py	Thu Feb 23 19:34:39 2023 -0500
+++ b/test/test_cgi.py	Fri Feb 24 23:47:28 2023 -0500
@@ -560,7 +560,7 @@
         # assume that the "best" algorithm is the first one and doesn't
         # need migration, all others should be migrated.
         cl.db.config.WEB_LOGIN_ATTEMPTS_MIN = 200
-
+        cl.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 10000
         # The third item always fails. Regardless of what is there.
         #  ['plaintext', 'SHA', 'crypt', 'MD5']:
         print(password.Password.deprecated_schemes)
@@ -571,23 +571,38 @@
                 continue  # crypt is not available on Windows
             pw1 = password.Password('foo', scheme=scheme)
             print(pw1)
-            self.assertEqual(pw1.needs_migration(), True)
+            self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
             self.db.user.set(chef, password=pw1)
             self.db.commit()
             actions.LoginAction(cl).handle()
             pw = cl.db.user.get(chef, 'password')
             print(pw)
             self.assertEqual(pw, 'foo')
-            self.assertEqual(pw.needs_migration(), False)
+            self.assertEqual(pw.needs_migration(config=cl.db.config), False)
         cl.db.Otk = self.db.Otk
         pw1 = pw
-        self.assertEqual(pw1.needs_migration(), False)
+        self.assertEqual(pw1.needs_migration(config=cl.db.config), False)
         scheme = password.Password.known_schemes[0]
         self.assertEqual(scheme, pw1.scheme)
         actions.LoginAction(cl).handle()
         pw = cl.db.user.get(chef, 'password')
         self.assertEqual(pw, 'foo')
         self.assertEqual(pw, pw1)
+
+        # migrate if rounds has increased above rounds was 10000
+        # below will be 100000
+        cl.db.Otk = self.db.Otk
+        pw1 = pw
+        cl.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 100000
+        self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
+        scheme = password.Password.known_schemes[0]
+        self.assertEqual(scheme, pw1.scheme)
+        actions.LoginAction(cl).handle()
+        pw = cl.db.user.get(chef, 'password')
+        self.assertEqual(pw, 'foo')
+        # do not assert self.assertEqual(pw, pw1) as pw is a 100,000
+        # cycle while pw1 is only 10,000. They won't compare equally.
+
         cl.db.close()
 
     def testPasswordConfigOption(self):
@@ -596,7 +611,7 @@
         cl = self._make_client(form)
         self.db.config.PASSWORD_PBKDF2_DEFAULT_ROUNDS = 1000
         pw1 = password.Password('foo', scheme='MD5')
-        self.assertEqual(pw1.needs_migration(), True)
+        self.assertEqual(pw1.needs_migration(config=cl.db.config), True)
         self.db.user.set(chef, password=pw1)
         self.db.commit()
         actions.LoginAction(cl).handle()

Roundup Issue Tracker: http://roundup-tracker.org/