Skip to content

Commit

Permalink
Issue 6092 - passwordHistory is not updated with a pre-hashed password (
Browse files Browse the repository at this point in the history
#6093)

Bug description: passwordHistory is not updated by with a pre-hashed password

Fix description: During a mod replace of the userpassword attribute, if an encoded
password value is detected and both pw_history and allow_hashed_pw are enabled, get
the present entry values which are used to update the password history.

Relates: #6092

Reviewed by: @tbordaz  (Thank you)
  • Loading branch information
jchapma committed Apr 5, 2024
1 parent 93d456d commit 04254a0
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 2 deletions.
83 changes: 83 additions & 0 deletions dirsrvtests/tests/suites/password/pwp_history_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import pytest
import time
import logging
import ldap
from lib389.tasks import *
from lib389.utils import ds_is_newer
from lib389.topologies import topology_st
from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
from lib389.idm.directorymanager import DirectoryManager
from lib389.idm.organizationalunit import OrganizationalUnits
from lib389.passwd import password_hash
from lib389._constants import DEFAULT_SUFFIX

pytestmark = pytest.mark.tier1
Expand Down Expand Up @@ -325,6 +327,87 @@ def test_basic(topology_st, user):
# Done
log.info('Test suite PASSED.')

def test_prehashed_pwd(topology_st):
"""Test password history is updated with a pre-hashed password change
:id: 24d08663-f36a-44ab-8f02-b8a3f502925b
:setup: Standalone instance
:steps:
1. Configure password history policy as bellow:
passwordHistory: on
passwordChange: on
nsslapd-allow-hashed-passwords: on
2. Create ACI to allow users change their password
3. Add a test user
4. Attempt to change password using non hased value
5. Bind with non hashed value
6. Create a hash value for update
7. Update user password with hash value
8. Bind with hashed password cleartext
9. Check users passwordHistory
:expectedresults:
1. Password history policy should be configured successfully
2. ACI applied correctly
3. User successfully added
4. Password change accepted
5. Successful bind
6. Hash value created
7. Password change accepted
8. Successful bind
9. Users passwordHistory should contain 2 enteries
"""

# Configure password history policy and add a test user
try:
topology_st.standalone.config.replace_many(('passwordHistory', 'on'),
('passwordChange', 'on'),
('nsslapd-allow-hashed-passwords', 'on'))
log.info('Configured password policy.')
except ldap.LDAPError as e:
log.fatal('Failed to configure password policy: ' + str(e))
assert False
time.sleep(1)

# Add aci so users can change their own password
USER_ACI = '(targetattr="userpassword || passwordHistory")(version 3.0; acl "pwp test"; allow (all) userdn="ldap:///self";)'
ous = OrganizationalUnits(topology_st.standalone, DEFAULT_SUFFIX)
ou = ous.get('people')
ou.add('aci', USER_ACI)

# Create user
users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX)
user = users.create(properties=TEST_USER_PROPERTIES)
user.set('userpassword', 'password')
user.rebind('password')

# Change user pwd to generate a history of 1 entry
user.replace('userpassword', 'password1')
user.rebind('password1')

#Create pwd hash
pwd_hash = password_hash('password2', scheme='PBKDF2_SHA256', bin_dir=topology_st.standalone.ds_paths.bin_dir)
#log.info(pwd_hash)

# Update user pwd hash
user.replace('userpassword', pwd_hash)
time.sleep(2)

# Bind with hashed password
user.rebind('password2')

# Check password history
pwds = user.get_attr_vals('passwordHistory')
if len(pwds) != 2:
log.fatal('Incorrect number of passwords stored in history: %d' %
len(pwds))
log.error('password history: ' + str(pwds))
assert False
else:
log.info('Correct number of passwords found in history.')

# Done
log.info('Test suite PASSED.')

if __name__ == '__main__':
# Run isolated
Expand Down
22 changes: 20 additions & 2 deletions ldap/servers/slapd/pw.c
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@ 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)
{
Slapi_Attr *attr;
Slapi_Value **va = NULL;
int i, pwresponse_req = 0;
int is_replication = 0;
int internal_op = 0;
Expand Down Expand Up @@ -1124,7 +1125,24 @@ check_pw_syntax_ext(Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, c
report_pw_violation(pb, dn, pwresponse_req, "invalid password syntax - passwords with storage scheme are not allowed");
return (1);
} else {
/* We want to skip syntax checking since this is a pre-hashed password */
/* We want to skip syntax checking since this is a pre-hashed password. But if the user
* has thrown caution to wind and allowed hashed passwords, we capture the history
*/
if (config_get_allow_hashed_pw() && pwpolicy->pw_history) {
e = get_entry(pb, dn);
if (e == NULL) {
return -1;
}
attr = attrlist_find(e->e_attrs, SLAPI_USERPWD_ATTR);
if (attr && !valueset_isempty(&attr->a_present_values)) {
if (old_pw && (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);
}
return (0);
}
}
Expand Down Expand Up @@ -1335,7 +1353,6 @@ check_pw_syntax_ext(Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, c

/* check for password history */
if (pwpolicy->pw_history == 1) {
Slapi_Value **va = NULL;
attr = attrlist_find(e->e_attrs, "passwordHistory");
if (pwpolicy->pw_inhistory && attr && !valueset_isempty(&attr->a_present_values)) {
/* Resetting password history array if necessary. */
Expand Down Expand Up @@ -1908,6 +1925,7 @@ pw_is_pwp_admin(Slapi_PBlock *pb, passwdPolicy *pwp, int rootdn_flag)
/* first check if it's root */
if (is_requestor_root && rootdn_flag == PWP_ADMIN_OR_ROOTDN) {
return 1;

}
/* now check if it's a Password Policy Administrator */
slapi_pblock_get(pb, SLAPI_REQUESTOR_SDN, &bind_sdn);
Expand Down

0 comments on commit 04254a0

Please sign in to comment.