[PATCH] Use correct memory context for messaging context for ldb audit

Andrew Bartlett abartlet at samba.org
Mon Jun 25 04:50:36 UTC 2018


G'Day Gary,

The attached patch ensures the messaging context you use to send audit
messages to the test script is destroyed at the end of the ldb
lifetime, not the overall event context.

For now, we continue to feed the messaging code the outside event
context.  Hopefully that is OK, because it won't be calling
tevent_loop(), and this saves setting it back up each transaction.

Also I've re-named some structures and variables in a second patch
here, to be more consistent with the rest of Samba's modules.  Please
take a look.

CI: https://gitlab.com/catalyst-samba/samba/pipelines/24476085

Please review and push!

Thanks,

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From c85ee72a43c28286cef8b987dba77f846cafdcf7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 25 Jun 2018 15:42:42 +1200
Subject: [PATCH 1/4] dsdb: Use correct memory context for
 imessaging_client_init() in audit logging

This is only used for selftest, to send out the log messages for checking.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/audit_log.c   | 4 +++-
 source4/dsdb/samdb/ldb_modules/group_audit.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index 80914cb8350..6edef618509 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -1565,7 +1565,9 @@ static int log_init(struct ldb_module *module)
 	if (sdb_events || pwd_events) {
 		context->send_samdb_events = sdb_events;
 		context->send_password_events = pwd_events;
-		context->msg_ctx = imessaging_client_init(ec, lp_ctx, ec);
+		context->msg_ctx = imessaging_client_init(context,
+							  lp_ctx,
+							  ec);
 	}
 
 	ldb_module_set_private(module, context);
diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
index dc586777736..247d79a41ba 100644
--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
@@ -1340,7 +1340,9 @@ static int group_init(struct ldb_module *module)
 
 	if (lp_ctx && lpcfg_dsdb_group_change_notification(lp_ctx)) {
 		context->send_events = true;
-		context->msg_ctx = imessaging_client_init(ec, lp_ctx, ec);
+		context->msg_ctx = imessaging_client_init(context,
+							  lp_ctx,
+							  ec);
 	}
 
 	ldb_module_set_private(module, context);
-- 
2.11.0


From bf8fa58cfe1807c4d7306a90b6f73df3d4768a86 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 25 Jun 2018 16:23:00 +1200
Subject: [PATCH 2/4] dsdb: Use customary variable names for audit event
 contexts

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/audit_log.c   | 4 ++--
 source4/dsdb/samdb/ldb_modules/group_audit.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index 6edef618509..c9b2adfbef4 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -1549,7 +1549,7 @@ static int log_init(struct ldb_module *module)
 	struct loadparm_context *lp_ctx
 		= talloc_get_type_abort(ldb_get_opaque(ldb, "loadparm"),
 					struct loadparm_context);
-	struct tevent_context *ec = ldb_get_event_context(ldb);
+	struct tevent_context *ev = ldb_get_event_context(ldb);
 	bool sdb_events = false;
 	bool pwd_events = false;
 
@@ -1567,7 +1567,7 @@ static int log_init(struct ldb_module *module)
 		context->send_password_events = pwd_events;
 		context->msg_ctx = imessaging_client_init(context,
 							  lp_ctx,
-							  ec);
+							  ev);
 	}
 
 	ldb_module_set_private(module, context);
diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
index 247d79a41ba..6da7068df75 100644
--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
@@ -1331,7 +1331,7 @@ static int group_init(struct ldb_module *module)
 		= talloc_get_type_abort(
 			ldb_get_opaque(ldb, "loadparm"),
 			struct loadparm_context);
-	struct tevent_context *ec = ldb_get_event_context(ldb);
+	struct tevent_context *ev = ldb_get_event_context(ldb);
 
 	context = talloc_zero(module, struct audit_context);
 	if (context == NULL) {
@@ -1342,7 +1342,7 @@ static int group_init(struct ldb_module *module)
 		context->send_events = true;
 		context->msg_ctx = imessaging_client_init(context,
 							  lp_ctx,
-							  ec);
+							  ev);
 	}
 
 	ldb_module_set_private(module, context);
-- 
2.11.0


From a0a2cd0321552c7638ebed5571c225d29fb35000 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 25 Jun 2018 16:43:38 +1200
Subject: [PATCH 3/4] dsdb: Use customary variable names for the audit private
 context

The variable name "ac" typically implies the async context, and the long-life
private context is normally denoted private, not context.  This aligns better
with other modules.

talloc_get_type_abort() is now also used.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/audit_log.c         | 152 +++++++++++----------
 .../dsdb/samdb/ldb_modules/tests/test_audit_log.c  | 110 +++++++--------
 2 files changed, 137 insertions(+), 125 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index c9b2adfbef4..23020b77738 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -68,7 +68,7 @@
 /*
  * Private data for the module, stored in the ldb_module private data
  */
-struct audit_context {
+struct audit_private {
 	/*
 	 * Should details of database operations be sent over the
 	 * messaging bus.
@@ -191,9 +191,9 @@ static struct json_object operation_json(
 	const char* operation = NULL;
 	const struct GUID *unique_session_token = NULL;
 	const struct ldb_message *message = NULL;
-	struct audit_context *ac = talloc_get_type(
-		ldb_module_get_private(module),
-		struct audit_context);
+	struct audit_private *audit_private
+		= talloc_get_type_abort(ldb_module_get_private(module),
+					struct audit_private);
 
 	ldb = ldb_module_get_ctx(module);
 
@@ -220,7 +220,9 @@ static struct json_object operation_json(
 	json_add_bool(&audit, "performedAsSystem", as_system);
 	json_add_sid(&audit, "userSid", sid);
 	json_add_string(&audit, "dn", dn);
-	json_add_guid(&audit, "transactionId", &ac->transaction_guid);
+	json_add_guid(&audit,
+		      "transactionId",
+		      &audit_private->transaction_guid);
 	json_add_guid(&audit, "sessionId", unique_session_token);
 
 	message = dsdb_audit_get_message(request);
@@ -258,9 +260,9 @@ static struct json_object replicated_update_json(
 {
 	struct json_object wrapper;
 	struct json_object audit;
-	struct audit_context *ac = talloc_get_type(
-		ldb_module_get_private(module),
-		struct audit_context);
+	struct audit_private *audit_private
+		= talloc_get_type_abort(ldb_module_get_private(module),
+					struct audit_private);
 	struct dsdb_extended_replicated_objects *ro = talloc_get_type(
 		request->op.extended.data,
 		struct dsdb_extended_replicated_objects);
@@ -274,7 +276,9 @@ static struct json_object replicated_update_json(
 	json_add_version(&audit, REPLICATION_MAJOR, REPLICATION_MINOR);
 	json_add_int(&audit, "statusCode", reply->error);
 	json_add_string(&audit, "status", ldb_strerror(reply->error));
-	json_add_guid(&audit, "transactionId", &ac->transaction_guid);
+	json_add_guid(&audit,
+		      "transactionId",
+		      &audit_private->transaction_guid);
 	json_add_int(&audit, "objectCount", ro->num_objects);
 	json_add_int(&audit, "linkCount", ro->linked_attributes_count);
 	json_add_string(&audit, "partitionDN", partition_dn);
@@ -322,9 +326,9 @@ static struct json_object password_change_json(
 	const struct tsocket_address *remote = NULL;
 	const char* action = NULL;
 	const struct GUID *unique_session_token = NULL;
-	struct audit_context *ac = talloc_get_type(
-		ldb_module_get_private(module),
-		struct audit_context);
+	struct audit_private *audit_private
+		= talloc_get_type_abort(ldb_module_get_private(module),
+					struct audit_private);
 
 
 	ldb = ldb_module_get_ctx(module);
@@ -343,7 +347,9 @@ static struct json_object password_change_json(
 	json_add_sid(&audit, "userSid", sid);
 	json_add_string(&audit, "dn", dn);
 	json_add_string(&audit, "action", action);
-	json_add_guid(&audit, "transactionId", &ac->transaction_guid);
+	json_add_guid(&audit,
+		      "transactionId",
+		      &audit_private->transaction_guid);
 	json_add_guid(&audit, "sessionId", unique_session_token);
 
 	wrapper = json_new_object();
@@ -815,9 +821,9 @@ static void log_standard_operation(
 
 	const struct ldb_message *message = dsdb_audit_get_message(request);
 	bool password_changed = has_password_changed(message);
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
@@ -853,7 +859,8 @@ static void log_standard_operation(
 	}
 #ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_AUDIT_JSON, OPERATION_LOG_LVL) ||
-		(ac->msg_ctx && ac->send_samdb_events)) {
+		(audit_private->msg_ctx
+		 && audit_private->send_samdb_events)) {
 		struct json_object json;
 		json = operation_json(module, request, reply);
 		audit_log_json(
@@ -861,9 +868,10 @@ static void log_standard_operation(
 			&json,
 			DBGC_DSDB_AUDIT_JSON,
 			OPERATION_LOG_LVL);
-		if (ac->msg_ctx && ac->send_password_events) {
+		if (audit_private->msg_ctx
+		    && audit_private->send_samdb_events) {
 			audit_message_send(
-				ac->msg_ctx,
+				audit_private->msg_ctx,
 				DSDB_EVENT_NAME,
 				MSG_DSDB_LOG,
 				&json);
@@ -871,7 +879,8 @@ static void log_standard_operation(
 		json_free(&json);
 	}
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_PWD_AUDIT_JSON, PASSWORD_LOG_LVL) ||
-		(ac->msg_ctx && ac->send_password_events)) {
+		(audit_private->msg_ctx
+		 && audit_private->send_password_events)) {
 		if (password_changed) {
 			struct json_object json;
 			json = password_change_json(module, request, reply);
@@ -880,9 +889,9 @@ static void log_standard_operation(
 				&json,
 				DBGC_DSDB_PWD_AUDIT_JSON,
 				PASSWORD_LOG_LVL);
-			if (ac->send_password_events) {
+			if (audit_private->send_password_events) {
 				audit_message_send(
-					ac->msg_ctx,
+					audit_private->msg_ctx,
 					DSDB_PWD_EVENT_NAME,
 					MSG_DSDB_PWD_LOG,
 					&json);
@@ -911,9 +920,9 @@ static void log_replicated_operation(
 	const struct ldb_reply *reply)
 {
 
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				struct audit_private);
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
@@ -933,7 +942,7 @@ static void log_replicated_operation(
 	}
 #ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_AUDIT_JSON, REPLICATION_LOG_LVL) ||
-		(ac->msg_ctx && ac->send_samdb_events)) {
+		(audit_private->msg_ctx && audit_private->send_samdb_events)) {
 		struct json_object json;
 		json = replicated_update_json(module, request, reply);
 		audit_log_json(
@@ -941,9 +950,9 @@ static void log_replicated_operation(
 			&json,
 			DBGC_DSDB_AUDIT_JSON,
 			REPLICATION_LOG_LVL);
-		if (ac->send_samdb_events) {
+		if (audit_private->send_samdb_events) {
 			audit_message_send(
-				ac->msg_ctx,
+				audit_private->msg_ctx,
 				DSDB_EVENT_NAME,
 				MSG_DSDB_LOG,
 				&json);
@@ -1000,11 +1009,11 @@ static void log_transaction(
 	int log_level)
 {
 
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 	const struct timeval now = timeval_current();
-	const int64_t duration = usec_time_diff(&now, &ac->transaction_start);
+	const int64_t duration = usec_time_diff(&now, &audit_private->transaction_start);
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
@@ -1020,20 +1029,20 @@ static void log_transaction(
 	}
 #ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_TXN_AUDIT_JSON, log_level) ||
-		(ac->msg_ctx && ac->send_samdb_events)) {
+		(audit_private->msg_ctx && audit_private->send_samdb_events)) {
 		struct json_object json;
 		json = transaction_json(
 			action,
-			&ac->transaction_guid,
+			&audit_private->transaction_guid,
 			duration);
 		audit_log_json(
 			TRANSACTION_JSON_TYPE,
 			&json,
 			DBGC_DSDB_TXN_AUDIT_JSON,
 			log_level);
-		if (ac->send_samdb_events) {
+		if (audit_private->send_samdb_events) {
 			audit_message_send(
-				ac->msg_ctx,
+				audit_private->msg_ctx,
 				DSDB_EVENT_NAME,
 				MSG_DSDB_LOG,
 				&json);
@@ -1061,13 +1070,14 @@ static void log_commit_failure(
 	int status)
 {
 
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 	const char* reason = dsdb_audit_get_ldb_error_string(module, status);
 	const int log_level = TRANSACTION_LOG_FAILURE_LVL;
 	const struct timeval now = timeval_current();
-	const int64_t duration = usec_time_diff(&now, &ac->transaction_start);
+	const int64_t duration = usec_time_diff(&now,
+						&audit_private->transaction_start);
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
@@ -1089,21 +1099,22 @@ static void log_commit_failure(
 	}
 #ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_TXN_AUDIT_JSON, log_level) ||
-		(ac->msg_ctx && ac->send_samdb_events)) {
+		(audit_private->msg_ctx
+		 && audit_private->send_samdb_events)) {
 		struct json_object json;
 		json = commit_failure_json(
 			action,
 			duration,
 			status,
 			reason,
-			&ac->transaction_guid);
+			&audit_private->transaction_guid);
 		audit_log_json(
 			TRANSACTION_JSON_TYPE,
 			&json,
 			DBGC_DSDB_TXN_AUDIT_JSON,
 			log_level);
-		if (ac->send_samdb_events) {
-			audit_message_send(ac->msg_ctx,
+		if (audit_private->send_samdb_events) {
+			audit_message_send(audit_private->msg_ctx,
 					   DSDB_EVENT_NAME,
 					   MSG_DSDB_LOG,
 					   &json);
@@ -1195,9 +1206,9 @@ static int add_transaction_id(
 	struct ldb_module *module,
 	struct ldb_request *req)
 {
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 	struct dsdb_control_transaction_identifier *transaction_id;
 	int ret;
 
@@ -1208,7 +1219,7 @@ static int add_transaction_id(
 		struct ldb_context *ldb = ldb_module_get_ctx(module);
 		return ldb_oom(ldb);
 	}
-	transaction_id->transaction_guid = ac->transaction_guid;
+	transaction_id->transaction_guid = audit_private->transaction_guid;
 	ret = ldb_request_add_control(req,
 				      DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID,
 				      false,
@@ -1382,9 +1393,9 @@ static int log_modify(
  */
 static int log_start_transaction(struct ldb_module *module)
 {
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 
 	/*
 	 * We do not log transaction begins
@@ -1392,8 +1403,8 @@ static int log_start_transaction(struct ldb_module *module)
 	 * time so that we can log the transaction duration.
 	 *
 	 */
-	ac->transaction_guid = GUID_random();
-	ac->transaction_start = timeval_current();
+	audit_private->transaction_guid = GUID_random();
+	audit_private->transaction_start = timeval_current();
 	return ldb_next_start_trans(module);
 }
 
@@ -1434,9 +1445,9 @@ static int log_prepare_commit(struct ldb_module *module)
  */
 static int log_end_transaction(struct ldb_module *module)
 {
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 	int ret = 0;
 
 
@@ -1452,7 +1463,7 @@ static int log_end_transaction(struct ldb_module *module)
 	/*
 	 * Clear the transaction id inserted by log_start_transaction
 	 */
-	memset(&ac->transaction_guid, 0, sizeof(struct GUID));
+	memset(&audit_private->transaction_guid, 0, sizeof(struct GUID));
 	return ret;
 }
 
@@ -1468,12 +1479,12 @@ static int log_end_transaction(struct ldb_module *module)
  */
 static int log_del_transaction(struct ldb_module *module)
 {
-	struct audit_context *ac =
-		talloc_get_type(ldb_module_get_private(module),
-				struct audit_context);
+	struct audit_private *audit_private =
+		talloc_get_type_abort(ldb_module_get_private(module),
+				      struct audit_private);
 
 	log_transaction(module, "rollback", TRANSACTION_LOG_FAILURE_LVL);
-	memset(&ac->transaction_guid, 0, sizeof(struct GUID));
+	memset(&audit_private->transaction_guid, 0, sizeof(struct GUID));
 	return ldb_next_del_trans(module);
 }
 
@@ -1545,7 +1556,7 @@ static int log_init(struct ldb_module *module)
 {
 
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
-	struct audit_context *context = NULL;
+	struct audit_private *audit_private = NULL;
 	struct loadparm_context *lp_ctx
 		= talloc_get_type_abort(ldb_get_opaque(ldb, "loadparm"),
 					struct loadparm_context);
@@ -1553,8 +1564,8 @@ static int log_init(struct ldb_module *module)
 	bool sdb_events = false;
 	bool pwd_events = false;
 
-	context = talloc_zero(module, struct audit_context);
-	if (context == NULL) {
+	audit_private = talloc_zero(module, struct audit_private);
+	if (audit_private == NULL) {
 		return ldb_module_oom(module);
 	}
 
@@ -1563,14 +1574,15 @@ static int log_init(struct ldb_module *module)
 		pwd_events = lpcfg_dsdb_password_event_notification(lp_ctx);
 	}
 	if (sdb_events || pwd_events) {
-		context->send_samdb_events = sdb_events;
-		context->send_password_events = pwd_events;
-		context->msg_ctx = imessaging_client_init(context,
-							  lp_ctx,
-							  ev);
+		audit_private->send_samdb_events = sdb_events;
+		audit_private->send_password_events = pwd_events;
+		audit_private->msg_ctx
+			= imessaging_client_init(audit_private,
+						 lp_ctx,
+						 ev);
 	}
 
-	ldb_module_set_private(module, context);
+	ldb_module_set_private(module, audit_private);
 	return ldb_next_init(module);
 }
 
diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_audit_log.c b/source4/dsdb/samdb/ldb_modules/tests/test_audit_log.c
index 6ceea80f155..6bb79cf5175 100644
--- a/source4/dsdb/samdb/ldb_modules/tests/test_audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_audit_log.c
@@ -283,7 +283,7 @@ static void test_operation_json_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct json_object json;
 	json_t *audit = NULL;
@@ -295,11 +295,11 @@ static void test_operation_json_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	req = talloc_zero(ctx, struct ldb_request);
 	reply = talloc_zero(ctx, struct ldb_reply);
@@ -390,7 +390,7 @@ static void test_operation_json(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -427,13 +427,13 @@ static void test_operation_json(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -581,7 +581,7 @@ static void test_as_system_operation_json(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -622,13 +622,13 @@ static void test_as_system_operation_json(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -784,7 +784,7 @@ static void test_password_change_json_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct json_object json;
 	json_t *audit = NULL;
@@ -796,11 +796,11 @@ static void test_password_change_json_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	req = talloc_zero(ctx, struct ldb_request);
 	reply = talloc_zero(ctx, struct ldb_reply);
@@ -867,7 +867,7 @@ static void test_password_change_json(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -896,13 +896,13 @@ static void test_password_change_json(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -1151,7 +1151,7 @@ static void test_replicated_update_json_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 	struct dsdb_extended_replicated_objects *ro = NULL;
 	struct repsFromTo1 *source_dsa = NULL;
 
@@ -1165,11 +1165,11 @@ static void test_replicated_update_json_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	source_dsa = talloc_zero(ctx, struct repsFromTo1);
 	ro = talloc_zero(ctx, struct dsdb_extended_replicated_objects);
@@ -1275,7 +1275,7 @@ static void test_replicated_update_json(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 	struct dsdb_extended_replicated_objects *ro = NULL;
 	struct repsFromTo1 *source_dsa = NULL;
 
@@ -1302,13 +1302,13 @@ static void test_replicated_update_json(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	dn = ldb_dn_new(ctx, ldb, DN);
 	GUID_from_string(SOURCE_DSA, &source_dsa_obj_guid);
@@ -1422,7 +1422,7 @@ static void test_operation_hr_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	char *line = NULL;
 	const char *rs = NULL;
@@ -1433,11 +1433,11 @@ static void test_operation_hr_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	req = talloc_zero(ctx, struct ldb_request);
 	reply = talloc_zero(ctx, struct ldb_reply);
@@ -1476,7 +1476,7 @@ static void test_operation_hr(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -1506,13 +1506,13 @@ static void test_operation_hr(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -1574,7 +1574,7 @@ static void test_as_system_operation_hr(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -1608,13 +1608,13 @@ static void test_as_system_operation_hr(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -1684,7 +1684,7 @@ static void test_password_change_hr_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	char *line = NULL;
 	const char *rs = NULL;
@@ -1694,11 +1694,11 @@ static void test_password_change_hr_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	req = talloc_zero(ctx, struct ldb_request);
 	reply = talloc_zero(ctx, struct ldb_reply);
@@ -1736,7 +1736,7 @@ static void test_password_change_hr(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 
 	struct tsocket_address *ts = NULL;
 
@@ -1764,13 +1764,13 @@ static void test_password_change_hr(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
 	ldb_set_opaque(ldb, "remoteAddress", ts);
@@ -1909,7 +1909,7 @@ static void test_add_transaction_id(void **state)
 {
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 	struct GUID guid;
 	const char * const GUID = "7130cb06-2062-6a1b-409e-3514c26b1773";
 	struct ldb_control * control = NULL;
@@ -1917,12 +1917,12 @@ static void test_add_transaction_id(void **state)
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(GUID, &guid);
-	ac->transaction_guid = guid;
+	audit_private->transaction_guid = guid;
 
 	module = talloc_zero(ctx, struct ldb_module);
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	req = talloc_zero(ctx, struct ldb_request);
 
@@ -1934,7 +1934,7 @@ static void test_add_transaction_id(void **state)
 		DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID);
 	assert_non_null(control);
 	assert_memory_equal(
-		&ac->transaction_guid,
+		&audit_private->transaction_guid,
 		control->data,
 		sizeof(struct GUID));
 
@@ -2085,7 +2085,7 @@ static void test_replicated_update_hr_empty(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 	struct dsdb_extended_replicated_objects *ro = NULL;
 	struct repsFromTo1 *source_dsa = NULL;
 
@@ -2097,11 +2097,11 @@ static void test_replicated_update_hr_empty(void **state)
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
 	ldb = ldb_init(ctx, NULL);
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	source_dsa = talloc_zero(ctx, struct repsFromTo1);
 	ro = talloc_zero(ctx, struct dsdb_extended_replicated_objects);
@@ -2145,7 +2145,7 @@ static void test_replicated_update_hr(void **state)
 	struct ldb_module  *module = NULL;
 	struct ldb_request *req = NULL;
 	struct ldb_reply *reply = NULL;
-	struct audit_context *ac = NULL;
+	struct audit_private *audit_private = NULL;
 	struct dsdb_extended_replicated_objects *ro = NULL;
 	struct repsFromTo1 *source_dsa = NULL;
 
@@ -2172,13 +2172,13 @@ static void test_replicated_update_hr(void **state)
 
 	ldb = ldb_init(ctx, NULL);
 
-	ac = talloc_zero(ctx, struct audit_context);
+	audit_private = talloc_zero(ctx, struct audit_private);
 	GUID_from_string(TRANSACTION, &transaction_id);
-	ac->transaction_guid = transaction_id;
+	audit_private->transaction_guid = transaction_id;
 
 	module = talloc_zero(ctx, struct ldb_module);
 	module->ldb = ldb;
-	ldb_module_set_private(module, ac);
+	ldb_module_set_private(module, audit_private);
 
 	dn = ldb_dn_new(ctx, ldb, DN);
 	GUID_from_string(SOURCE_DSA, &source_dsa_obj_guid);
-- 
2.11.0


From 728a2253e8865db646005e0882f6fe2963c005cd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 25 Jun 2018 16:46:29 +1200
Subject: [PATCH 4/4] dsdb: Use GUID_zero() rather than memset in dsdb audit
 code

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/audit_log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index 23020b77738..ad71fd55df8 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -1463,7 +1463,7 @@ static int log_end_transaction(struct ldb_module *module)
 	/*
 	 * Clear the transaction id inserted by log_start_transaction
 	 */
-	memset(&audit_private->transaction_guid, 0, sizeof(struct GUID));
+	audit_private->transaction_guid = GUID_zero();
 	return ret;
 }
 
@@ -1484,7 +1484,7 @@ static int log_del_transaction(struct ldb_module *module)
 				      struct audit_private);
 
 	log_transaction(module, "rollback", TRANSACTION_LOG_FAILURE_LVL);
-	memset(&audit_private->transaction_guid, 0, sizeof(struct GUID));
+	audit_private->transaction_guid = GUID_zero();
 	return ldb_next_del_trans(module);
 }
 
-- 
2.11.0



More information about the samba-technical mailing list