From b8e6b1394ee20434f2ecf4c70451f4f0bcc2b6bf Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Apr 02 2012 23:51:02 +0000 Subject: Trac Ticket #45 - Fine Grained Password policy: if passwordHistory is on, deleting the password fails. https://fedorahosted.org/389/ticket/45 Bug description: To allow replicating unhashed password, an internal entry contains the key value pair when the entry is newly added or the password is updated. In that case, deleting the userpassword attribute leaves the unhashed password in the internal entry. If you attempt to add a new userpassword, the remaining unhashed password makes the attempt fail due to LDAP_TYPE_OR_VALUE_EXISTS. Fix description: This patch cleans up the unhashed password if a userpassword is deleted and the unhashed password is found in the internal entry. If it does not exist, the deletion does nothing. (If the entry is read from the database, the unhashed password does not exist in the internal entry since it is not stored in the database.) --- diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c index 5afba09..c235431 100644 --- a/ldap/servers/slapd/entry.c +++ b/ldap/servers/slapd/entry.c @@ -63,7 +63,6 @@ /* a helper function to set special rdn to a tombstone entry */ static int _entry_set_tombstone_rdn(Slapi_Entry *e, const char *normdn); -static int is_type_protected(const char *type); /* protected attributes which are not included in the flattened entry, * which will be stored in the db. */ @@ -1613,7 +1612,7 @@ entry2str_internal_put_valueset( const char *attrtype, const CSN *attrcsn, CSNTy } } -static int +int is_type_protected(const char *type) { char **paap = NULL; @@ -3393,24 +3392,37 @@ slapi_entry_delete_values( return(rc); } - static int delete_values_sv_internal( Slapi_Entry *e, const char *type, Slapi_Value **valuestodelete, - int flags + int flags ) { Slapi_Attr *a; int retVal= LDAP_SUCCESS; + /* + * If type is in the protected_attrs_all list, we could ignore the failure, + * as the attribute could only exist in the entry in the memory when the + * add/mod operation is done, while the retried entry from the db does not + * contain the attribute. + */ + if (is_type_protected(type)) { + flags |= SLAPI_VALUE_FLAG_IGNOREERROR; + } + /* delete the entire attribute */ if ( valuestodelete == NULL || valuestodelete[0] == NULL ){ LDAPDebug( LDAP_DEBUG_ARGS, "removing entire attribute %s\n", type, 0, 0 ); - return( attrlist_delete( &e->e_attrs, type) ? - LDAP_NO_SUCH_ATTRIBUTE : LDAP_SUCCESS ); + retVal = attrlist_delete( &e->e_attrs, type); + if (flags & SLAPI_VALUE_FLAG_IGNOREERROR) { + return LDAP_SUCCESS; + } else { + } + return(retVal ? LDAP_NO_SUCH_ATTRIBUTE : LDAP_SUCCESS); } /* delete specific values - find the attribute first */ diff --git a/ldap/servers/slapd/entrywsi.c b/ldap/servers/slapd/entrywsi.c index a749cee..05dbb36 100644 --- a/ldap/servers/slapd/entrywsi.c +++ b/ldap/servers/slapd/entrywsi.c @@ -638,12 +638,22 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval } else if (attr_state==ATTRIBUTE_NOTFOUND) { - if (!urp) - { - /* Only warn if not urping */ - LDAPDebug( LDAP_DEBUG_ARGS, "could not find attribute %s\n", type, 0, 0 ); + /* + * If type is in the protected_attrs_all list, we could ignore the + * failure, as the attribute could only exist in the entry in the + * memory when the add/mod operation is done, while the retried entry + * from the db does not contain the attribute. + */ + if (is_type_protected(type)) { + retVal = LDAP_SUCCESS; + } else { + if (!urp) { + /* Only warn if not urping */ + LDAPDebug1Arg(LDAP_DEBUG_ARGS, "could not find attribute %s\n", + type); + } + retVal = LDAP_NO_SUCH_ATTRIBUTE; } - retVal= LDAP_NO_SUCH_ATTRIBUTE; /* NOTE: LDAP says that a MOD REPLACE with no vals of a non-existent attribute is a no-op - MOD REPLACE with some vals will add the attribute */ /* if we are doing a replace with actual values, meaning the result diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 7ec9ff9..73752ed 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -299,9 +299,8 @@ do_modify( Slapi_PBlock *pb ) continue; } - /* check for password change */ - if ( mod->mod_bvalues != NULL && - strcasecmp( mod->mod_type, SLAPI_USERPWD_ATTR ) == 0 ){ + /* check for password change (including deletion) */ + if ( strcasecmp( mod->mod_type, SLAPI_USERPWD_ATTR ) == 0 ){ has_password_mod++; } @@ -357,11 +356,11 @@ do_modify( Slapi_PBlock *pb ) for (mod = slapi_mods_get_first_mod(&smods); mod; mod = slapi_mods_get_next_mod(&smods)) { - if ( mod->mod_bvalues != NULL && - strcasecmp( mod->mod_type, SLAPI_USERPWD_ATTR ) == 0 ) { + /* check for password change (including deletion) */ + if ( strcasecmp( mod->mod_type, SLAPI_USERPWD_ATTR ) == 0 ) { /* assumes controls have already been decoded and placed in the pblock */ - pw_change = op_shared_allow_pw_change (pb, mod, &old_pw, &smods); + pw_change = op_shared_allow_pw_change(pb, mod, &old_pw, &smods); if (pw_change == -1) { goto free_and_return; } @@ -821,8 +820,9 @@ static void op_shared_modify (Slapi_PBlock *pb, int pw_change, char *old_pw) } /* - * Add the unhashed password pseudo-attribute before - * calling the preop plugins + * Add the unhashed password pseudo-attribute (for add) OR + * Delete the unhashed password pseudo-attribute (for delete) + * before calling the preop plugins */ if (pw_change && !repl_op) @@ -837,10 +837,20 @@ static void op_shared_modify (Slapi_PBlock *pb, int pw_change, char *old_pw) if (strcasecmp (pw_mod->mod_type, SLAPI_USERPWD_ATTR) != 0) continue; - /* add pseudo password attribute */ - valuearray_init_bervalarray(pw_mod->mod_bvalues, &va); - slapi_mods_add_mod_values(&smods, pw_mod->mod_op, unhashed_pw_attr, va); - valuearray_free(&va); + if (LDAP_MOD_DELETE == pw_mod->mod_op) { + Slapi_Attr *a = NULL; + /* delete pseudo password attribute if it exists in the entry */ + if (!slapi_entry_attr_find(e, unhashed_pw_attr, &a)) { + slapi_mods_add_mod_values(&smods, pw_mod->mod_op, + unhashed_pw_attr, va); + } + } else { + /* add pseudo password attribute */ + valuearray_init_bervalarray(pw_mod->mod_bvalues, &va); + slapi_mods_add_mod_values(&smods, pw_mod->mod_op, + unhashed_pw_attr, va); + valuearray_free(&va); + } /* Init new value array for hashed value */ valuearray_init_bervalarray(pw_mod->mod_bvalues, &va); @@ -1199,7 +1209,8 @@ static int op_shared_allow_pw_change (Slapi_PBlock *pb, LDAPMod *mod, char **old /* check password syntax; remember the old password; error sent directly from check_pw_syntax function */ valuearray_init_bervalarray(mod->mod_bvalues, &values); - switch (check_pw_syntax_ext (pb, &sdn, values, old_pw, NULL, 1, smods)) + switch (check_pw_syntax_ext (pb, &sdn, values, old_pw, NULL, + mod->mod_op, smods)) { case 0: /* success */ rc = 1; diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 3bc7334..0dac10b 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -788,6 +788,20 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, passwdPolicy *pwpolicy = NULL; Slapi_Operation *operation = NULL; + /* + * check_pw_syntax_ext could be called with mod_op == LDAP_MOD_DELETE. + * In that case, no need to check the password syntax, but just returns + * PASS == 0. + */ + if (LDAP_MOD_DELETE == (mod_op & LDAP_MOD_OP)) { + return 0; + } + if (NULL == vals) { + slapi_log_error( SLAPI_LOG_FATAL, NULL, + "check_pw_syntax_ext: no passwords to check\n" ); + return -1; + } + pwpolicy = new_passwdPolicy(pb, dn); slapi_pblock_get ( pb, SLAPI_PWPOLICY, &pwresponse_req ); @@ -967,7 +981,7 @@ check_pw_syntax_ext ( Slapi_PBlock *pb, const Slapi_DN *sdn, Slapi_Value **vals, } /* get the entry and check for the password history if this is called by a modify operation */ - if ( mod_op ) { + if ( mod_op ) { /* retrieve the entry */ e = get_entry ( pb, dn ); if ( e == NULL ) { diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 7d4e944..75f8e8f 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -330,6 +330,7 @@ int entry_next_deleted_attribute( const Slapi_Entry *e, Slapi_Attr **a); /* entry.c */ int entry_apply_mods( Slapi_Entry *e, LDAPMod **mods ); +int is_type_protected(const char *type); int slapi_entries_diff(Slapi_Entry **old_entries, Slapi_Entry **new_entries, int testall, const char *logging_prestr, const int force_update, void *plg_id);