Attachment 14741 Details for Bug 13714 – patch for 4.9

[patch] patch for 4.9

json-audit-improvements.patch (text/plain), 87.72 KB, created by Andrew Bartlett on 2018-12-17 04:23:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2018-12-17 04:23:21 UTC

Size: 87.72 KB

patch

obsolete

>From 6a6bd5d6b971368429388d94321812bd236b2e28 Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Fri, 13 Jul 2018 09:14:09 +1200
>Subject: [PATCH 1/3] json: Modify API to use return codes
>
>Modify the auditing JSON API to return a response code, as the consensus
>was that the existing error handling was aesthetically displeasing.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13714
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 79f494e51eabb5176747fcf3b9f2efad10ec7f97 and
>adapted to compile on 4.9 by abartlet)
>---
> auth/auth_log.c                                    | 307 ++++++++----
> lib/audit_logging/audit_logging.c                  | 527 ++++++++++++++-------
> lib/audit_logging/audit_logging.h                  |  61 +--
> lib/audit_logging/tests/audit_logging_test.c       | 252 +++++++---
> source4/dsdb/samdb/ldb_modules/audit_log.c         | 436 +++++++++++++----
> source4/dsdb/samdb/ldb_modules/audit_util.c        | 148 +++++-
> source4/dsdb/samdb/ldb_modules/group_audit.c       |  91 +++-
> .../samdb/ldb_modules/tests/test_group_audit.c     |   2 +-
> 8 files changed, 1350 insertions(+), 474 deletions(-)
>
>diff --git a/auth/auth_log.c b/auth/auth_log.c
>index 67d23c12a1b..9e57b1d3968 100644
>--- a/auth/auth_log.c
>+++ b/auth/auth_log.c
>@@ -123,63 +123,134 @@ static void log_authentication_event_json(
> 	struct dom_sid *sid,
> 	int debug_level)
> {
>-	struct json_object wrapper = json_new_object();
>-	struct json_object authentication;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object authentication = json_empty_object;
> 	char negotiate_flags[11];
>-
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", AUTH_JSON_TYPE);
>+	int rc = 0;
> 
> 	authentication = json_new_object();
>-	json_add_version(&authentication, AUTH_MAJOR, AUTH_MINOR);
>-	json_add_string(&authentication, "status", nt_errstr(status));
>-	json_add_address(&authentication, "localAddress", ui->local_host);
>-	json_add_address(&authentication, "remoteAddress", ui->remote_host);
>-	json_add_string(&authentication,
>-			"serviceDescription",
>-			ui->service_description);
>-	json_add_string(&authentication,
>-			"authDescription",
>-			ui->auth_description);
>-	json_add_string(&authentication,
>-			"clientDomain",
>-			ui->client.domain_name);
>-	json_add_string(&authentication,
>-			"clientAccount",
>-			ui->client.account_name);
>-	json_add_string(&authentication,
>-			"workstation",
>-			ui->workstation_name);
>-	json_add_string(&authentication, "becameAccount", account_name);
>-	json_add_string(&authentication, "becameDomain", domain_name);
>-	json_add_sid(&authentication, "becameSid", sid);
>-	json_add_string(&authentication,
>-			"mappedAccount",
>-			ui->mapped.account_name);
>-	json_add_string(&authentication,
>-			"mappedDomain",
>-			ui->mapped.domain_name);
>-	json_add_string(&authentication,
>-			"netlogonComputer",
>-			ui->netlogon_trust_account.computer_name);
>-	json_add_string(&authentication,
>-			"netlogonTrustAccount",
>-			ui->netlogon_trust_account.account_name);
>+	if (json_is_invalid(&authentication)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&authentication, AUTH_MAJOR, AUTH_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authentication, "status", nt_errstr(status));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&authentication, "localAddress", ui->local_host);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc =
>+	    json_add_address(&authentication, "remoteAddress", ui->remote_host);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "serviceDescription", ui->service_description);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "authDescription", ui->auth_description);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "clientDomain", ui->client.domain_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "clientAccount", ui->client.account_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "workstation", ui->workstation_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authentication, "becameAccount", account_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authentication, "becameDomain", domain_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(&authentication, "becameSid", sid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "mappedAccount", ui->mapped.account_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "mappedDomain", ui->mapped.domain_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authentication,
>+			     "netlogonComputer",
>+			     ui->netlogon_trust_account.computer_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authentication,
>+			     "netlogonTrustAccount",
>+			     ui->netlogon_trust_account.account_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 	snprintf(negotiate_flags,
> 		 sizeof( negotiate_flags),
> 		 "0x%08X",
> 		 ui->netlogon_trust_account.negotiate_flags);
>-	json_add_string(&authentication,
>-			"netlogonNegotiateFlags",
>-			negotiate_flags);
>-	json_add_int(&authentication,
>-		     "netlogonSecureChannelType",
>-		     ui->netlogon_trust_account.secure_channel_type);
>-	json_add_sid(&authentication,
>-		     "netlogonTrustAccountSid",
>-		     ui->netlogon_trust_account.sid);
>-	json_add_string(&authentication, "passwordType", get_password_type(ui));
>-	json_add_object(&wrapper, AUTH_JSON_TYPE, &authentication);
>+	rc = json_add_string(
>+	    &authentication, "netlogonNegotiateFlags", negotiate_flags);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&authentication,
>+			  "netlogonSecureChannelType",
>+			  ui->netlogon_trust_account.secure_channel_type);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(&authentication,
>+			  "netlogonTrustAccountSid",
>+			  ui->netlogon_trust_account.sid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authentication, "passwordType", get_password_type(ui));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+
>+	wrapper = json_new_object();
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", AUTH_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, AUTH_JSON_TYPE, &authentication);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	/*
> 	 * While not a general-purpose profiling solution this will
>@@ -192,9 +263,10 @@ static void log_authentication_event_json(
> 		struct timeval current_time = timeval_current();
> 		uint64_t duration =  usec_time_diff(&current_time,
> 						    start_time);
>-		json_add_int(&authentication,
>-			     "duration",
>-			     duration);
>+		rc = json_add_int(&authentication, "duration", duration);
>+		if (rc != 0) {
>+			goto failure;
>+		}
> 	}
> 
> 	log_json(msg_ctx,
>@@ -204,6 +276,16 @@ static void log_authentication_event_json(
> 		 DBGC_AUTH_AUDIT,
> 		 debug_level);
> 	json_free(&wrapper);
>+	return;
>+failure:
>+	/*
>+	 * On a failure authentication will not have been added to wrapper so it
>+	 * needs to be freed to avoid a leak.
>+	 *
>+	 */
>+	json_free(&authentication);
>+	json_free(&wrapper);
>+	DBG_ERR("Failed to write authentication event JSON log message\n");
> }
> 
> /*
>@@ -237,45 +319,92 @@ static void log_successful_authz_event_json(
> 	struct auth_session_info *session_info,
> 	int debug_level)
> {
>-	struct json_object wrapper = json_new_object();
>-	struct json_object authorization;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object authorization = json_empty_object;
> 	char account_flags[11];
>+	int rc = 0;
> 
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", AUTHZ_JSON_TYPE);
> 	authorization = json_new_object();
>-	json_add_version(&authorization, AUTHZ_MAJOR, AUTHZ_MINOR);
>-	json_add_address(&authorization, "localAddress", local);
>-	json_add_address(&authorization, "remoteAddress", remote);
>-	json_add_string(&authorization,
>-			"serviceDescription",
>-			service_description);
>-	json_add_string(&authorization, "authType", auth_type);
>-	json_add_string(&authorization,
>-			"domain",
>-			session_info->info->domain_name);
>-	json_add_string(&authorization,
>-			"account",
>-			session_info->info->account_name);
>-	json_add_sid(&authorization,
>-		     "sid",
>-		     &session_info->security_token->sids[0]);
>-	json_add_guid(&authorization,
>-		      "sessionId",
>-		      &session_info->unique_session_token);
>-	json_add_string(&authorization,
>-			"logonServer",
>-			session_info->info->logon_server);
>-	json_add_string(&authorization,
>-			"transportProtection",
>-			transport_protection);
>+	if (json_is_invalid(&authorization)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&authorization, AUTHZ_MAJOR, AUTHZ_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&authorization, "localAddress", local);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&authorization, "remoteAddress", remote);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authorization, "serviceDescription", service_description);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&authorization, "authType", auth_type);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authorization, "domain", session_info->info->domain_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authorization, "account", session_info->info->account_name);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(
>+	    &authorization, "sid", &session_info->security_token->sids[0]);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &authorization, "sessionId", &session_info->unique_session_token);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authorization, "logonServer", session_info->info->logon_server);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(
>+	    &authorization, "transportProtection", transport_protection);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	snprintf(account_flags,
> 		 sizeof(account_flags),
> 		 "0x%08X",
> 		 session_info->info->acct_flags);
>-	json_add_string(&authorization, "accountFlags", account_flags);
>-	json_add_object(&wrapper, AUTHZ_JSON_TYPE, &authorization);
>+	rc = json_add_string(&authorization, "accountFlags", account_flags);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+
>+	wrapper = json_new_object();
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", AUTHZ_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, AUTHZ_JSON_TYPE, &authorization);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	log_json(msg_ctx,
> 		 lp_ctx,
>@@ -284,6 +413,16 @@ static void log_successful_authz_event_json(
> 		 DBGC_AUTH_AUDIT,
> 		 debug_level);
> 	json_free(&wrapper);
>+	return;
>+failure:
>+	/*
>+	 * On a failure authorization will not have been added to wrapper so it
>+	 * needs to be freed to avoid a leak.
>+	 *
>+	 */
>+	json_free(&authorization);
>+	json_free(&wrapper);
>+	DBG_ERR("Unable to log Authentication event JSON audit message\n");
> }
> 
> #else
>diff --git a/lib/audit_logging/audit_logging.c b/lib/audit_logging/audit_logging.c
>index f94f2c2a839..ac08863129a 100644
>--- a/lib/audit_logging/audit_logging.c
>+++ b/lib/audit_logging/audit_logging.c
>@@ -20,31 +20,6 @@
> /*
>  * Error handling:
>  *
>- * The json_object structure contains a boolean 'error'.  This is set whenever
>- * an error is detected. All the library functions check this flag and return
>- * immediately if it is set.
>- *
>- *	if (object->error) {
>- *		return;
>- *	}
>- *
>- * This allows the operations to be sequenced naturally with out the clutter
>- * of error status checks.
>- *
>- *	audit = json_new_object();
>- *	json_add_version(&audit, OPERATION_MAJOR, OPERATION_MINOR);
>- *	json_add_int(&audit, "statusCode", ret);
>- *	json_add_string(&audit, "status", ldb_strerror(ret));
>- *	json_add_string(&audit, "operation", operation);
>- *	json_add_address(&audit, "remoteAddress", remote);
>- *	json_add_sid(&audit, "userSid", sid);
>- *	json_add_string(&audit, "dn", dn);
>- *	json_add_guid(&audit, "transactionId", &ac->transaction_guid);
>- *	json_add_guid(&audit, "sessionId", unique_session_token);
>- *
>- * The assumptions are that errors will be rare, and that the audit logging
>- * code should not cause failures. So errors are logged but processing
>- * continues on a best effort basis.
>  */
> 
> #include "includes.h"
>@@ -67,7 +42,7 @@
>  *
>  * @param mem_ctx talloc memory context that owns the returned string.
>  *
>- * @return a human readable time stamp.
>+ * @return a human readable time stamp, or NULL in the event of an error.
>  *
>  */
> char* audit_get_timestamp(TALLOC_CTX *frame)
>@@ -76,11 +51,11 @@ char* audit_get_timestamp(TALLOC_CTX *frame)
> 	char tz[10];		/* formatted time zone			 */
> 	struct tm* tm_info;	/* current local time			 */
> 	struct timeval tv;	/* current system time			 */
>-	int r;			/* response code from gettimeofday	 */
>+	int ret;		/* response code			 */
> 	char * ts;		/* formatted time stamp			 */
> 
>-	r = gettimeofday(&tv, NULL);
>-	if (r) {
>+	ret = gettimeofday(&tv, NULL);
>+	if (ret != 0) {
> 		DBG_ERR("Unable to get time of day: (%d) %s\n",
> 			errno,
> 			strerror(errno));
>@@ -122,6 +97,10 @@ void audit_log_human_text(const char* prefix,
> 
> #ifdef HAVE_JANSSON
> /*
>+ * Constant for empty json object initialisation
>+ */
>+const struct json_object json_empty_object = {.valid = false, .root = NULL};
>+/*
>  * @brief write a json object to the samba audit logs.
>  *
>  * Write the json object to the audit logs as a formatted string
>@@ -136,8 +115,23 @@ void audit_log_json(const char* prefix,
> 		    int debug_class,
> 		    int debug_level)
> {
>-	TALLOC_CTX *ctx = talloc_new(NULL);
>-	char *s = json_to_string(ctx, message);
>+	TALLOC_CTX *ctx = NULL;
>+	char *s = NULL;
>+
>+	if (json_is_invalid(message)) {
>+		DBG_ERR("Invalid JSON object, unable to log\n");
>+		return;
>+	}
>+
>+	ctx = talloc_new(NULL);
>+	s = json_to_string(ctx, message);
>+	if (s == NULL) {
>+		DBG_ERR("json_to_string for (%s) returned NULL, "
>+			"JSON audit message could not written\n",
>+			prefix);
>+		TALLOC_FREE(ctx);
>+		return;
>+	}
> 	DEBUGC(debug_class, debug_level, ("JSON %s: %s\n", prefix, s));
> 	TALLOC_FREE(ctx);
> }
>@@ -235,11 +229,14 @@ void audit_message_send(
> 
> 	const char *message_string = NULL;
> 	DATA_BLOB message_blob = data_blob_null;
>-	TALLOC_CTX *ctx = talloc_new(NULL);
>+	TALLOC_CTX *ctx = NULL;
> 
>+	if (json_is_invalid(message)) {
>+		DBG_ERR("Invalid JSON object, unable to send\n");
>+		return;
>+	}
> 	if (msg_ctx == NULL) {
> 		DBG_DEBUG("No messaging context\n");
>-		TALLOC_FREE(ctx);
> 		return;
> 	}
> 
>@@ -288,20 +285,21 @@ void audit_message_send(
>  * Free with a call to json_free_object, note that the jansson inplementation
>  * allocates memory with malloc and not talloc.
>  *
>- * @return a struct json_object, error will be set to true if the object
>+ * @return a struct json_object, valid will be set to false if the object
>  *         could not be created.
>  *
>  */
> struct json_object json_new_object(void) {
> 
>-	struct json_object object;
>-	object.error = false;
>+	struct json_object object = json_empty_object;
> 
> 	object.root = json_object();
> 	if (object.root == NULL) {
>-		object.error = true;
>-		DBG_ERR("Unable to create json_object\n");
>+		object.valid = false;
>+		DBG_ERR("Unable to create JSON object\n");
>+		return object;
> 	}
>+	object.valid = true;
> 	return object;
> }
> 
>@@ -320,14 +318,15 @@ struct json_object json_new_object(void) {
>  */
> struct json_object json_new_array(void) {
> 
>-	struct json_object array;
>-	array.error = false;
>+	struct json_object array = json_empty_object;
> 
> 	array.root = json_array();
> 	if (array.root == NULL) {
>-		array.error = true;
>-		DBG_ERR("Unable to create json_array\n");
>+		array.valid = false;
>+		DBG_ERR("Unable to create JSON array\n");
>+		return array;
> 	}
>+	array.valid = true;
> 	return array;
> }
> 
>@@ -345,7 +344,7 @@ void json_free(struct json_object *object)
> 		json_decref(object->root);
> 	}
> 	object->root = NULL;
>-	object->error = true;
>+	object->valid = false;
> }
> 
> /*
>@@ -358,99 +357,131 @@ void json_free(struct json_object *object)
>  */
> bool json_is_invalid(struct json_object *object)
> {
>-	return object->error;
>+	return !object->valid;
> }
> 
> /*
>  * @brief Add an integer value to a JSON object.
>  *
>  * Add an integer value named 'name' to the json object.
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name of the value.
>  * @param value the value.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_int(struct json_object *object,
>-		  const char* name,
>-		  const int value)
>+int json_add_int(struct json_object *object, const char *name, const int value)
> {
>-	int rc = 0;
>+	int ret = 0;
>+	json_t *integer = NULL;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add int [%s] value [%d], "
>+			"target object is invalid\n",
>+			name,
>+			value);
>+		return JSON_ERROR;
>+	}
>+
>+	integer = json_integer(value);
>+	if (integer == NULL) {
>+		DBG_ERR("Unable to create integer value [%s] value [%d]\n",
>+			name,
>+			value);
>+		return JSON_ERROR;
> 	}
> 
>-	rc = json_object_set_new(object->root, name, json_integer(value));
>-	if (rc) {
>-		DBG_ERR("Unable to set name [%s] value [%d]\n", name, value);
>-		object->error = true;
>+	ret = json_object_set_new(object->root, name, integer);
>+	if (ret != 0) {
>+		json_decref(integer);
>+		DBG_ERR("Unable to add int [%s] value [%d]\n", name, value);
> 	}
>+	return ret;
> }
> 
> /*
>  * @brief Add a boolean value to a JSON object.
>  *
>  * Add a boolean value named 'name' to the json object.
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param value the value.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_bool(struct json_object *object,
>-		   const char* name,
>-		   const bool value)
>+int json_add_bool(struct json_object *object,
>+		  const char *name,
>+		  const bool value)
> {
>-	int rc = 0;
>+	int ret = 0;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add boolean [%s] value [%d], "
>+			"target object is invalid\n",
>+			name,
>+			value);
>+		return JSON_ERROR;
> 	}
> 
>-	rc = json_object_set_new(object->root, name, json_boolean(value));
>-	if (rc) {
>-		DBG_ERR("Unable to set name [%s] value [%d]\n", name, value);
>-		object->error = true;
>+	ret = json_object_set_new(object->root, name, json_boolean(value));
>+	if (ret != 0) {
>+		DBG_ERR("Unable to add boolean [%s] value [%d]\n", name, value);
> 	}
>-
>+	return ret;
> }
> 
> /*
>  * @brief Add a string value to a JSON object.
>  *
>  * Add a string value named 'name' to the json object.
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param value the value.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_string(struct json_object *object,
>-		     const char* name,
>-		     const char* value)
>+int json_add_string(struct json_object *object,
>+		    const char *name,
>+		    const char *value)
> {
>-	int rc = 0;
>+	int ret = 0;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add string [%s], target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
>-
> 	if (value) {
>-		rc = json_object_set_new(
>-			object->root,
>-			name,
>-			json_string(value));
>+		json_t *string = json_string(value);
>+		if (string == NULL) {
>+			DBG_ERR("Unable to add string [%s], "
>+				"could not create string object\n",
>+				name);
>+			return JSON_ERROR;
>+		}
>+		ret = json_object_set_new(object->root, name, string);
>+		if (ret != 0) {
>+			json_decref(string);
>+			DBG_ERR("Unable to add string [%s]\n", name);
>+			return ret;
>+		}
> 	} else {
>-		rc = json_object_set_new(object->root, name, json_null());
>-	}
>-	if (rc) {
>-		DBG_ERR("Unable to set name [%s] value [%s]\n", name, value);
>-		object->error = true;
>+		ret = json_object_set_new(object->root, name, json_null());
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add null string [%s]\n", name);
>+			return ret;
>+		}
> 	}
>+	return ret;
> }
> 
> /*
>@@ -464,13 +495,13 @@ void json_add_string(struct json_object *object,
>  */
> void json_assert_is_array(struct json_object *array) {
> 
>-	if (array->error) {
>+	if (json_is_invalid(array)) {
> 		return;
> 	}
> 
> 	if (json_is_array(array->root) == false) {
> 		DBG_ERR("JSON object is not an array\n");
>-		array->error = true;
>+		array->valid = false;
> 		return;
> 	}
> }
>@@ -479,43 +510,46 @@ void json_assert_is_array(struct json_object *array) {
>  * @brief Add a JSON object to a JSON object.
>  *
>  * Add a JSON object named 'name' to the json object.
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param value the value.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_object(struct json_object *object,
>-		     const char* name,
>-		     struct json_object *value)
>+int json_add_object(struct json_object *object,
>+		    const char *name,
>+		    struct json_object *value)
> {
>-	int rc = 0;
>+	int ret = 0;
> 	json_t *jv = NULL;
> 
>-	if (object->error) {
>-		return;
>+	if (value != NULL && json_is_invalid(value)) {
>+		DBG_ERR("Invalid JSON object [%s] supplied\n", name);
>+		return JSON_ERROR;
> 	}
>-
>-	if (value != NULL && value->error) {
>-		object->error = true;
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add object [%s], target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
> 
> 	jv = value == NULL ? json_null() : value->root;
> 
> 	if (json_is_array(object->root)) {
>-		rc = json_array_append_new(object->root, jv);
>+		ret = json_array_append_new(object->root, jv);
> 	} else if (json_is_object(object->root)) {
>-		rc = json_object_set_new(object->root, name,  jv);
>+		ret = json_object_set_new(object->root, name, jv);
> 	} else {
> 		DBG_ERR("Invalid JSON object type\n");
>-		object->error = true;
>+		ret = JSON_ERROR;
> 	}
>-	if (rc) {
>+	if (ret != 0) {
> 		DBG_ERR("Unable to add object [%s]\n", name);
>-		object->error = true;
> 	}
>+	return ret;
> }
> 
> /*
>@@ -526,39 +560,57 @@ void json_add_object(struct json_object *object,
>  * truncated if it is more than len characters long. If len is 0 the value
>  * is encoded as a JSON null.
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param value the value.
>  * @param len the maximum number of characters to be copied.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_stringn(struct json_object *object,
>-		      const char *name,
>-		      const char *value,
>-		      const size_t len)
>+int json_add_stringn(struct json_object *object,
>+		     const char *name,
>+		     const char *value,
>+		     const size_t len)
> {
> 
>-	int rc = 0;
>-	if (object->error) {
>-		return;
>+	int ret = 0;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add string [%s], target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
> 
> 	if (value != NULL && len > 0) {
>+		json_t *string = NULL;
> 		char buffer[len+1];
>+
> 		strncpy(buffer, value, len);
> 		buffer[len] = '\0';
>-		rc = json_object_set_new(object->root,
>-					 name,
>-					 json_string(buffer));
>+
>+		string = json_string(buffer);
>+		if (string == NULL) {
>+			DBG_ERR("Unable to add string [%s], "
>+				"could not create string object\n",
>+				name);
>+			return JSON_ERROR;
>+		}
>+		ret = json_object_set_new(object->root, name, string);
>+		if (ret != 0) {
>+			json_decref(string);
>+			DBG_ERR("Unable to add string [%s]\n", name);
>+			return ret;
>+		}
> 	} else {
>-		rc = json_object_set_new(object->root, name, json_null());
>-	}
>-	if (rc) {
>-		DBG_ERR("Unable to set name [%s] value [%s]\n", name, value);
>-		object->error = true;
>+		ret = json_object_set_new(object->root, name, json_null());
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add null string [%s]\n", name);
>+			return ret;
>+		}
> 	}
>+	return ret;
> }
> 
> /*
>@@ -576,18 +628,45 @@ void json_add_stringn(struct json_object *object,
>  * The minor version should change whenever a new attribute is added and for
>  * minor bug fixes to an attributes content.
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param major the major version number
>  * @param minor the minor version number
>+ *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>  */
>-void json_add_version(struct json_object *object, int major, int minor)
>+int json_add_version(struct json_object *object, int major, int minor)
> {
>-	struct json_object version = json_new_object();
>-	json_add_int(&version, "major", major);
>-	json_add_int(&version, "minor", minor);
>-	json_add_object(object, "version", &version);
>+	int ret = 0;
>+	struct json_object version;
>+
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add version, target object is invalid\n");
>+		return JSON_ERROR;
>+	}
>+
>+	version = json_new_object();
>+	if (json_is_invalid(&version)) {
>+		DBG_ERR("Unable to add version, failed to create object\n");
>+		return JSON_ERROR;
>+	}
>+	ret = json_add_int(&version, "major", major);
>+	if (ret != 0) {
>+		json_free(&version);
>+		return ret;
>+	}
>+	ret = json_add_int(&version, "minor", minor);
>+	if (ret != 0) {
>+		json_free(&version);
>+		return ret;
>+	}
>+	ret = json_add_object(object, "version", &version);
>+	if (ret != 0) {
>+		json_free(&version);
>+		return ret;
>+	}
>+	return ret;
> }
> 
> /*
>@@ -598,11 +677,13 @@ void json_add_version(struct json_object *object, int major, int minor)
>  *
>  * "timestamp":"2017-03-06T17:18:04.455081+1300"
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>+ *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>  */
>-void json_add_timestamp(struct json_object *object)
>+int json_add_timestamp(struct json_object *object)
> {
> 	char buffer[40];	/* formatted time less usec and timezone */
> 	char timestamp[65];	/* the formatted ISO 8601 time stamp	 */
>@@ -610,9 +691,11 @@ void json_add_timestamp(struct json_object *object)
> 	struct tm* tm_info;	/* current local time			 */
> 	struct timeval tv;	/* current system time			 */
> 	int r;			/* response code from gettimeofday	 */
>+	int ret;		/* return code from json operations	*/
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add time stamp, target object is invalid\n");
>+		return JSON_ERROR;
> 	}
> 
> 	r = gettimeofday(&tv, NULL);
>@@ -620,15 +703,13 @@ void json_add_timestamp(struct json_object *object)
> 		DBG_ERR("Unable to get time of day: (%d) %s\n",
> 			errno,
> 			strerror(errno));
>-		object->error = true;
>-		return;
>+		return JSON_ERROR;
> 	}
> 
> 	tm_info = localtime(&tv.tv_sec);
> 	if (tm_info == NULL) {
> 		DBG_ERR("Unable to determine local time\n");
>-		object->error = true;
>-		return;
>+		return JSON_ERROR;
> 	}
> 
> 	strftime(buffer, sizeof(buffer)-1, "%Y-%m-%dT%T", tm_info);
>@@ -640,10 +721,13 @@ void json_add_timestamp(struct json_object *object)
> 		buffer,
> 		tv.tv_usec,
> 		tz);
>-	json_add_string(object, "timestamp", timestamp);
>+	ret = json_add_string(object, "timestamp", timestamp);
>+	if (ret != 0) {
>+		DBG_ERR("Unable to add time stamp to JSON object\n");
>+	}
>+	return ret;
> }
> 
>-
> /*
>  *@brief Add a tsocket_address to a JSON object
>  *
>@@ -651,35 +735,59 @@ void json_add_timestamp(struct json_object *object)
>  *
>  * "localAddress":"ipv6::::0"
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param address the tsocket_address.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_address(struct json_object *object,
>-		      const char *name,
>-		      const struct tsocket_address *address)
>+int json_add_address(struct json_object *object,
>+		     const char *name,
>+		     const struct tsocket_address *address)
> {
>+	int ret = 0;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add address [%s], "
>+			"target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
>+
> 	if (address == NULL) {
>-		int rc = json_object_set_new(object->root, name, json_null());
>-		if (rc) {
>-			DBG_ERR("Unable to set address [%s] to null\n", name);
>-			object->error = true;
>+		ret = json_object_set_new(object->root, name, json_null());
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add null address [%s]\n", name);
>+			return JSON_ERROR;
> 		}
> 	} else {
> 		TALLOC_CTX *ctx = talloc_new(NULL);
> 		char *s = NULL;
> 
>+		if (ctx == NULL) {
>+			DBG_ERR("Out of memory adding address [%s]\n", name);
>+			return JSON_ERROR;
>+		}
>+
> 		s = tsocket_address_string(address, ctx);
>-		json_add_string(object, name, s);
>+		if (s == NULL) {
>+			DBG_ERR("Out of memory adding address [%s]\n", name);
>+			TALLOC_FREE(ctx);
>+			return JSON_ERROR;
>+		}
>+		ret = json_add_string(object, name, s);
>+		if (ret != 0) {
>+			DBG_ERR(
>+			    "Unable to add address [%s] value [%s]\n", name, s);
>+			TALLOC_FREE(ctx);
>+			return JSON_ERROR;
>+		}
> 		TALLOC_FREE(ctx);
> 	}
>+	return ret;
> }
> 
> /*
>@@ -689,33 +797,47 @@ void json_add_address(struct json_object *object,
>  *
>  * "sid":"S-1-5-18"
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param sid the sid
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  */
>-void json_add_sid(struct json_object *object,
>-		  const char *name,
>-		  const struct dom_sid *sid)
>+int json_add_sid(struct json_object *object,
>+		 const char *name,
>+		 const struct dom_sid *sid)
> {
>+	int ret = 0;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add SID [%s], "
>+			"target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
>+
> 	if (sid == NULL) {
>-		int rc = json_object_set_new(object->root, name, json_null());
>-		if (rc) {
>-			DBG_ERR("Unable to set SID [%s] to null\n", name);
>-			object->error = true;
>+		ret = json_object_set_new(object->root, name, json_null());
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add null SID [%s]\n", name);
>+			return ret;
> 		}
> 	} else {
> 		char sid_buf[DOM_SID_STR_BUFLEN];
> 
> 		dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
>-		json_add_string(object, name, sid_buf);
>+		ret = json_add_string(object, name, sid_buf);
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add SID [%s] value [%s]\n",
>+				name,
>+				sid_buf);
>+			return ret;
>+		}
> 	}
>+	return ret;
> }
> 
> /*
>@@ -725,46 +847,59 @@ void json_add_sid(struct json_object *object,
>  *
>  * "guid":"1fb9f2ee-2a4d-4bf8-af8b-cb9d4529a9ab"
>  *
>- * In the event of an error object will be invalidated.
>  *
>  * @param object the JSON object to be updated.
>  * @param name the name.
>  * @param guid the guid.
>  *
>+ * @return 0 the operation was successful
>+ *        -1 the operation failed
>+ *
>  *
>  */
>-void json_add_guid(struct json_object *object,
>-		   const char *name,
>-		   const struct GUID *guid)
>+int json_add_guid(struct json_object *object,
>+		  const char *name,
>+		  const struct GUID *guid)
> {
> 
>+	int ret = 0;
> 
>-	if (object->error) {
>-		return;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Unable to add GUID [%s], "
>+			"target object is invalid\n",
>+			name);
>+		return JSON_ERROR;
> 	}
>+
> 	if (guid == NULL) {
>-		int rc = json_object_set_new(object->root, name, json_null());
>-		if (rc) {
>-			DBG_ERR("Unable to set GUID [%s] to null\n", name);
>-			object->error = true;
>+		ret = json_object_set_new(object->root, name, json_null());
>+		if (ret != 0) {
>+			DBG_ERR("Unable to add null GUID [%s]\n", name);
>+			return ret;
> 		}
> 	} else {
> 		char *guid_str;
> 		struct GUID_txt_buf guid_buff;
> 
> 		guid_str = GUID_buf_string(guid, &guid_buff);
>-		json_add_string(object, name, guid_str);
>+		ret = json_add_string(object, name, guid_str);
>+		if (ret != 0) {
>+			DBG_ERR("Unable to guid GUID [%s] value [%s]\n",
>+				name,
>+				guid_str);
>+			return ret;
>+		}
> 	}
>+	return ret;
> }
> 
>-
> /*
>  * @brief Convert a JSON object into a string
>  *
>  * Convert the jsom object into a string suitable for printing on a log line,
>  * i.e. with no embedded line breaks.
>  *
>- * If the object is invalid it returns NULL.
>+ * If the object is invalid it logs an error and returns NULL.
>  *
>  * @param mem_ctx the talloc memory context owning the returned string
>  * @param object the json object.
>@@ -772,13 +907,17 @@ void json_add_guid(struct json_object *object,
>  * @return A string representation of the object or NULL if the object
>  *         is invalid.
>  */
>-char *json_to_string(TALLOC_CTX *mem_ctx,
>-		     struct json_object *object)
>+char *json_to_string(TALLOC_CTX *mem_ctx, struct json_object *object)
> {
> 	char *json = NULL;
> 	char *json_string = NULL;
> 
>-	if (object->error) {
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Invalid JSON object, unable to convert to string\n");
>+		return NULL;
>+	}
>+
>+	if (object->root == NULL) {
> 		return NULL;
> 	}
> 
>@@ -813,15 +952,23 @@ char *json_to_string(TALLOC_CTX *mem_ctx,
>  *
>  * @return The array object, will be created if it did not exist.
>  */
>-struct json_object json_get_array(struct json_object *object,
>-				  const char* name)
>+struct json_object json_get_array(struct json_object *object, const char *name)
> {
> 
>-	struct json_object array = json_new_array();
>+	struct json_object array = json_empty_object;
> 	json_t *a = NULL;
>+	int ret = 0;
>+
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Invalid JSON object, unable to get array [%s]\n",
>+			name);
>+		json_free(&array);
>+		return array;
>+	}
> 
>-	if (object->error) {
>-		array.error = true;
>+	array = json_new_array();
>+	if (json_is_invalid(&array)) {
>+		DBG_ERR("Unable to create new array for [%s]\n", name);
> 		return array;
> 	}
> 
>@@ -829,7 +976,13 @@ struct json_object json_get_array(struct json_object *object,
> 	if (a == NULL) {
> 		return array;
> 	}
>-	json_array_extend(array.root, a);
>+
>+	ret = json_array_extend(array.root, a);
>+	if (ret != 0) {
>+		DBG_ERR("Unable to get array [%s]\n", name);
>+		json_free(&array);
>+		return array;
>+	}
> 
> 	return array;
> }
>@@ -844,15 +997,17 @@ struct json_object json_get_array(struct json_object *object,
>  *
>  * @return The object, will be created if it did not exist.
>  */
>-struct json_object json_get_object(struct json_object *object,
>-				   const char* name)
>+struct json_object json_get_object(struct json_object *object, const char *name)
> {
> 
> 	struct json_object o = json_new_object();
> 	json_t *v = NULL;
>+	int ret = 0;
> 
>-	if (object->error) {
>-		o.error = true;
>+	if (json_is_invalid(object)) {
>+		DBG_ERR("Invalid JSON object, unable to get object [%s]\n",
>+			name);
>+		json_free(&o);
> 		return o;
> 	}
> 
>@@ -860,8 +1015,12 @@ struct json_object json_get_object(struct json_object *object,
> 	if (v == NULL) {
> 		return o;
> 	}
>-	json_object_update(o.root, v);
>-
>+	ret = json_object_update(o.root, v);
>+	if (ret != 0) {
>+		DBG_ERR("Unable to get object [%s]\n", name);
>+		json_free(&o);
>+		return o;
>+	}
> 	return o;
> }
> #endif
>diff --git a/lib/audit_logging/audit_logging.h b/lib/audit_logging/audit_logging.h
>index 4af743a4102..84738d2bb93 100644
>--- a/lib/audit_logging/audit_logging.h
>+++ b/lib/audit_logging/audit_logging.h
>@@ -16,7 +16,8 @@
>    You should have received a copy of the GNU General Public License
>    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
>-
>+#ifndef _AUDIT_LOGGING_H_
>+#define _AUDIT_LOGGING_H_
> #include <talloc.h>
> #include "lib/messaging/irpc.h"
> #include "lib/tsocket/tsocket.h"
>@@ -35,8 +36,11 @@ void audit_log_human_text(const char *prefix,
>  */
> struct json_object {
> 	json_t *root;
>-	bool error;
>+	bool valid;
> };
>+extern const struct json_object json_empty_object;
>+
>+#define JSON_ERROR -1
> 
> void audit_log_json(const char *prefix,
> 		    struct json_object *message,
>@@ -52,35 +56,31 @@ void json_free(struct json_object *object);
> void json_assert_is_array(struct json_object *array);
> bool json_is_invalid(struct json_object *object);
> 
>-void json_add_int(struct json_object *object,
>-		  const char* name,
>-		  const int value);
>-void json_add_bool(struct json_object *object,
>-		   const char* name,
>-		   const bool value);
>-void json_add_string(struct json_object *object,
>-		     const char* name,
>-		     const char* value);
>-void json_add_object(struct json_object *object,
>-		     const char* name,
>-		     struct json_object *value);
>-void json_add_stringn(struct json_object *object,
>-		      const char *name,
>-		      const char *value,
>-		      const size_t len);
>-void json_add_version(struct json_object *object,
>-		      int major,
>-		      int minor);
>-void json_add_timestamp(struct json_object *object);
>-void json_add_address(struct json_object *object,
>-		      const char *name,
>-		      const struct tsocket_address *address);
>-void json_add_sid(struct json_object *object,
>+int json_add_int(struct json_object *object, const char *name, const int value);
>+int json_add_bool(struct json_object *object,
> 		  const char *name,
>-		  const struct dom_sid *sid);
>-void json_add_guid(struct json_object *object,
>-		   const char *name,
>-		   const struct GUID *guid);
>+		  const bool value);
>+int json_add_string(struct json_object *object,
>+		    const char *name,
>+		    const char *value);
>+int json_add_object(struct json_object *object,
>+		    const char *name,
>+		    struct json_object *value);
>+int json_add_stringn(struct json_object *object,
>+		     const char *name,
>+		     const char *value,
>+		     const size_t len);
>+int json_add_version(struct json_object *object, int major, int minor);
>+int json_add_timestamp(struct json_object *object);
>+int json_add_address(struct json_object *object,
>+		     const char *name,
>+		     const struct tsocket_address *address);
>+int json_add_sid(struct json_object *object,
>+		 const char *name,
>+		 const struct dom_sid *sid);
>+int json_add_guid(struct json_object *object,
>+		  const char *name,
>+		  const struct GUID *guid);
> 
> struct json_object json_get_array(struct json_object *object,
> 				  const char* name);
>@@ -89,3 +89,4 @@ struct json_object json_get_object(struct json_object *object,
> char *json_to_string(TALLOC_CTX *mem_ctx,
> 		     struct json_object *object);
> #endif
>+#endif
>diff --git a/lib/audit_logging/tests/audit_logging_test.c b/lib/audit_logging/tests/audit_logging_test.c
>index 26875c98944..07c52eada39 100644
>--- a/lib/audit_logging/tests/audit_logging_test.c
>+++ b/lib/audit_logging/tests/audit_logging_test.c
>@@ -64,12 +64,15 @@ static void test_json_add_int(void **state)
> 	struct json_object object;
> 	struct json_t *value = NULL;
> 	double n;
>+	int rc = 0;
> 
> 	object = json_new_object();
>-	json_add_int(&object, "positive_one", 1);
>-	json_add_int(&object, "zero", 0);
>-	json_add_int(&object, "negative_one", -1);
>-
>+	rc = json_add_int(&object, "positive_one", 1);
>+	assert_int_equal(0, rc);
>+	rc = json_add_int(&object, "zero", 0);
>+	assert_int_equal(0, rc);
>+	rc = json_add_int(&object, "negative_one", -1);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(3, json_object_size(object.root));
> 
>@@ -88,18 +91,27 @@ static void test_json_add_int(void **state)
> 	n = json_number_value(value);
> 	assert_true(n == -1.0);
> 
>+	object.valid = false;
>+	rc = json_add_int(&object, "should fail 1", 0xf1);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_int(&object, "should fail 2", 0xf2);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_bool(void **state)
> {
> 	struct json_object object;
> 	struct json_t *value = NULL;
>+	int rc = 0;
> 
> 	object = json_new_object();
>-	json_add_bool(&object, "true", true);
>-	json_add_bool(&object, "false", false);
>-
>+	rc = json_add_bool(&object, "true", true);
>+	assert_int_equal(0, rc);
>+	rc = json_add_bool(&object, "false", false);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(2, json_object_size(object.root));
> 
>@@ -111,7 +123,14 @@ static void test_json_add_bool(void **state)
> 	assert_true(json_is_boolean(value));
> 	assert_true(value == json_false());
> 
>+	object.valid = false;
>+	rc = json_add_bool(&object, "should fail 1", true);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_bool(&object, "should fail 2", false);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_string(void **state)
>@@ -119,13 +138,15 @@ static void test_json_add_string(void **state)
> 	struct json_object object;
> 	struct json_t *value = NULL;
> 	const char *s = NULL;
>+	int rc = 0;
> 
> 	object = json_new_object();
>-	json_add_string(&object, "null", NULL);
>-	json_add_string(&object, "empty", "");
>-	json_add_string(&object, "name", "value");
>-
>-
>+	rc = json_add_string(&object, "null", NULL);
>+	assert_int_equal(0, rc);
>+	rc = json_add_string(&object, "empty", "");
>+	assert_int_equal(0, rc);
>+	rc = json_add_string(&object, "name", "value");
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(3, json_object_size(object.root));
> 
>@@ -141,21 +162,32 @@ static void test_json_add_string(void **state)
> 	assert_true(json_is_string(value));
> 	s = json_string_value(value);
> 	assert_string_equal("value", s);
>+
>+	object.valid = false;
>+	rc = json_add_string(&object, "should fail 1", "A value");
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_string(&object, "should fail 2", "Another value");
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_object(void **state)
> {
> 	struct json_object object;
> 	struct json_object other;
>+	struct json_object after;
>+	struct json_object invalid = json_empty_object;
> 	struct json_t *value = NULL;
>+	int rc = 0;
> 
> 	object = json_new_object();
> 	other  = json_new_object();
>-	json_add_object(&object, "null", NULL);
>-	json_add_object(&object, "other", &other);
>-
>-
>+	rc = json_add_object(&object, "null", NULL);
>+	assert_int_equal(0, rc);
>+	rc = json_add_object(&object, "other", &other);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(2, json_object_size(object.root));
> 
>@@ -166,7 +198,20 @@ static void test_json_add_object(void **state)
> 	assert_true(json_is_object(value));
> 	assert_ptr_equal(other.root, value);
> 
>+	rc = json_add_object(&object, "invalid", &invalid);
>+	assert_int_equal(JSON_ERROR, rc);
>+
>+	object.valid = false;
>+	after = json_new_object();
>+	rc = json_add_object(&object, "after", &after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_object(&object, "after", &after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
>+	json_free(&after);
> }
> 
> static void test_json_add_to_array(void **state)
>@@ -175,7 +220,10 @@ static void test_json_add_to_array(void **state)
> 	struct json_object o1;
> 	struct json_object o2;
> 	struct json_object o3;
>+	struct json_object after;
>+	struct json_object invalid = json_empty_object;
> 	struct json_t *value = NULL;
>+	int rc = 0;
> 
> 	array = json_new_array();
> 	assert_true(json_is_array(array.root));
>@@ -184,10 +232,14 @@ static void test_json_add_to_array(void **state)
> 	o2 = json_new_object();
> 	o3 = json_new_object();
> 
>-	json_add_object(&array, NULL, &o3);
>-	json_add_object(&array, "", &o2);
>-	json_add_object(&array, "will-be-ignored", &o1);
>-	json_add_object(&array, NULL, NULL);
>+	rc = json_add_object(&array, NULL, &o3);
>+	assert_int_equal(0, rc);
>+	rc = json_add_object(&array, "", &o2);
>+	assert_int_equal(0, rc);
>+	rc = json_add_object(&array, "will-be-ignored", &o1);
>+	assert_int_equal(0, rc);
>+	rc = json_add_object(&array, NULL, NULL);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(4, json_array_size(array.root));
> 
>@@ -203,8 +255,20 @@ static void test_json_add_to_array(void **state)
> 	value = json_array_get(array.root, 3);
> 	assert_true(json_is_null(value));
> 
>+	rc = json_add_object(&array, "invalid", &invalid);
>+	assert_int_equal(JSON_ERROR, rc);
>+
>+	array.valid = false;
>+	after = json_new_object();
>+	rc = json_add_object(&array, "after", &after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&array);
> 
>+	rc = json_add_object(&array, "after", &after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
>+	json_free(&after);
> }
> 
> static void test_json_add_timestamp(void **state)
>@@ -224,7 +288,8 @@ static void test_json_add_timestamp(void **state)
> 
> 	object = json_new_object();
> 	before = time(NULL);
>-	json_add_timestamp(&object);
>+	rc = json_add_timestamp(&object);
>+	assert_int_equal(0, rc);
> 	after = time(NULL);
> 
> 	ts = json_object_get(object.root, "timestamp");
>@@ -263,7 +328,14 @@ static void test_json_add_timestamp(void **state)
> 	assert_true(difftime(actual, before) >= 0);
> 	assert_true(difftime(after, actual) >= 0);
> 
>+	object.valid = false;
>+	rc = json_add_timestamp(&object);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_timestamp(&object);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_stringn(void **state)
>@@ -271,17 +343,26 @@ static void test_json_add_stringn(void **state)
> 	struct json_object object;
> 	struct json_t *value = NULL;
> 	const char *s = NULL;
>+	int rc = 0;
> 
> 	object = json_new_object();
>-	json_add_stringn(&object, "null", NULL, 10);
>-	json_add_stringn(&object, "null-zero-len", NULL, 0);
>-	json_add_stringn(&object, "empty", "", 1);
>-	json_add_stringn(&object, "empty-zero-len", "", 0);
>-	json_add_stringn(&object, "value-less-than-len", "123456", 7);
>-	json_add_stringn(&object, "value-greater-than-len", "abcd", 3);
>-	json_add_stringn(&object, "value-equal-len", "ZYX", 3);
>-	json_add_stringn(&object, "value-len-is-zero", "this will be null", 0);
>-
>+	rc = json_add_stringn(&object, "null", NULL, 10);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "null-zero-len", NULL, 0);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "empty", "", 1);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "empty-zero-len", "", 0);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "value-less-than-len", "123456", 7);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "value-greater-than-len", "abcd", 3);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(&object, "value-equal-len", "ZYX", 3);
>+	assert_int_equal(0, rc);
>+	rc = json_add_stringn(
>+	    &object, "value-len-is-zero", "this will be null", 0);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(8, json_object_size(object.root));
> 
>@@ -314,7 +395,14 @@ static void test_json_add_stringn(void **state)
> 	value = json_object_get(object.root, "value-len-is-zero");
> 	assert_true(json_is_null(value));
> 
>+	object.valid = false;
>+	rc = json_add_stringn(&object, "fail-01", "xxxxxxx", 1);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_stringn(&object, "fail-02", "xxxxxxx", 1);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_version(void **state)
>@@ -323,9 +411,11 @@ static void test_json_add_version(void **state)
> 	struct json_t *version = NULL;
> 	struct json_t *v = NULL;
> 	double n;
>+	int rc;
> 
> 	object = json_new_object();
>-	json_add_version(&object, 3, 1);
>+	rc = json_add_version(&object, 3, 1);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(1, json_object_size(object.root));
> 
>@@ -343,7 +433,14 @@ static void test_json_add_version(void **state)
> 	n = json_number_value(v);
> 	assert_true(n == 1.0);
> 
>+	object.valid = false;
>+	rc = json_add_version(&object, 3, 1);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_version(&object, 3, 1);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_address(void **state)
>@@ -353,6 +450,8 @@ static void test_json_add_address(void **state)
> 	struct tsocket_address *ip4  = NULL;
> 	struct tsocket_address *ip6  = NULL;
> 	struct tsocket_address *pipe = NULL;
>+
>+	struct tsocket_address *after = NULL;
> 	const char *s = NULL;
> 	int rc;
> 
>@@ -360,7 +459,8 @@ static void test_json_add_address(void **state)
> 
> 	object = json_new_object();
> 
>-	json_add_address(&object, "null", NULL);
>+	rc = json_add_address(&object, "null", NULL);
>+	assert_int_equal(0, rc);
> 
> 	rc = tsocket_address_inet_from_strings(
> 		ctx,
>@@ -369,7 +469,8 @@ static void test_json_add_address(void **state)
> 		21,
> 		&ip4);
> 	assert_int_equal(0, rc);
>-	json_add_address(&object, "ip4", ip4);
>+	rc = json_add_address(&object, "ip4", ip4);
>+	assert_int_equal(0, rc);
> 
> 	rc = tsocket_address_inet_from_strings(
> 		ctx,
>@@ -378,11 +479,13 @@ static void test_json_add_address(void **state)
> 		42,
> 		&ip6);
> 	assert_int_equal(0, rc);
>-	json_add_address(&object, "ip6", ip6);
>+	rc = json_add_address(&object, "ip6", ip6);
>+	assert_int_equal(0, rc);
> 
> 	rc = tsocket_address_unix_from_path(ctx, "/samba/pipe", &pipe);
> 	assert_int_equal(0, rc);
>-	json_add_address(&object, "pipe", pipe);
>+	rc = json_add_address(&object, "pipe", pipe);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(4, json_object_size(object.root));
> 
>@@ -404,7 +507,18 @@ static void test_json_add_address(void **state)
> 	s = json_string_value(value);
> 	assert_string_equal("unix:/samba/pipe", s);
> 
>+	object.valid = false;
>+	rc = tsocket_address_inet_from_strings(
>+	    ctx, "ip", "127.0.0.11", 23, &after);
>+	assert_int_equal(0, rc);
>+	rc = json_add_address(&object, "invalid_object", after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_address(&object, "freed object", after);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	TALLOC_FREE(ctx);
> }
> 
>@@ -415,14 +529,16 @@ static void test_json_add_sid(void **state)
> 	const char *SID = "S-1-5-21-2470180966-3899876309-2637894779";
> 	struct dom_sid sid;
> 	const char *s = NULL;
>-
>+	int rc;
> 
> 	object = json_new_object();
> 
>-	json_add_sid(&object, "null", NULL);
>+	rc = json_add_sid(&object, "null", NULL);
>+	assert_int_equal(0, rc);
> 
> 	assert_true(string_to_sid(&sid, SID));
>-	json_add_sid(&object, "sid", &sid);
>+	rc = json_add_sid(&object, "sid", &sid);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(2, json_object_size(object.root));
> 
>@@ -433,7 +549,15 @@ static void test_json_add_sid(void **state)
> 	assert_true(json_is_string(value));
> 	s = json_string_value(value);
> 	assert_string_equal(SID, s);
>+
>+	object.valid = false;
>+	rc = json_add_sid(&object, "invalid_object", &sid);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_sid(&object, "freed_object", &sid);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_add_guid(void **state)
>@@ -444,15 +568,17 @@ static void test_json_add_guid(void **state)
> 	struct GUID guid;
> 	const char *s = NULL;
> 	NTSTATUS status;
>-
>+	int rc;
> 
> 	object = json_new_object();
> 
>-	json_add_guid(&object, "null", NULL);
>+	rc = json_add_guid(&object, "null", NULL);
>+	assert_int_equal(0, rc);
> 
> 	status = GUID_from_string(GUID, &guid);
> 	assert_true(NT_STATUS_IS_OK(status));
>-	json_add_guid(&object, "guid", &guid);
>+	rc = json_add_guid(&object, "guid", &guid);
>+	assert_int_equal(0, rc);
> 
> 	assert_int_equal(2, json_object_size(object.root));
> 
>@@ -464,7 +590,14 @@ static void test_json_add_guid(void **state)
> 	s = json_string_value(value);
> 	assert_string_equal(GUID, s);
> 
>+	object.valid = false;
>+	rc = json_add_guid(&object, "invalid_object", &guid);
>+	assert_int_equal(JSON_ERROR, rc);
>+
> 	json_free(&object);
>+
>+	rc = json_add_guid(&object, "freed_object", &guid);
>+	assert_int_equal(JSON_ERROR, rc);
> }
> 
> static void test_json_to_string(void **state)
>@@ -475,12 +608,7 @@ static void test_json_to_string(void **state)
> 	TALLOC_CTX *ctx = talloc_new(NULL);
> 
> 	object = json_new_object();
>-	object.error = true;
>-
>-	s = json_to_string(ctx, &object);
>-	assert_null(s);
> 
>-	object.error = false;
> 	s = json_to_string(ctx, &object);
> 	assert_string_equal("{}", s);
> 	TALLOC_FREE(s);
>@@ -490,7 +618,17 @@ static void test_json_to_string(void **state)
> 	assert_string_equal("{\"name\": \"value\"}", s);
> 	TALLOC_FREE(s);
> 
>+	object.valid = false;
>+	s = json_to_string(ctx, &object);
>+	assert_null(s);
>+
> 	json_free(&object);
>+
>+	object.valid = true;
>+	object.root = NULL;
>+
>+	s = json_to_string(ctx, &object);
>+	assert_null(s);
> 	TALLOC_FREE(ctx);
> }
> 
>@@ -507,7 +645,7 @@ static void test_json_get_array(void **state)
> 	object = json_new_object();
> 
> 	array = json_get_array(&object, "not-there");
>-	assert_false(array.error);
>+	assert_true(array.valid);
> 	assert_non_null(array.root);
> 	assert_true(json_is_array(array.root));
> 	json_free(&array);
>@@ -518,7 +656,7 @@ static void test_json_get_array(void **state)
> 	json_add_object(&object, "stored_array", &stored_array);
> 
> 	array = json_get_array(&object, "stored_array");
>-	assert_false(array.error);
>+	assert_true(array.valid);
> 	assert_non_null(array.root);
> 	assert_true(json_is_array(array.root));
> 
>@@ -542,7 +680,7 @@ static void test_json_get_array(void **state)
> 	assert_true(json_is_array(array.root));
> 	o2 = json_new_object();
> 	json_add_string(&o2, "value", "value-two");
>-	assert_false(o2.error);
>+	assert_true(o2.valid);
> 	json_add_object(&array, NULL, &o2);
> 	assert_true(json_is_array(array.root));
> 	json_add_object(&object, "stored_array", &array);
>@@ -551,7 +689,7 @@ static void test_json_get_array(void **state)
> 	array = json_get_array(&object, "stored_array");
> 	assert_non_null(array.root);
> 	assert_true(json_is_array(array.root));
>-	assert_false(array.error);
>+	assert_true(array.valid);
> 	assert_true(json_is_array(array.root));
> 
> 	assert_int_equal(2, json_array_size(array.root));
>@@ -577,6 +715,10 @@ static void test_json_get_array(void **state)
> 
> 	json_free(&array);
> 	json_free(&object);
>+
>+	array = json_get_array(&object, "stored_array");
>+	assert_false(array.valid);
>+	json_free(&array);
> }
> 
> static void test_json_get_object(void **state)
>@@ -590,7 +732,7 @@ static void test_json_get_object(void **state)
> 	object = json_new_object();
> 
> 	o1 = json_get_object(&object, "not-there");
>-	assert_false(o1.error);
>+	assert_true(o1.valid);
> 	assert_non_null(o1.root);
> 	assert_true(json_is_object(o1.root));
> 	json_free(&o1);
>@@ -600,7 +742,7 @@ static void test_json_get_object(void **state)
> 	json_add_object(&object, "stored_object", &o1);
> 
> 	o2 = json_get_object(&object, "stored_object");
>-	assert_false(o2.error);
>+	assert_true(o2.valid);
> 	assert_non_null(o2.root);
> 	assert_true(json_is_object(o2.root));
> 
>@@ -615,7 +757,7 @@ static void test_json_get_object(void **state)
> 
> 
> 	o3 = json_get_object(&object, "stored_object");
>-	assert_false(o3.error);
>+	assert_true(o3.valid);
> 	assert_non_null(o3.root);
> 	assert_true(json_is_object(o3.root));
> 
>@@ -627,6 +769,10 @@ static void test_json_get_object(void **state)
> 
> 	json_free(&o3);
> 	json_free(&object);
>+
>+	o3 = json_get_object(&object, "stored_object");
>+	assert_false(o3.valid);
>+	json_free(&o3);
> }
> 
> static void test_audit_get_timestamp(void **state)
>diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
>index 581f2f235eb..dd714bca4e1 100644
>--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
>+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
>@@ -176,6 +176,7 @@ static const char *get_password_action(
>  *
>  * @return the generated JSON object, should be freed with json_free.
>  *
>+ *
>  */
> static struct json_object operation_json(
> 	struct ldb_module *module,
>@@ -185,8 +186,8 @@ static struct json_object operation_json(
> 	struct ldb_context *ldb = NULL;
> 	const struct dom_sid *sid = NULL;
> 	bool as_system = false;
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
> 	const struct tsocket_address *remote = NULL;
> 	const char *dn = NULL;
> 	const char* operation = NULL;
>@@ -195,6 +196,7 @@ static struct json_object operation_json(
> 	struct audit_private *audit_private
> 		= talloc_get_type_abort(ldb_module_get_private(module),
> 					struct audit_private);
>+	int rc = 0;
> 
> 	ldb = ldb_module_get_ctx(module);
> 
>@@ -213,18 +215,50 @@ static struct json_object operation_json(
> 	operation = dsdb_audit_get_operation_name(request);
> 
> 	audit = json_new_object();
>-	json_add_version(&audit, OPERATION_MAJOR, OPERATION_MINOR);
>-	json_add_int(&audit, "statusCode", reply->error);
>-	json_add_string(&audit, "status", ldb_strerror(reply->error));
>-	json_add_string(&audit, "operation", operation);
>-	json_add_address(&audit, "remoteAddress", remote);
>-	json_add_bool(&audit, "performedAsSystem", as_system);
>-	json_add_sid(&audit, "userSid", sid);
>-	json_add_string(&audit, "dn", dn);
>-	json_add_guid(&audit,
>-		      "transactionId",
>-		      &audit_private->transaction_guid);
>-	json_add_guid(&audit, "sessionId", unique_session_token);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&audit, OPERATION_MAJOR, OPERATION_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "statusCode", reply->error);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "status", ldb_strerror(reply->error));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "operation", operation);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&audit, "remoteAddress", remote);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_bool(&audit, "performedAsSystem", as_system);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(&audit, "userSid", sid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "dn", dn);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &audit, "transactionId", &audit_private->transaction_guid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "sessionId", unique_session_token);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	message = dsdb_audit_get_message(request);
> 	if (message != NULL) {
>@@ -232,13 +266,46 @@ static struct json_object operation_json(
> 			dsdb_audit_attributes_json(
> 				request->operation,
> 				message);
>-		json_add_object(&audit, "attributes", &attributes);
>+		if (json_is_invalid(&attributes)) {
>+			goto failure;
>+		}
>+		rc = json_add_object(&audit, "attributes", &attributes);
>+		if (rc != 0) {
>+			goto failure;
>+		}
> 	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", OPERATION_JSON_TYPE);
>-	json_add_object(&wrapper, OPERATION_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", OPERATION_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, OPERATION_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	return wrapper;
>+
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to free it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&audit);
>+	json_free(&wrapper);
>+	DBG_ERR("Unable to create ldb operation JSON audit message\n");
> 	return wrapper;
> }
> 
>@@ -252,6 +319,7 @@ static struct json_object operation_json(
>  * @paran reply the result of the operation
>  *
>  * @return the generated JSON object, should be freed with json_free.
>+ *         NULL if there was an error generating the message.
>  *
>  */
> static struct json_object replicated_update_json(
>@@ -259,8 +327,8 @@ static struct json_object replicated_update_json(
> 	const struct ldb_request *request,
> 	const struct ldb_reply *reply)
> {
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
> 	struct audit_private *audit_private
> 		= talloc_get_type_abort(ldb_module_get_private(module),
> 					struct audit_private);
>@@ -269,35 +337,93 @@ static struct json_object replicated_update_json(
> 		struct dsdb_extended_replicated_objects);
> 	const char *partition_dn = NULL;
> 	const char *error = NULL;
>+	int rc = 0;
> 
> 	partition_dn = ldb_dn_get_linearized(ro->partition_dn);
> 	error = get_friendly_werror_msg(ro->error);
> 
> 	audit = json_new_object();
>-	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",
>-		      &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);
>-	json_add_string(&audit, "error", error);
>-	json_add_int(&audit, "errorCode", W_ERROR_V(ro->error));
>-	json_add_guid(
>-		&audit,
>-		"sourceDsa",
>-		&ro->source_dsa->source_dsa_obj_guid);
>-	json_add_guid(
>-		&audit,
>-		"invocationId",
>-		&ro->source_dsa->source_dsa_invocation_id);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&audit, REPLICATION_MAJOR, REPLICATION_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "statusCode", reply->error);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "status", ldb_strerror(reply->error));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &audit, "transactionId", &audit_private->transaction_guid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "objectCount", ro->num_objects);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "linkCount", ro->linked_attributes_count);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "partitionDN", partition_dn);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "error", error);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "errorCode", W_ERROR_V(ro->error));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &audit, "sourceDsa", &ro->source_dsa->source_dsa_obj_guid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &audit, "invocationId", &ro->source_dsa->source_dsa_invocation_id);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", REPLICATION_JSON_TYPE);
>-	json_add_object(&wrapper, REPLICATION_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", REPLICATION_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, REPLICATION_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	return wrapper;
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to be freed it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&audit);
>+	json_free(&wrapper);
>+	DBG_ERR("Unable to create replicated update JSON audit message\n");
> 	return wrapper;
> }
> 
>@@ -321,16 +447,16 @@ static struct json_object password_change_json(
> {
> 	struct ldb_context *ldb = NULL;
> 	const struct dom_sid *sid = NULL;
>-	const char* dn = NULL;
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	const char *dn = NULL;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
> 	const struct tsocket_address *remote = NULL;
> 	const char* action = NULL;
> 	const struct GUID *unique_session_token = NULL;
> 	struct audit_private *audit_private
> 		= talloc_get_type_abort(ldb_module_get_private(module),
> 					struct audit_private);
>-
>+	int rc = 0;
> 
> 	ldb = ldb_module_get_ctx(module);
> 
>@@ -341,24 +467,79 @@ static struct json_object password_change_json(
> 	unique_session_token = dsdb_audit_get_unique_session_token(module);
> 
> 	audit = json_new_object();
>-	json_add_version(&audit, PASSWORD_MAJOR, PASSWORD_MINOR);
>-	json_add_int(&audit, "statusCode", reply->error);
>-	json_add_string(&audit, "status", ldb_strerror(reply->error));
>-	json_add_address(&audit, "remoteAddress", remote);
>-	json_add_sid(&audit, "userSid", sid);
>-	json_add_string(&audit, "dn", dn);
>-	json_add_string(&audit, "action", action);
>-	json_add_guid(&audit,
>-		      "transactionId",
>-		      &audit_private->transaction_guid);
>-	json_add_guid(&audit, "sessionId", unique_session_token);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&audit, PASSWORD_MAJOR, PASSWORD_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "statusCode", reply->error);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "status", ldb_strerror(reply->error));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&audit, "remoteAddress", remote);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(&audit, "userSid", sid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "dn", dn);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "action", action);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(
>+	    &audit, "transactionId", &audit_private->transaction_guid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "sessionId", unique_session_token);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", PASSWORD_JSON_TYPE);
>-	json_add_object(&wrapper, PASSWORD_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", PASSWORD_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, PASSWORD_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	return wrapper;
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to free it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&wrapper);
>+	json_free(&audit);
>+	DBG_ERR("Unable to create password change JSON audit message\n");
>+	return wrapper;
> }
> 
> 
>@@ -380,22 +561,64 @@ static struct json_object transaction_json(
> 	struct GUID *transaction_id,
> 	const int64_t duration)
> {
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
>+	int rc = 0;
> 
> 	audit = json_new_object();
>-	json_add_version(&audit, TRANSACTION_MAJOR, TRANSACTION_MINOR);
>-	json_add_string(&audit, "action", action);
>-	json_add_guid(&audit, "transactionId", transaction_id);
>-	json_add_int(&audit, "duration", duration);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
> 
>+	rc = json_add_version(&audit, TRANSACTION_MAJOR, TRANSACTION_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "action", action);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "transactionId", transaction_id);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "duration", duration);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", TRANSACTION_JSON_TYPE);
>-	json_add_object(&wrapper, TRANSACTION_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", TRANSACTION_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, TRANSACTION_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	return wrapper;
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to free it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&wrapper);
>+	json_free(&audit);
>+	DBG_ERR("Unable to create transaction JSON audit message\n");
>+	return wrapper;
> }
> 
> 
>@@ -416,24 +639,75 @@ static struct json_object commit_failure_json(
> 	const char *reason,
> 	struct GUID *transaction_id)
> {
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
>+	int rc = 0;
> 
> 	audit = json_new_object();
>-	json_add_version(&audit, TRANSACTION_MAJOR, TRANSACTION_MINOR);
>-	json_add_string(&audit, "action", action);
>-	json_add_guid(&audit, "transactionId", transaction_id);
>-	json_add_int(&audit, "duration", duration);
>-	json_add_int(&audit, "statusCode", status);
>-	json_add_string(&audit, "status", ldb_strerror(status));
>-	json_add_string(&audit, "reason", reason);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&audit, TRANSACTION_MAJOR, TRANSACTION_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "action", action);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "transactionId", transaction_id);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "duration", duration);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "statusCode", status);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "status", ldb_strerror(status));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "reason", reason);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", TRANSACTION_JSON_TYPE);
>-	json_add_object(&wrapper, TRANSACTION_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", TRANSACTION_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, TRANSACTION_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	return wrapper;
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to free it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&audit);
>+	json_free(&wrapper);
>+	DBG_ERR("Unable to create commit failure JSON audit message\n");
>+	return wrapper;
> }
> 
> #endif
>diff --git a/source4/dsdb/samdb/ldb_modules/audit_util.c b/source4/dsdb/samdb/ldb_modules/audit_util.c
>index 766c34c1e23..edf3c5e4b46 100644
>--- a/source4/dsdb/samdb/ldb_modules/audit_util.c
>+++ b/source4/dsdb/samdb/ldb_modules/audit_util.c
>@@ -467,30 +467,39 @@ const char *dsdb_audit_get_modification_action(unsigned int flags)
>  * @param lv the ldb_val to convert and append to the array.
>  *
>  */
>-static void dsdb_audit_add_ldb_value(
>-	struct json_object *array,
>-	const struct ldb_val lv)
>+static int dsdb_audit_add_ldb_value(struct json_object *array,
>+				    const struct ldb_val lv)
> {
> 	bool base64;
> 	int len;
>-	struct json_object value;
>+	struct json_object value = json_empty_object;
>+	int rc = 0;
> 
> 	json_assert_is_array(array);
> 	if (json_is_invalid(array)) {
>-		return;
>+		return -1;
> 	}
> 
> 	if (lv.length == 0 || lv.data == NULL) {
>-		json_add_object(array, NULL, NULL);
>-		return;
>+		rc = json_add_object(array, NULL, NULL);
>+		if (rc != 0) {
>+			goto failure;
>+		}
>+		return 0;
> 	}
> 
> 	base64 = ldb_should_b64_encode(NULL, &lv);
> 	len = min(lv.length, MAX_LENGTH);
> 	value = json_new_object();
>+	if (json_is_invalid(&value)) {
>+		goto failure;
>+	}
> 
> 	if (lv.length > MAX_LENGTH) {
>-		json_add_bool(&value, "truncated", true);
>+		rc = json_add_bool(&value, "truncated", true);
>+		if (rc != 0) {
>+			goto failure;
>+		}
> 	}
> 	if (base64) {
> 		TALLOC_CTX *ctx = talloc_new(NULL);
>@@ -499,16 +508,43 @@ static void dsdb_audit_add_ldb_value(
> 			(char*) lv.data,
> 			len);
> 
>-		json_add_bool(&value, "base64", true);
>-		json_add_string(&value, "value", encoded);
>+		if (ctx == NULL) {
>+			goto failure;
>+		}
>+
>+		rc = json_add_bool(&value, "base64", true);
>+		if (rc != 0) {
>+			TALLOC_FREE(ctx);
>+			goto failure;
>+		}
>+		rc = json_add_string(&value, "value", encoded);
>+		if (rc != 0) {
>+			TALLOC_FREE(ctx);
>+			goto failure;
>+		}
> 		TALLOC_FREE(ctx);
> 	} else {
>-		json_add_stringn(&value, "value", (char *)lv.data, len);
>+		rc = json_add_stringn(&value, "value", (char *)lv.data, len);
>+		if (rc != 0) {
>+			goto failure;
>+		}
> 	}
> 	/*
> 	 * As array is a JSON array the element name is NULL
> 	 */
>-	json_add_object(array, NULL, &value);
>+	rc = json_add_object(array, NULL, &value);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	return 0;
>+failure:
>+	/*
>+	 * In the event of a failure value will not have been added to array
>+	 * so it needs to be freed to prevent a leak.
>+	 */
>+	json_free(&value);
>+	DBG_ERR("unable to add ldb value to JSON audit message");
>+	return -1;
> }
> 
> /*
>@@ -550,13 +586,23 @@ struct json_object dsdb_audit_attributes_json(
> 	const struct ldb_message* message)
> {
> 
>-	struct json_object attributes = json_new_object();
> 	int i, j;
>+	struct json_object attributes = json_new_object();
>+
>+	if (json_is_invalid(&attributes)) {
>+		goto failure;
>+	}
> 	for (i=0;i<message->num_elements;i++) {
>-		struct json_object actions;
>-		struct json_object attribute;
>-		struct json_object action = json_new_object();
>+		struct json_object actions = json_empty_object;
>+		struct json_object attribute = json_empty_object;
>+		struct json_object action = json_empty_object;
> 		const char *name = message->elements[i].name;
>+		int rc = 0;
>+
>+		action = json_new_object();
>+		if (json_is_invalid(&action)) {
>+			goto failure;
>+		}
> 
> 		/*
> 		 * If this is a modify operation tag the attribute with
>@@ -566,10 +612,18 @@ struct json_object dsdb_audit_attributes_json(
> 			const char *act = NULL;
> 			const int flags =  message->elements[i].flags;
> 			act = dsdb_audit_get_modification_action(flags);
>-			json_add_string(&action, "action" , act);
>+			rc = json_add_string(&action, "action", act);
>+			if (rc != 0) {
>+				json_free(&action);
>+				goto failure;
>+			}
> 		}
> 		if (operation == LDB_ADD) {
>-			json_add_string(&action, "action" , "add");
>+			rc = json_add_string(&action, "action", "add");
>+			if (rc != 0) {
>+				json_free(&action);
>+				goto failure;
>+			}
> 		}
> 
> 		/*
>@@ -577,25 +631,67 @@ struct json_object dsdb_audit_attributes_json(
> 		 * and don't include the values
> 		 */
> 		if (dsdb_audit_redact_attribute(name)) {
>-			json_add_bool(&action, "redacted", true);
>+			rc = json_add_bool(&action, "redacted", true);
>+			if (rc != 0) {
>+				json_free(&action);
>+				goto failure;
>+			}
> 		} else {
> 			struct json_object values;
> 			/*
> 			 * Add the values for the action
> 			 */
> 			values = json_new_array();
>+			if (json_is_invalid(&values)) {
>+				json_free(&action);
>+				goto failure;
>+			}
>+
> 			for (j=0;j<message->elements[i].num_values;j++) {
>-				dsdb_audit_add_ldb_value(
>-					&values,
>-					message->elements[i].values[j]);
>+				rc = dsdb_audit_add_ldb_value(
>+				    &values, message->elements[i].values[j]);
>+				if (rc != 0) {
>+					json_free(&values);
>+					json_free(&action);
>+					goto failure;
>+				}
>+			}
>+			rc = json_add_object(&action, "values", &values);
>+			if (rc != 0) {
>+				json_free(&values);
>+				json_free(&action);
>+				goto failure;
> 			}
>-			json_add_object(&action, "values", &values);
> 		}
> 		attribute = json_get_object(&attributes, name);
>+		if (json_is_invalid(&attribute)) {
>+			json_free(&action);
>+			goto failure;
>+		}
> 		actions = json_get_array(&attribute, "actions");
>-		json_add_object(&actions, NULL, &action);
>-		json_add_object(&attribute, "actions", &actions);
>-		json_add_object(&attributes, name, &attribute);
>+		if (json_is_invalid(&actions)) {
>+			json_free(&action);
>+			goto failure;
>+		}
>+		rc = json_add_object(&actions, NULL, &action);
>+		if (rc != 0) {
>+			json_free(&action);
>+			goto failure;
>+		}
>+		rc = json_add_object(&attribute, "actions", &actions);
>+		if (rc != 0) {
>+			json_free(&actions);
>+			goto failure;
>+		}
>+		rc = json_add_object(&attributes, name, &attribute);
>+		if (rc != 0) {
>+			json_free(&attribute);
>+			goto failure;
>+		}
> 	}
> 	return attributes;
>+failure:
>+	json_free(&attributes);
>+	DBG_ERR("Unable to create ldb attributes JSON audit message\n");
>+	return attributes;
> }
>diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
>index eb592adc336..d5c9bbdaa1e 100644
>--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
>@@ -104,6 +104,7 @@ static struct GUID *get_transaction_id(
>  * @param status the ldb status code for the ldb operation.
>  *
>  * @return A json object containing the details.
>+ * 	   NULL if an error was detected
>  */
> static struct json_object audit_group_json(
> 	const struct ldb_module *module,
>@@ -115,11 +116,12 @@ static struct json_object audit_group_json(
> {
> 	struct ldb_context *ldb = NULL;
> 	const struct dom_sid *sid = NULL;
>-	struct json_object wrapper;
>-	struct json_object audit;
>+	struct json_object wrapper = json_empty_object;
>+	struct json_object audit = json_empty_object;
> 	const struct tsocket_address *remote = NULL;
> 	const struct GUID *unique_session_token = NULL;
> 	struct GUID *transaction_id = NULL;
>+	int rc = 0;
> 
> 	ldb = ldb_module_get_ctx(discard_const(module));
> 
>@@ -129,23 +131,82 @@ static struct json_object audit_group_json(
> 	transaction_id = get_transaction_id(request);
> 
> 	audit = json_new_object();
>-	json_add_version(&audit, AUDIT_MAJOR, AUDIT_MINOR);
>-	json_add_int(&audit, "statusCode", status);
>-	json_add_string(&audit, "status", ldb_strerror(status));
>-	json_add_string(&audit, "action", action);
>-	json_add_address(&audit, "remoteAddress", remote);
>-	json_add_sid(&audit, "userSid", sid);
>-	json_add_string(&audit, "group", group);
>-	json_add_guid(&audit, "transactionId", transaction_id);
>-	json_add_guid(&audit, "sessionId", unique_session_token);
>-	json_add_string(&audit, "user", user);
>+	if (json_is_invalid(&audit)) {
>+		goto failure;
>+	}
>+	rc = json_add_version(&audit, AUDIT_MAJOR, AUDIT_MINOR);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_int(&audit, "statusCode", status);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "status", ldb_strerror(status));
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "action", action);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_address(&audit, "remoteAddress", remote);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_sid(&audit, "userSid", sid);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "group", group);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "transactionId", transaction_id);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_guid(&audit, "sessionId", unique_session_token);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&audit, "user", user);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	wrapper = json_new_object();
>-	json_add_timestamp(&wrapper);
>-	json_add_string(&wrapper, "type", AUDIT_JSON_TYPE);
>-	json_add_object(&wrapper, AUDIT_JSON_TYPE, &audit);
>+	if (json_is_invalid(&wrapper)) {
>+		goto failure;
>+	}
>+	rc = json_add_timestamp(&wrapper);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_string(&wrapper, "type", AUDIT_JSON_TYPE);
>+	if (rc != 0) {
>+		goto failure;
>+	}
>+	rc = json_add_object(&wrapper, AUDIT_JSON_TYPE, &audit);
>+	if (rc != 0) {
>+		goto failure;
>+	}
> 
> 	return wrapper;
>+failure:
>+	/*
>+	 * On a failure audit will not have been added to wrapper so it
>+	 * needs to free it to avoid a leak.
>+	 *
>+	 * wrapper is freed to invalidate it as it will have only been
>+	 * partially constructed and may be inconsistent.
>+	 *
>+	 * All the json manipulation routines handle a freed object correctly
>+	 */
>+	json_free(&audit);
>+	json_free(&wrapper);
>+	DBG_ERR("Failed to create group change JSON log message\n");
>+	return wrapper;
> }
> #endif
> 
>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 7857ad5b56f..98cd95064be 100644
>--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
>@@ -74,7 +74,7 @@ void audit_message_send(
> 	struct json_object *message)
> {
> 	messages[messages_sent].root = json_deep_copy(message->root);
>-	messages[messages_sent].error = message->error;
>+	messages[messages_sent].valid = message->valid;
> 	messages_sent++;
> }
> 
>-- 
>2.11.0
>
>
>From ac4a139bdcac93451435643b50e1f286399632d8 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 13 Dec 2018 13:53:08 +1300
>Subject: [PATCH 2/3] audit_logging: Remove debug log header and JSON
> Authentication: prefix
>
>Feedback from real-world users is that they really want raw JSON
>strings in the log.
>
>We can not easily remove the leading "  " but the other strings above
>and before the JSON are really annoying to strip back off
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13714
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>(cherry picked from commit edab1318f9138c0d87de7cc7cfa5da8e29c906f8)
>---
> auth/auth_log.c                              | 13 ++++---------
> lib/audit_logging/audit_logging.c            | 19 ++++++++++++-------
> lib/audit_logging/audit_logging.h            |  3 +--
> source4/dsdb/samdb/ldb_modules/audit_log.c   |  5 -----
> source4/dsdb/samdb/ldb_modules/group_audit.c |  2 --
> 5 files changed, 17 insertions(+), 25 deletions(-)
>
>diff --git a/auth/auth_log.c b/auth/auth_log.c
>index 9e57b1d3968..9a873561284 100644
>--- a/auth/auth_log.c
>+++ b/auth/auth_log.c
>@@ -78,11 +78,10 @@ static const char* get_password_type(const struct auth_usersupplied_info *ui);
> static void log_json(struct imessaging_context *msg_ctx,
> 		     struct loadparm_context *lp_ctx,
> 		     struct json_object *object,
>-		     const char *type,
> 		     int debug_class,
> 		     int debug_level)
> {
>-	audit_log_json(type, object, debug_class, debug_level);
>+	audit_log_json(object, debug_class, debug_level);
> 	if (msg_ctx && lp_ctx && lpcfg_auth_event_notification(lp_ctx)) {
> 		audit_message_send(msg_ctx,
> 				   AUTH_EVENT_NAME,
>@@ -102,9 +101,8 @@ static void log_json(struct imessaging_context *msg_ctx,
>  *  To process the resulting log lines from the commend line use jq to
>  *  parse the json.
>  *
>- *  grep "JSON Authentication" log file |
>- *  sed 's;^[^{]*;;' |
>- * jq -rc  '"\(.timestamp)\t\(.Authentication.status)\t
>+ *  grep "^  {" log file |
>+ *  jq -rc '"\(.timestamp)\t\(.Authentication.status)\t
>  *           \(.Authentication.clientDomain)\t
>  *           \(.Authentication.clientAccount)
>  *           \t\(.Authentication.workstation)
>@@ -272,7 +270,6 @@ static void log_authentication_event_json(
> 	log_json(msg_ctx,
> 		 lp_ctx,
> 		 &wrapper,
>-		 AUTH_JSON_TYPE,
> 		 DBGC_AUTH_AUDIT,
> 		 debug_level);
> 	json_free(&wrapper);
>@@ -300,8 +297,7 @@ failure:
>  *  To process the resulting log lines from the commend line use jq to
>  *  parse the json.
>  *
>- *  grep "JSON Authentication" log_file |\
>- *  sed "s;^[^{]*;;" |\
>+ *  grep "^  {" log_file |\
>  *  jq -rc '"\(.timestamp)\t
>  *           \(.Authorization.domain)\t
>  *           \(.Authorization.account)\t
>@@ -409,7 +405,6 @@ static void log_successful_authz_event_json(
> 	log_json(msg_ctx,
> 		 lp_ctx,
> 		 &wrapper,
>-		 AUTHZ_JSON_TYPE,
> 		 DBGC_AUTH_AUDIT,
> 		 debug_level);
> 	json_free(&wrapper);
>diff --git a/lib/audit_logging/audit_logging.c b/lib/audit_logging/audit_logging.c
>index ac08863129a..4ae18fb773b 100644
>--- a/lib/audit_logging/audit_logging.c
>+++ b/lib/audit_logging/audit_logging.c
>@@ -105,13 +105,11 @@ const struct json_object json_empty_object = {.valid = false, .root = NULL};
>  *
>  * Write the json object to the audit logs as a formatted string
>  *
>- * @param prefix Text to be printed at the start of the log line
>  * @param message The content of the log line.
>  * @param debub_class The debug class to log the message with.
>  * @param debug_level The debug level to log the message with.
>  */
>-void audit_log_json(const char* prefix,
>-		    struct json_object* message,
>+void audit_log_json(struct json_object* message,
> 		    int debug_class,
> 		    int debug_level)
> {
>@@ -126,13 +124,20 @@ void audit_log_json(const char* prefix,
> 	ctx = talloc_new(NULL);
> 	s = json_to_string(ctx, message);
> 	if (s == NULL) {
>-		DBG_ERR("json_to_string for (%s) returned NULL, "
>-			"JSON audit message could not written\n",
>-			prefix);
>+		DBG_ERR("json_to_string returned NULL, "
>+			"JSON audit message could not written\n");
> 		TALLOC_FREE(ctx);
> 		return;
> 	}
>-	DEBUGC(debug_class, debug_level, ("JSON %s: %s\n", prefix, s));
>+	/*
>+	 * This is very strange, but we call this routine to get a log
>+	 * output without the header.  JSON logs all have timestamps
>+	 * so this only makes parsing harder.
>+	 *
>+	 * We push out the raw JSON blob without a prefix, consumers
>+	 * can find such lines by the leading {
>+	 */
>+	DEBUGADDC(debug_class, debug_level, ("%s\n", s));
> 	TALLOC_FREE(ctx);
> }
> 
>diff --git a/lib/audit_logging/audit_logging.h b/lib/audit_logging/audit_logging.h
>index 84738d2bb93..f91efc39478 100644
>--- a/lib/audit_logging/audit_logging.h
>+++ b/lib/audit_logging/audit_logging.h
>@@ -42,8 +42,7 @@ extern const struct json_object json_empty_object;
> 
> #define JSON_ERROR -1
> 
>-void audit_log_json(const char *prefix,
>-		    struct json_object *message,
>+void audit_log_json(struct json_object *message,
> 		    int debug_class,
> 		    int debug_level);
> void audit_message_send(struct imessaging_context *msg_ctx,
>diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
>index dd714bca4e1..16acf146194 100644
>--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
>+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
>@@ -1139,7 +1139,6 @@ static void log_standard_operation(
> 		struct json_object json;
> 		json = operation_json(module, request, reply);
> 		audit_log_json(
>-			OPERATION_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_AUDIT_JSON,
> 			OPERATION_LOG_LVL);
>@@ -1160,7 +1159,6 @@ static void log_standard_operation(
> 			struct json_object json;
> 			json = password_change_json(module, request, reply);
> 			audit_log_json(
>-				PASSWORD_JSON_TYPE,
> 				&json,
> 				DBGC_DSDB_PWD_AUDIT_JSON,
> 				PASSWORD_LOG_LVL);
>@@ -1221,7 +1219,6 @@ static void log_replicated_operation(
> 		struct json_object json;
> 		json = replicated_update_json(module, request, reply);
> 		audit_log_json(
>-			REPLICATION_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_AUDIT_JSON,
> 			REPLICATION_LOG_LVL);
>@@ -1311,7 +1308,6 @@ static void log_transaction(
> 			&audit_private->transaction_guid,
> 			duration);
> 		audit_log_json(
>-			TRANSACTION_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_TXN_AUDIT_JSON,
> 			log_level);
>@@ -1384,7 +1380,6 @@ static void log_commit_failure(
> 			reason,
> 			&audit_private->transaction_guid);
> 		audit_log_json(
>-			TRANSACTION_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_TXN_AUDIT_JSON,
> 			log_level);
>diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
>index d5c9bbdaa1e..2a6c1163c86 100644
>--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
>+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
>@@ -507,7 +507,6 @@ static void log_primary_group_change(
> 			group,
> 			status);
> 		audit_log_json(
>-			AUDIT_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_GROUP_AUDIT_JSON,
> 			GROUP_LOG_LVL);
>@@ -582,7 +581,6 @@ static void log_membership_change(
> 			group,
> 			status);
> 		audit_log_json(
>-			AUDIT_JSON_TYPE,
> 			&json,
> 			DBGC_DSDB_GROUP_AUDIT_JSON,
> 			GROUP_LOG_LVL);
>-- 
>2.11.0
>
>
>From 7e48d746aa2c7d3e0455e9815da64348c1872b73 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 14 Dec 2018 16:05:33 +1300
>Subject: [PATCH 3/3] audit_logging: auth_json_audit required auth_json
>
>To log JSON the human-readable logs must also have been enabled
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13715
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Fri Dec 14 14:32:25 CET 2018 on sn-devel-144
>
>(cherry picked from commit 31957c7fe9d0f67bef08177e982043a23b172c7d)
>---
> auth/auth_log.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/auth/auth_log.c b/auth/auth_log.c
>index 9a873561284..1df112d9a8b 100644
>--- a/auth/auth_log.c
>+++ b/auth/auth_log.c
>@@ -270,7 +270,7 @@ static void log_authentication_event_json(
> 	log_json(msg_ctx,
> 		 lp_ctx,
> 		 &wrapper,
>-		 DBGC_AUTH_AUDIT,
>+		 DBGC_AUTH_AUDIT_JSON,
> 		 debug_level);
> 	json_free(&wrapper);
> 	return;
>@@ -405,7 +405,7 @@ static void log_successful_authz_event_json(
> 	log_json(msg_ctx,
> 		 lp_ctx,
> 		 &wrapper,
>-		 DBGC_AUTH_AUDIT,
>+		 DBGC_AUTH_AUDIT_JSON,
> 		 debug_level);
> 	json_free(&wrapper);
> 	return;
>-- 
>2.11.0
>

Flags:

gary: review+

Actions: View

Attachments on bug 13714: 14741 | 14757 | 14770