Mercurial > p > roundup > code
changeset 4484:52e13bf0bb40
Add new config-option 'migrate_passwords' in section 'web'...
...to auto-migrate passwords at web-login time. Default for the new
option 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.
| author | Ralf Schlatterbeck <schlatterbeck@users.sourceforge.net> |
|---|---|
| date | Thu, 14 Apr 2011 18:10:58 +0000 |
| parents | 22bc0426e348 |
| children | 95aace124a8e |
| files | CHANGES.txt doc/upgrading.txt roundup/cgi/actions.py roundup/configuration.py roundup/password.py test/test_cgi.py |
| diffstat | 6 files changed, 82 insertions(+), 19 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Thu Apr 14 15:42:41 2011 +0000 +++ b/CHANGES.txt Thu Apr 14 18:10:58 2011 +0000 @@ -79,6 +79,11 @@ - Fix Password handling security issue2550688 (thanks Joseph Myers for reporting and Eli Collins for fixing) -- this fixes all observations by Joseph Myers except for auto-migration of existing passwords. +- Add new config-option 'migrate_passwords' in section 'web' to + auto-migrate passwords at web-login time. Default for the new option + 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. 2010-10-08 1.4.16 (r4541)
--- a/doc/upgrading.txt Thu Apr 14 15:42:41 2011 +0000 +++ b/doc/upgrading.txt Thu Apr 14 18:10:58 2011 +0000 @@ -16,6 +16,13 @@ Migrating from 1.4.x to 1.4.17 ============================== +There is a new config-option 'migrate_passwords' in section 'web' to +auto-migrate passwords at web-login time to a more secure storage +scheme. Default for the new option 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. + Searching now requires either read-permission without a check method, or you will have to add a "Search" permission for a class or a list of properties for a class (if you want to allow searching). For the classic
--- a/roundup/cgi/actions.py Thu Apr 14 15:42:41 2011 +0000 +++ b/roundup/cgi/actions.py Thu Apr 14 18:10:58 2011 +0000 @@ -1005,12 +1005,18 @@ raise exceptions.LoginError(self._( "You do not have permission to login")) - def verifyPassword(self, userid, password): - '''Verify the password that the user has supplied''' - stored = self.db.user.get(userid, 'password') - if password == stored: + def verifyPassword(self, userid, givenpw): + '''Verify the password that the user has supplied. + Optionally migrate to new password scheme if configured + ''' + db = self.db + 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)) + db.commit() return 1 - if not password and not stored: + if not givenpw and not stored: return 1 return 0
--- a/roundup/configuration.py Thu Apr 14 15:42:41 2011 +0000 +++ b/roundup/configuration.py Thu Apr 14 18:10:58 2011 +0000 @@ -579,6 +579,10 @@ "Setting this option makes Roundup display error tracebacks\n" "in the user's browser rather than emailing them to the\n" "tracker admin."), + (BooleanOption, "migrate_passwords", "yes", + "Setting this option makes Roundup migrate passwords with\n" + "an insecure password-scheme to a more secure scheme\n" + "when the user logs in via the web-interface."), )), ("rdbms", ( (Option, 'name', 'roundup',
--- a/roundup/password.py Thu Apr 14 15:42:41 2011 +0000 +++ b/roundup/password.py Thu Apr 14 18:10:58 2011 +0000 @@ -116,6 +116,25 @@ """ The password value is not valid """ pass +def pbkdf2_unpack(pbkdf2): + """ unpack pbkdf2 encrypted password into parts, + assume it has format "{rounds}${salt}${digest} + """ + if isinstance(pbkdf2, unicode): + pbkdf2 = pbkdf2.encode("ascii") + try: + rounds, salt, digest = pbkdf2.split("$") + except ValueError: + raise PasswordValueError, "invalid PBKDF2 hash (wrong number of separators)" + if rounds.startswith("0"): + raise PasswordValueError, "invalid PBKDF2 hash (zero-padded rounds)" + try: + rounds = int(rounds) + except ValueError: + raise PasswordValueError, "invalid PBKDF2 hash (invalid rounds)" + raw_salt = h64decode(salt) + return rounds, salt, raw_salt, digest + def encodePassword(plaintext, scheme, other=None): """Encrypt the plaintext password. """ @@ -123,20 +142,7 @@ plaintext = "" if scheme == "PBKDF2": if other: - #assume it has format "{rounds}${salt}${digest}" - if isinstance(other, unicode): - other = other.encode("ascii") - try: - rounds, salt, digest = other.split("$") - except ValueError: - raise PasswordValueError, "invalid PBKDF2 hash (wrong number of separators)" - if rounds.startswith("0"): - raise PasswordValueError, "invalid PBKDF2 hash (zero-padded rounds)" - try: - rounds = int(rounds) - except ValueError: - raise PasswordValueError, "invalid PBKDF2 hash (invalid rounds)" - raw_salt = h64decode(salt) + rounds, salt, raw_salt, digest = pbkdf2_unpack(other) else: raw_salt = getrandbytes(20) salt = h64encode(raw_salt) @@ -249,6 +255,17 @@ self.password = None self.plaintext = None + def needs_migration(self): + """ Password has insecure scheme or other insecure parameters + and needs migration to new password scheme + """ + if self.scheme != 'PBKDF2': + return True + rounds, salt, raw_salt, digest = pbkdf2_unpack(self.password) + if rounds < 1000: + return True + return False + def unpack(self, encrypted, scheme=None, strict=False): """Set the password info from the scheme:<encryted info> string (the inverse of __str__)
--- a/test/test_cgi.py Thu Apr 14 15:42:41 2011 +0000 +++ b/test/test_cgi.py Thu Apr 14 18:10:58 2011 +0000 @@ -425,6 +425,30 @@ ':confirm:password': ''}, 'user', nodeid), ({('user', nodeid): {}}, [])) + def testPasswordMigration(self): + chef = self.db.user.lookup('Chef') + form = dict(__login_name='Chef', __login_password='foo') + cl = self._make_client(form) + # assume that the "best" algorithm is the first one and doesn't + # need migration, all others should be migrated. + for scheme in password.Password.known_schemes[1:]: + pw1 = password.Password('foo', scheme=scheme) + 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(pw, 'foo') + self.assertEqual(pw.needs_migration(), False) + pw1 = pw + self.assertEqual(pw1.needs_migration(), False) + scheme = password.Password.known_schemes[0] + self.assertEqual(scheme, pw1.scheme) + actions.LoginAction(cl).handle() + pw = self.db.user.get(chef, 'password') + self.assertEqual(pw, 'foo') + self.assertEqual(pw, pw1) + # # Boolean #
