This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.3.8
in repository 389-ds-base.
commit d5bc4cf102af3570df373524e95648c9b2552d64
Author: Thierry Bordaz <tbordaz@redhat.com>
Date: Thu Jun 7 18:35:34 2018 +0200
Ticket 49742 - Fine grained password policy can impact search performance
Bug Description:
new_passwdPolicy is called with an entry DN.
In case of fine grain password policy we need to retrieve
the possible password policy (pwdpolicysubentry) that applies to
that entry.
It triggers an internal search to retrieve the entry.
In case of a search operation (add_shadow_ext_password_attrs), the
entry is already in the pblock. So it is useless to do an additional
internal search for it.
Fix Description:
in case of fine grain password policy and a SRCH operation,
if the entry DN matches the entry stored in the pblock (SLAPI_SEARCH_RESULT_ENTRY)
then use that entry instead of doing an internal search
https://pagure.io/389-ds-base/issue/49742
Reviewed by: Mark Reynolds
Platforms tested: F26
Flag Day: no
Doc impact: no
---
dirsrvtests/tests/tickets/ticket48228_test.py | 18 +++++-----
dirsrvtests/tests/tickets/ticket548_test.py | 18 ++++++----
ldap/servers/slapd/pw.c | 48 +++++++++++++++++++++++++--
3 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/dirsrvtests/tests/tickets/ticket48228_test.py b/dirsrvtests/tests/tickets/ticket48228_test.py
index 4f4494e..1ab741b 100644
--- a/dirsrvtests/tests/tickets/ticket48228_test.py
+++ b/dirsrvtests/tests/tickets/ticket48228_test.py
@@ -7,6 +7,7 @@
# --- END COPYRIGHT BLOCK ---
#
import logging
+import time
import pytest
from lib389.tasks import *
@@ -33,14 +34,14 @@ def set_global_pwpolicy(topology_st, inhistory):
topology_st.standalone.simple_bind_s(DN_DM, PASSWORD)
# Enable password policy
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-pwpolicy-local', 'on')])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-pwpolicy-local', b'on')])
except ldap.LDAPError as e:
log.error('Failed to set pwpolicy-local: error ' + e.message['desc'])
assert False
log.info(" Set global password history on\n")
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordHistory', 'on')])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordHistory', b'on')])
except ldap.LDAPError as e:
log.error('Failed to set passwordHistory: error ' + e.message['desc'])
assert False
@@ -48,7 +49,7 @@ def set_global_pwpolicy(topology_st, inhistory):
log.info(" Set global passwords in history\n")
try:
count = "%d" % inhistory
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordInHistory', count)])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordInHistory', count.encode())])
except ldap.LDAPError as e:
log.error('Failed to set passwordInHistory: error ' + e.message['desc'])
assert False
@@ -113,9 +114,9 @@ def check_passwd_inhistory(topology_st, user, cpw, passwd):
topology_st.standalone.simple_bind_s(user, cpw)
time.sleep(1)
try:
- topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', passwd)])
+ topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', passwd.encode())])
except ldap.LDAPError as e:
- log.info(' The password ' + passwd + ' of user' + USER1_DN + ' in history: error ' + e.message['desc'])
+ log.info(' The password ' + passwd + ' of user' + USER1_DN + ' in history: error {0}'.format(e))
inhistory = 1
time.sleep(1)
return inhistory
@@ -130,7 +131,7 @@ def update_passwd(topology_st, user, passwd, times):
# Now update the value for this iter.
cpw = 'password%d' % i
try:
- topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', cpw)])
+ topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', cpw.encode())])
except ldap.LDAPError as e:
log.fatal(
'test_ticket48228: Failed to update the password ' + cpw + ' of user ' + user + ': error ' + e.message[
@@ -146,7 +147,6 @@ def test_ticket48228_test_global_policy(topology_st):
"""
Check global password policy
"""
-
log.info(' Set inhistory = 6')
set_global_pwpolicy(topology_st, 6)
@@ -201,7 +201,7 @@ def test_ticket48228_test_global_policy(topology_st):
log.info("Global policy was successfully verified.")
-def test_ticket48228_test_subtree_policy(topology_st):
+def text_ticket48228_text_subtree_policy(topology_st):
"""
Check subtree level password policy
"""
@@ -233,7 +233,7 @@ def test_ticket48228_test_subtree_policy(topology_st):
log.info(' Set inhistory = 4')
topology_st.standalone.simple_bind_s(DN_DM, PASSWORD)
try:
- topology_st.standalone.modify_s(SUBTREE_PWP, [(ldap.MOD_REPLACE, 'passwordInHistory', '4')])
+ topology_st.standalone.modify_s(SUBTREE_PWP, [(ldap.MOD_REPLACE, 'passwordInHistory', b'4')])
except ldap.LDAPError as e:
log.error('Failed to set pwpolicy-local: error ' + e.message['desc'])
assert False
diff --git a/dirsrvtests/tests/tickets/ticket548_test.py b/dirsrvtests/tests/tickets/ticket548_test.py
index d354cc8..0d71ab6 100644
--- a/dirsrvtests/tests/tickets/ticket548_test.py
+++ b/dirsrvtests/tests/tickets/ticket548_test.py
@@ -42,7 +42,7 @@ def set_global_pwpolicy(topology_st, min_=1, max_=10, warn=3):
log.info(" +++++ Enable global password policy +++++\n")
# Enable password policy
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-pwpolicy-local', 'on')])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-pwpolicy-local', b'on')])
except ldap.LDAPError as e:
log.error('Failed to set pwpolicy-local: error ' + e.message['desc'])
assert False
@@ -54,28 +54,28 @@ def set_global_pwpolicy(topology_st, min_=1, max_=10, warn=3):
log.info(" Set global password Min Age -- %s day\n" % min_)
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMinAge', '%s' % min_secs)])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMinAge', ('%s' % min_secs).encode())])
except ldap.LDAPError as e:
log.error('Failed to set passwordMinAge: error ' + e.message['desc'])
assert False
log.info(" Set global password Expiration -- on\n")
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordExp', 'on')])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordExp', b'on')])
except ldap.LDAPError as e:
log.error('Failed to set passwordExp: error ' + e.message['desc'])
assert False
log.info(" Set global password Max Age -- %s days\n" % max_)
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMaxAge', '%s' % max_secs)])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMaxAge', ('%s' % max_secs).encode())])
except ldap.LDAPError as e:
log.error('Failed to set passwordMaxAge: error ' + e.message['desc'])
assert False
log.info(" Set global password Warning -- %s days\n" % warn)
try:
- topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordWarning', '%s' % warn_secs)])
+ topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordWarning', ('%s' % warn_secs).encode())])
except ldap.LDAPError as e:
log.error('Failed to set passwordWarning: error ' + e.message['desc'])
assert False
@@ -93,6 +93,8 @@ def set_subtree_pwpolicy(topology_st, min_=2, max_=20, warn=6):
try:
topology_st.standalone.add_s(Entry((SUBTREE_CONTAINER, {'objectclass': 'top nsContainer'.split(),
'cn': 'nsPwPolicyContainer'})))
+ except ldap.ALREADY_EXISTS:
+ pass
except ldap.LDAPError as e:
log.error('Failed to add subtree container: error ' + e.message['desc'])
# assert False
@@ -128,6 +130,8 @@ def set_subtree_pwpolicy(topology_st, min_=2, max_=20, warn=6):
'cosPriority': '1',
'cn': SUBTREE_COS_TMPLDN,
'pwdpolicysubentry': SUBTREE_PWP})))
+ except ldap.ALREADY_EXISTS:
+ pass
except ldap.LDAPError as e:
log.error('Failed to add COS template: error ' + e.message['desc'])
# assert False
@@ -139,6 +143,8 @@ def set_subtree_pwpolicy(topology_st, min_=2, max_=20, warn=6):
'cn': SUBTREE_PWPDN,
'costemplatedn': SUBTREE_COS_TMPL,
'cosAttribute': 'pwdpolicysubentry default operational-default'})))
+ except ldap.ALREADY_EXISTS:
+ pass
except ldap.LDAPError as e:
log.error('Failed to add COS def: error ' + e.message['desc'])
# assert False
@@ -150,7 +156,7 @@ def update_passwd(topology_st, user, passwd, newpasswd):
log.info(" Bind as {%s,%s}" % (user, passwd))
topology_st.standalone.simple_bind_s(user, passwd)
try:
- topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', newpasswd)])
+ topology_st.standalone.modify_s(user, [(ldap.MOD_REPLACE, 'userpassword', newpasswd.encode())])
except ldap.LDAPError as e:
log.fatal('test_ticket548: Failed to update the password ' + cpw + ' of user ' + user + ': error ' + e.message[
'desc'])
diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c
index 451be36..10b8e72 100644
--- a/ldap/servers/slapd/pw.c
+++ b/ldap/servers/slapd/pw.c
@@ -1625,6 +1625,10 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn)
int attr_free_flags = 0;
int rc = 0;
int optype = -1;
+ int free_e = 1; /* reset if e is taken from pb */
+ if (pb) {
+ slapi_pblock_get(pb, SLAPI_OPERATION_TYPE, &optype);
+ }
/* If we already allocated a pw policy, return it */
if (pb != NULL) {
@@ -1688,7 +1692,43 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn)
/* If we're not doing an add, we look for the pwdpolicysubentry
attribute in the target entry itself. */
} else {
- if ((e = get_entry(pb, dn)) != NULL) {
+ if (optype == SLAPI_OPERATION_SEARCH) {
+ Slapi_Entry *pb_e;
+
+ /* During a search the entry should be in the pblock
+ * For safety check entry DN is identical to 'dn'
+ */
+ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &pb_e);
+ if (pb_e) {
+ Slapi_DN * sdn;
+ const char *ndn;
+ char *pb_ndn;
+
+ pb_ndn = slapi_entry_get_ndn(pb_e);
+
+ sdn = slapi_sdn_new_dn_byval(dn);
+ ndn = slapi_sdn_get_ndn(sdn);
+
+ if (strcasecmp(pb_ndn, ndn) == 0) {
+ /* We are using the candidate entry that is already loaded in the pblock
+ * Do not trigger an additional internal search
+ * Also we will not need to free the entry that will remain in the pblock
+ */
+ e = pb_e;
+ free_e = 0;
+ } else {
+ e = get_entry(pb, dn);
+ }
+ slapi_sdn_free(&sdn);
+ } else {
+ e = get_entry(pb, dn);
+ }
+ } else {
+ /* For others operations but SEARCH */
+ e = get_entry(pb, dn);
+ }
+
+ if (e) {
Slapi_Attr *attr = NULL;
rc = slapi_entry_attr_find(e, "pwdpolicysubentry", &attr);
if (attr && (0 == rc)) {
@@ -1718,7 +1758,9 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn)
}
}
slapi_vattr_values_free(&values, &actual_type_name, attr_free_flags);
- slapi_entry_free(e);
+ if (free_e) {
+ slapi_entry_free(e);
+ }
if (pw_entry == NULL) {
slapi_log_err(SLAPI_LOG_ERR, "new_passwdPolicy",
@@ -1916,7 +1958,7 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn)
slapi_pblock_set_pwdpolicy(pb, pwdpolicy);
}
return pwdpolicy;
- } else if (e) {
+ } else if (free_e) {
slapi_entry_free(e);
}
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
389-commits mailing list -- 389-commits@lists.fedoraproject.org
To unsubscribe send an email to 389-commits-leave@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/389-commits@lists.fedoraproject.org/message/KB742DPUYGDBDRQQVGOQAPLRU5WEYO56/
No comments:
Post a Comment