CVE-2023-0614 ldb: Make ldb_filter_attrs_in_place() work in place

ldb_filter_attrs() previously did too much. Now its replacement,
ldb_filter_attrs_in_place(), only does the actual filtering, while
taking ownership of each element's values is handled in a separate
function, ldb_msg_elements_take_ownership().

Also, ldb_filter_attrs_in_place() no longer adds the distinguishedName
to the message if it is missing. That is handled in another function,
ldb_msg_add_distinguished_name().

As we're now modifying the original message rather than copying it into
a new one, we no longer need the filtered_msg parameter.

We adapt a test, based on ldb_filter_attrs_test, to exercise the new
function.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

3 files changed, 307 insertions, 442 deletions

diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index f19ac73fa5e..28b9a8dfe07 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -1264,19 +1264,16 @@ failed:
 
 /*
  * filter the specified list of attributes from msg,
- * adding requested attributes, and perhaps all for *,
- * but not the DN to filtered_msg.
+ * adding requested attributes, and perhaps all for *.
+ * Unlike ldb_filter_attrs(), the DN will not be added
+ * if it is missing.
  */
-int ldb_filter_attrs_in_place(struct ldb_context *ldb,
-		     const struct ldb_message *msg,
-		     const char *const *attrs,
-		     struct ldb_message *filtered_msg)
+int ldb_filter_attrs_in_place(struct ldb_message *msg,
+			      const char *const *attrs)
 {
-	unsigned int i;
+	unsigned int i = 0;
 	bool keep_all = false;
-	bool add_dn = false;
-	uint32_t num_elements;
-	uint32_t elements_size;
+	unsigned int num_del = 0;
 
 	if (attrs) {
 		/* check for special attrs */
@@ -1286,123 +1283,41 @@ int ldb_filter_attrs_in_place(struct ldb_context *ldb,
 				keep_all = true;
 				break;
 			}
-			cmp = ldb_attr_cmp(attrs[i], "distinguishedName");
-			if (cmp == 0) {
-				add_dn = true;
-			}
 		}
-	} else {
-		keep_all = true;
-	}
-
-	if (keep_all) {
-		add_dn = true;
-		elements_size = msg->num_elements + 1;
-
-	/* Shortcuts for the simple cases */
-	} else if (add_dn && i == 1) {
-		if (ldb_msg_add_distinguished_name(filtered_msg) != 0) {
-			goto failed;
+		if (!keep_all && i == 0) {
+			msg->num_elements = 0;
+			return LDB_SUCCESS;
 		}
-		return 0;
-	} else if (i == 0) {
-		return 0;
-
-	/*
-	 * Otherwise we are copying at most as many elements as we
-	 * have attributes
-	 */
 	} else {
-		elements_size = i;
+		keep_all = true;
 	}
 
-	filtered_msg->elements = talloc_array(filtered_msg,
-					      struct ldb_message_element,
-					      elements_size);
-	if (filtered_msg->elements == NULL) goto failed;
-
-	num_elements = 0;
-
 	for (i = 0; i < msg->num_elements; i++) {
-		struct ldb_message_element *el = &msg->elements[i];
-
-		/*
-		 * el2 is assigned after the Pigeonhole principle
-		 * check below for clarity
-		 */
-		struct ldb_message_element *el2 = NULL;
+		bool found = false;
 		unsigned int j;
 
-		if (keep_all == false) {
-			bool found = false;
+		if (keep_all) {
+			found = true;
+		} else {
 			for (j = 0; attrs[j]; j++) {
-				int cmp = ldb_attr_cmp(el->name, attrs[j]);
+				int cmp = ldb_attr_cmp(msg->elements[i].name, attrs[j]);
 				if (cmp == 0) {
 					found = true;
 					break;
 				}
 			}
-			if (found == false) {
-				continue;
-			}
-		}
-
-		/*
-		 * Pigeonhole principle: we can't have more elements
-		 * than the number of attributes if they are unique in
-		 * the DB.
-		 */
-		if (num_elements >= elements_size) {
-			goto failed;
 		}
 
-		el2 = &filtered_msg->elements[num_elements];
-
-		*el2 = *el;
-		el2->name = talloc_strdup(filtered_msg->elements,
-					  el->name);
-		if (el2->name == NULL) {
-			goto failed;
-		}
-		el2->values = talloc_array(filtered_msg->elements,
-					   struct ldb_val, el->num_values);
-		if (el2->values == NULL) {
-			goto failed;
+		if (!found) {
+			++num_del;
+		} else if (num_del != 0) {
+			msg->elements[i - num_del] = msg->elements[i];
 		}
-		for (j=0;j<el->num_values;j++) {
-			el2->values[j] = ldb_val_dup(el2->values, &el->values[j]);
-			if (el2->values[j].data == NULL && el->values[j].length != 0) {
-				goto failed;
-			}
-		}
-		num_elements++;
 	}
 
-	filtered_msg->num_elements = num_elements;
-
-	if (add_dn) {
-		if (ldb_msg_add_distinguished_name(filtered_msg) != 0) {
-			goto failed;
-		}
-	}
+	msg->num_elements -= num_del;
 
-	if (filtered_msg->num_elements > 0) {
-		filtered_msg->elements
-			= talloc_realloc(filtered_msg,
-					 filtered_msg->elements,
-					 struct ldb_message_element,
-					 filtered_msg->num_elements);
-		if (filtered_msg->elements == NULL) {
-			goto failed;
-		}
-	} else {
-		TALLOC_FREE(filtered_msg->elements);
-	}
-
-	return 0;
-failed:
-	TALLOC_FREE(filtered_msg->elements);
-	return -1;
+	return LDB_SUCCESS;
 }
 
 /* Have an unpacked ldb message take talloc ownership of its elements. */
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 105093cf38c..4ae381ba5be 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -545,13 +545,12 @@ int ldb_filter_attrs(struct ldb_context *ldb,
 
 /*
  * filter the specified list of attributes from msg,
- * adding requested attributes, and perhaps all for *,
- * but not the DN to filtered_msg.
+ * adding requested attributes, and perhaps all for *.
+ * Unlike ldb_filter_attrs(), the DN will not be added
+ * if it is missing.
  */
-int ldb_filter_attrs_in_place(struct ldb_context *ldb,
-		     const struct ldb_message *msg,
-		     const char *const *attrs,
-		     struct ldb_message *filtered_msg);
+int ldb_filter_attrs_in_place(struct ldb_message *msg,
+			      const char *const *attrs);
 
 /* Have an unpacked ldb message take talloc ownership of its elements. */
 int ldb_msg_elements_take_ownership(struct ldb_message *msg);
diff --git a/lib/ldb/tests/ldb_filter_attrs_in_place_test.c b/lib/ldb/tests/ldb_filter_attrs_in_place_test.c
index bef961f8f9c..da333c73c99 100644
--- a/lib/ldb/tests/ldb_filter_attrs_in_place_test.c
+++ b/lib/ldb/tests/ldb_filter_attrs_in_place_test.c
@@ -83,17 +83,41 @@ static int teardown(void **state)
 	return 0;
 }
 
+static void msg_add_dn(struct ldb_message *msg)
+{
+	const char *dn_attr = "distinguishedName";
+	char *dn = NULL;
+	int ret;
+
+	assert_null(ldb_msg_find_element(msg, dn_attr));
+
+	assert_non_null(msg->dn);
+	dn = ldb_dn_alloc_linearized(msg, msg->dn);
+	assert_non_null(dn);
+
+	/*
+	 * The message's elements must be talloc allocated to call
+	 * ldb_msg_add_steal_string().
+	 */
+	msg->elements = talloc_memdup(msg,
+				      msg->elements,
+				      msg->num_elements * sizeof(msg->elements[0]));
+	assert_non_null(msg->elements);
+
+	ret = ldb_msg_add_steal_string(msg, dn_attr, dn);
+	assert_int_equal(ret, LDB_SUCCESS);
+}
 
 /*
  * Test against a record with only one attribute, matching the one in
  * the list
  */
-static void test_filter_attrs_one_attr_matched(void **state)
+static void test_filter_attrs_in_place_one_attr_matched(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"foo", NULL};
 
@@ -107,32 +131,25 @@ static void test_filter_attrs_one_attr_matched(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not read or modify
-	 * filtered_msg.dn in this case
-	 */
-	assert_null(filtered_msg->dn);
-	assert_int_equal(filtered_msg->num_elements, 1);
-	assert_string_equal(filtered_msg->elements[0].name, "foo");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_non_null(msg->dn);
+	assert_int_equal(msg->num_elements, 1);
+	assert_string_equal(msg->elements[0].name, "foo");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value, strlen(value));
 }
 
@@ -140,12 +157,12 @@ static void test_filter_attrs_one_attr_matched(void **state)
  * Test against a record with only one attribute, matching the one of
  * the multiple attributes in the list
  */
-static void test_filter_attrs_one_attr_matched_of_many(void **state)
+static void test_filter_attrs_in_place_one_attr_matched_of_many(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"foo", "bar", "baz", NULL};
 
@@ -159,32 +176,25 @@ static void test_filter_attrs_one_attr_matched_of_many(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not read or modify
-	 * filtered_msg.dn in this case
-	 */
-	assert_null(filtered_msg->dn);
-	assert_int_equal(filtered_msg->num_elements, 1);
-	assert_string_equal(filtered_msg->elements[0].name, "foo");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_non_null(msg->dn);
+	assert_int_equal(msg->num_elements, 1);
+	assert_string_equal(msg->elements[0].name, "foo");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value, strlen(value));
 }
 
@@ -192,12 +202,12 @@ static void test_filter_attrs_one_attr_matched_of_many(void **state)
  * Test against a record with only one attribute, matching both
  * attributes in the list
  */
-static void test_filter_attrs_two_attr_matched_attrs(void **state)
+static void test_filter_attrs_in_place_two_attr_matched_attrs(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	/* deliberatly the other order */
 	const char *attrs[] = {"bar", "foo", NULL};
@@ -226,40 +236,33 @@ static void test_filter_attrs_two_attr_matched_attrs(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 2);
+	assert_int_equal(msg->num_elements, 2);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not read or modify
-	 * filtered_msg.dn in this case
-	 */
-	assert_null(filtered_msg->dn);
+	assert_non_null(msg->dn);
 
 	/* Assert that DB order is preserved */
-	assert_string_equal(filtered_msg->elements[0].name, "foo");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_string_equal(msg->elements[0].name, "foo");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value1));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value1, strlen(value1));
-	assert_string_equal(filtered_msg->elements[1].name, "bar");
-	assert_int_equal(filtered_msg->elements[1].num_values, 1);
-	assert_int_equal(filtered_msg->elements[1].values[0].length,
+	assert_string_equal(msg->elements[1].name, "bar");
+	assert_int_equal(msg->elements[1].num_values, 1);
+	assert_int_equal(msg->elements[1].values[0].length,
 			 strlen(value2));
-	assert_memory_equal(filtered_msg->elements[1].values[0].data,
+	assert_memory_equal(msg->elements[1].values[0].data,
 			    value2, strlen(value2));
 }
 
@@ -267,14 +270,13 @@ static void test_filter_attrs_two_attr_matched_attrs(void **state)
  * Test against a record with two attributes, only of which is in
  * the list
  */
-static void test_filter_attrs_two_attr_matched_one_attr(void **state)
+static void test_filter_attrs_in_place_two_attr_matched_one_attr(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
-	/* deliberatly the other order */
 	const char *attrs[] = {"bar", NULL};
 
 	char value1[] = "The value.......end";
@@ -288,7 +290,6 @@ static void test_filter_attrs_two_attr_matched_one_attr(void **state)
 		.length = strlen(value2)
 	};
 
-	/* foo and bar are the other order to in attrs */
 	struct ldb_message_element elements[] = {
 		{
 			.name = "foo",
@@ -301,34 +302,27 @@ static void test_filter_attrs_two_attr_matched_one_attr(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 1);
+	assert_int_equal(msg->num_elements, 1);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not read or modify
-	 * filtered_msg.dn in this case
-	 */
-	assert_null(filtered_msg->dn);
+	assert_non_null(msg->dn);
 
 	/* Assert that DB order is preserved */
-	assert_string_equal(filtered_msg->elements[0].name, "bar");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_string_equal(msg->elements[0].name, "bar");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value2));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value2, strlen(value2));
 }
 
@@ -336,14 +330,13 @@ static void test_filter_attrs_two_attr_matched_one_attr(void **state)
  * Test against a record with two attributes, both matching the one
  * specified attribute in the list (a corrupt record)
  */
-static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
+static void test_filter_attrs_in_place_two_dup_attr_matched_one_attr(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
-	/* deliberatly the other order */
 	const char *attrs[] = {"bar", NULL};
 
 	char value1[] = "The value.......end";
@@ -357,7 +350,6 @@ static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
 		.length = strlen(value2)
 	};
 
-	/* foo and bar are the other order to in attrs */
 	struct ldb_message_element elements[] = {
 		{
 			.name = "bar",
@@ -370,34 +362,49 @@ static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
+
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	/* Both elements match the filter */
+	assert_int_equal(ret, LDB_SUCCESS);
+	assert_int_equal(msg->num_elements, 2);
+
+	assert_non_null(msg->dn);
 
-	/* This should fail the pidgenhole test */
-	assert_int_equal(ret, -1);
-	assert_null(filtered_msg->elements);
+	/* Assert that DB order is preserved */
+	assert_string_equal(msg->elements[0].name, "bar");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
+			 strlen(value1));
+	assert_memory_equal(msg->elements[0].values[0].data,
+			    value1, strlen(value1));
+
+	assert_string_equal(msg->elements[1].name, "bar");
+	assert_int_equal(msg->elements[1].num_values, 1);
+	assert_int_equal(msg->elements[1].values[0].length,
+			 strlen(value2));
+	assert_memory_equal(msg->elements[1].values[0].data,
+			    value2, strlen(value2));
 }
 
 /*
  * Test against a record with two attributes, both matching the one
  * specified attribute in the list (a corrupt record)
  */
-static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
+static void test_filter_attrs_in_place_two_dup_attr_matched_dup(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"bar", "bar", NULL};
 
@@ -412,7 +419,6 @@ static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
 		.length = strlen(value2)
 	};
 
-	/* foo and bar are the other order to in attrs */
 	struct ldb_message_element elements[] = {
 		{
 			.name = "bar",
@@ -425,35 +431,33 @@ static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
+
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 
 	/* This does not fail the pidgenhole test */
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_int_equal(filtered_msg->num_elements, 2);
+	assert_int_equal(msg->num_elements, 2);
 
 	/* Assert that DB order is preserved */
-	assert_string_equal(filtered_msg->elements[0].name, "bar");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_string_equal(msg->elements[0].name, "bar");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value1));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value1, strlen(value1));
-	assert_string_equal(filtered_msg->elements[1].name, "bar");
-	assert_int_equal(filtered_msg->elements[1].num_values, 1);
-	assert_int_equal(filtered_msg->elements[1].values[0].length,
+	assert_string_equal(msg->elements[1].name, "bar");
+	assert_int_equal(msg->elements[1].num_values, 1);
+	assert_int_equal(msg->elements[1].values[0].length,
 			 strlen(value2));
-	assert_memory_equal(filtered_msg->elements[1].values[0].data,
+	assert_memory_equal(msg->elements[1].values[0].data,
 			    value2, strlen(value2));
 }
 
@@ -461,12 +465,12 @@ static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
  * Test against a record with two attributes, both matching one of the
  * specified attributes in the list (a corrupt record)
  */
-static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
+static void test_filter_attrs_in_place_two_dup_attr_matched_one_of_two(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"bar", "foo", NULL};
 
@@ -481,7 +485,6 @@ static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
 		.length = strlen(value2)
 	};
 
-	/* foo and bar are the other order to in attrs */
 	struct ldb_message_element elements[] = {
 		{
 			.name = "bar",
@@ -494,35 +497,33 @@ static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
+
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 
 	/* This does not fail the pidgenhole test */
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_int_equal(filtered_msg->num_elements, 2);
+	assert_int_equal(msg->num_elements, 2);
 
 	/* Assert that DB order is preserved */
-	assert_string_equal(filtered_msg->elements[0].name, "bar");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_string_equal(msg->elements[0].name, "bar");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value1));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value1, strlen(value1));
-	assert_string_equal(filtered_msg->elements[1].name, "bar");
-	assert_int_equal(filtered_msg->elements[1].num_values, 1);
-	assert_int_equal(filtered_msg->elements[1].values[0].length,
+	assert_string_equal(msg->elements[1].name, "bar");
+	assert_int_equal(msg->elements[1].num_values, 1);
+	assert_int_equal(msg->elements[1].values[0].length,
 			 strlen(value2));
-	assert_memory_equal(filtered_msg->elements[1].values[0].data,
+	assert_memory_equal(msg->elements[1].values[0].data,
 			    value2, strlen(value2));
 }
 
@@ -530,12 +531,12 @@ static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
  * Test against a record with two attributes against * (but not the
  * other named attribute) (a corrupt record)
  */
-static void test_filter_attrs_two_dup_attr_matched_star(void **state)
+static void test_filter_attrs_in_place_two_dup_attr_matched_star(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"*", "foo", NULL};
 
@@ -550,7 +551,6 @@ static void test_filter_attrs_two_dup_attr_matched_star(void **state)
 		.length = strlen(value2)
 	};
 
-	/* foo and bar are the other order to in attrs */
 	struct ldb_message_element elements[] = {
 		{
 			.name = "bar",
@@ -563,60 +563,52 @@ static void test_filter_attrs_two_dup_attr_matched_star(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
 
-	/* Needed as * implies distinguishedName */
-	filtered_msg->dn = in.dn;
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 
 	/* This does not fail the pidgenhole test */
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_int_equal(filtered_msg->num_elements, 3);
+	assert_int_equal(msg->num_elements, 3);
 
 	/* Assert that DB order is preserved */
-	assert_string_equal(filtered_msg->elements[0].name, "bar");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_int_equal(filtered_msg->elements[0].values[0].length,
+	assert_string_equal(msg->elements[0].name, "bar");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_int_equal(msg->elements[0].values[0].length,
 			 strlen(value1));
-	assert_memory_equal(filtered_msg->elements[0].values[0].data,
+	assert_memory_equal(msg->elements[0].values[0].data,
 			    value1, strlen(value1));
-	assert_string_equal(filtered_msg->elements[1].name, "bar");
-	assert_int_equal(filtered_msg->elements[1].num_values, 1);
-	assert_int_equal(filtered_msg->elements[1].values[0].length,
+	assert_string_equal(msg->elements[1].name, "bar");
+	assert_int_equal(msg->elements[1].num_values, 1);
+	assert_int_equal(msg->elements[1].values[0].length,
 			 strlen(value2));
-	assert_memory_equal(filtered_msg->elements[1].values[0].data,
+	assert_memory_equal(msg->elements[1].values[0].data,
 			    value2, strlen(value2));
-	/*
-	 * assert the ldb_filter_attrs_in_place does not modify filtered_msg.dn
-	 * in this case
-	 */
-	assert_ptr_equal(filtered_msg->dn, in.dn);
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+
+	assert_non_null(msg->dn);
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"distinguishedName",
 							NULL),
-			    ldb_dn_get_linearized(in.dn));
+			    ldb_dn_get_linearized(msg->dn));
 }
 
 /*
  * Test against a record with only one attribute, matching the * in
  * the list
  */
-static void test_filter_attrs_one_attr_matched_star(void **state)
+static void test_filter_attrs_in_place_one_attr_matched_star(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"*", NULL};
 
@@ -630,35 +622,25 @@ static void test_filter_attrs_one_attr_matched_star(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
 
-	/* Needed as * implies distinguishedName */
-	filtered_msg->dn = in.dn;
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 2);
+	assert_int_equal(msg->num_elements, 2);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not modify filtered_msg.dn
-	 * in this case
-	 */
-	assert_ptr_equal(filtered_msg->dn, in.dn);
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+	assert_non_null(msg->dn);
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"distinguishedName",
 							NULL),
-			    ldb_dn_get_linearized(in.dn));
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+			    ldb_dn_get_linearized(msg->dn));
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"foo",
 							NULL),
 			    value);
@@ -668,12 +650,12 @@ static void test_filter_attrs_one_attr_matched_star(void **state)
  * Test against a record with two attributes, matching the * in
  * the list
  */
-static void test_filter_attrs_two_attr_matched_star(void **state)
+static void test_filter_attrs_in_place_two_attr_matched_star(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"*", NULL};
 
@@ -699,39 +681,29 @@ static void test_filter_attrs_two_attr_matched_star(void **state)
 			.values = &value_2
 		}
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 2,
-		.elements = elements,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 2;
+	msg->elements = elements;
 
-	/* Needed as * implies distinguishedName */
-	filtered_msg->dn = in.dn;
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 3);
+	assert_int_equal(msg->num_elements, 3);
 
-	/*
-	 * assert the ldb_filter_attrs_in_place does not modify filtered_msg.dn
-	 * in this case
-	 */
-	assert_ptr_equal(filtered_msg->dn, in.dn);
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+	assert_non_null(msg->dn);
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"distinguishedName",
 							NULL),
-			    ldb_dn_get_linearized(in.dn));
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+			    ldb_dn_get_linearized(msg->dn));
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"foo",
 							NULL),
 			    value1);
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"bar",
 							NULL),
 			    value2);
@@ -739,15 +711,15 @@ static void test_filter_attrs_two_attr_matched_star(void **state)
 
 /*
  * Test against a record with only one attribute, matching the * in
- * the list, but without the DN being pre-filled.  Fails due to need
- * to contstruct the distinguishedName
+ * the list, but without the DN being pre-filled.  Succeeds, but the
+ * distinguishedName is not added.
  */
-static void test_filter_attrs_one_attr_matched_star_no_dn(void **state)
+static void test_filter_attrs_in_place_one_attr_matched_star_no_dn(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"*", NULL};
 
@@ -761,32 +733,29 @@ static void test_filter_attrs_one_attr_matched_star_no_dn(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = NULL;
+	msg->num_elements = 1;
+	msg->elements = &element_1;
+
+	assert_null(msg->dn);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
-	assert_int_equal(ret, -1);
-	assert_null(filtered_msg->elements);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
+	assert_int_equal(ret, LDB_SUCCESS);
+	assert_int_equal(msg->num_elements, 1);
 }
 
 /*
  * Test against a record with only one attribute, matching the * in
  * the list plus requsesting distinguishedName
  */
-static void test_filter_attrs_one_attr_matched_star_dn(void **state)
+static void test_filter_attrs_in_place_one_attr_matched_star_dn(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"*", "distinguishedName", NULL};
 
@@ -800,33 +769,26 @@ static void test_filter_attrs_one_attr_matched_star_dn(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
 
-	/* Needed for distinguishedName */
-	filtered_msg->dn = in.dn;
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 2);
+	assert_int_equal(msg->num_elements, 2);
 
-	/* show that ldb_filter_attrs_in_place does not modify in.dn */
-	assert_ptr_equal(filtered_msg->dn, in.dn);
+	assert_non_null(msg->dn);
 
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"distinguishedName",
 							NULL),
-			    ldb_dn_get_linearized(in.dn));
-	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
+			    ldb_dn_get_linearized(msg->dn));
+	assert_string_equal(ldb_msg_find_attr_as_string(msg,
 							"foo",
 							NULL),
 			    value);
@@ -836,12 +798,12 @@ static void test_filter_attrs_one_attr_matched_star_dn(void **state)
  * Test against a record with only one attribute, but returning
  * distinguishedName from the list (only)
  */
-static void test_filter_attrs_one_attr_matched_dn(void **state)
+static void test_filter_attrs_in_place_one_attr_matched_dn(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {"distinguishedName", NULL};
 
@@ -855,43 +817,36 @@ static void test_filter_attrs_one_attr_matched_dn(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
 
-	/* Needed for distinguishedName */
-	filtered_msg->dn = in.dn;
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 1);
-
-	/* show that ldb_filter_attrs_in_place does not modify in.dn */
-	assert_ptr_equal(filtered_msg->dn, in.dn);
-	assert_string_equal(filtered_msg->elements[0].name, "distinguishedName");
-	assert_int_equal(filtered_msg->elements[0].num_values, 1);
-	assert_string_equal(filtered_msg->elements[0].values[0].data,
-			    ldb_dn_get_linearized(in.dn));
+	assert_int_equal(msg->num_elements, 1);
+
+	assert_non_null(msg->dn);
+	assert_string_equal(msg->elements[0].name, "distinguishedName");
+	assert_int_equal(msg->elements[0].num_values, 1);
+	assert_string_equal(msg->elements[0].values[0].data,
+			    ldb_dn_get_linearized(msg->dn));
 }
 
 /*
  * Test against a record with only one attribute, not matching the
  * empty attribute list
  */
-static void test_filter_attrs_one_attr_empty_list(void **state)
+static void test_filter_attrs_in_place_one_attr_empty_list(void **state)
 {
 	struct ldbtest_ctx *ctx = *state;
 	int ret;
 
-	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
+	struct ldb_message *msg = ldb_msg_new(ctx);
 
 	const char *attrs[] = {NULL};
 
@@ -905,82 +860,78 @@ static void test_filter_attrs_one_attr_empty_list(void **state)
 		.num_values = 1,
 		.values = &value_1
 	};
-	struct ldb_message in = {
-		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
-		.num_elements = 1,
-		.elements = &element_1,
-	};
 
-	assert_non_null(in.dn);
+	assert_non_null(msg);
+	msg->dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org");
+	msg->num_elements = 1;
+	msg->elements = &element_1;
+
+	assert_non_null(msg->dn);
+	msg_add_dn(msg);
 
-	ret = ldb_filter_attrs_in_place(ctx->ldb,
-			       &in,
-			       attrs,
-			       filtered_msg);
+	ret = ldb_filter_attrs_in_place(msg, attrs);
 	assert_int_equal(ret, LDB_SUCCESS);
-	assert_non_null(filtered_msg);
-	assert_int_equal(filtered_msg->num_elements, 0);
-	assert_null(filtered_msg->dn);
-	assert_null(filtered_msg->elements);
+	assert_int_equal(msg->num_elements, 0);
+	assert_non_null(msg->dn);
 }
 
 int main(int argc, const char **argv)
 {
 	const struct CMUnitTest tests[] = {
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched,
+			test_filter_attrs_in_place_one_attr_matched,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched_of_many,
+			test_filter_attrs_in_place_one_attr_matched_of_many,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_attr_matched_attrs,
+			test_filter_attrs_in_place_two_attr_matched_attrs,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_attr_matched_one_attr,
+			test_filter_attrs_in_place_two_attr_matched_one_attr,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_dup_attr_matched_one_attr,
+			test_filter_attrs_in_place_two_dup_attr_matched_one_attr,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_dup_attr_matched_dup,
+			test_filter_attrs_in_place_two_dup_attr_matched_dup,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_dup_attr_matched_one_of_two,
+			test_filter_attrs_in_place_two_dup_attr_matched_one_of_two,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_dup_attr_matched_star,
+			test_filter_attrs_in_place_two_dup_attr_matched_star,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched_star,
+			test_filter_attrs_in_place_one_attr_matched_star,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_two_attr_matched_star,
+			test_filter_attrs_in_place_two_attr_matched_star,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched_star_no_dn,
+			test_filter_attrs_in_place_one_attr_matched_star_no_dn,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched_star_dn,
+			test_filter_attrs_in_place_one_attr_matched_star_dn,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_matched_dn,
+			test_filter_attrs_in_place_one_attr_matched_dn,
 			setup,
 			teardown),
 		cmocka_unit_test_setup_teardown(
-			test_filter_attrs_one_attr_empty_list,
+			test_filter_attrs_in_place_one_attr_empty_list,
 			setup,
 			teardown),
 	};