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.)

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 */

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

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;

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 ) {

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);