pbkdf2 hardcoded parameters not secure anymore

[furznir]

This is a security issue. It leads to weak password storage compared to current standards and computation times.
This commit in 2017 reduced the security parameters for pbkdf2 algorithm. In 2022, it's time to follow the code comment's recommendation and raise the default parameters.

These parameters are not considered secure anymore, OWASP recommends 310.000 iterations for the algorithm (in HMAC case).

The current implementation targets 2 ms computation time for one hash, so attackers won't be able to easily brute-force the password hash. The number of iteratons is dinamically set: If the benchmark functions for this target yields more time on the current machine, the number of iterations will be reduced. The hard-minimum is 2048 iterations.

I ran the benchmark myself on a laptop with 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz CPU. On this laptop 1500 iterations yield the target 2 ms computation time, so the code set the 2048 minimum, I saw this in the logs: "Based on CPU performance, chose 2048 rounds", and the base64 decoded user password hash in 389ds also contained the 2048 iteration parameter.

With some calculation, we can see that with a password of 12 characters (A-Z a-z 0-9 and 10 possible specials) with 2048 iterations can be broken on this laptop (8 threads) with 50% chance in 726 days, with 64 threads in 91 days.
Calculation time for 2048 iterations on my computer with 1 thread: 0,003 sec
Number of all possible input passwords: (26+26+10+10)^12
Trials needed for 50% chance of password break due to birthday paradox: 1,2×√((26+26+10+10)^12) = 167176883404,8
Needed time in seconds with 8 cores: 167176883404,8*0,003/8 = 62691331,2768 sec
In days: 62691331,2768/60/60/24 = 726 days
On a 64 core machine with the same CPU (more dedicated attacker): 726/8 = 91 days.
We can conclude, if the password expiry policy is more than 91 days, the algorithm cannot be considered mathematically secure (less than 50% chance of breakage) against semi-pro attackers.

I suggest raising the default 2048 iterations to at least 10.000, this would mean 303 days to break with 50% chance by a 64 thread attacker. The computation time target parameter should be 10 ms in this case instead of 2 ms.
I think these parameters would be reasonable new defaults, considering today's CPU computation strength while not slowing down the functionality too much (in human perception). To put it in perspective, if we followed the OWASP recommendation of 310.000 iterations, it would mean 0.5 sec for every password check on this CPU.
The calculations still not consider the usage of GPUs for hash breaking, which is a common attack technique - to match their performance with CPUs, these default parameters would need to be set so high, that normal password checks wouldn't be performed under around a minute in normal use cases, which is unacceptable.

[Firstyear]

Thanks for this @furznir. We are extremely aware of the details around how password hashing works.

When pbkdf was introduced it was identified that there were two issues.

First, is that our cryptography provider (nss) uses an un-optimised pkbdf2 routine which takes about 2x to 3x longer, so 2048 rounds, takes the time of ~4500 to compute. i.e. if we set this to 10,000 rounds, it would take 2x to 3x longer than it should to compute. The NSS project declined to resolve as they did not believe this was an issue.

The second issue is the freeipa project. When this was added they were "scared" of a performance regression, due to the already slow nature of the project and it's many performance barriers. In fact, even when adding this, there was resistance even at 2048 rounds. It was ... quite an experience.

However, since then we have added the pwdchan plugin, which allows importing and use of the openldap formatted versions of pbkdf2. These are cryptograhically the same, but in a slightly different storage format.

If you look, you'll notice:

https://github.com/389ds/389-ds-base/blob/master/src/plugins/pwdchan/src/lib.rs#L9

These are backed by openssl (still fips compliant) which has the faster routine implemented, written in rust, and generally just seem to be better.

Anyway, the OWASP guidelines for HMAC are not what we are following, but the (significantly better) rules from NIST sp800-63B. https://pages.nist.gov/800-63-3/sp800-63b.html . This still recomends 10,000 rounds, but we could expand this.

I think we could consider making this version of pbkdf2 the default implementation. It would mean enforced rust by default which I think has given some vendors grief though.

[furznir]

Thanks for the clarification @Firstyear.

I understand the performance concerns, but if an attacker steals a password hash, he/she's not restricted to use the suboptimal NSS implementation, they can choose a more optimized one (like hashlib) instead. This is a reason why the benchmark logic now is flawed, it adjusts iteration count (quite smartly anyways) based on the (unfortunately slow) algorithm performance, while the attacker may have better resources. I understand that on the other hand this tunes for the local machine's performance too, which is important of course.

I still think the minimum values should be adjusted/adjustable though, this may be important for vendors who can't allow the rust dependency too. A golden middle way may be to allow the users to tweak these minimum values themselves from parameter, as some (e.g. us) can't sacrifice security even if it takes away some milliseconds of performance. Now, the hardcoded values force us to do so. I know the current code comments say you don't want to trust the users' judgement on these parameters, but you could expose the option for the more experienced users, while setting a hard minimum in code, and via the parameter the values can of course be raised anywhere higher than that by the users who are willing to sacrifice performance for security.

I'll check the possibility to migrate our project to a newer version of 389ds with pwdchan plugin and rust, which can work though. But not all projects using 389ds are able to, e.g. strict SLES 15 users.

One more detail, in 1.3.6, the iterations parameter was 30.000 according to documentation, then it was reduced in code, while the documentation says it will be in fact increased - so the doc may be misleading too.

[Firstyear]

Thanks for the clarification @Firstyear.

I understand the performance concerns, but if an attacker steals a password hash, he/she's not restricted to use the suboptimal NSS implementation, they can choose a more optimized one (like hashlib) instead. This is a reason why the benchmark logic now is flawed, it adjusts iteration count (quite smartly anyways) based on the (unfortunately slow) algorithm performance, while the attacker may have better resources. I understand that on the other hand this tunes for the local machine's performance too, which is important of course.

• NSS refused to adopt the optimised algorithm despite me providing the same arguments
• FreeIPA is the project with the performance concerns, not us in 389-ds, but FreeIPA is a "major consumer" of the project and very ... interesting.

I still think the minimum values should be adjusted/adjustable though, this may be important for vendors who can't allow the rust dependency too. A golden middle way may be to allow the users to tweak these minimum values themselves from parameter, as some (e.g. us) can't sacrifice security even if it takes away some milliseconds of performance. Now, the hardcoded values force us to do so. I know the current code comments say you don't want to trust the users' judgement on these parameters, but you could expose the option for the more experienced users, while setting a hard minimum in code, and via the parameter the values can of course be raised anywhere higher than that by the users who are willing to sacrifice performance for security.

I'd rather invest in pwdchan than the existing PBKDF2 and it would be possible to extend this to support configurable minimum rounds.

I'll check the possibility to migrate our project to a newer version of 389ds with pwdchan plugin and rust, which can work though. But not all projects using 389ds are able to, e.g. strict SLES 15 users.

SLE15SP3 and SP4 have pwdchan available :) I maintain 389-ds for SUSE, so I know what they support.

One more detail, in 1.3.6, the iterations parameter was 30.000 according to documentation, then it was reduced in code, while the documentation says it will be in fact increased - so the doc may be misleading too.

[furznir]

SLE15SP3 and SP4 have pwdchan available :) I maintain 389-ds for SUSE, so I know what they support.

The SUSE site says: "There is no official package available for SUSE SLE-15-SP3"
I see the official release for openSUSE 15.3 is 1.4.4.19~git38.9951c1... isn't the pwdchan introduced around 2.0.2? I'm not sure our project can take non-official community packages.

[Firstyear]

@furznir The reason for that is we want people to use 15SP4 which has 2.x as well as a number of other improvements. Mostly that's also related to documentation and some other issues.

So if I were you, I'd try to use 15SP4 if possible.

[mreynolds389]

@Firstyear

I think we could consider making this version of pbkdf2 the default implementation. It would mean enforced rust by default which I think has given some vendors grief though.

I know I have seen some complaints about Rust with Debian, but is it that Rust is not available on debian or that the user builds DS without Rust? Not sure, but I would prefer to use the Rust storage scheme by default, but that means making Rust 100% required for everyone.

If we make that change it would be in the main branch. However, we still need to do something for older versions that are not requiring Rust (might have to add that config setting you've been wanting to avoid).

[mreynolds389]

Open bugzilla for downstream work:

https://bugzilla.redhat.com/show_bug.cgi?id=2114039

[Firstyear]

(might have to add that config setting you've been wanting to avoid).

I thought we'd agreed to make rust a requirement from 2.1?

[mreynolds389]

(might have to add that config setting you've been wanting to avoid).

I thought we'd agreed to make rust a requirement from 2.1?

It's still a #define option, and some people are not building with it. You yourself said it was giving grief to some people. Anyway I was thinking of 1.4.3 releases in regards to the config option, maybe it's not a concern after all. I'd love to remove all the #rust defines, just want to make sure we aren't breaking anyone.

[Firstyear]

well at the same time, we also can't "wait forever" because some distros are too slow to keep up. It's been years at this point, even rhel and suse have this worked out ...

[mreynolds389]

For 2.1 and older we are just using Rust defines to set the proper PBKDF2 plugin (which uses 10000 iterations)

6bdc5d0..7d381b9 389-ds-base-2.1 -> 389-ds-base-2.1
219f047..120a477 389-ds-base-2.0 -> 389-ds-base-2.0
1d8036a..9b34201 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

[droideck]

@mreynolds389 do we want to backport #5431 to 389-ds-base-2.0 and 389-ds-base-2.1?
I've checked and the backport fixes the test failures there.

The question is if we want to make Rust non-optional and update default password storage for 2.0 and 2.1 too?

[mreynolds389]

@mreynolds389 do we want to backport #5431 to 389-ds-base-2.0 and 389-ds-base-2.1? I've checked and the backport fixes the test failures there.

The question is if we want to make Rust non-optional and update default password storage for 2.0 and 2.1 too?

Rust must remain optional in all the older releases. We made an announcement on this, and we can not go back on what we promised.

But I think I updated the storage scheme if Rust is enabled.

[droideck]

Rust must remain optional in all the older releases. We made an announcement on this, and we can not go back on what we promised.

But I think I updated the storage scheme if Rust is enabled.

Okay, just checking. :)
I'll see what can be fixed in tests and if we need to cherry-pick just a Rust Cargo.lock to older branches (as now it fails for the Rust related code).

[Firstyear]

I think you just regenerate the cargo.lock with a "cargo update" instead of backporting the .lock.

[droideck]

Fix tests in 2.0 and 2.1 branches:
b576ddc..00c374c 389-ds-base-2.0 -> origin/389-ds-base-2.0
a929745..41c8e41 389-ds-base-2.1 -> 389-ds-base-2.1