Roundup Tracker - Issues

Issue 2551253

classification
Modify password PBKDF2 method to use SHA512
Type: security Severity: normal
Components: Web interface Versions: 2.4.0
process
Status: open remind
:
: rouilj : rouilj
Priority: : Blocker, Effort-Medium

Created on 2022-12-23 05:01 by rouilj, last changed 2023-06-26 19:21 by rouilj.

Messages
msg7700 Author: [hidden] (rouilj) Date: 2022-12-23 05:01
We use SHA1 with PBKDF2 for hashing passwords. This requires 720,000 rounds/iterations.

Using sha512 the recommended number of rounds (setting: password_pbkdf2_default_rounds)
is 120,000 according to:

   https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2

Also OWASP suggests deprecating sha1 for sha512.

The roundup database stored password entry looks like:

  {PBKDF2}rounds$salt$password_digest_in_h_base64

e.g.

  {PBKDF2}120000$W1.gcLfmA6JKaeNhT3XPiPOZoiU$NR9QRy9VQWRUVix6cgl6cysrItA

In the rdbms databases (backends/rdbms_common.py and backends/back_*), the password
class is defined as varchar(255). I think this should allow enough space for a sha512
digest and the auxiliary fields as the digest size of sha1 is 20 and of sha512 is 64.
If we multiply the encoded digest length of 28 by 4 (since 4 * 20 > 64) I expect
the new encoded digest for sha512 to be less than 112. The rest of the fields consume
44 characters. So the total is 156 < 256. We can even add a few more bytes to
change the label {PBKDF2} to {PBKDF2-512} to mark the new format if needed.

It looks like the stored entry is using a form of the modular PHC string format
https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md.

Also there are some magic numbers in the code. keylen = 20 not sure why or if it should
be changed. This seems to just be the default value for the digest length of the
underlying hash function but I am not sure if they should be changed to use the
digest length or not.
msg7737 Author: [hidden] (rouilj) Date: 2023-02-28 21:01
The keylen should be increased to the digest length of 64 for SHA512.

Multiple refs but:

  https://security.stackexchange.com/questions/110084/parameters-for-pbkdf2-for-password-
hashing

The output looks like,

  
{PBKDF2S5}2000000$n/Ut1zX1XWdsi38jhMs6gnDUKFU$BPG9ubjHv9jlGEhWTwDe1wB7OWKRZdnb2lVogm9UgIjVCu4g
oQOmiugO.5rPU1Q..SEqMpbnd21R9fLWagpv2A

it has a length of 132 chars which can be stored in the db w/o issue.
Note that 2 millions rounds is excessive for PBDKF2-SHA512. As I write this,
the recommended is 210,000 but we have room to grow.

Committed changes in rev da751d3a2138.

Note that it is not the default. To make it the default, you need to add this code to
the tracker's interfaces.py:

from roundup.password import Password
# force password with scheme PBDFK2 to get re-hashed
Password.deprecated_schemes.insert(0, Password.known_schemes[0])
# choose PBDFK2S5 as the scheme to use for reshashing.
Password.default_scheme = Password.experimental_schemes[0]

After some discussion/feedback, I'll see if this SHA512 version should be the default,
if PBDKF2-SHA1 should be migrated etc.

Also need to change the note in CHANGES.txt that references this ticket when I close it
and possibly add to upgrading.txt.
msg7766 Author: [hidden] (rouilj) Date: 2023-05-20 19:36
changeset:   7375:9bd7ed918121

Documented using PBKDF2-SHA512 in 2.3.0 before it becomes default.
History
Date User Action Args
2023-06-26 19:21:04rouiljsetversions: + 2.4.0
2023-05-20 19:36:35rouiljsetmessages: + msg7766
2023-02-28 21:01:04rouiljsetkeywords: + Blocker
assignee: rouilj
messages: + msg7737
status: new -> open
resolution: remind
2022-12-23 05:01:18rouiljcreate