Issue 6269 - RFE - Add nsslapd-pwdPBKDF2Rounds configuration to PBKDF2-* plugins #6447
Conversation
droideck
added
the
work in progress
src/plugins/pwdchan/src/lib.rs
Outdated
| const MAX_PBKDF2_ROUNDS: usize = 1_000_000; | ||
|
|
||
| const PBKDF2_ROUNDS_ATTR: &str = "nsslapd-pwdPBKDF2Rounds"; | ||
| static PBKDF2_ROUNDS: Lazy<RwLock<usize>> = Lazy::new(|| RwLock::new(DEFAULT_PBKDF2_ROUNDS)); |
There was a problem hiding this comment.
Consider using https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html with Ordering::Relaxed
There was a problem hiding this comment.
I just realized we need to make the PBKDF2_ROUNDS a HashMap, as we might use different values in different schemes.
Do you know what the best option for our case is?
I found dashmap option Lazy<DashMap<MessageDigest, usize>> or we can make a RwLock HashMap like this RwLock<HashMap<MessageDigest, usize>>...
And I think there can be more...
There was a problem hiding this comment.
Okay, I tried a few approaches (even Rust's generics...), and I think the one in the last commit will work the best.
It's the simple AtomicUsize variables - one for each digest.
droideck
force-pushed
the
rust-pbkdf2-add-param
branch
3 times, most recently
from
d1025bd to
2f0db06
Compare
…2-* plugins Description: Add nsslapd-pwdPBKDF2Rounds attribute that can be configured in PBKDF2-* password storage plugin entries. This was password hashing round value can be adjusted. Certain compliance requirements (like from BSI) require specific hashing round values greater than what we currently provide. Add CLI, Web UI option, and CI tests. Fixes: 389ds#6269 Reviewed by: ?
droideck
force-pushed
the
rust-pbkdf2-add-param
branch
from
2f0db06 to
3603e54
Compare
|
Okay, it's ready for the final review! Design doc: 389ds/389ds.github.io#17 Please check! Thank you! |
droideck
removed
the
work in progress
dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py
Outdated
Show resolved
Hide resolved
| @@ -106,27 +389,27 @@ def test_check_two_scheme(topo): | |||
| user.delete() | |||
|
|
|||
| @pytest.mark.skipif(ds_is_older('1.4'), reason="Not implemented") | |||
| def test_check_pbkdf2_sha256(topo): | |||
| """Check password scheme PBKDF2_SHA256. | |||
| def test_check_pbkdf2_sha512(topo): | |||
There was a problem hiding this comment.
Should we really remove the test about the old scheme ?
IMHO it should be better to keep it and add a similar test for the new scheme
There was a problem hiding this comment.
Okay, first of all, the initial test case was invalid (it removed the PBKDF2-SHA256 plugin, but it checked for PBKDF2_SHA256).
I tried to make the test about both PBKDF2-SHA256 first and then about PBKDF2_SHA256 consistently. And if we set the password scheme to it and restart it, the plugin entry will not be bootstraped.
It seems intentional, as only the latest recommended password plugin is bootstrapped correctly: https://github.com/389ds/389-ds-base/blob/main/ldap/servers/slapd/config.c#L42-L50
So, I changed the test to PBKDF2-SHA512.
P.S. the test in dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py checks for PBKDF2-SHA256, though and it is restored if it's not set to password storage scheme in cn=config
|
Changes are made, please, review |
droideck
force-pushed
the
rust-pbkdf2-add-param
branch
from
6496b99 to
802fb91
Compare
Description: Add nsslapd-pwdPBKDF2Rounds attribute that can be configured in
PBKDF2-* password storage plugin entries. This was password hashing round value can be adjusted.
Certain compliance requirements (like from BSI) require specific hashing round values greater than
what we currently provide.
Fixes: #6269
Reviewed by: ?