From 7451322d58b2527eabb34fc494c6ecf9376f859c Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Mon, 16 Dec 2024 16:17:35 -0800 Subject: [PATCH] Fix comments and improve processing for each plugin Add tests to storage_test.py It has debug logs for now --- .../suites/openldap_2_389/migrate_hdb_test.py | 1 - .../openldap_2_389/migrate_memberof_test.py | 1 - .../openldap_2_389/migrate_monitor_test.py | 1 - .../suites/openldap_2_389/migrate_test.py | 1 - .../password/pbkdf2_upgrade_plugin_test.py | 8 +- .../tests/suites/pwp_storage/storage_test.py | 162 +++++++++++++- .../389-console/src/lib/server/settings.jsx | 1 + src/lib389/lib389/password_plugins.py | 58 ++++- src/plugins/pwdchan/src/lib.rs | 200 +++++++++++------- src/slapi_r_plugin/src/error.rs | 4 +- src/slapi_r_plugin/src/plugin.rs | 4 - 11 files changed, 335 insertions(+), 106 deletions(-) diff --git a/dirsrvtests/tests/suites/openldap_2_389/migrate_hdb_test.py b/dirsrvtests/tests/suites/openldap_2_389/migrate_hdb_test.py index 95597898e6..da67ed244c 100644 --- a/dirsrvtests/tests/suites/openldap_2_389/migrate_hdb_test.py +++ b/dirsrvtests/tests/suites/openldap_2_389/migrate_hdb_test.py @@ -9,7 +9,6 @@ import pytest import os from lib389.topologies import topology_st -from lib389.password_plugins import PBKDF2Plugin from lib389.utils import ds_is_older from lib389.migrate.openldap.config import olConfig from lib389.migrate.openldap.config import olOverlayType diff --git a/dirsrvtests/tests/suites/openldap_2_389/migrate_memberof_test.py b/dirsrvtests/tests/suites/openldap_2_389/migrate_memberof_test.py index 4092bb36df..e5f749c01a 100644 --- a/dirsrvtests/tests/suites/openldap_2_389/migrate_memberof_test.py +++ b/dirsrvtests/tests/suites/openldap_2_389/migrate_memberof_test.py @@ -9,7 +9,6 @@ import pytest import os from lib389.topologies import topology_st -from lib389.password_plugins import PBKDF2Plugin from lib389.utils import ds_is_older from lib389.migrate.openldap.config import olConfig from lib389.migrate.openldap.config import olOverlayType diff --git a/dirsrvtests/tests/suites/openldap_2_389/migrate_monitor_test.py b/dirsrvtests/tests/suites/openldap_2_389/migrate_monitor_test.py index bf056f0e05..6935900a55 100644 --- a/dirsrvtests/tests/suites/openldap_2_389/migrate_monitor_test.py +++ b/dirsrvtests/tests/suites/openldap_2_389/migrate_monitor_test.py @@ -9,7 +9,6 @@ import pytest import os from lib389.topologies import topology_st -from lib389.password_plugins import PBKDF2Plugin from lib389.utils import ds_is_older from lib389.migrate.openldap.config import olConfig from lib389.migrate.openldap.config import olOverlayType diff --git a/dirsrvtests/tests/suites/openldap_2_389/migrate_test.py b/dirsrvtests/tests/suites/openldap_2_389/migrate_test.py index 492c94fc8f..fa41a9daf3 100644 --- a/dirsrvtests/tests/suites/openldap_2_389/migrate_test.py +++ b/dirsrvtests/tests/suites/openldap_2_389/migrate_test.py @@ -9,7 +9,6 @@ import pytest import os from lib389.topologies import topology_st -from lib389.password_plugins import PBKDF2Plugin from lib389.utils import ds_is_older from lib389.migrate.openldap.config import olConfig from lib389.migrate.openldap.config import olOverlayType diff --git a/dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py b/dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py index 90dae36ec7..65c9bed939 100644 --- a/dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py +++ b/dirsrvtests/tests/suites/password/pbkdf2_upgrade_plugin_test.py @@ -8,7 +8,7 @@ # import pytest from lib389.topologies import topology_st -from lib389.password_plugins import PBKDF2Plugin +from lib389.password_plugins import PBKDF2SHA512Plugin from lib389.utils import ds_is_older pytestmark = pytest.mark.tier1 @@ -35,18 +35,18 @@ def test_pbkdf2_upgrade(topology_st): """ # Remove the pbkdf2 plugin config - p1 = PBKDF2Plugin(topology_st.standalone) + p1 = PBKDF2SHA512Plugin(topology_st.standalone) assert(p1.exists()) p1._protected = False p1.delete() # Restart topology_st.standalone.restart() # check it's been readded. - p2 = PBKDF2Plugin(topology_st.standalone) + p2 = PBKDF2SHA512Plugin(topology_st.standalone) assert(p2.exists()) # Now restart to make sure we still work from the non-bootstrap form topology_st.standalone.restart() - p3 = PBKDF2Plugin(topology_st.standalone) + p3 = PBKDF2SHA512Plugin(topology_st.standalone) assert(p3.exists()) diff --git a/dirsrvtests/tests/suites/pwp_storage/storage_test.py b/dirsrvtests/tests/suites/pwp_storage/storage_test.py index ed0dd9e533..a725219202 100644 --- a/dirsrvtests/tests/suites/pwp_storage/storage_test.py +++ b/dirsrvtests/tests/suites/pwp_storage/storage_test.py @@ -18,14 +18,49 @@ from lib389.topologies import topology_st as topo from lib389.idm.user import UserAccounts, UserAccount -from lib389._constants import DEFAULT_SUFFIX +from lib389._constants import DEFAULT_SUFFIX, DN_DM, PASSWORD, ErrorLog from lib389.config import Config -from lib389.password_plugins import PBKDF2Plugin, SSHA512Plugin +from lib389.password_plugins import ( + SSHA512Plugin, + PBKDF2SHA1Plugin, + PBKDF2SHA256Plugin, + PBKDF2SHA512Plugin +) from lib389.utils import ds_is_older pytestmark = pytest.mark.tier1 +@pytest.fixture(scope="function") +def test_user(request, topo): + """Fixture to create and clean up a test user for each test""" + # Generate unique user ID based on test name + uid = f'test_user_{request.node.name[:20]}' + + # Create user + users = UserAccounts(topo.standalone, DEFAULT_SUFFIX) + user = users.create(properties={ + 'uid': uid, + 'cn': 'Test User', + 'sn': 'User', + 'uidNumber': '1000', + 'gidNumber': '2000', + 'homeDirectory': f'/home/{uid}' + }) + + def fin(): + try: + # Ensure we're bound as DM before cleanup + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + if user.exists(): + user.delete() + except Exception as e: + log.error(f"Error during user cleanup: {e}") + + request.addfinalizer(fin) + return user + + def user_config(topo, field_value): """ Will set storage schema and create user. @@ -106,27 +141,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): + """Check password scheme PBKDF2-SHA512. :id: 31612e7e-33a6-11ea-a750-8c16451d917b :setup: Standalone :steps: - 1. Try to delete PBKDF2_SHA256. - 2. Should not deleted PBKDF2_SHA256 and server should up. + 1. Try to delete PBKDF2-SHA512. + 2. Should not deleted PBKDF2-SHA512 and server should up. :expectedresults: 1. Pass 2. Pass """ - value = 'PBKDF2_SHA256' + value = 'PBKDF2-SHA512' user = user_config(topo, value) assert '{' + f'{value.lower()}' + '}' in \ UserAccount(topo.standalone, user.dn).get_attr_val_utf8('userpassword').lower() - plg = PBKDF2Plugin(topo.standalone) + plg = PBKDF2SHA512Plugin(topo.standalone) plg._protected = False plg.delete() topo.standalone.restart() - assert Config(topo.standalone).get_attr_val_utf8('passwordStorageScheme') == 'PBKDF2_SHA256' + assert Config(topo.standalone).get_attr_val_utf8('passwordStorageScheme') == 'PBKDF2-SHA512' assert topo.standalone.status() user.delete() @@ -160,6 +195,115 @@ def test_check_ssha512(topo): user.delete() +@pytest.mark.parametrize('plugin_class,plugin_name', [ + (PBKDF2SHA1Plugin, 'PBKDF2-SHA1'), + (PBKDF2SHA256Plugin, 'PBKDF2-SHA256'), + (PBKDF2SHA512Plugin, 'PBKDF2-SHA512') +]) +def test_pbkdf2_rounds_configuration(topo, test_user, plugin_class, plugin_name): + """Test PBKDF2 rounds configuration for different variants""" + try: + # Enable plugin logging + topo.standalone.config.loglevel((ErrorLog.DEFAULT, ErrorLog.PLUGIN)) + + # Configure plugin + plugin = plugin_class(topo.standalone) + plugin.enable() + + # Test rounds configuration + test_rounds = 20000 + plugin.set_rounds(test_rounds) + # Restart after changing rounds + topo.standalone.restart() + assert plugin.get_rounds() == test_rounds + + # Verify invalid rounds are rejected + with pytest.raises(ValueError): + plugin.set_rounds(5000) # Too low + with pytest.raises(ValueError): + plugin.set_rounds(2000000) # Too high + + # Configure as password storage scheme + topo.standalone.config.replace('passwordStorageScheme', plugin_name) + topo.standalone.deleteErrorLogs() + + TEST_PASSWORD = 'Secret123' + test_user.set('userPassword', TEST_PASSWORD) + + # Verify password hash format + pwd_hash = test_user.get_attr_val_utf8('userPassword') + assert pwd_hash.startswith('{' + plugin_name.upper() + '}') + + # Test authentication + topo.standalone.simple_bind_s(test_user.dn, TEST_PASSWORD) + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + + # Restart to flush logs + topo.standalone.restart() + + # Verify logs + assert topo.standalone.searchErrorsLog(f'PBKDF2 rounds for {plugin_name.split("-")[1]} is {test_rounds}') + + finally: + # Always rebind as Directory Manager + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + + +@pytest.mark.parametrize('plugin_class,plugin_name', [ + (PBKDF2SHA1Plugin, 'PBKDF2-SHA1'), + (PBKDF2SHA256Plugin, 'PBKDF2-SHA256'), + (PBKDF2SHA512Plugin, 'PBKDF2-SHA512') +]) +def test_pbkdf2_rounds_modification(topo, test_user, plugin_class, plugin_name): + """Test PBKDF2 rounds modification behavior""" + try: + # Enable plugin logging + topo.standalone.config.loglevel((ErrorLog.DEFAULT, ErrorLog.PLUGIN)) + + plugin = plugin_class(topo.standalone) + plugin.enable() + + # Set initial rounds and restart + initial_rounds = 15000 + plugin.set_rounds(initial_rounds) + topo.standalone.restart() + + # Configure as password storage scheme + topo.standalone.config.replace('passwordStorageScheme', plugin_name) + topo.standalone.deleteErrorLogs() + + INITIAL_PASSWORD = 'Initial123' + NEW_PASSWORD = 'New123' + + test_user.set('userPassword', INITIAL_PASSWORD) + + # Modify rounds and restart + new_rounds = 25000 + plugin.set_rounds(new_rounds) + topo.standalone.restart() + + # Verify old password still works + topo.standalone.simple_bind_s(test_user.dn, INITIAL_PASSWORD) + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + + # Set new password + test_user.set('userPassword', NEW_PASSWORD) + + # Verify new password works + topo.standalone.simple_bind_s(test_user.dn, NEW_PASSWORD) + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + + # Restart to flush logs + topo.standalone.restart() + + # Verify logs show new rounds value + assert topo.standalone.searchErrorsLog(f'PBKDF2 rounds for {plugin_name.split("-")[1]} is {new_rounds}') + + finally: + # Always rebind as Directory Manager + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + + if __name__ == "__main__": CURRENT_FILE = os.path.realpath(__file__) pytest.main("-s -v %s" % CURRENT_FILE) diff --git a/src/cockpit/389-console/src/lib/server/settings.jsx b/src/cockpit/389-console/src/lib/server/settings.jsx index f6432ecd41..5caa5ed818 100644 --- a/src/cockpit/389-console/src/lib/server/settings.jsx +++ b/src/cockpit/389-console/src/lib/server/settings.jsx @@ -160,6 +160,7 @@ export class ServerSettings extends React.Component { }; this.options = [ + { value: 'PBKDF2-SHA512', label: 'PBKDF2-SHA512', disabled: false }, { value: 'PBKDF2_SHA256', label: 'PBKDF2_SHA256', disabled: false }, { value: 'SSHA512', label: 'SSHA512', disabled: false }, { value: 'SSHA384', label: 'SSHA384', disabled: false }, diff --git a/src/lib389/lib389/password_plugins.py b/src/lib389/lib389/password_plugins.py index a912407046..ef84d683f9 100644 --- a/src/lib389/lib389/password_plugins.py +++ b/src/lib389/lib389/password_plugins.py @@ -31,10 +31,6 @@ def __init__(self, instance, dn=None): # We'll mark this protected, and people can just disable the plugins. self._protected = True -class PBKDF2Plugin(PasswordPlugin): - def __init__(self, instance, dn="cn=PBKDF2-SHA256,cn=Password Storage Schemes,cn=plugins,cn=config"): - super(PBKDF2Plugin, self).__init__(instance, dn) - class SSHA512Plugin(PasswordPlugin): def __init__(self, instance, dn=f'cn=SSHA512,{DN_PWDSTORAGE_SCHEMES}'): @@ -56,6 +52,60 @@ def __init__(self, instance, dn=f'cn=SSHA,{DN_PWDSTORAGE_SCHEMES}'): super(SSHAPlugin, self).__init__(instance, dn) +class PBKDF2BasePlugin(PasswordPlugin): + """Base class for all PBKDF2 variants""" + def __init__(self, instance, dn): + super(PBKDF2BasePlugin, self).__init__(instance, dn) + self._create_objectclasses.append('pwdPBKDF2PluginConfig') + + def set_rounds(self, rounds): + """Set the number of rounds for PBKDF2 hashing (requires restart) + + :param rounds: Number of rounds (10000-1000000) + :type rounds: int + """ + rounds = int(rounds) + if rounds < 10000 or rounds > 1000000: + raise ValueError("PBKDF2 rounds must be between 10000 and 1000000") + self.replace('nsslapd-pwdPBKDF2Rounds', str(rounds)) + + def get_rounds(self): + """Get the current number of rounds + + :returns: Current rounds setting or 10000 if not set + :rtype: int + """ + rounds = self.get_attr_val_utf8('nsslapd-pwdPBKDF2Rounds') + return int(rounds) if rounds else 10000 + + +class PBKDF2SHA1Plugin(PBKDF2BasePlugin): + """PBKDF2-SHA1 password storage scheme""" + def __init__(self, instance, dn=f'cn=PBKDF2-SHA1,{DN_PWDSTORAGE_SCHEMES}'): + super(PBKDF2SHA1Plugin, self).__init__(instance, dn) + self._plugin_properties.update({ + 'nsslapd-pluginInitfunc': 'pwdchan_pbkdf2_sha1_init' + }) + + +class PBKDF2SHA256Plugin(PBKDF2BasePlugin): + """PBKDF2-SHA256 password storage scheme""" + def __init__(self, instance, dn=f'cn=PBKDF2-SHA256,{DN_PWDSTORAGE_SCHEMES}'): + super(PBKDF2SHA256Plugin, self).__init__(instance, dn) + self._plugin_properties.update({ + 'nsslapd-pluginInitfunc': 'pwdchan_pbkdf2_sha256_init' + }) + + +class PBKDF2SHA512Plugin(PBKDF2BasePlugin): + """PBKDF2-SHA512 password storage scheme""" + def __init__(self, instance, dn=f'cn=PBKDF2-SHA512,{DN_PWDSTORAGE_SCHEMES}'): + super(PBKDF2SHA512Plugin, self).__init__(instance, dn) + self._plugin_properties.update({ + 'nsslapd-pluginInitfunc': 'pwdchan_pbkdf2_sha512_init' + }) + + class PasswordPlugins(Plugins): def __init__(self, instance): super(PasswordPlugins, self).__init__(instance=instance) diff --git a/src/plugins/pwdchan/src/lib.rs b/src/plugins/pwdchan/src/lib.rs index c254496ea3..6f014dde9a 100644 --- a/src/plugins/pwdchan/src/lib.rs +++ b/src/plugins/pwdchan/src/lib.rs @@ -5,8 +5,7 @@ use base64; use openssl::{hash::MessageDigest, pkcs5::pbkdf2_hmac, rand::rand_bytes}; use slapi_r_plugin::prelude::*; use std::fmt::Write; -use once_cell::sync::Lazy; -use std::sync::RwLock; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::convert::TryInto; use std::os::raw::c_char; @@ -15,7 +14,11 @@ const MIN_PBKDF2_ROUNDS: usize = 10_000; const MAX_PBKDF2_ROUNDS: usize = 1_000_000; const PBKDF2_ROUNDS_ATTR: &str = "nsslapd-pwdPBKDF2Rounds"; -static PBKDF2_ROUNDS: Lazy> = Lazy::new(|| RwLock::new(DEFAULT_PBKDF2_ROUNDS)); +// Each algorithm gets its own atomic counter for thread-safe round configuration +static PBKDF2_ROUNDS: AtomicUsize = AtomicUsize::new(DEFAULT_PBKDF2_ROUNDS); +static PBKDF2_ROUNDS_SHA1: AtomicUsize = AtomicUsize::new(DEFAULT_PBKDF2_ROUNDS); +static PBKDF2_ROUNDS_SHA256: AtomicUsize = AtomicUsize::new(DEFAULT_PBKDF2_ROUNDS); +static PBKDF2_ROUNDS_SHA512: AtomicUsize = AtomicUsize::new(DEFAULT_PBKDF2_ROUNDS); const PBKDF2_SALT_LEN: usize = 24; const PBKDF2_SHA1_EXTRACT: usize = 20; @@ -48,7 +51,7 @@ macro_rules! ab64_to_b64 { } // Create a module for each plugin type to avoid name conflicts -mod pbkdf2_default { +mod pbkdf2 { use super::*; pub struct PwdChanPbkdf2; @@ -110,7 +113,7 @@ macro_rules! impl_slapi_pbkdf2_plugin { fn start(pb: &mut PblockRef) -> Result<(), PluginError> { log_error!(ErrorLevel::Trace, "{} plugin start", Self::scheme_name()); - PwdChanCrypto::handle_pbkdf2_rounds_config(pb)?; + PwdChanCrypto::handle_pbkdf2_rounds_config(pb, Self::digest_type())?; Ok(()) } @@ -139,7 +142,7 @@ macro_rules! impl_slapi_pbkdf2_plugin { } // Apply the implementation to all plugin types -impl_slapi_pbkdf2_plugin!(pbkdf2_default::PwdChanPbkdf2); +impl_slapi_pbkdf2_plugin!(pbkdf2::PwdChanPbkdf2); impl_slapi_pbkdf2_plugin!(pbkdf2_sha1::PwdChanPbkdf2Sha1); impl_slapi_pbkdf2_plugin!(pbkdf2_sha256::PwdChanPbkdf2Sha256); impl_slapi_pbkdf2_plugin!(pbkdf2_sha512::PwdChanPbkdf2Sha512); @@ -214,6 +217,8 @@ impl PwdChanCrypto { } fn pbkdf2_encrypt(cleartext: &str, digest: MessageDigest) -> Result { + let rounds = Self::get_pbkdf2_rounds(digest)?; + let (hash_length, str_length, header) = if digest == MessageDigest::sha1() { (PBKDF2_SHA1_EXTRACT, 80, "{PBKDF2-SHA1}") } else if digest == MessageDigest::sha256() { @@ -233,8 +238,6 @@ impl PwdChanCrypto { let mut hash_input: Vec = (0..hash_length).map(|_| 0).collect(); - let rounds = Self::get_pbkdf2_rounds()?; - pbkdf2_hmac( cleartext.as_bytes(), &salt, @@ -265,7 +268,17 @@ impl PwdChanCrypto { Ok(output) } - pub fn handle_pbkdf2_rounds_config(pb: &mut PblockRef) -> Result<(), PluginError> { + // Private helper method to get the appropriate AtomicUsize reference + fn get_rounds_atomic(digest: MessageDigest) -> &'static AtomicUsize { + match digest { + d if d == MessageDigest::sha1() => &PBKDF2_ROUNDS_SHA1, + d if d == MessageDigest::sha256() => &PBKDF2_ROUNDS_SHA256, + d if d == MessageDigest::sha512() => &PBKDF2_ROUNDS_SHA512, + _ => &PBKDF2_ROUNDS, + } + } + + fn handle_pbkdf2_rounds_config(pb: &mut PblockRef, digest: MessageDigest) -> Result<(), PluginError> { let mut rounds = DEFAULT_PBKDF2_ROUNDS; let mut source = "default"; @@ -301,19 +314,29 @@ impl PwdChanCrypto { } } - // Use the existing set_pbkdf2_rounds function to validate and set the rounds - Self::set_pbkdf2_rounds(rounds)?; - + Self::set_pbkdf2_rounds(digest, rounds)?; + + let digest_name = if digest == MessageDigest::sha1() { + "SHA1" + } else if digest == MessageDigest::sha256() { + "SHA256" + } else if digest == MessageDigest::sha512() { + "SHA512" + } else { + "Unknown" + }; + log_error!( - ErrorLevel::Plugin, - "handle_pbkdf2_rounds_config -> PBKDF2 rounds set to {} from {}", + ErrorLevel::Error, + "handle_pbkdf2_rounds_config -> Rounds for PBKDF2-{} password scheme set to {} from {}", + digest_name, rounds, source ); Ok(()) } - pub fn set_pbkdf2_rounds(rounds: usize) -> Result<(), PluginError> { + fn set_pbkdf2_rounds(digest: MessageDigest, rounds: usize) -> Result<(), PluginError> { if rounds < MIN_PBKDF2_ROUNDS || rounds > MAX_PBKDF2_ROUNDS { log_error!( ErrorLevel::Error, @@ -325,34 +348,30 @@ impl PwdChanCrypto { return Err(PluginError::InvalidConfiguration); } - match PBKDF2_ROUNDS.write() { - Ok(mut rounds_guard) => { - *rounds_guard = rounds; - Ok(()) - } - Err(e) => { - log_error!( - ErrorLevel::Error, - "Failed to acquire write lock for PBKDF2 rounds: {}", - e - ); - Err(PluginError::LockError) - } - } + Self::get_rounds_atomic(digest).store(rounds, Ordering::Relaxed); + Ok(()) } - fn get_pbkdf2_rounds() -> Result { - match PBKDF2_ROUNDS.read() { - Ok(rounds_guard) => Ok(*rounds_guard), - Err(e) => { - log_error!( - ErrorLevel::Error, - "Failed to acquire read lock for PBKDF2 rounds: {}", - e - ); - Err(PluginError::LockError) - } - } + fn get_pbkdf2_rounds(digest: MessageDigest) -> Result { + let rounds = Self::get_rounds_atomic(digest).load(Ordering::Relaxed); + let digest_name = if digest == MessageDigest::sha1() { + "SHA1" + } else if digest == MessageDigest::sha256() { + "SHA256" + } else if digest == MessageDigest::sha512() { + "SHA512" + } else { + "Unknown" + }; + + log_error!( + ErrorLevel::Error, + "get_pbkdf2_rounds -> PBKDF2 rounds for {} is {}", + digest_name, + rounds + ); + Ok(rounds) + // Ok(Self::get_rounds_atomic(digest).load(Ordering::Relaxed)) } } @@ -367,17 +386,17 @@ mod tests { impl Pbkdf2Plugin for TestPbkdf2Sha1 { fn digest_type() -> MessageDigest { MessageDigest::sha1() } - fn scheme_suffix() -> &'static str { "-SHA1" } + fn scheme_name() -> &'static str { "PBKDF2-SHA1" } } impl Pbkdf2Plugin for TestPbkdf2Sha256 { fn digest_type() -> MessageDigest { MessageDigest::sha256() } - fn scheme_suffix() -> &'static str { "-SHA256" } + fn scheme_name() -> &'static str { "PBKDF2-SHA256" } } impl Pbkdf2Plugin for TestPbkdf2Sha512 { fn digest_type() -> MessageDigest { MessageDigest::sha512() } - fn scheme_suffix() -> &'static str { "-SHA512" } + fn scheme_name() -> &'static str { "PBKDF2-SHA512" } } /* @@ -390,62 +409,79 @@ mod tests { #[test] fn test_pbkdf2_rounds_configuration() { - // Test valid rounds configuration - assert!(PwdChanCrypto::set_pbkdf2_rounds(15000).is_ok()); - assert_eq!(PwdChanCrypto::get_pbkdf2_rounds().unwrap(), 15000); + // Test different rounds for each algorithm + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha1(), 15000).is_ok()); + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha256(), 20000).is_ok()); + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha512(), 25000).is_ok()); + + // Verify each algorithm has its own rounds setting + assert_eq!(PwdChanCrypto::get_pbkdf2_rounds(MessageDigest::sha1()).unwrap(), 15000); + assert_eq!(PwdChanCrypto::get_pbkdf2_rounds(MessageDigest::sha256()).unwrap(), 20000); + assert_eq!(PwdChanCrypto::get_pbkdf2_rounds(MessageDigest::sha512()).unwrap(), 25000); } #[test] fn test_pbkdf2_rounds_limits() { - // Test maximum rounds + // Test limits for SHA1 assert!(matches!( - PwdChanCrypto::set_pbkdf2_rounds(MAX_PBKDF2_ROUNDS + 1), + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha1(), MAX_PBKDF2_ROUNDS + 1), Err(PluginError::InvalidConfiguration) )); + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha1(), MIN_PBKDF2_ROUNDS).is_ok()); - // Test valid minimum - assert!(PwdChanCrypto::set_pbkdf2_rounds(MIN_PBKDF2_ROUNDS).is_ok()); - - // Test valid maximum - assert!(PwdChanCrypto::set_pbkdf2_rounds(MAX_PBKDF2_ROUNDS).is_ok()); - - // Test invalid rounds - too low + // Test invalid rounds for SHA256 - too low assert!(matches!( - PwdChanCrypto::set_pbkdf2_rounds(5000), + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha256(), 5000), Err(PluginError::InvalidConfiguration) )); + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha256(), MAX_PBKDF2_ROUNDS).is_ok()); - // Test invalid rounds - too high + // Test invalid rounds for SHA512 - too high assert!(matches!( - PwdChanCrypto::set_pbkdf2_rounds(2_000_000), + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha512(), 2_000_000), Err(PluginError::InvalidConfiguration) )); + assert!(PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha512(), MIN_PBKDF2_ROUNDS).is_ok()); } #[test] - fn test_pbkdf2_encrypt_with_rounds() { - // Set a specific number of rounds - PwdChanCrypto::set_pbkdf2_rounds(15000).unwrap(); + fn test_pbkdf2_encrypt_with_different_rounds() { + // Set different rounds for each algorithm + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha1(), 15000).unwrap(); + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha256(), 20000).unwrap(); + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha512(), 25000).unwrap(); - // Test each hash type let test_password = "test_password"; - // Test using generic functions through traits - for (plugin_type, header) in [ - (TestPbkdf2Sha1::digest_type(), "{PBKDF2-SHA1}"), - (TestPbkdf2Sha256::digest_type(), "{PBKDF2-SHA256}"), - (TestPbkdf2Sha512::digest_type(), "{PBKDF2-SHA512}") - ] { - let result = PwdChanCrypto::pbkdf2_encrypt(test_password, plugin_type).unwrap(); - assert!(result.contains("15000$")); - - let encrypted = result.replace(header, ""); - assert!(PwdChanCrypto::pbkdf2_compare( - test_password, - &encrypted, - plugin_type - ).unwrap()); - } + // Test SHA1 + let result = PwdChanCrypto::pbkdf2_encrypt(test_password, MessageDigest::sha1()).unwrap(); + assert!(result.contains("15000$")); + let encrypted = result.replace("{PBKDF2-SHA1}", ""); + assert!(PwdChanCrypto::pbkdf2_compare( + test_password, + &encrypted, + MessageDigest::sha1() + ).unwrap()); + + // Test SHA256 + let result = PwdChanCrypto::pbkdf2_encrypt(test_password, MessageDigest::sha256()).unwrap(); + assert!(result.contains("20000$")); + let encrypted = result.replace("{PBKDF2-SHA256}", ""); + assert!(PwdChanCrypto::pbkdf2_compare( + test_password, + &encrypted, + MessageDigest::sha256() + ).unwrap()); + + // Test SHA512 + let result = PwdChanCrypto::pbkdf2_encrypt(test_password, MessageDigest::sha512()).unwrap(); + assert!(result.contains("25000$")); + let encrypted = result.replace("{PBKDF2-SHA512}", ""); + assert!(PwdChanCrypto::pbkdf2_compare( + test_password, + &encrypted, + MessageDigest::sha512() + ).unwrap()); } #[test] @@ -463,6 +499,8 @@ mod tests { #[test] fn pwdchan_pbkdf2_sha1_basic() { + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha1(), 10000).unwrap(); + let encrypted = "10000$IlfapjA351LuDSwYC0IQ8Q$saHqQTuYnjJN/tmAndT.8mJt.6w"; assert!(PwdChanCrypto::pbkdf2_compare("password", encrypted, MessageDigest::sha1()) == Ok(true)); assert!(PwdChanCrypto::pbkdf2_compare("password!", encrypted, MessageDigest::sha1()) == Ok(false)); @@ -480,6 +518,8 @@ mod tests { #[test] fn pwdchan_pbkdf2_sha256_basic() { + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha256(), 10000).unwrap(); + let encrypted = "10000$henZGfPWw79Cs8ORDeVNrQ$1dTJy73v6n3bnTmTZFghxHXHLsAzKaAy8SksDfZBPIw"; assert!(PwdChanCrypto::pbkdf2_compare("password", encrypted, MessageDigest::sha256()) == Ok(true)); assert!(PwdChanCrypto::pbkdf2_compare("password!", encrypted, MessageDigest::sha256()) == Ok(false)); @@ -505,6 +545,8 @@ mod tests { #[test] fn pwdchan_pbkdf2_sha512_basic() { + PwdChanCrypto::set_pbkdf2_rounds(MessageDigest::sha512(), 10000).unwrap(); + let encrypted = "10000$Je1Uw19Bfv5lArzZ6V3EPw$g4T/1sqBUYWl9o93MVnyQ/8zKGSkPbKaXXsT8WmysXQJhWy8MRP2JFudSL.N9RklQYgDPxPjnfum/F2f/TrppA"; assert!(PwdChanCrypto::pbkdf2_compare("password", encrypted, MessageDigest::sha512()) == Ok(true)); assert!(PwdChanCrypto::pbkdf2_compare("password!", encrypted, MessageDigest::sha512()) == Ok(false)); diff --git a/src/slapi_r_plugin/src/error.rs b/src/slapi_r_plugin/src/error.rs index 0c7f2cd553..be8a1cb1d3 100644 --- a/src/slapi_r_plugin/src/error.rs +++ b/src/slapi_r_plugin/src/error.rs @@ -24,8 +24,8 @@ pub enum PluginError { InvalidBase64 = 1009, OpenSSL = 1010, Format = 1011, - LockError, - InvalidConfiguration, + LockError = 1012, + InvalidConfiguration = 1013, } #[derive(Debug)] diff --git a/src/slapi_r_plugin/src/plugin.rs b/src/slapi_r_plugin/src/plugin.rs index adbc309245..3790c40439 100644 --- a/src/slapi_r_plugin/src/plugin.rs +++ b/src/slapi_r_plugin/src/plugin.rs @@ -130,8 +130,4 @@ pub trait SlapiPlugin3 { fn pwd_storage_compare(_cleartext: &str, _encrypted: &str) -> Result { Err(PluginError::Unimplemented) } - - fn handle_pbkdf2_rounds_config(_pb: &mut PblockRef) -> Result<(), PluginError> { - Err(PluginError::Unimplemented) - } }