Blame SOURCES/0095-Ticket-49742-Fine-grained-password-policy-can-impact.patch

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