559016 - Attempting to rename suffix returns inappropriate errors

https://bugzilla.redhat.com/show_bug.cgi?id=559016

[Fix Description] If the target dn of the modrdn operation is a suffix,
check if the new dn already exists or not.  If it exists, it returns
LDAP_ALREADY_EXISTS.  If the backend associated with the new dn does
not exist, it returns LDAP_NO_SUCH_OBJECT.  Otherwise, it returns
LDAP_NAMING_VIOLATION.

If the target dn of the modrdn is attempted to move across backends,
it returns LDAP_AFFECTS_MULTIPLE_DSAS instead of LDAP_UNWILLING_TO_PERFORM.

Modrdn (op_shared_rename) was logging the parameter errors in the
clients request as SLAPI_LOG_FATAL.  Reduced the level to SLAPI_LOG_ARGS.
Also, replaced ldap_explode_dn with slapi_dn_syntax_check to verify
the newsuperior.

By the replacement, 2 bugs in slapi_dn_syntax_check were found.
1) The key for the DN in the hashtable of the attribute syntax has
to be "distinguishedName".  2) Argument type for plg_syntax_validate
was not correct.

3 files changed, 60 insertions, 20 deletions

diff --git a/ldap/servers/slapd/mapping_tree.c b/ldap/servers/slapd/mapping_tree.c
index 3b082074..8b4c541b 100644
--- a/ldap/servers/slapd/mapping_tree.c
+++ b/ldap/servers/slapd/mapping_tree.c
@@ -2262,11 +2262,54 @@ int slapi_mapping_tree_select_and_check(Slapi_PBlock *pb,char *newdn, Slapi_Back
     if (ret)
         goto unlock_and_return;
 
-    if ((*be) && ((*be != new_be) || mtn_sdn_has_child(target_sdn)))
+    if (*be)
     {
-        ret = LDAP_UNWILLING_TO_PERFORM;
-        PR_snprintf(errorbuf, BUFSIZ, "Cannot move entries accross backends\n");
-        goto unlock_and_return;
+        /* suffix is a part of mapping tree. We should not free it */
+        const Slapi_DN *suffix = slapi_get_suffix_by_dn(target_sdn);
+        if (NULL == suffix)
+        {
+            ret = LDAP_NO_SUCH_OBJECT;
+            PR_snprintf(errorbuf, BUFSIZ,
+                        "Target entry \"%s\" does not exist\n", 
+                        slapi_sdn_get_dn(target_sdn));
+            goto unlock_and_return;
+        }
+        if (0 == slapi_sdn_compare(target_sdn, suffix))
+        {
+            /* target_sdn is a suffix */
+            const Slapi_DN *new_suffix = NULL;
+            /* new_suffix is a part of mapping tree. We should not free it */
+            new_suffix = slapi_get_suffix_by_dn(&dn_newdn);
+            if (!slapi_be_exist((const Slapi_DN *)&dn_newdn))
+            {
+                /* new_be is an empty backend */
+                ret = LDAP_NO_SUCH_OBJECT;
+                PR_snprintf(errorbuf, BUFSIZ,
+                           "Backend for suffix \"%s\" does not exist\n", newdn);
+                goto unlock_and_return;
+            }
+            if (0 == slapi_sdn_compare(&dn_newdn, new_suffix))
+            {
+                ret = LDAP_ALREADY_EXISTS;
+                PR_snprintf(errorbuf, BUFSIZ,
+                            "Suffix \"%s\" already exists\n", newdn);
+                goto unlock_and_return;
+            }
+            ret = LDAP_NAMING_VIOLATION;
+            PR_snprintf(errorbuf, BUFSIZ, "Cannot rename suffix \"%s\"\n",
+                        slapi_sdn_get_dn(target_sdn));
+            goto unlock_and_return;
+        }
+        else
+        {
+            if ((*be != new_be) || mtn_sdn_has_child(target_sdn))
+            {
+                ret = LDAP_AFFECTS_MULTIPLE_DSAS;
+                PR_snprintf(errorbuf, BUFSIZ,
+                                "Cannot move entries accross backends\n");
+                goto unlock_and_return;
+            }
+        }
     }
 
 unlock_and_return:
diff --git a/ldap/servers/slapd/modrdn.c b/ldap/servers/slapd/modrdn.c
index 8f332a10..38d13000 100644
--- a/ldap/servers/slapd/modrdn.c
+++ b/ldap/servers/slapd/modrdn.c
@@ -352,7 +352,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args)
 	{
 		if ( !internal_op )
 		{
-			slapi_log_access(LDAP_DEBUG_STATS,
+			slapi_log_access(SLAPI_LOG_ARGS,
 					 "conn=%" NSPRIu64 " op=%d MODRDN dn=\"%s\" newrdn=\"%s\" newsuperior=\"%s\"\n",
 					 pb->pb_conn->c_connid, 
 					 pb->pb_op->o_opid,
@@ -362,7 +362,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args)
 		}
 		else
 		{
-			slapi_log_access(LDAP_DEBUG_ARGS,
+			slapi_log_access(SLAPI_LOG_ARGS,
 					 "conn=%s op=%d MODRDN dn=\"%s\" newrdn=\"%s\" newsuperior=\"%s\"\n",
 					 LOG_INTERNAL_OP_CON_ID,
 					 LOG_INTERNAL_OP_OP_ID,
@@ -376,13 +376,13 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args)
 	if ((rdns = ldap_explode_rdn(newrdn, 0)) == NULL) 
 	{
 		if ( !internal_op ) {
-			slapi_log_error(SLAPI_LOG_FATAL, NULL, 
+			slapi_log_error(SLAPI_LOG_ARGS, NULL, 
 				 "conn=%" NSPRIu64 " op=%d MODRDN invalid new RDN (\"%s\")\n",
 				 pb->pb_conn->c_connid,
 				 pb->pb_op->o_opid,
 				 (NULL == newrdn) ? "(null)" : newrdn);
 		} else {
-			slapi_log_error(SLAPI_LOG_FATAL, NULL, 
+			slapi_log_error(SLAPI_LOG_ARGS, NULL, 
 				 "conn=%" NSPRIu64 " op=%d MODRDN invalid new RDN (\"%s\")\n",
 				 LOG_INTERNAL_OP_CON_ID,
 				 LOG_INTERNAL_OP_OP_ID,
@@ -403,30 +403,27 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args)
 	}
 
 	/* check that the dn is formatted correctly */
-	if ((rdns = ldap_explode_dn(newsuperior, 0)) == NULL) 
+	err = slapi_dn_syntax_check(pb, newsuperior, 1);
+	if (err)
 	{
-		LDAPDebug(LDAP_DEBUG_ARGS, "ldap_explode_dn of newSuperior failed\n", 0, 0, 0);
+		LDAPDebug0Args(LDAP_DEBUG_ARGS, "Syntax check of newSuperior failed\n");
 		if (!internal_op) {
-			slapi_log_error(SLAPI_LOG_FATAL, NULL,
+			slapi_log_error(SLAPI_LOG_ARGS, NULL,
 				 "conn=%" NSPRIu64 " op=%d MODRDN invalid new superior (\"%s\")",
 				 pb->pb_conn->c_connid,
 				 pb->pb_op->o_opid,
 				 (NULL == newsuperior) ? "(null)" : newsuperiorbuf);
 		} else {
-			slapi_log_error(SLAPI_LOG_FATAL, NULL,
+			slapi_log_error(SLAPI_LOG_ARGS, NULL,
 				 "conn=%" NSPRIu64 " op=%d MODRDN invalid new superior (\"%s\")",
 				 LOG_INTERNAL_OP_CON_ID,
 				 LOG_INTERNAL_OP_OP_ID,
 				 (NULL == newsuperior) ? "(null)" : newsuperiorbuf);
 		}
-		send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL,
+		send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, NULL,
 						 "newSuperior does not look like a DN", 0, NULL);
 		goto free_and_return_nolock;
 	} 
-	else 
-	{
-		slapi_ldap_value_free(rdns);
-	}
 
 	if (newsuperior != NULL) 
 	{
@@ -448,7 +445,7 @@ op_shared_rename(Slapi_PBlock *pb, int passin_args)
 	 * if we don't hold it.
 	 */
 	if ((err = slapi_mapping_tree_select_and_check(pb, newdn, &be, &referral, errorbuf)) != LDAP_SUCCESS)
-	 {
+	{
 		send_ldap_result(pb, err, NULL, errorbuf, 0, NULL);
 		goto free_and_return_nolock;
 	}
diff --git a/ldap/servers/slapd/plugin_syntax.c b/ldap/servers/slapd/plugin_syntax.c
index ecf999bc..e2cc7fb1 100644
--- a/ldap/servers/slapd/plugin_syntax.c
+++ b/ldap/servers/slapd/plugin_syntax.c
@@ -295,7 +295,7 @@ slapi_dn_syntax_check(
 	}
 
 	/* Locate the dn syntax plugin. */
-	slapi_attr_type2plugin("dn", (void **)&dn_plugin);
+	slapi_attr_type2plugin("distinguishedName", (void **)&dn_plugin);
 
 	/* Assume the value is valid if we don't find a dn validate function */
 	if (dn_plugin && dn_plugin->plg_syntax_validate != NULL) {
@@ -305,7 +305,7 @@ slapi_dn_syntax_check(
 			dn_bval.bv_len = strlen(dn);
 
 			/* Validate the value. */
-			if (dn_plugin->plg_syntax_validate(dn_bval) != 0) {
+			if (dn_plugin->plg_syntax_validate(&dn_bval) != 0) {
 				if (syntaxlogging) {
 					slapi_log_error( SLAPI_LOG_FATAL, "Syntax Check",
 						"DN value (%s) invalid per syntax\n", dn ? dn : "");