From 34f8a4d3538255a54268c8483ebbf6e0177868b6 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Mon, 22 Oct 2012 16:17:42 -0400 Subject: [PATCH] Ticket 147 - Internal Password Policy usage very inefficient Bug Description: When updating a userpassword, the passwordPolicy struct is allocated & freed 5 to 7 times. Fix Description: Store the passwordPolicy struct in the pblock, and when we try and create a new policy struct, return the one in the pblock. https://fedorahosted.org/389/ticket/147 Reviewed by: ? --- ldap/servers/slapd/log.c | 2 +- ldap/servers/slapd/modify.c | 1 - ldap/servers/slapd/passwd_extop.c | 1 - ldap/servers/slapd/pblock.c | 7 ++-- ldap/servers/slapd/pw.c | 47 ++++++++------------------- ldap/servers/slapd/pw_mgmt.c | 6 --- ldap/servers/slapd/pw_retry.c | 3 -- ldap/servers/slapd/result.c | 1 - ldap/servers/slapd/slap.h | 63 +++++++++++++++++++------------------ 9 files changed, 51 insertions(+), 80 deletions(-) diff --git a/ldap/servers/slapd/log.c b/ldap/servers/slapd/log.c index 2ffd1f6..b6b988e 100644 --- a/ldap/servers/slapd/log.c +++ b/ldap/servers/slapd/log.c @@ -2554,7 +2554,7 @@ log__delete_rotated_logs() log_convert_time (logp->l_ctime, tbuf, 1); PR_snprintf (buffer, sizeof(buffer), "%s.%s", loginfo.log_access_file, tbuf); - LDAPDebug(LDAP_DEBUG_ANY,"Deleted Rotated Log: %s\n",buffer,0,0); /* MARK */ + LDAPDebug(LDAP_DEBUG_ANY,"Deleted Rotated Log: %s\n",buffer,0,0); if (PR_Delete(buffer) != PR_SUCCESS) { logp = logp->l_next; diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index da742da..424badb 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -1256,7 +1256,6 @@ static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old done: slapi_entry_free( e ); slapi_sdn_done (&sdn); - delete_passwdPolicy(&pwpolicy); slapi_ch_free_string(&proxydn); slapi_ch_free_string(&proxystr); return rc; diff --git a/ldap/servers/slapd/passwd_extop.c b/ldap/servers/slapd/passwd_extop.c index 3c050d6..b103a14 100644 --- a/ldap/servers/slapd/passwd_extop.c +++ b/ldap/servers/slapd/passwd_extop.c @@ -869,7 +869,6 @@ free_and_return: slapi_pblock_set(pb, SLAPI_TARGET_SDN, NULL); slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, NULL ); slapi_ch_free_string(&authmethod); - delete_passwdPolicy(&pwpolicy); slapi_entry_free(referrals); if ( targetEntry != NULL ){ diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c index baee7a7..7a0d780 100644 --- a/ldap/servers/slapd/pblock.c +++ b/ldap/servers/slapd/pblock.c @@ -111,10 +111,11 @@ pblock_done( Slapi_PBlock *pb ) { if(pb->pb_op!=NULL) { - operation_free(&pb->pb_op,pb->pb_conn); + operation_free(&pb->pb_op,pb->pb_conn); } - slapi_ch_free((void**)&(pb->pb_vattr_context)); - slapi_ch_free((void**)&(pb->pb_result_text)); + delete_passwdPolicy(&pb->pwdpolicy); + slapi_ch_free((void**)&(pb->pb_vattr_context)); + slapi_ch_free((void**)&(pb->pb_result_text)); } void diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 93fc899..bc6a30f 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -197,7 +197,6 @@ char* slapi_encode_ext (Slapi_PBlock *pb, const Slapi_DN *sdn, char *value, char { pwpolicy = new_passwdPolicy(pb, (char*)slapi_sdn_get_ndn(sdn) ); pws_enc = pwpolicy->pw_storagescheme->pws_enc; - delete_passwdPolicy(&pwpolicy); if (pws_enc == NULL) { @@ -354,8 +353,6 @@ pw_encodevals_ext( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals ) if (pwpolicy->pw_storagescheme) { pws_enc = pwpolicy->pw_storagescheme->pws_enc; } - - delete_passwdPolicy(&pwpolicy); } /* Password scheme encryption function was not found */ @@ -675,7 +672,6 @@ update_pw_info ( Slapi_PBlock *pb , char *old_pw) { slapi_ch_free((void**)&prev_exp_date_str); pw_apply_mods(sdn, &smods); slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return 0; } @@ -692,12 +688,9 @@ update_pw_info ( Slapi_PBlock *pb , char *old_pw) { } else { pw_apply_mods(sdn, &smods); slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return 0; } - delete_passwdPolicy(&pwpolicy); - timestr = format_genTime ( pw_exp_date ); slapi_mods_add_string(&smods, LDAP_MOD_REPLACE, "passwordExpirationTime", timestr); slapi_ch_free((void **)×tr); @@ -732,7 +725,6 @@ check_pw_minage ( Slapi_PBlock *pb, const Slapi_DN *sdn, struct berval **vals) /* retrieve the entry */ e = get_entry ( pb, dn ); if ( e == NULL ) { - delete_passwdPolicy(&pwpolicy); return ( -1 ); } /* get passwordAllowChangeTime attribute */ @@ -760,14 +752,12 @@ check_pw_minage ( Slapi_PBlock *pb, const Slapi_DN *sdn, struct berval **vals) "within password minimum age", 0, NULL ); slapi_entry_free( e ); slapi_ch_free((void **) &cur_time_str ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } slapi_ch_free((void **) &cur_time_str ); } slapi_entry_free( e ); } - delete_passwdPolicy(&pwpolicy); return ( 0 ); } @@ -844,12 +834,10 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_PWPOLICY_INVALIDPWDSYNTAX ); } pw_send_ldap_result ( pb, LDAP_CONSTRAINT_VIOLATION, NULL, errormsg, 0, NULL ); - delete_passwdPolicy(&pwpolicy); return( 1 ); } else { /* We want to skip syntax checking since this is a pre-hashed * password from replication or the root DN. */ - delete_passwdPolicy(&pwpolicy); return( 0 ); } } @@ -866,7 +854,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_PWPOLICY_PWDTOOSHORT ); } pw_send_ldap_result ( pb, LDAP_CONSTRAINT_VIOLATION, NULL, errormsg, 0, NULL ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } @@ -981,7 +968,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_PWPOLICY_INVALIDPWDSYNTAX ); } pw_send_ldap_result ( pb, LDAP_CONSTRAINT_VIOLATION, NULL, errormsg, 0, NULL ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } } @@ -992,7 +978,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, /* retrieve the entry */ e = get_entry ( pb, dn ); if ( e == NULL ) { - delete_passwdPolicy(&pwpolicy); return ( -1 ); } @@ -1012,7 +997,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_CONSTRAINT_VIOLATION, NULL, "password in history", 0, NULL ); slapi_entry_free( e ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } } @@ -1030,7 +1014,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_CONSTRAINT_VIOLATION ,NULL, "password in history", 0, NULL); slapi_entry_free( e ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } } else @@ -1041,7 +1024,6 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, LDAP_CONSTRAINT_VIOLATION ,NULL, "password in history", 0, NULL); slapi_entry_free( e ); - delete_passwdPolicy(&pwpolicy); return ( 1 ); } } @@ -1070,13 +1052,10 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, slapi_entry_free( e ); } - delete_passwdPolicy(&pwpolicy); return 1; } } - delete_passwdPolicy(&pwpolicy); - if ( mod_op ) { /* free e only when called by modify operation */ slapi_entry_free( e ); @@ -1107,7 +1086,6 @@ update_pw_history( Slapi_PBlock *pb, const Slapi_DN *sdn, char *old_pw ) /* retrieve the entry */ e = get_entry ( pb, dn ); if ( e == NULL ) { - delete_passwdPolicy(&pwpolicy); return ( 1 ); } @@ -1173,7 +1151,6 @@ update_pw_history( Slapi_PBlock *pb, const Slapi_DN *sdn, char *old_pw ) slapi_ch_free((void **) &str ); slapi_ch_free((void **) &history_str ); slapi_entry_free( e ); - delete_passwdPolicy(&pwpolicy); return 0; } @@ -1412,8 +1389,6 @@ add_password_attrs( Slapi_PBlock *pb, Operation *op, Slapi_Entry *e ) slapi_entry_attr_merge( e, "passwordallowchangetime", bvals ); slapi_ch_free((void **) &bv.bv_val ); } - - delete_passwdPolicy(&pwpolicy); } static int @@ -1548,6 +1523,11 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn) slapdFrontendConfig_t *slapdFrontendConfig; int optype = -1; + /* If we already allocated a pw policy, return it */ + if(pb && pb->pwdpolicy){ + return pb->pwdpolicy; + } + slapdFrontendConfig = getFrontendConfig(); pwdpolicy = (passwdPolicy *)slapi_ch_calloc(1, sizeof(passwdPolicy)); @@ -1832,6 +1812,9 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn) if (pw_entry) { slapi_entry_free(pw_entry); } + if(pb){ + pb->pwdpolicy = pwdpolicy; + } return pwdpolicy; } else if ( e ) { slapi_entry_free( e ); @@ -1839,15 +1822,18 @@ new_passwdPolicy(Slapi_PBlock *pb, const char *dn) } done: - /* If we are here, that means we need to load the passwdPolicy + /* + * If we are here, that means we need to load the passwdPolicy * structure from slapdFrontendconfig */ - *pwdpolicy = slapdFrontendConfig->pw_policy; pwdscheme = (struct pw_scheme *)slapi_ch_calloc(1, sizeof(struct pw_scheme)); *pwdscheme = *slapdFrontendConfig->pw_storagescheme; pwdscheme->pws_name = strdup( slapdFrontendConfig->pw_storagescheme->pws_name ); pwdpolicy->pw_storagescheme = pwdscheme; + if(pb){ + pb->pwdpolicy = pwdpolicy; + } return pwdpolicy; @@ -2188,14 +2174,9 @@ slapi_check_account_lock ( Slapi_PBlock *pb, Slapi_Entry * bind_target_entry, in notlocked: /* account is not locked. */ - if(check_password_policy) - delete_passwdPolicy(&pwpolicy); - return ( 0 ); + return (0); locked: - if(check_password_policy) - delete_passwdPolicy(&pwpolicy); return (1); - } /* The idea here is that these functions could allow us to have password diff --git a/ldap/servers/slapd/pw_mgmt.c b/ldap/servers/slapd/pw_mgmt.c index f6f3cf3..7b2767e 100644 --- a/ldap/servers/slapd/pw_mgmt.c +++ b/ldap/servers/slapd/pw_mgmt.c @@ -107,7 +107,6 @@ need_new_pw( Slapi_PBlock *pb, long *t, Slapi_Entry *e, int pwresponse_req ) pw_apply_mods(sdn, &smods); } slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return ( 0 ); } @@ -152,7 +151,6 @@ skip: } pw_apply_mods(sdn, &smods); slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return ( 0 ); } @@ -191,7 +189,6 @@ skip: if (pb->pb_conn->c_needpw == 1) { slapi_add_pwd_control ( pb, LDAP_CONTROL_PWEXPIRED, 0); } - delete_passwdPolicy(&pwpolicy); return ( 0 ); } @@ -218,7 +215,6 @@ skip: /* Apply current modifications */ pw_apply_mods(sdn, &smods); slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return (-1); } slapi_ch_free((void **) &cur_time_str ); @@ -279,7 +275,6 @@ skip: if (pb->pb_conn->c_needpw == 1) { slapi_add_pwd_control ( pb, LDAP_CONTROL_PWEXPIRED, 0); } - delete_passwdPolicy(&pwpolicy); return (2); } @@ -289,7 +284,6 @@ skip: if (pb->pb_conn->c_needpw == 1) { slapi_add_pwd_control ( pb, LDAP_CONTROL_PWEXPIRED, 0); } - delete_passwdPolicy(&pwpolicy); /* passes checking, return 0 */ return( 0 ); } diff --git a/ldap/servers/slapd/pw_retry.c b/ldap/servers/slapd/pw_retry.c index 74e575e..47e5829 100644 --- a/ldap/servers/slapd/pw_retry.c +++ b/ldap/servers/slapd/pw_retry.c @@ -136,7 +136,6 @@ int set_retry_cnt_and_time ( Slapi_PBlock *pb, int count, time_t cur_time ) { slapi_pblock_get( pb, SLAPI_TARGET_SDN, &sdn ); dn = slapi_sdn_get_dn(sdn); pwpolicy = new_passwdPolicy(pb, dn); - slapi_mods_init(&smods, 0); reset_time = time_plus_sec ( cur_time, @@ -150,7 +149,6 @@ int set_retry_cnt_and_time ( Slapi_PBlock *pb, int count, time_t cur_time ) { pw_apply_mods(sdn, &smods); slapi_mods_done(&smods); - delete_passwdPolicy(&pwpolicy); return rc; } @@ -190,7 +188,6 @@ int set_retry_cnt_mods(Slapi_PBlock *pb, Slapi_Mods *smods, int count) rc = LDAP_CONSTRAINT_VIOLATION; } } - delete_passwdPolicy(&pwpolicy); return rc; } diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index 09d6b90..e124d0b 100644 --- a/ldap/servers/slapd/result.c +++ b/ldap/servers/slapd/result.c @@ -555,7 +555,6 @@ log_and_return: log_result( pb, operation, err, tag, nentries ); } - delete_passwdPolicy (&pwpolicy); LDAPDebug( LDAP_DEBUG_TRACE, "<= send_ldap_result\n", 0, 0, 0 ); } diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index d0f2b33..2d923f8 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1505,6 +1505,37 @@ struct slapi_task { } slapi_task; /* End of interface to support online tasks **********************************/ +typedef struct passwordpolicyarray { + int pw_change; /* 1 - indicates that users are allowed to change the pwd */ + int pw_must_change; /* 1 - indicates that users must change pwd upon reset */ + int pw_syntax; + int pw_minlength; + int pw_mindigits; + int pw_minalphas; + int pw_minuppers; + int pw_minlowers; + int pw_minspecials; + int pw_min8bit; + int pw_maxrepeats; + int pw_mincategories; + int pw_mintokenlength; + int pw_exp; + long pw_maxage; + long pw_minage; + long pw_warning; + int pw_history; + int pw_inhistory; + int pw_lockout; + int pw_maxfailure; + int pw_unlock; + long pw_lockduration; + long pw_resetfailurecount; + int pw_gracelimit; + int pw_is_legacy; + int pw_track_update_time; + struct pw_scheme *pw_storagescheme; +} passwdPolicy; + typedef struct slapi_pblock { /* common */ Slapi_Backend *pb_backend; @@ -1657,6 +1688,7 @@ typedef struct slapi_pblock { int pb_syntax_filter_normalized; /* the syntax filter types/values are already normalized */ void *pb_syntax_filter_data; /* extra data to pass to a syntax plugin function */ int pb_paged_results_index; /* stash SLAPI_PAGED_RESULTS_INDEX */ + passwdPolicy *pwdpolicy; } slapi_pblock; /* index if substrlens */ @@ -2022,37 +2054,6 @@ typedef struct _slapdEntryPoints { #define MAX_ALLOWED_TIME_IN_SECS 2147483647 -typedef struct passwordpolicyarray { - int pw_change; /* 1 - indicates that users are allowed to change the pwd */ - int pw_must_change; /* 1 - indicates that users must change pwd upon reset */ - int pw_syntax; - int pw_minlength; - int pw_mindigits; - int pw_minalphas; - int pw_minuppers; - int pw_minlowers; - int pw_minspecials; - int pw_min8bit; - int pw_maxrepeats; - int pw_mincategories; - int pw_mintokenlength; - int pw_exp; - long pw_maxage; - long pw_minage; - long pw_warning; - int pw_history; - int pw_inhistory; - int pw_lockout; - int pw_maxfailure; - int pw_unlock; - long pw_lockduration; - long pw_resetfailurecount; - int pw_gracelimit; - int pw_is_legacy; - int pw_track_update_time; - struct pw_scheme *pw_storagescheme; -} passwdPolicy; - typedef struct _slapdFrontendConfig { Slapi_RWLock *cfg_rwlock; /* read/write lock to serialize access */ struct pw_scheme *rootpwstoragescheme; -- 1.7.1