Attachment 14560 Details for Bug 13664 – Backported fix for version 4.9

Backported fix for version 4.9

bug-13664-v4-9.patch.txt (text/plain), 31.52 KB, created by Gary Lockyer on 2018-11-02 01:46:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Gary Lockyer

Created: 2018-11-02 01:46:33 UTC

Size: 31.52 KB

patch

obsolete

>From 749b145630febd232b50540f29b858443fe0e42a Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Tue, 23 Oct 2018 17:14:34 +1300
>Subject: [PATCH 1/5] dsdb group_audit: Test to replicate BUG 13664
>
>The group audit code incorrectly logs member additions and deletions.
>
>Thanks to metze for the debugging that isolated the issue, and for
>suggesting the fix to dn_compare.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13664
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit eeb4089dafc45277d8af19073ef9348451c1836a)
>---
> selftest/knownfail.d/bug13664                 |   1 +
> .../ldb_modules/tests/test_group_audit.c      | 259 +++++++++++++++++-
> 2 files changed, 248 insertions(+), 12 deletions(-)
> create mode 100644 selftest/knownfail.d/bug13664
>
>diff --git a/selftest/knownfail.d/bug13664 b/selftest/knownfail.d/bug13664
>new file mode 100644
>index 00000000000..6ffbf1297c3
>--- /dev/null
>+++ b/selftest/knownfail.d/bug13664
>@@ -0,0 +1 @@
>+^samba4.dsdb.samdb.ldb_modules.group_audit.test_log_membership_changes_removed
>diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>index 2c32b658a21..1c1bd29958e 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -91,6 +91,138 @@ void audit_log_human_text(
> 	audit_log_hr_debug_level = debug_level;
> }
> 
>+#define MAX_EXPECTED_MESSAGES 16
>+static struct json_object messages[MAX_EXPECTED_MESSAGES];
>+static size_t messages_sent = 0;
>+
>+void audit_message_send(
>+	struct imessaging_context *msg_ctx,
>+	const char *server_name,
>+	uint32_t message_type,
>+	struct json_object *message)
>+{
>+	messages[messages_sent].root = json_deep_copy(message->root);
>+	messages[messages_sent].error = message->error;
>+	messages_sent++;
>+}
>+
>+#define check_group_change_message(m, u, a)\
>+	_check_group_change_message(m, u, a, __FILE__, __LINE__);
>+/*
>+ * declare the internal cmocka cm_print_error so that we can output messages
>+ * in sub unit format
>+ */
>+void cm_print_error(const char * const format, ...);
>+
>+/*
>+ * Validate a group change JSON audit message
>+ *
>+ * It should contain 3 elements.
>+ * Have a type of "groupChange"
>+ * Have a groupChange element
>+ *
>+ * The group change element should have 10 elements.
>+ *
>+ * There should be a user element matching the expected value
>+ * There should be an action matching the expected value
>+ */
>+static void _check_group_change_message(
>+	const int message,
>+	const char *user,
>+	const char *action,
>+	const char *file,
>+	const int line)
>+{
>+	struct json_object json;
>+	json_t *audit = NULL;
>+	json_t *v = NULL;
>+	const char* value;
>+	json = messages[message];
>+
>+	/*
>+	 * Validate the root JSON element
>+	 * check the number of elements
>+	 */
>+	if (json_object_size(json.root) != 3) {
>+		cm_print_error(
>+		    "Unexpected number of elements in root %zu != %d\n",
>+		    json_object_size(json.root),
>+		    3);
>+		_fail(file, line);
>+	}
>+
>+	/*
>+	 * Check the type element
>+	 */
>+	v = json_object_get(json.root, "type");
>+	if (v == NULL) {
>+		cm_print_error( "No \"type\" element\n");
>+		_fail(file, line);
>+	}
>+
>+	value = json_string_value(v);
>+	if (strncmp("groupChange", value, strlen("groupChange") != 0)) {
>+		cm_print_error(
>+		    "Unexpected type \"%s\" != \"groupChange\"\n",
>+		    value);
>+		_fail(file, line);
>+	}
>+
>+
>+	audit = json_object_get(json.root, "groupChange");
>+	if (audit == NULL) {
>+		cm_print_error("No groupChange element\n");
>+		_fail(file, line);
>+	}
>+
>+	/*
>+	 * Validate the groupChange element
>+	 */
>+	if (json_object_size(audit) != 10) {
>+		cm_print_error(
>+		    "Unexpected number of elements in groupChange "
>+		    "%zu != %d\n",
>+		    json_object_size(audit),
>+		    10);
>+		_fail(file, line);
>+	}
>+	/*
>+	 * Validate the user element
>+	 */
>+	v = json_object_get(audit, "user");
>+	if (v == NULL) {
>+		cm_print_error( "No user element\n");
>+		_fail(file, line);
>+	}
>+
>+	value = json_string_value(v);
>+	if (strncmp(user, value, strlen(user) != 0)) {
>+		cm_print_error(
>+		    "Unexpected user name \"%s\" != \"%s\"\n",
>+		    value,
>+		    user);
>+		_fail(file, line);
>+	}
>+
>+	/*
>+	 * Validate the action element
>+	 */
>+	v = json_object_get(audit, "action");
>+	if (v == NULL) {
>+		cm_print_error( "No action element\n");
>+		_fail(file, line);
>+	}
>+
>+	value = json_string_value(v);
>+	if (strncmp(action, value, strlen(action) != 0)) {
>+		print_error(
>+		    "Unexpected action \"%s\" != \"%s\"\n",
>+		    value,
>+		    action);
>+		_fail(file, line);
>+	}
>+}
>+
> /*
>  * Test helper to check ISO 8601 timestamps for validity
>  */
>@@ -593,17 +725,6 @@ static void audit_message_send_init(void) {
> 	audit_message_send_message_type = 0;
> 	audit_message_send_message = NULL;
> }
>-void audit_message_send(
>-	struct imessaging_context *msg_ctx,
>-	const char *server_name,
>-	uint32_t message_type,
>-	struct json_object *message)
>-{
>-	audit_message_send_msg_ctx = msg_ctx;
>-	audit_message_send_server_name = server_name;
>-	audit_message_send_message_type = message_type;
>-	audit_message_send_message = message;
>-}
> 
> static void test_audit_group_json(void **state)
> {
>@@ -700,7 +821,121 @@ static void test_audit_group_json(void **state)
> 
> 	json_free(&json);
> 	TALLOC_FREE(ctx);
>+}
>+
>+static void setup_ldb(
>+	TALLOC_CTX *ctx,
>+	struct ldb_context **ldb,
>+	struct ldb_module **module,
>+	const char *ip,
>+	const char *session,
>+	const char *sid)
>+{
>+	struct tsocket_address *ts = NULL;
>+	struct audit_context *context = NULL;
>+
>+	*ldb = ldb_init(ctx, NULL);
>+	ldb_register_samba_handlers(*ldb);
>+
>+
>+	*module = talloc_zero(ctx, struct ldb_module);
>+	(*module)->ldb = *ldb;
>+
>+	context = talloc_zero(*module, struct audit_context);
>+	context->send_events = true;
>+	context->msg_ctx = (struct imessaging_context *) 0x01;
>+
>+	ldb_module_set_private(*module, context);
>+
>+	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
>+	ldb_set_opaque(*ldb, "remoteAddress", ts);
> 
>+	add_session_data(ctx, *ldb, session, sid);
>+}
>+
>+/*
>+ * Test the removal of a user from a group.
>+ *
>+ * The new element contains one group member
>+ * The old element contains two group member
>+ *
>+ * Expect to see the removed entry logged.
>+ *
>+ * This test confirms bug 13664
>+ * https://bugzilla.samba.org/show_bug.cgi?id=13664
>+ */
>+static void test_log_membership_changes_removed(void **state)
>+{
>+	struct ldb_context *ldb = NULL;
>+	struct ldb_module  *module = NULL;
>+	const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779";
>+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const IP = "127.0.0.1";
>+	struct ldb_request *req = NULL;
>+	struct ldb_message_element *new_el = NULL;
>+	struct ldb_message_element *old_el = NULL;
>+	int status = 0;
>+	TALLOC_CTX *ctx = talloc_new(NULL);
>+
>+	setup_ldb(ctx, &ldb, &module, IP, SESSION, SID);
>+
>+	/*
>+	 * Build the ldb_request
>+	 */
>+	req = talloc_zero(ctx, struct ldb_request);
>+	req->operation =  LDB_ADD;
>+	add_transaction_id(req, TRANSACTION);
>+
>+	/*
>+	 * Populate the new elements, containing one entry.
>+	 * Indicating that one element has been removed
>+	 */
>+	new_el = talloc_zero(ctx, struct ldb_message_element);
>+	new_el->num_values = 1;
>+	new_el->values = talloc_zero_array(ctx, DATA_BLOB, 1);
>+	new_el->values[0] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * Populate the old elements, with two elements
>+	 * The first is the same as the one in new elements.
>+	 */
>+	old_el = talloc_zero(ctx, struct ldb_message_element);
>+	old_el->num_values = 2;
>+	old_el->values = talloc_zero_array(ctx, DATA_BLOB, 2);
>+	old_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	old_el->values[1] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * call log_membership_changes
>+	 */
>+	messages_sent = 0;
>+	log_membership_changes(module, req, new_el, old_el, status);
>+
>+	/*
>+	 * Check the results
>+	 */
>+	assert_int_equal(1, messages_sent);
>+
>+	check_group_change_message(
>+	    0,
>+	    "cn=grpadttstuser01,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Removed");
>+
>+	/*
>+	 * Clean up
>+	 */
>+	json_free(&messages[0]);
>+	TALLOC_FREE(ctx);
> }
> 
> static void test_place_holder(void **state)
>@@ -726,7 +961,7 @@ int main(void) {
> 		cmocka_unit_test(test_get_parsed_dns),
> 		cmocka_unit_test(test_dn_compare),
> 		cmocka_unit_test(test_get_primary_group_dn),
>-
>+		cmocka_unit_test(test_log_membership_changes_removed),
> 	};
> 
> 	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
>-- 
>2.17.1
>
>
>From fbd2320ee7b712548dd82b1570b1e5409c1f1971 Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Thu, 25 Oct 2018 10:52:27 +1300
>Subject: [PATCH 2/5] dsdb group audit: align dn_compare with memcmp
>
>Rename the parameter names and adjust the  return codes from dn_compare
>so that:
>dn_compare(a, b) =>
>
>LESS_THAN means a is less than b.
>GREATER_THAN means a is greater than b.
>
>Thanks to metze for suggesting the correct semantics for dn_compare
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13664
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 8420a4d0fddd71af608635a707ef20f37fa9b627)
>---
> selftest/knownfail.d/bug13664                 |  1 -
> source4/dsdb/samdb/ldb_modules/group_audit.c  | 31 ++++++++++---------
> .../ldb_modules/tests/test_group_audit.c      |  4 +--
> 3 files changed, 18 insertions(+), 18 deletions(-)
> delete mode 100644 selftest/knownfail.d/bug13664
>
>diff --git a/selftest/knownfail.d/bug13664 b/selftest/knownfail.d/bug13664
>deleted file mode 100644
>index 6ffbf1297c3..00000000000
>--- a/selftest/knownfail.d/bug13664
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba4.dsdb.samdb.ldb_modules.group_audit.test_log_membership_changes_removed
>diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
>index 47929aa8764..eb592adc336 100644
>--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
>@@ -252,35 +252,36 @@ enum dn_compare_result {
> 	GREATER_THAN
> };
> /*
>- * @brief compare parsed_dns
>+ * @brief compare parsed_dn, using GUID ordering
>  *
>- * Compare two parsed_dn structures, parsing the entries if necessary.
>+ * Compare two parsed_dn structures, using GUID ordering.
>  * To avoid the overhead of parsing the DN's this function does a binary
>- * compare first. Only parsing the DN's they are not equal at a binary level.
>+ * compare first. The DN's tre only parsed if they are not equal at a binary
>+ * level.
>  *
>  * @param ctx talloc context that will own the parsed dsdb_dn
>  * @param ldb ldb_context
>- * @param old_val The old value
>- * @param new_val The old value
>+ * @param dn1 The first dn
>+ * @param dn2 The second dn
>  *
>  * @return BINARY_EQUAL values are equal at a binary level
>  *         EQUAL        DN's are equal but the meta data is different
>- *         LESS_THAN    old value < new value
>- *         GREATER_THAN old value > new value
>+ *         LESS_THAN    dn1's GUID is less than dn2's GUID
>+ *         GREATER_THAN dn1's GUID is greater than  dn2's GUID
>  *
>  */
> static enum dn_compare_result dn_compare(
> 	TALLOC_CTX *mem_ctx,
> 	struct ldb_context *ldb,
>-	struct parsed_dn *old_val,
>-	struct parsed_dn *new_val) {
>+	struct parsed_dn *dn1,
>+	struct parsed_dn *dn2) {
> 
> 	int res = 0;
> 
> 	/*
> 	 * Do a binary compare first to avoid unnecessary parsing
> 	 */
>-	if (data_blob_cmp(new_val->v, old_val->v) == 0) {
>+	if (data_blob_cmp(dn1->v, dn2->v) == 0) {
> 		/*
> 		 * Values are equal at a binary level so no need
> 		 * for further processing
>@@ -292,22 +293,22 @@ static enum dn_compare_result dn_compare(
> 	 * do a GUID ordering compare. To do this we will need to ensure
> 	 * that the dn's have been parsed.
> 	 */
>-	if (old_val->dsdb_dn == NULL) {
>+	if (dn1->dsdb_dn == NULL) {
> 		really_parse_trusted_dn(
> 			mem_ctx,
> 			ldb,
>-			old_val,
>+			dn1,
> 			LDB_SYNTAX_DN);
> 	}
>-	if (new_val->dsdb_dn == NULL) {
>+	if (dn2->dsdb_dn == NULL) {
> 		really_parse_trusted_dn(
> 			mem_ctx,
> 			ldb,
>-			new_val,
>+			dn2,
> 			LDB_SYNTAX_DN);
> 	}
> 
>-	res = ndr_guid_compare(&new_val->guid, &old_val->guid);
>+	res = ndr_guid_compare(&dn1->guid, &dn2->guid);
> 	if (res < 0) {
> 		return LESS_THAN;
> 	} else if (res == 0) {
>diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>index 1c1bd29958e..47ffb4be7a8 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -586,7 +586,7 @@ static void test_dn_compare(void **state)
> 	b->v = &bb;
> 
> 	res = dn_compare(ctx, ldb, a, b);
>-	assert_int_equal(GREATER_THAN, res);
>+	assert_int_equal(LESS_THAN, res);
> 	/*
> 	 * DN's should have been parsed
> 	 */
>@@ -614,7 +614,7 @@ static void test_dn_compare(void **state)
> 	b->v = &bb;
> 
> 	res = dn_compare(ctx, ldb, a, b);
>-	assert_int_equal(LESS_THAN, res);
>+	assert_int_equal(GREATER_THAN, res);
> 	/*
> 	 * DN's should have been parsed
> 	 */
>-- 
>2.17.1
>
>
>From 069c22799c369516fa53e46327e6b52ee0d6edf9 Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Thu, 25 Oct 2018 13:28:09 +1300
>Subject: [PATCH 3/5] dsdb group audit tests: check_timestamp improve
> diagnostics
>
>Change check_timestamp to display the expected, actual along with the
>line and name of the failing test, rather than the line in
>check_timestamp.
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit e2970887140d558c6359fd9b3f8c2a4c26d2cf35)
>---
> .../ldb_modules/tests/test_group_audit.c      | 36 ++++++++++++++++---
> 1 file changed, 32 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>index 47ffb4be7a8..13e8e2985f6 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -223,10 +223,16 @@ static void _check_group_change_message(
> 	}
> }
> 
>+#define check_timestamp(b, t)\
>+	_check_timestamp(b, t, __FILE__, __LINE__);
> /*
>  * Test helper to check ISO 8601 timestamps for validity
>  */
>-static void check_timestamp(time_t before, const char *timestamp)
>+static void _check_timestamp(
>+	time_t before,
>+	const char *timestamp,
>+	const char *file,
>+	const int line)
> {
> 	int rc;
> 	int usec, tz;
>@@ -262,10 +268,32 @@ static void check_timestamp(time_t before, const char *timestamp)
> 	actual = mktime(&tm);
> 
> 	/*
>-	 * The timestamp should be before <= actual <= after
>+	 * The time stamp should be before <= actual <= after
> 	 */
>-	assert_true(difftime(actual, before) >= 0);
>-	assert_true(difftime(after, actual) >= 0);
>+	if (difftime(actual, before) < 0) {
>+		char buffer[40];
>+		strftime(buffer,
>+			 sizeof(buffer)-1,
>+			 "%Y-%m-%dT%T",
>+			 localtime(&before));
>+		cm_print_error(
>+		    "time stamp \"%s\" is before start time \"%s\"\n",
>+		    timestamp,
>+		    buffer);
>+		_fail(file, line);
>+	}
>+	if (difftime(after, actual) < 0) {
>+		char buffer[40];
>+		strftime(buffer,
>+			 sizeof(buffer)-1,
>+			 "%Y-%m-%dT%T",
>+			 localtime(&after));
>+		cm_print_error(
>+		    "time stamp \"%s\" is after finish time \"%s\"\n",
>+		    timestamp,
>+		    buffer);
>+		_fail(file, line);
>+	}
> }
> 
> /*
>-- 
>2.17.1
>
>
>From 9d61b9461ab5437ae5d966998c5dd65c7233dab9 Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Thu, 25 Oct 2018 14:38:31 +1300
>Subject: [PATCH 4/5] dsdb group audit tests: check_version improve diagnostics
>
>Change check_version to display the expected, actual along with the
>line and name of the failing test, rather than the line in check_version
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit c952fc1273397c04fddf177bcd809551d6324bdd)
>---
> .../ldb_modules/tests/test_group_audit.c      | 60 ++++++++++++++++---
> 1 file changed, 53 insertions(+), 7 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>index 13e8e2985f6..ce7e485b9b0 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -296,23 +296,69 @@ static void _check_timestamp(
> 	}
> }
> 
>+#define check_version(v, m, n)\
>+	_check_version(v, m, n, __FILE__, __LINE__);
> /*
>  * Test helper to validate a version object.
>  */
>-static void check_version(struct json_t *version, int major, int minor)
>+static void _check_version(
>+	struct json_t *version,
>+	int major,
>+	int minor,
>+	const char* file,
>+	const int line)
> {
> 	struct json_t *v = NULL;
>+	int value;
>+
>+	if (!json_is_object(version)) {
>+		cm_print_error("version is not a JSON object\n");
>+		_fail(file, line);
>+	}
> 
>-	assert_true(json_is_object(version));
>-	assert_int_equal(2, json_object_size(version));
>+	if (json_object_size(version) != 2) {
>+		cm_print_error(
>+		    "Unexpected number of elements in version %zu != %d\n",
>+		    json_object_size(version),
>+		    2);
>+		_fail(file, line);
>+	}
> 
>+	/*
>+	 * Validate the major version number element
>+	 */
> 	v = json_object_get(version, "major");
>-	assert_non_null(v);
>-	assert_int_equal(major, json_integer_value(v));
>+	if (v == NULL) {
>+		cm_print_error( "No major element\n");
>+		_fail(file, line);
>+	}
> 
>+	value = json_integer_value(v);
>+	if (value != major) {
>+		print_error(
>+		    "Unexpected major version number \"%d\" != \"%d\"\n",
>+		    value,
>+		    major);
>+		_fail(file, line);
>+	}
>+
>+	/*
>+	 * Validate the minor version number element
>+	 */
> 	v = json_object_get(version, "minor");
>-	assert_non_null(v);
>-	assert_int_equal(minor, json_integer_value(v));
>+	if (v == NULL) {
>+		cm_print_error( "No minor element\n");
>+		_fail(file, line);
>+	}
>+
>+	value = json_integer_value(v);
>+	if (value != minor) {
>+		print_error(
>+		    "Unexpected minor version number \"%d\" != \"%d\"\n",
>+		    value,
>+		    minor);
>+		_fail(file, line);
>+	}
> }
> 
> /*
>-- 
>2.17.1
>
>
>From 8e269cbd3498381e379ddde8c6f54d6528c223ee Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Thu, 25 Oct 2018 10:52:55 +1300
>Subject: [PATCH 5/5] dsdb group audit tests: log_membership_changes extra
> tests
>
>Add extra tests to ensure better test coverage of log_membership_changes
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit aeef8b41fa03a32859f824f4a09560ad83bd2b50)
>---
> .../ldb_modules/tests/test_group_audit.c      | 388 ++++++++++++++++--
> 1 file changed, 361 insertions(+), 27 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>index ce7e485b9b0..75e14fed2e2 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -71,14 +71,6 @@ const char *audit_log_hr_message = NULL;
> int audit_log_hr_debug_class = 0;
> int audit_log_hr_debug_level = 0;
> 
>-static void audit_log_hr_init(void)
>-{
>-	audit_log_hr_prefix = NULL;
>-	audit_log_hr_message = NULL;
>-	audit_log_hr_debug_class = 0;
>-	audit_log_hr_debug_level = 0;
>-}
>-
> void audit_log_human_text(
> 	const char *prefix,
> 	const char *message,
>@@ -765,13 +757,6 @@ struct json_object *audit_log_json_message = NULL;
> int audit_log_json_debug_class = 0;
> int audit_log_json_debug_level = 0;
> 
>-static void audit_log_json_init(void)
>-{
>-	audit_log_json_prefix = NULL;
>-	audit_log_json_message = NULL;
>-	audit_log_json_debug_class = 0;
>-	audit_log_json_debug_level = 0;
>-}
> 
> void audit_log_json(
> 	const char* prefix,
>@@ -793,13 +778,6 @@ const char *audit_message_send_server_name = NULL;
> uint32_t audit_message_send_message_type = 0;
> struct json_object *audit_message_send_message = NULL;
> 
>-static void audit_message_send_init(void) {
>-	audit_message_send_msg_ctx = NULL;
>-	audit_message_send_server_name = NULL;
>-	audit_message_send_message_type = 0;
>-	audit_message_send_message = NULL;
>-}
>-
> static void test_audit_group_json(void **state)
> {
> 	struct ldb_context *ldb = NULL;
>@@ -1012,11 +990,364 @@ static void test_log_membership_changes_removed(void **state)
> 	TALLOC_FREE(ctx);
> }
> 
>-static void test_place_holder(void **state)
>+/* test log_membership_changes
>+ *
>+ * old contains 2 user dn's
>+ * new contains 0 user dn's
>+ *
>+ * Expect to see both dn's logged as deleted.
>+ */
>+static void test_log_membership_changes_remove_all(void **state)
>+{
>+	struct ldb_context *ldb = NULL;
>+	struct ldb_module  *module = NULL;
>+	const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779";
>+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const IP = "127.0.0.1";
>+	struct ldb_request *req = NULL;
>+	struct ldb_message_element *new_el = NULL;
>+	struct ldb_message_element *old_el = NULL;
>+	int status = 0;
>+	TALLOC_CTX *ctx = talloc_new(NULL);
>+
>+	setup_ldb(ctx, &ldb, &module, IP, SESSION, SID);
>+
>+	/*
>+	 * Build the ldb_request
>+	 */
>+	req = talloc_zero(ctx, struct ldb_request);
>+	req->operation =  LDB_ADD;
>+	add_transaction_id(req, TRANSACTION);
>+
>+	/*
>+	 * Populate the new elements, containing no entries.
>+	 * Indicating that all elements have been removed
>+	 */
>+	new_el = talloc_zero(ctx, struct ldb_message_element);
>+	new_el->num_values = 0;
>+	new_el->values = NULL;
>+
>+	/*
>+	 * Populate the old elements, with two elements
>+	 */
>+	old_el = talloc_zero(ctx, struct ldb_message_element);
>+	old_el->num_values = 2;
>+	old_el->values = talloc_zero_array(ctx, DATA_BLOB, 2);
>+	old_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	old_el->values[1] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * call log_membership_changes
>+	 */
>+	messages_sent = 0;
>+	log_membership_changes( module, req, new_el, old_el, status);
>+
>+	/*
>+	 * Check the results
>+	 */
>+	assert_int_equal(2, messages_sent);
>+
>+	check_group_change_message(
>+	    0,
>+	    "cn=grpadttstuser01,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Removed");
>+
>+	check_group_change_message(
>+	    1,
>+	    "CN=testuser131953,CN=Users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Removed");
>+
>+	/*
>+	 * Clean up
>+	 */
>+	json_free(&messages[0]);
>+	json_free(&messages[1]);
>+	TALLOC_FREE(ctx);
>+}
>+
>+/* test log_membership_changes
>+ *
>+ * Add an entry.
>+ *
>+ * Old entries contains a single user dn
>+ * New entries contains 2 user dn's, one matching the dn in old entries
>+ *
>+ * Should see a single new entry logged.
>+ */
>+static void test_log_membership_changes_added(void **state)
>+{
>+	struct ldb_context *ldb = NULL;
>+	struct ldb_module  *module = NULL;
>+	const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779";
>+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const IP = "127.0.0.1";
>+	struct ldb_request *req = NULL;
>+	struct ldb_message_element *new_el = NULL;
>+	struct ldb_message_element *old_el = NULL;
>+	int status = 0;
>+	TALLOC_CTX *ctx = talloc_new(NULL);
>+
>+	setup_ldb(ctx, &ldb, &module, IP, SESSION, SID);
>+
>+	/*
>+	 * Build the ldb_request
>+	 */
>+	req = talloc_zero(ctx, struct ldb_request);
>+	req->operation =  LDB_ADD;
>+	add_transaction_id(req, TRANSACTION);
>+
>+	/*
>+	 * Populate the old elements adding a single entry.
>+	 */
>+	old_el = talloc_zero(ctx, struct ldb_message_element);
>+	old_el->num_values = 1;
>+	old_el->values = talloc_zero_array(ctx, DATA_BLOB, 1);
>+	old_el->values[0] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * Populate the new elements adding two entries. One matches the entry
>+	 * in old elements. We expect to see the other element logged as Added
>+	 */
>+	new_el = talloc_zero(ctx, struct ldb_message_element);
>+	new_el->num_values = 2;
>+	new_el->values = talloc_zero_array(ctx, DATA_BLOB, 2);
>+	new_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	new_el->values[1] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * call log_membership_changes
>+	 */
>+	messages_sent = 0;
>+	log_membership_changes( module, req, new_el, old_el, status);
>+
>+	/*
>+	 * Check the results
>+	 */
>+	assert_int_equal(1, messages_sent);
>+
>+	check_group_change_message(
>+	    0,
>+	    "cn=grpadttstuser01,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Added");
>+
>+	/*
>+	 * Clean up
>+	 */
>+	json_free(&messages[0]);
>+	TALLOC_FREE(ctx);
>+}
>+
>+/*
>+ * test log_membership_changes.
>+ *
>+ * Old entries is empty
>+ * New entries contains 2 user dn's
>+ *
>+ * Expect to see log messages for two added users
>+ */
>+static void test_log_membership_changes_add_to_empty(void **state)
>+{
>+	struct ldb_context *ldb = NULL;
>+	struct ldb_module  *module = NULL;
>+	const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779";
>+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const IP = "127.0.0.1";
>+	struct ldb_request *req = NULL;
>+	struct ldb_message_element *new_el = NULL;
>+	struct ldb_message_element *old_el = NULL;
>+	int status = 0;
>+	TALLOC_CTX *ctx = talloc_new(NULL);
>+
>+	/*
>+	 * Set up the ldb and module structures
>+	 */
>+	setup_ldb(ctx, &ldb, &module, IP, SESSION, SID);
>+
>+	/*
>+	 * Build the request structure
>+	 */
>+	req = talloc_zero(ctx, struct ldb_request);
>+	req->operation =  LDB_ADD;
>+	add_transaction_id(req, TRANSACTION);
>+
>+	/*
>+	 * Build the element containing the old values
>+	 */
>+	old_el = talloc_zero(ctx, struct ldb_message_element);
>+	old_el->num_values = 0;
>+	old_el->values = NULL;
>+
>+	/*
>+	 * Build the element containing the new values
>+	 */
>+	new_el = talloc_zero(ctx, struct ldb_message_element);
>+	new_el->num_values = 2;
>+	new_el->values = talloc_zero_array(ctx, DATA_BLOB, 2);
>+	new_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	new_el->values[1] = data_blob_string_const(
>+		"<GUID=081519b5-a709-44a0-bc95-dd4bfe809bf8>;"
>+		"CN=testuser131953,CN=Users,DC=addom,DC=samba,"
>+		"DC=example,DC=com");
>+
>+	/*
>+	 * Run log membership changes
>+	 */
>+	messages_sent = 0;
>+	log_membership_changes( module, req, new_el, old_el, status);
>+	assert_int_equal(2, messages_sent);
>+
>+	check_group_change_message(
>+	    0,
>+	    "cn=grpadttstuser01,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Added");
>+
>+	check_group_change_message(
>+	    1,
>+            "CN=testuser131953,CN=Users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Added");
>+
>+	json_free(&messages[0]);
>+	json_free(&messages[1]);
>+	TALLOC_FREE(ctx);
>+}
>+
>+/* test log_membership_changes
>+ *
>+ * Test Replication Meta Data flag handling.
>+ *
>+ * 4 entries in old and new entries with their RMD_FLAGS set as below:
>+ *    old   new
>+ * 1)  0     0    Not logged
>+ * 2)  1     1    Both deleted, no change not logged
>+ * 3)  0     1    New tagged as deleted, log as deleted
>+ * 4)  1     0    Has been undeleted, log as an add
>+ *
>+ * Should see a single new entry logged.
>+ */
>+static void test_log_membership_changes_rmd_flags(void **state)
> {
>-	audit_log_json_init();
>-	audit_log_hr_init();
>-	audit_message_send_init();
>+	struct ldb_context *ldb = NULL;
>+	struct ldb_module  *module = NULL;
>+	const char * const SID = "S-1-5-21-2470180966-3899876309-2637894779";
>+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
>+	const char * const IP = "127.0.0.1";
>+	struct ldb_request *req = NULL;
>+	struct ldb_message_element *new_el = NULL;
>+	struct ldb_message_element *old_el = NULL;
>+	int status = 0;
>+	TALLOC_CTX *ctx = talloc_new(NULL);
>+
>+	setup_ldb(ctx, &ldb, &module, IP, SESSION, SID);
>+
>+	/*
>+	 * Build the ldb_request
>+	 */
>+	req = talloc_zero(ctx, struct ldb_request);
>+	req->operation =  LDB_ADD;
>+	add_transaction_id(req, TRANSACTION);
>+
>+	/*
>+	 * Populate the old elements.
>+	 */
>+	old_el = talloc_zero(ctx, struct ldb_message_element);
>+	old_el->num_values = 4;
>+	old_el->values = talloc_zero_array(ctx, DATA_BLOB, 4);
>+	old_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"<RMD_FLAGS=0>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	old_el->values[1] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681c>;"
>+		"<RMD_FLAGS=1>;"
>+		"cn=grpadttstuser02,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	old_el->values[2] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681d>;"
>+		"<RMD_FLAGS=0>;"
>+		"cn=grpadttstuser03,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	old_el->values[3] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681e>;"
>+		"<RMD_FLAGS=1>;"
>+		"cn=grpadttstuser04,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+
>+	/*
>+	 * Populate the new elements.
>+	 */
>+	new_el = talloc_zero(ctx, struct ldb_message_element);
>+	new_el->num_values = 4;
>+	new_el->values = talloc_zero_array(ctx, DATA_BLOB, 4);
>+	new_el->values[0] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681b>;"
>+		"<RMD_FLAGS=0>;"
>+		"cn=grpadttstuser01,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	new_el->values[1] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681c>;"
>+		"<RMD_FLAGS=1>;"
>+		"cn=grpadttstuser02,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	new_el->values[2] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681d>;"
>+		"<RMD_FLAGS=1>;"
>+		"cn=grpadttstuser03,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+	new_el->values[3] = data_blob_string_const(
>+		"<GUID=cb8c2777-dcf5-419c-ab57-f645dbdf681e>;"
>+		"<RMD_FLAGS=0>;"
>+		"cn=grpadttstuser04,cn=users,DC=addom,"
>+		"DC=samba,DC=example,DC=com");
>+
>+	/*
>+	 * call log_membership_changes
>+	 */
>+	messages_sent = 0;
>+	log_membership_changes( module, req, new_el, old_el, status);
>+
>+	/*
>+	 * Check the results
>+	 */
>+	assert_int_equal(2, messages_sent);
>+
>+	check_group_change_message(
>+	    0,
>+	    "cn=grpadttstuser03,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Removed");
>+	check_group_change_message(
>+	    1,
>+	    "cn=grpadttstuser04,cn=users,DC=addom,DC=samba,DC=example,DC=com",
>+	    "Added");
>+
>+	/*
>+	 * Clean up
>+	 */
>+	json_free(&messages[0]);
>+	json_free(&messages[1]);
>+	TALLOC_FREE(ctx);
> }
> 
> /*
>@@ -1029,13 +1360,16 @@ static void test_place_holder(void **state)
> int main(void) {
> 	const struct CMUnitTest tests[] = {
> 		cmocka_unit_test(test_audit_group_json),
>-		cmocka_unit_test(test_place_holder),
> 		cmocka_unit_test(test_get_transaction_id),
> 		cmocka_unit_test(test_audit_group_hr),
> 		cmocka_unit_test(test_get_parsed_dns),
> 		cmocka_unit_test(test_dn_compare),
> 		cmocka_unit_test(test_get_primary_group_dn),
> 		cmocka_unit_test(test_log_membership_changes_removed),
>+		cmocka_unit_test(test_log_membership_changes_remove_all),
>+		cmocka_unit_test(test_log_membership_changes_added),
>+		cmocka_unit_test(test_log_membership_changes_add_to_empty),
>+		cmocka_unit_test(test_log_membership_changes_rmd_flags),
> 	};
> 
> 	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
>-- 
>2.17.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 13664: 14548 | 14560