From 492a1d826e8a78207deeeebbcb45902a91d2de35 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: May 03 2016 19:27:19 +0000 Subject: Ticket 48813 - password history is not updated when an admin resets the password Bug Description: When an admin resets a password the current password is not stored in the password history. This incorrectly allows the user to reuse the previous password after the reset. Fix Description: When a password is being reset by an "admin", still grab the old password so we can correctly update the password history. https://fedorahosted.org/389/ticket/48813 Reviewed by: nhosoi(Thanks!) (cherry picked from commit 9c310b09c481a32b1a012371c688c65156b33472) --- diff --git a/dirsrvtests/tests/suites/password/pwp_history_test.py b/dirsrvtests/tests/suites/password/pwp_history_test.py new file mode 100644 index 0000000..3f66efd --- /dev/null +++ b/dirsrvtests/tests/suites/password/pwp_history_test.py @@ -0,0 +1,264 @@ +import os +import ldap +import logging +import pytest +from lib389 import DirSrv, Entry +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from lib389.utils import * + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + + +class TopologyStandalone(object): + """ Topology class """ + def __init__(self, standalone): + """ init """ + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + """ + Creating standalone instance ... + """ + standalone = DirSrv(verbose=False) + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + instance_standalone = standalone.exists() + if instance_standalone: + standalone.delete() + standalone.create() + standalone.open() + + # Delete each instance in the end + def fin(): + """ Clean up instance """ + standalone.delete() + request.addfinalizer(fin) + + # Clear out the tmp dir + standalone.clearTmpDir(__file__) + + return TopologyStandalone(standalone) + + +def test_pwp_history_test(topology): + """ + Test password policy history feature: + - Test password history is enforced + - Test password history works after an Admin resets the password + - Test that the correct number of passwords are stored in history + """ + + USER_DN = 'uid=testuser,' + DEFAULT_SUFFIX + + # + # Configure password history policy and add a test user + # + try: + topology.standalone.modify_s("cn=config", + [(ldap.MOD_REPLACE, + 'passwordHistory', 'on'), + (ldap.MOD_REPLACE, + 'passwordInHistory', '3'), + (ldap.MOD_REPLACE, + 'passwordChange', 'on'), + (ldap.MOD_REPLACE, + 'passwordStorageScheme', 'CLEAR')]) + log.info('Configured password policy.') + except ldap.LDAPError as e: + log.fatal('Failed to configure password policy: ' + str(e)) + assert False + + try: + topology.standalone.add_s(Entry((USER_DN, { + 'objectclass': ['top', 'extensibleObject'], + 'sn': 'user', + 'cn': 'test user', + 'uid': 'testuser', + 'userpassword': 'password'}))) + except ldap.LDAPError as e: + log.fatal('Failed to add test user' + USER_DN + ': error ' + str(e)) + assert False + + # + # Test that password history is enforced. + # + try: + topology.standalone.simple_bind_s(USER_DN, 'password') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user: ' + str(e)) + assert False + + # Attempt to change password to the same password + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password')]) + log.info('Incorrectly able to to set password to existing password.') + assert False + except ldap.CONSTRAINT_VIOLATION: + log.info('Password change correctly rejected') + except ldap.LDAPError as e: + log.fatal('Failed to attempt to change password: ' + str(e)) + assert False + + # + # Keep changing password until we fill the password history (3) + # + + # password1 + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password1')]) + except ldap.LDAPError as e: + log.fatal('Failed to change password: ' + str(e)) + assert False + try: + topology.standalone.simple_bind_s(USER_DN, 'password1') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user using "password1": ' + str(e)) + assert False + + # password2 + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password2')]) + except ldap.LDAPError as e: + log.fatal('Failed to change password: ' + str(e)) + assert False + try: + topology.standalone.simple_bind_s(USER_DN, 'password2') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user using "password2": ' + str(e)) + assert False + + # password3 + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password3')]) + except ldap.LDAPError as e: + log.fatal('Failed to change password: ' + str(e)) + assert False + try: + topology.standalone.simple_bind_s(USER_DN, 'password3') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user using "password3": ' + str(e)) + assert False + + # password4 + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password4')]) + except ldap.LDAPError as e: + log.fatal('Failed to change password: ' + str(e)) + assert False + try: + topology.standalone.simple_bind_s(USER_DN, 'password4') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user using "password4": ' + str(e)) + assert False + + # + # Check that we only have 3 passwords stored in history\ + # + try: + entry = topology.standalone.search_s(USER_DN, ldap.SCOPE_BASE, + 'objectclass=*', + ['passwordHistory']) + pwds = entry[0].getValues('passwordHistory') + if len(pwds) != 3: + log.fatal('Incorrect number of passwords stored in histry: %d' % + len(pwds)) + assert False + else: + log.info('Correct number of passwords found in history.') + except ldap.LDAPError as e: + log.fatal('Failed to get user entry: ' + str(e)) + assert False + + # + # Attempt to change the password to previous passwords + # + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password1')]) + log.info('Incorrectly able to to set password to previous password1.') + assert False + except ldap.CONSTRAINT_VIOLATION: + log.info('Password change correctly rejected') + except ldap.LDAPError as e: + log.fatal('Failed to attempt to change password: ' + str(e)) + assert False + + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password2')]) + log.info('Incorrectly able to to set password to previous password2.') + assert False + except ldap.CONSTRAINT_VIOLATION: + log.info('Password change correctly rejected') + except ldap.LDAPError as e: + log.fatal('Failed to attempt to change password: ' + str(e)) + assert False + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password3')]) + log.info('Incorrectly able to to set password to previous password3.') + assert False + except ldap.CONSTRAINT_VIOLATION: + log.info('Password change correctly rejected') + except ldap.LDAPError as e: + log.fatal('Failed to attempt to change password: ' + str(e)) + assert False + + # + # Reset password by Directory Manager(admin reset) + # + try: + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + except ldap.LDAPError as e: + log.fatal('Failed to bind as rootDN: ' + str(e)) + assert False + + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', + 'password-reset')]) + except ldap.LDAPError as e: + log.fatal('Failed to attempt to reset password: ' + str(e)) + assert False + + # Try and change the password to the previous password before the reset + try: + topology.standalone.simple_bind_s(USER_DN, 'password-reset') + except ldap.LDAPError as e: + log.fatal('Failed to bind as user: ' + str(e)) + assert False + + try: + topology.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, + 'userpassword', 'password4')]) + log.info('Incorrectly able to to set password to previous password4.') + assert False + except ldap.CONSTRAINT_VIOLATION: + log.info('Password change correctly rejected') + except ldap.LDAPError as e: + log.fatal('Failed to attempt to change password: ' + str(e)) + assert False + + log.info('Test suite PASSED.') + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE) diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 9225d31..c1d0cff 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -1277,6 +1277,10 @@ static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old * just return success. */ if(pw_is_pwp_admin(pb, pwpolicy)){ + if (!SLAPI_IS_MOD_DELETE(mod->mod_op) && pwpolicy->pw_history){ + /* Updating pw history, get the old password */ + get_old_pw(pb, &sdn, old_pw); + } rc = 1; goto done; } diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index f745852..b74c3c5 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -915,6 +915,7 @@ int check_pw_syntax( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, char **old_pw, Slapi_Entry *e, int mod_op ); int check_pw_syntax_ext( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, char **old_pw, Slapi_Entry *e, int mod_op, Slapi_Mods *smods ); +void get_old_pw( Slapi_PBlock *pb, const Slapi_DN *sdn, char **old_pw); int check_account_lock( Slapi_PBlock *pb, Slapi_Entry * bind_target_entry, int pwresponse_req, int account_inactivation_only /*no wire/no pw policy*/); int check_pw_minage( Slapi_PBlock *pb, const Slapi_DN *sdn, struct berval **vals) ; void add_password_attrs( Slapi_PBlock *pb, Operation *op, Slapi_Entry *e ); diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 4e222d7..09a0a6b 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -1080,6 +1080,35 @@ retry: } /* + * Get the old password -used by password admin so we properly + * update pw history when reseting a password. + */ +void +get_old_pw( Slapi_PBlock *pb, const Slapi_DN *sdn, char **old_pw ) +{ + Slapi_Entry *e = NULL; + Slapi_Value **va = NULL; + Slapi_Attr *attr = NULL; + char *dn = (char*)slapi_sdn_get_ndn(sdn); + + e = get_entry ( pb, dn ); + if ( e == NULL ) { + return; + } + + /* get current password, and remember it */ + attr = attrlist_find(e->e_attrs, "userpassword"); + if ( attr && !valueset_isempty(&attr->a_present_values) ) { + va = valueset_get_valuearray(&attr->a_present_values); + *old_pw = slapi_ch_strdup(slapi_value_get_string(va[0])); + } else { + *old_pw = NULL; + } + + slapi_entry_free(e); +} + +/* * Basically, h0 and h1 must be longer than GENERALIZED_TIME_LENGTH. */ static int