Patch Fix JSON API

Gary Lockyer gary at catalyst.net.nz
Tue Jul 24 18:59:00 UTC 2018


Hopefully the final patch set, have added _WARN_UNUSED_RESULT_ to the
function prototypes.

Could I please get a review on this.

Thanks

Gary

On 18/07/18 15:59, Gary Lockyer via samba-technical wrote:
> Latest patch incorporating Philipp's and Ralph's feedback.
> 
> As for Philipp's question about warn_unused_result, I see there is a
> macro _WARN_UNUSED_RESULT_ defined in attr.h, but this does not appear
> to be used anywhere.  Is this the correct way to do this in samba, and
> also should it be done. I think it makes sense to do this.
> 
> Gary
> 
> On 18/07/18 00:38, Philipp Gesang via samba-technical wrote:
>>
>> Btw. how about tagging the json_* functions with
>> __attribute__((warn_unused_result)) ?
>>
>> Philipp
>>
-------------- next part --------------
From c37820cfc7f172151cda5a0935baa07b7da53997 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 13 Jul 2018 09:14:09 +1200
Subject: [PATCH 1/7] 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.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 auth/auth_log.c                              | 307 +++++++++++-----
 lib/audit_logging/audit_logging.c            | 520 +++++++++++++++++----------
 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 ++++-
 7 files changed, 1342 insertions(+), 473 deletions(-)

diff --git a/auth/auth_log.c b/auth/auth_log.c
index 67d23c1..9e57b1d 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 f94f2c2..1952914 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,16 @@ 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);
 	DEBUGC(debug_class, debug_level, ("JSON %s: %s\n", prefix, s));
 	TALLOC_FREE(ctx);
 }
@@ -235,11 +222,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 +278,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 +311,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 +337,7 @@ void json_free(struct json_object *object)
 		json_decref(object->root);
 	}
 	object->root = NULL;
-	object->error = true;
+	object->valid = false;
 }
 
 /*
@@ -358,99 +350,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;
 	}
 
-	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;
+	integer = json_integer(value);
+	if (integer == NULL) {
+		DBG_ERR("Unable to create integer value [%s] value [%d]\n",
+			name,
+			value);
+		return JSON_ERROR;
 	}
+
+	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 +488,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 +503,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 +553,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 +621,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 +670,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 +684,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 +696,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 +714,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 +728,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 +790,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 +840,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 +900,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 +945,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 +969,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 +990,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 +1008,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 4af743a..84738d2 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 26875c9..07c52ea 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 800f8e8..81d4931 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 766c34c..edf3c5e 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 47929aa..1166c69 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
 
-- 
2.7.4


From 31f9c9f2288bbaa65e7b59e191a262e30fb63197 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 13 Jul 2018 09:15:34 +1200
Subject: [PATCH 2/7] json: Add unit tests for error handling

Add cmocka unit tests to exercise the error handling in the JSON
routines.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 lib/audit_logging/tests/audit_logging_error_test.c | 869 +++++++++++++++++++++
 lib/audit_logging/wscript_build                    |  31 +
 .../ldb_modules/tests/test_audit_log_errors.c      | 639 +++++++++++++++
 .../ldb_modules/tests/test_group_audit_errors.c    | 265 +++++++
 .../dsdb/samdb/ldb_modules/wscript_build_server    |  38 +
 source4/selftest/tests.py                          |  10 +-
 6 files changed, 1851 insertions(+), 1 deletion(-)
 create mode 100644 lib/audit_logging/tests/audit_logging_error_test.c
 create mode 100644 source4/dsdb/samdb/ldb_modules/tests/test_audit_log_errors.c
 create mode 100644 source4/dsdb/samdb/ldb_modules/tests/test_group_audit_errors.c

diff --git a/lib/audit_logging/tests/audit_logging_error_test.c b/lib/audit_logging/tests/audit_logging_error_test.c
new file mode 100644
index 0000000..1c0929a
--- /dev/null
+++ b/lib/audit_logging/tests/audit_logging_error_test.c
@@ -0,0 +1,869 @@
+/*
+ * Unit tests for the audit_logging library.
+ *
+ *  Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+
+/*
+ * Unit tests for lib/audit_logging/audit_logging.c
+ *
+ * These tests exercise the error handling code and mock the jannson functions
+ * to trigger errors.
+ *
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "includes.h"
+
+#include "librpc/ndr/libndr.h"
+#include "lib/tsocket/tsocket.h"
+#include "libcli/security/dom_sid.h"
+#include "lib/messaging/messaging.h"
+#include "auth/common_auth.h"
+
+#include "lib/audit_logging/audit_logging.h"
+
+const int JANNASON_FAILURE = -1;
+const int CALL_ORIG = -2;
+
+/*
+ * cmocka wrappers for json_object
+ */
+json_t *__wrap_json_object(void);
+json_t *__real_json_object(void);
+json_t *__wrap_json_object(void)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_json_object();
+}
+
+/*
+ * cmocka wrappers for json_array
+ */
+json_t *__wrap_json_array(void);
+json_t *__real_json_array(void);
+json_t *__wrap_json_array(void)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_json_array();
+}
+
+/*
+ * cmoka wrappers for json_integer
+ */
+json_t *__wrap_json_integer(json_int_t value);
+json_t *__real_json_integer(json_int_t value);
+json_t *__wrap_json_integer(json_int_t value)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_json_integer(value);
+}
+
+/*
+ * cmocka wrappers for json_string
+ */
+json_t *__wrap_json_string(const char *value);
+json_t *__real_json_string(const char *value);
+json_t *__wrap_json_string(const char *value)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_json_string(value);
+}
+
+/*
+ * cmocka wrappers for json_dumps
+ */
+char *__wrap_json_dumps(const json_t *json, size_t flags);
+char *__real_json_dumps(const json_t *json, size_t flags);
+char *__wrap_json_dumps(const json_t *json, size_t flags)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_json_dumps(json, flags);
+}
+
+/*
+ * cmocka wrappers for json_object_set_new
+ */
+int __wrap_json_object_set_new(json_t *object, const char *key, json_t *value);
+int __real_json_object_set_new(json_t *object, const char *key, json_t *value);
+int __wrap_json_object_set_new(json_t *object, const char *key, json_t *value)
+{
+	int rc = (int)mock();
+	if (rc != CALL_ORIG) {
+		return rc;
+	}
+	return __real_json_object_set_new(object, key, value);
+}
+
+/*
+ * cmocka wrappers for json_array_append_new
+ */
+int __wrap_json_array_append_new(json_t *object,
+				 const char *key,
+				 json_t *value);
+int __real_json_array_append_new(json_t *object,
+				 const char *key,
+				 json_t *value);
+int __wrap_json_array_append_new(json_t *object, const char *key, json_t *value)
+{
+	int rc = (int)mock();
+	if (rc != CALL_ORIG) {
+		return rc;
+	}
+	return __real_json_array_append_new(object, key, value);
+}
+
+/*
+ * cmocka wrappers for json_array_extend
+ */
+int __wrap_json_array_extend(json_t *array, json_t *other_array);
+int __real_json_array_extend(json_t *array, json_t *other_array);
+int __wrap_json_array_extend(json_t *array, json_t *other_array)
+{
+
+	int rc = (int)mock();
+	if (rc != CALL_ORIG) {
+		return rc;
+	}
+	return __real_json_array_extend(array, other_array);
+}
+
+/*
+ * cmocka wrappers for json_object_update
+ */
+int __wrap_json_object_update(json_t *object, json_t *other_object);
+int __real_json_object_update(json_t *object, json_t *other_object);
+int __wrap_json_object_update(json_t *object, json_t *other_object)
+{
+
+	int rc = (int)mock();
+	if (rc != CALL_ORIG) {
+		return rc;
+	}
+	return __real_json_array_extend(object, other_object);
+}
+
+/*
+ * cmocka wrappers for gettimeofday
+ */
+int __wrap_gettimeofday(struct timeval *tv, struct timezone *tz);
+int __real_gettimeofday(struct timeval *tv, struct timezone *tz);
+int __wrap_gettimeofday(struct timeval *tv, struct timezone *tz)
+{
+
+	int rc = (int)mock();
+	if (rc != 0) {
+		return rc;
+	}
+	return __real_gettimeofday(tv, tz);
+}
+
+/*
+ * cmocka wrappers for localtime
+ */
+struct tm *__wrap_localtime(const time_t *timep);
+struct tm *__real_localtime(const time_t *timep);
+struct tm *__wrap_localtime(const time_t *timep)
+{
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_localtime(timep);
+}
+
+/*
+ * cmocka wrappers for talloc_named_const
+ */
+static const void *REAL_TALLOC = "Here";
+
+void *__wrap_talloc_named_const(const void *context,
+				size_t size,
+				const char *name);
+void *__real_talloc_named_const(const void *context,
+				size_t size,
+				const char *name);
+void *__wrap_talloc_named_const(const void *context,
+				size_t size,
+				const char *name)
+{
+
+	void *ret = (void *)mock();
+
+	if (ret == NULL) {
+		return NULL;
+	}
+	return __real_talloc_named_const(context, size, name);
+}
+
+/*
+ * cmocka wrappers for talloc_strdup
+ */
+char *__wrap_talloc_strdup(const void *t, const char *p);
+char *__real_talloc_strdup(const void *t, const char *p);
+char *__wrap_talloc_strdup(const void *t, const char *p)
+{
+
+	void *ret = (void *)mock();
+
+	if (ret == NULL) {
+		return NULL;
+	}
+	return __real_talloc_strdup(t, p);
+}
+
+char *__wrap_tsocket_address_string(const struct tsocket_address *addr,
+				    TALLOC_CTX *mem_ctx);
+char *__real_tsocket_address_string(const struct tsocket_address *addr,
+				    TALLOC_CTX *mem_ctx);
+char *__wrap_tsocket_address_string(const struct tsocket_address *addr,
+				    TALLOC_CTX *mem_ctx)
+{
+
+	bool fail = (bool)mock();
+	if (fail) {
+		return NULL;
+	}
+	return __real_tsocket_address_string(addr, mem_ctx);
+}
+
+static void test_json_add_int(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	/*
+	 * Test json integer failure
+	 */
+	will_return(__wrap_json_integer, true);
+	rc = json_add_int(&object, "name", 2);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure
+	 */
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_int(&object, "name", 2);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+	json_free(&object);
+}
+
+static void test_json_add_bool(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	/*
+	 * json_boolean does not return an error code.
+	 * Test json object set new failure
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_bool(&object, "name", true);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_add_string(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+	assert_false(json_is_invalid(&object));
+
+	/*
+	 * Test json string failure
+	 */
+	will_return(__wrap_json_string, true);
+	rc = json_add_string(&object, "name", "value");
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure
+	 */
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_string(&object, "name", "value");
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure for a NULL string
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_string(&object, "null", NULL);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_add_object(void **state)
+{
+	struct json_object object;
+	struct json_object value;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	will_return(__wrap_json_object, false);
+
+	object = json_new_object();
+	value = json_new_object();
+
+	/*
+	 * Test json object set new failure
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_object(&object, "name", &value);
+
+	assert_false(json_is_invalid(&object));
+	assert_false(json_is_invalid(&value));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure for a NULL value
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_object(&object, "null", NULL);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+	json_free(&value);
+}
+
+static void test_json_add_to_array(void **state)
+{
+	struct json_object array;
+	struct json_object value;
+	int rc = 0;
+
+	will_return(__wrap_json_array, false);
+	will_return(__wrap_json_object, false);
+
+	array = json_new_array();
+	value = json_new_object();
+
+	/*
+	 * Test json array append new failure
+	 */
+	will_return(__wrap_json_array_append_new, JANNASON_FAILURE);
+	rc = json_add_object(&array, "name", &value);
+
+	assert_false(json_is_invalid(&array));
+	assert_false(json_is_invalid(&value));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json append new failure with a NULL value
+	 */
+	will_return(__wrap_json_array_append_new, JANNASON_FAILURE);
+	rc = json_add_object(&array, "null", NULL);
+
+	assert_false(json_is_invalid(&array));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&array);
+	json_free(&value);
+}
+
+static void test_json_add_timestamp(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	/*
+	 * Test json string failure
+	 */
+	will_return(__wrap_gettimeofday, 0);
+	will_return(__wrap_localtime, false);
+	will_return(__wrap_json_string, true);
+	rc = json_add_timestamp(&object);
+
+	/*
+	 * Test json_object_set_new failure
+	 */
+	will_return(__wrap_gettimeofday, 0);
+	will_return(__wrap_localtime, false);
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_timestamp(&object);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test gettimeofday failure
+	 */
+	will_return(__wrap_gettimeofday, -1);
+	rc = json_add_timestamp(&object);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test local time failure
+	 */
+	will_return(__wrap_gettimeofday, 0);
+	will_return(__wrap_localtime, true);
+	rc = json_add_timestamp(&object);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_add_stringn(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+	assert_false(json_is_invalid(&object));
+
+	/*
+	 * Test json string failure
+	 */
+	will_return(__wrap_json_string, true);
+	rc = json_add_stringn(&object, "name", "value", 3);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure
+	 */
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_stringn(&object, "name", "value", 3);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure for a NULL string
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_stringn(&object, "null", NULL, 2);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Test json object set new failure for a zero string size
+	 */
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_stringn(&object, "zero", "no value", 0);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+	json_free(&object);
+}
+
+static void test_json_add_version(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+
+	/*
+	 * Fail creating the version object
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object, true);
+	rc = json_add_version(&object, 1, 11);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+
+	/*
+	 * Fail adding the major version
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object, false);
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_version(&object, 2, 12);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+
+	/*
+	 * Fail adding the minor version
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object, false);
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, CALL_ORIG);
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_version(&object, 3, 13);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+
+	/*
+	 * Fail adding the version object
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object, false);
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, CALL_ORIG);
+	will_return(__wrap_json_integer, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_version(&object, 4, 14);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_add_address(void **state)
+{
+	struct json_object object;
+	int rc = 0;
+	struct tsocket_address *ip = NULL;
+
+	TALLOC_CTX *ctx = NULL;
+
+	will_return(__wrap_talloc_named_const, REAL_TALLOC);
+	ctx = talloc_new(NULL);
+
+	/*
+	 * Add a null address
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_address(&object, "name", NULL);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Add a non null address, json_object_set_new failure
+	 */
+	rc = tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 21, &ip);
+	assert_int_equal(0, rc);
+
+	will_return(__wrap_talloc_named_const, REAL_TALLOC);
+	will_return(__wrap_tsocket_address_string, false);
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_address(&object, "name", ip);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Add a non null address, with a talloc failure
+	 */
+	rc = tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 21, &ip);
+	assert_int_equal(0, rc);
+
+	will_return(__wrap_talloc_named_const, NULL);
+	rc = json_add_address(&object, "name", ip);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Add a non null address, tsocket_address_string failure
+	 */
+	rc = tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 21, &ip);
+	assert_int_equal(0, rc);
+
+	will_return(__wrap_talloc_named_const, REAL_TALLOC);
+	will_return(__wrap_tsocket_address_string, true);
+	rc = json_add_address(&object, "name", ip);
+
+	assert_false(json_is_invalid(&object));
+	assert_int_equal(JSON_ERROR, rc);
+
+	TALLOC_FREE(ctx);
+	json_free(&object);
+}
+
+static void test_json_add_sid(void **state)
+{
+	struct json_object object;
+	const char *SID = "S-1-5-21-2470180966-3899876309-2637894779";
+	struct dom_sid sid;
+	int rc;
+
+	/*
+	 * Add a null SID
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_sid(&object, "null", NULL);
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Add a non null SID
+	 */
+	assert_true(string_to_sid(&sid, SID));
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_sid(&object, "sid", &sid);
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_add_guid(void **state)
+{
+	struct json_object object;
+	const char *GUID = "3ab88633-1e57-4c1a-856c-d1bc4b15bbb1";
+	struct GUID guid;
+	NTSTATUS status;
+	int rc;
+
+	/*
+	 * Add a null GUID
+	 */
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_guid(&object, "null", NULL);
+	assert_int_equal(JSON_ERROR, rc);
+
+	/*
+	 * Add a non null GUID
+	 */
+	status = GUID_from_string(GUID, &guid);
+	assert_true(NT_STATUS_IS_OK(status));
+	will_return(__wrap_json_string, false);
+	will_return(__wrap_json_object_set_new, JANNASON_FAILURE);
+	rc = json_add_guid(&object, "guid", &guid);
+	assert_int_equal(JSON_ERROR, rc);
+
+	json_free(&object);
+}
+
+static void test_json_to_string(void **state)
+{
+	struct json_object object;
+	char *s = NULL;
+	TALLOC_CTX *ctx = NULL;
+
+	will_return(__wrap_talloc_named_const, REAL_TALLOC);
+	ctx = talloc_new(NULL);
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+
+	/*
+	 * json_dumps failure
+	 */
+	will_return(__wrap_json_dumps, true);
+	s = json_to_string(ctx, &object);
+	assert_null(s);
+
+	/*
+	 * talloc failure
+	 */
+	will_return(__wrap_json_dumps, false);
+	will_return(__wrap_talloc_strdup, NULL);
+	s = json_to_string(ctx, &object);
+	assert_null(s);
+	TALLOC_FREE(ctx);
+	json_free(&object);
+}
+
+static void test_json_get_array(void **state)
+{
+	struct json_object object;
+	struct json_object stored_array;
+	struct json_object array;
+
+	int rc;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+	assert_false(json_is_invalid(&object));
+
+	will_return(__wrap_json_array, false);
+	stored_array = json_new_array();
+	assert_false(json_is_invalid(&stored_array));
+
+	will_return(__wrap_json_object_set_new, CALL_ORIG);
+	rc = json_add_object(&object, "array", &stored_array);
+	assert_int_equal(0, rc);
+
+	/*
+	 * json array failure
+	 */
+	will_return(__wrap_json_array, true);
+	array = json_get_array(&object, "array");
+	assert_true(json_is_invalid(&array));
+
+	/*
+	 * json array extend failure
+	 */
+	will_return(__wrap_json_array, false);
+	will_return(__wrap_json_array_extend, true);
+	array = json_get_array(&object, "array");
+	assert_true(json_is_invalid(&array));
+
+	json_free(&stored_array);
+	json_free(&object);
+}
+
+static void test_json_get_object(void **state)
+{
+	struct json_object object;
+	struct json_object stored;
+	struct json_object retreived;
+
+	int rc;
+
+	will_return(__wrap_json_object, false);
+	object = json_new_object();
+	assert_false(json_is_invalid(&object));
+
+	will_return(__wrap_json_object, false);
+	stored = json_new_object();
+	assert_false(json_is_invalid(&stored));
+
+	will_return(__wrap_json_object_set_new, CALL_ORIG);
+	rc = json_add_object(&object, "stored", &stored);
+	assert_int_equal(0, rc);
+
+	/*
+	 * json object update failure
+	 */
+	will_return(__wrap_json_object, false);
+	will_return(__wrap_json_object_update, true);
+	retreived = json_get_object(&object, "stored");
+	assert_true(json_is_invalid(&retreived));
+
+	json_free(&object);
+}
+
+int main(int argc, const char **argv)
+{
+	const struct CMUnitTest tests[] = {
+	    cmocka_unit_test(test_json_add_int),
+	    cmocka_unit_test(test_json_add_bool),
+	    cmocka_unit_test(test_json_add_string),
+	    cmocka_unit_test(test_json_add_object),
+	    cmocka_unit_test(test_json_add_to_array),
+	    cmocka_unit_test(test_json_add_timestamp),
+	    cmocka_unit_test(test_json_add_stringn),
+	    cmocka_unit_test(test_json_add_version),
+	    cmocka_unit_test(test_json_add_address),
+	    cmocka_unit_test(test_json_add_sid),
+	    cmocka_unit_test(test_json_add_guid),
+	    cmocka_unit_test(test_json_to_string),
+	    cmocka_unit_test(test_json_get_array),
+	    cmocka_unit_test(test_json_get_object),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/audit_logging/wscript_build b/lib/audit_logging/wscript_build
index 24ac8bb..4811e05 100644
--- a/lib/audit_logging/wscript_build
+++ b/lib/audit_logging/wscript_build
@@ -23,3 +23,34 @@ if bld.AD_DC_BUILD_IS_ENABLED() and bld.CONFIG_GET('ENABLE_SELFTEST'):
         ''',
         install=False
     )
+
+if bld.AD_DC_BUILD_IS_ENABLED() and bld.CONFIG_GET('ENABLE_SELFTEST'):
+    bld.SAMBA_BINARY(
+        'audit_logging_error_test',
+        source='tests/audit_logging_error_test.c',
+        deps='''
+             audit_logging
+             jansson
+             cmocka
+             talloc
+             samba-util
+             LIBTSOCKET
+        ''',
+        install=False,
+        ldflags='''
+            -Wl,--wrap,json_object_set_new
+            -Wl,--wrap,json_object_update
+            -Wl,--wrap,json_array_append_new
+            -Wl,--wrap,json_array_extend
+            -Wl,--wrap,json_object
+            -Wl,--wrap,json_string
+            -Wl,--wrap,json_integer
+            -Wl,--wrap,json_array
+            -Wl,--wrap,json_dumps
+            -Wl,--wrap,gettimeofday
+            -Wl,--wrap,localtime
+            -Wl,--wrap,talloc_named_const
+            -Wl,--wrap,talloc_strdup
+            -Wl,--wrap,tsocket_address_string
+        '''
+    )
diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_audit_log_errors.c b/source4/dsdb/samdb/ldb_modules/tests/test_audit_log_errors.c
new file mode 100644
index 0000000..2931768
--- /dev/null
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_audit_log_errors.c
@@ -0,0 +1,639 @@
+/*
+   Unit tests for the dsdb audit logging code code in audit_log.c
+
+   Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/*
+ * These tests exercise the error handling code
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <unistd.h>
+#include <cmocka.h>
+
+int ldb_audit_log_module_init(const char *version);
+#include "../audit_log.c"
+#include "lib/ldb/include/ldb_private.h"
+
+/*
+ * cmocka wrappers for json_new_object
+ */
+struct json_object __wrap_json_new_object(void);
+struct json_object __real_json_new_object(void);
+struct json_object __wrap_json_new_object(void)
+{
+
+	bool use_real = (bool)mock();
+	if (!use_real) {
+		return json_empty_object;
+	}
+	return __real_json_new_object();
+}
+
+/*
+ * cmocka wrappers for json_add_version
+ */
+int __wrap_json_add_version(struct json_object *object, int major, int minor);
+int __real_json_add_version(struct json_object *object, int major, int minor);
+int __wrap_json_add_version(struct json_object *object, int major, int minor)
+{
+
+	int ret = (int)mock();
+	if (ret) {
+		return ret;
+	}
+	return __real_json_add_version(object, major, minor);
+}
+
+/*
+ * cmocka wrappers for json_add_version
+ */
+int __wrap_json_add_timestamp(struct json_object *object);
+int __real_json_add_timestamp(struct json_object *object);
+int __wrap_json_add_timestamp(struct json_object *object)
+{
+
+	int ret = (int)mock();
+	if (ret) {
+		return ret;
+	}
+	return __real_json_add_timestamp(object);
+}
+/*
+ * unit test of operation_json, that ensures that all the expected
+ * attributes and objects are in the json object.
+ */
+static void test_operation_json(void **state)
+{
+	struct ldb_context *ldb = NULL;
+	struct ldb_module  *module = NULL;
+	struct ldb_request *req = NULL;
+	struct ldb_reply *reply = NULL;
+	struct audit_private *audit_private = NULL;
+
+	struct tsocket_address *ts = NULL;
+
+	struct auth_session_info *sess = NULL;
+	struct security_token *token = NULL;
+	struct dom_sid sid;
+	const char *const SID = "S-1-5-21-2470180966-3899876309-2637894779";
+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+	struct GUID session_id;
+
+	struct GUID transaction_id;
+	const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct ldb_dn *dn = NULL;
+	const char *const DN = "dn=CN=USER,CN=Users,DC=SAMBA,DC=ORG";
+
+	struct ldb_message *msg = NULL;
+
+	struct json_object json;
+
+
+	/*
+	 * Test setup
+	 */
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	ldb = ldb_init(ctx, NULL);
+
+	audit_private = talloc_zero(ctx, struct audit_private);
+	GUID_from_string(TRANSACTION, &transaction_id);
+	audit_private->transaction_guid = transaction_id;
+
+	module = talloc_zero(ctx, struct ldb_module);
+	module->ldb = ldb;
+	ldb_module_set_private(module, audit_private);
+
+	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
+	ldb_set_opaque(ldb, "remoteAddress", ts);
+
+	sess = talloc_zero(ctx, struct auth_session_info);
+	token = talloc_zero(ctx, struct security_token);
+	string_to_sid(&sid, SID);
+	token->num_sids = 1;
+	token->sids = &sid;
+	sess->security_token = token;
+	GUID_from_string(SESSION, &session_id);
+	sess->unique_session_token = session_id;
+	ldb_set_opaque(ldb, DSDB_SESSION_INFO, sess);
+
+	msg = talloc_zero(ctx, struct ldb_message);
+	dn = ldb_dn_new(ctx, ldb, DN);
+	msg->dn = dn;
+	ldb_msg_add_string(msg, "attribute", "the-value");
+
+	req = talloc_zero(ctx, struct ldb_request);
+	req->operation =  LDB_ADD;
+	req->op.add.message = msg;
+
+	reply = talloc_zero(ctx, struct ldb_reply);
+	reply->error = LDB_ERR_OPERATIONS_ERROR;
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+
+	json = operation_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = operation_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, false);
+
+	json = operation_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the timestamp to the wrapper object.
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = operation_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Now test the happy path
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = operation_json(module, req, reply);
+	assert_false(json_is_invalid(&json));
+	json_free(&json);
+
+	TALLOC_FREE(ctx);
+
+}
+
+/*
+ * minimal unit test of password_change_json, that ensures that all the expected
+ * attributes and objects are in the json object.
+ */
+static void test_password_change_json(void **state)
+{
+	struct ldb_context *ldb = NULL;
+	struct ldb_module  *module = NULL;
+	struct ldb_request *req = NULL;
+	struct ldb_reply *reply = NULL;
+	struct audit_private *audit_private = NULL;
+
+	struct tsocket_address *ts = NULL;
+
+	struct auth_session_info *sess = NULL;
+	struct security_token *token = NULL;
+	struct dom_sid sid;
+	const char *const SID = "S-1-5-21-2470180966-3899876309-2637894779";
+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+	struct GUID session_id;
+
+	struct GUID transaction_id;
+	const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct ldb_dn *dn = NULL;
+	const char *const DN = "dn=CN=USER,CN=Users,DC=SAMBA,DC=ORG";
+
+	struct ldb_message *msg = NULL;
+
+	struct json_object json;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	ldb = ldb_init(ctx, NULL);
+
+	audit_private = talloc_zero(ctx, struct audit_private);
+	GUID_from_string(TRANSACTION, &transaction_id);
+	audit_private->transaction_guid = transaction_id;
+
+	module = talloc_zero(ctx, struct ldb_module);
+	module->ldb = ldb;
+	ldb_module_set_private(module, audit_private);
+
+	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
+	ldb_set_opaque(ldb, "remoteAddress", ts);
+
+	sess = talloc_zero(ctx, struct auth_session_info);
+	token = talloc_zero(ctx, struct security_token);
+	string_to_sid(&sid, SID);
+	token->num_sids = 1;
+	token->sids = &sid;
+	sess->security_token = token;
+	GUID_from_string(SESSION, &session_id);
+	sess->unique_session_token = session_id;
+	ldb_set_opaque(ldb, DSDB_SESSION_INFO, sess);
+
+	msg = talloc_zero(ctx, struct ldb_message);
+	dn = ldb_dn_new(ctx, ldb, DN);
+	msg->dn = dn;
+	ldb_msg_add_string(msg, "planTextPassword", "super-secret");
+
+	req = talloc_zero(ctx, struct ldb_request);
+	req->operation =  LDB_ADD;
+	req->op.add.message = msg;
+	reply = talloc_zero(ctx, struct ldb_reply);
+	reply->error = LDB_SUCCESS;
+
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+	json = password_change_json(module, req, reply);
+
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = password_change_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, false);
+
+	json = password_change_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the time stamp.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = password_change_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Now test the happy path
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = password_change_json(module, req, reply);
+	assert_false(json_is_invalid(&json));
+	json_free(&json);
+
+	TALLOC_FREE(ctx);
+}
+
+
+/*
+ * minimal unit test of transaction_json, that ensures that all the expected
+ * attributes and objects are in the json object.
+ */
+static void test_transaction_json(void **state)
+{
+
+	struct GUID guid;
+	const char * const GUID = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct json_object json;
+
+	GUID_from_string(GUID, &guid);
+
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+
+	json = transaction_json("delete", &guid, 10000099);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = transaction_json("delete", &guid, 10000099);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, false);
+
+	json = transaction_json("delete", &guid, 10000099);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the time stamp.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = transaction_json("delete", &guid, 10000099);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Now test the happy path
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = transaction_json("delete", &guid, 10000099);
+	assert_false(json_is_invalid(&json));
+	json_free(&json);
+}
+
+/*
+ * minimal unit test of commit_failure_json, that ensures that all the
+ * expected attributes and objects are in the json object.
+ */
+static void test_commit_failure_json(void **state)
+{
+
+	struct GUID guid;
+	const char * const GUID = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct json_object json;
+
+	GUID_from_string(GUID, &guid);
+
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+
+	json = commit_failure_json(
+		"prepare",
+		987876,
+		LDB_ERR_OPERATIONS_ERROR,
+		"because",
+		&guid);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = commit_failure_json(
+		"prepare",
+		987876,
+		LDB_ERR_OPERATIONS_ERROR,
+		"because",
+		&guid);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, false);
+
+	json = commit_failure_json(
+		"prepare",
+		987876,
+		LDB_ERR_OPERATIONS_ERROR,
+		"because",
+		&guid);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the time stamp.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = commit_failure_json(
+		"prepare",
+		987876,
+		LDB_ERR_OPERATIONS_ERROR,
+		"because",
+		&guid);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Now test the happy path
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = commit_failure_json(
+		"prepare",
+		987876,
+		LDB_ERR_OPERATIONS_ERROR,
+		"because",
+		&guid);
+	assert_false(json_is_invalid(&json));
+	json_free(&json);
+}
+
+/*
+ * unit test of replicated_update_json, that ensures that all the expected
+ * attributes and objects are in the json object.
+ */
+static void test_replicated_update_json(void **state)
+{
+	struct ldb_context *ldb = NULL;
+	struct ldb_module  *module = NULL;
+	struct ldb_request *req = NULL;
+	struct ldb_reply *reply = NULL;
+	struct audit_private *audit_private = NULL;
+	struct dsdb_extended_replicated_objects *ro = NULL;
+	struct repsFromTo1 *source_dsa = NULL;
+
+	struct GUID transaction_id;
+	const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct ldb_dn *dn = NULL;
+	const char *const DN = "dn=CN=USER,CN=Users,DC=SAMBA,DC=ORG";
+
+	struct GUID source_dsa_obj_guid;
+	const char *const SOURCE_DSA = "7130cb06-2062-6a1b-409e-3514c26b1793";
+
+	struct GUID invocation_id;
+	const char *const INVOCATION_ID =
+		"7130cb06-2062-6a1b-409e-3514c26b1893";
+	struct json_object json;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	ldb = ldb_init(ctx, NULL);
+
+	audit_private = talloc_zero(ctx, struct audit_private);
+	GUID_from_string(TRANSACTION, &transaction_id);
+	audit_private->transaction_guid = transaction_id;
+
+	module = talloc_zero(ctx, struct ldb_module);
+	module->ldb = ldb;
+	ldb_module_set_private(module, audit_private);
+
+	dn = ldb_dn_new(ctx, ldb, DN);
+	GUID_from_string(SOURCE_DSA, &source_dsa_obj_guid);
+	GUID_from_string(INVOCATION_ID, &invocation_id);
+	source_dsa = talloc_zero(ctx, struct repsFromTo1);
+	source_dsa->source_dsa_obj_guid = source_dsa_obj_guid;
+	source_dsa->source_dsa_invocation_id = invocation_id;
+
+	ro = talloc_zero(ctx, struct dsdb_extended_replicated_objects);
+	ro->source_dsa = source_dsa;
+	ro->num_objects = 808;
+	ro->linked_attributes_count = 2910;
+	ro->partition_dn = dn;
+	ro->error = WERR_NOT_SUPPORTED;
+
+
+	req = talloc_zero(ctx, struct ldb_request);
+	req->op.extended.data = ro;
+	req->operation = LDB_EXTENDED;
+
+	reply = talloc_zero(ctx, struct ldb_reply);
+	reply->error = LDB_ERR_NO_SUCH_OBJECT;
+
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+
+	json = replicated_update_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = replicated_update_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, false);
+
+	json = replicated_update_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the time stamp.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = replicated_update_json(module, req, reply);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Now test the happy path.
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = replicated_update_json(module, req, reply);
+	assert_false(json_is_invalid(&json));
+	json_free(&json);
+
+	TALLOC_FREE(ctx);
+}
+
+int main(void) {
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_operation_json),
+		cmocka_unit_test(test_password_change_json),
+		cmocka_unit_test(test_transaction_json),
+		cmocka_unit_test(test_commit_failure_json),
+		cmocka_unit_test(test_replicated_update_json),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit_errors.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit_errors.c
new file mode 100644
index 0000000..3c8829e
--- /dev/null
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit_errors.c
@@ -0,0 +1,265 @@
+/*
+   Unit tests for the dsdb group auditing code in group_audit.c
+
+   Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/*
+ * These tests exercise the error handling routines.
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <unistd.h>
+#include <cmocka.h>
+
+int ldb_group_audit_log_module_init(const char *version);
+#include "../group_audit.c"
+
+#include "lib/ldb/include/ldb_private.h"
+
+/*
+ * cmocka wrappers for json_new_object
+ */
+struct json_object __wrap_json_new_object(void);
+struct json_object __real_json_new_object(void);
+struct json_object __wrap_json_new_object(void)
+{
+
+	bool use_real = (bool)mock();
+	if (!use_real) {
+		return json_empty_object;
+	}
+	return __real_json_new_object();
+}
+
+/*
+ * cmocka wrappers for json_add_version
+ */
+int __wrap_json_add_version(struct json_object *object, int major, int minor);
+int __real_json_add_version(struct json_object *object, int major, int minor);
+int __wrap_json_add_version(struct json_object *object, int major, int minor)
+{
+
+	int ret = (int)mock();
+	if (ret) {
+		return ret;
+	}
+	return __real_json_add_version(object, major, minor);
+}
+
+/*
+ * cmocka wrappers for json_add_version
+ */
+int __wrap_json_add_timestamp(struct json_object *object);
+int __real_json_add_timestamp(struct json_object *object);
+int __wrap_json_add_timestamp(struct json_object *object)
+{
+
+	int ret = (int)mock();
+	if (ret) {
+		return ret;
+	}
+	return __real_json_add_timestamp(object);
+}
+
+/*
+ * Test helper to add a session id and user SID
+ */
+static void add_session_data(
+	TALLOC_CTX *ctx,
+	struct ldb_context *ldb,
+	const char *session,
+	const char *user_sid)
+{
+	struct auth_session_info *sess = NULL;
+	struct security_token *token = NULL;
+	struct dom_sid *sid = NULL;
+	struct GUID session_id;
+	bool ok;
+
+	sess = talloc_zero(ctx, struct auth_session_info);
+	token = talloc_zero(ctx, struct security_token);
+	sid = talloc_zero(ctx, struct dom_sid);
+	ok = string_to_sid(sid, user_sid);
+	assert_true(ok);
+	token->sids = sid;
+	sess->security_token = token;
+	GUID_from_string(session, &session_id);
+	sess->unique_session_token = session_id;
+	ldb_set_opaque(ldb, DSDB_SESSION_INFO, sess);
+}
+
+/*
+ * Test helper to insert a transaction_id into a request.
+ */
+static void add_transaction_id(struct ldb_request *req, const char *id)
+{
+	struct GUID guid;
+	struct dsdb_control_transaction_identifier *transaction_id = NULL;
+
+	transaction_id = talloc_zero(
+		req,
+		struct dsdb_control_transaction_identifier);
+	assert_non_null(transaction_id);
+	GUID_from_string(id, &guid);
+	transaction_id->transaction_guid = guid;
+	ldb_request_add_control(
+		req,
+		DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID,
+		false,
+		transaction_id);
+}
+
+static void test_audit_group_json(void **state)
+{
+	struct ldb_context *ldb = NULL;
+	struct ldb_module  *module = NULL;
+	struct ldb_request *req = NULL;
+
+	struct tsocket_address *ts = NULL;
+
+	const char *const SID = "S-1-5-21-2470180966-3899876309-2637894779";
+	const char * const SESSION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+	struct GUID transaction_id;
+	const char *const TRANSACTION = "7130cb06-2062-6a1b-409e-3514c26b1773";
+
+
+	struct json_object json;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	ldb = ldb_init(ctx, NULL);
+
+	GUID_from_string(TRANSACTION, &transaction_id);
+
+	module = talloc_zero(ctx, struct ldb_module);
+	module->ldb = ldb;
+
+	tsocket_address_inet_from_strings(ctx, "ip", "127.0.0.1", 0, &ts);
+	ldb_set_opaque(ldb, "remoteAddress", ts);
+
+	add_session_data(ctx, ldb, SESSION, SID);
+
+	req = talloc_zero(ctx, struct ldb_request);
+	req->operation =  LDB_ADD;
+	add_transaction_id(req, TRANSACTION);
+
+	/*
+	 * Fail on the creation of the audit json object
+	 */
+
+	will_return(__wrap_json_new_object, false);
+
+	json = audit_group_json(
+		module,
+		req,
+		"the-action",
+		"the-user-name",
+		"the-group-name",
+		LDB_ERR_OPERATIONS_ERROR);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the version object .
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, JSON_ERROR);
+
+	json = audit_group_json(
+		module,
+		req,
+		"the-action",
+		"the-user-name",
+		"the-group-name",
+		LDB_ERR_OPERATIONS_ERROR);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail on creation of the wrapper.
+	 */
+
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, false);
+
+	json = audit_group_json(
+		module,
+		req,
+		"the-action",
+		"the-user-name",
+		"the-group-name",
+		LDB_ERR_OPERATIONS_ERROR);
+	assert_true(json_is_invalid(&json));
+
+	/*
+	 * Fail adding the timestamp to the wrapper object.
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, JSON_ERROR);
+
+	json = audit_group_json(
+		module,
+		req,
+		"the-action",
+		"the-user-name",
+		"the-group-name",
+		LDB_ERR_OPERATIONS_ERROR);
+	assert_true(json_is_invalid(&json));
+
+
+	/*
+	 * Now test the happy path
+	 */
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_version, 0);
+	will_return(__wrap_json_new_object, true);
+	will_return(__wrap_json_add_timestamp, 0);
+
+	json = audit_group_json(
+		module,
+		req,
+		"the-action",
+		"the-user-name",
+		"the-group-name",
+		LDB_ERR_OPERATIONS_ERROR);
+	assert_false(json_is_invalid(&json));
+
+	json_free(&json);
+	TALLOC_FREE(ctx);
+
+}
+
+/*
+ * Note: to run under valgrind us:
+ *       valgrind --suppressions=test_group_audit.valgrind bin/test_group_audit
+ *       This suppresses the errors generated because the ldb_modules are not
+ *       de-registered.
+ *
+ */
+int main(void) {
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_audit_group_json),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build_server b/source4/dsdb/samdb/ldb_modules/wscript_build_server
index 39c9477..a0bb0d4 100644
--- a/source4/dsdb/samdb/ldb_modules/wscript_build_server
+++ b/source4/dsdb/samdb/ldb_modules/wscript_build_server
@@ -37,6 +37,25 @@ bld.SAMBA_BINARY('test_audit_log',
                  ''',
                  install=False)
 
+bld.SAMBA_BINARY('test_audit_log_errors',
+                 source='tests/test_audit_log_errors.c',
+                 deps='''
+                 talloc
+                 samba-util
+                 samdb-common
+                 samdb
+                 cmocka
+                 audit_logging
+                 DSDB_MODULE_HELPERS
+                 DSDB_MODULE_HELPERS_AUDIT
+                 ''',
+                 ldflags='''
+                     -Wl,--wrap,json_new_object
+                     -Wl,--wrap,json_add_version
+                     -Wl,--wrap,json_add_timestamp
+                 ''',
+                 install=False)
+
 bld.SAMBA_BINARY('test_group_audit',
                  source='tests/test_group_audit.c',
                  deps='''
@@ -51,6 +70,25 @@ bld.SAMBA_BINARY('test_group_audit',
                  ''',
                  install=False)
 
+bld.SAMBA_BINARY('test_group_audit_errors',
+                 source='tests/test_group_audit_errors.c',
+                 deps='''
+                 talloc
+                 samba-util
+                 samdb-common
+                 samdb
+                 cmocka
+                 audit_logging
+                 DSDB_MODULE_HELPERS
+                 DSDB_MODULE_HELPERS_AUDIT
+                 ''',
+                 ldflags='''
+                     -Wl,--wrap,json_new_object
+                     -Wl,--wrap,json_add_version
+                     -Wl,--wrap,json_add_timestamp
+                 ''',
+                 install=False)
+
 bld.SAMBA_MODULE('ldb_samba_dsdb',
 	source='samba_dsdb.c',
 	subsystem='ldb',
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 978a520..4b77b7e 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1114,7 +1114,7 @@ for env in ["ad_dc_ntvfs", "ad_dc", "fl2000dc", "fl2003dc", "fl2008r2dc",
             'renamedc', 'labdc']:
     plantestsuite("samba4.blackbox.dbcheck(%s)" % env, env + ":local" , ["PYTHON=%s" % python, os.path.join(bbdir, "dbcheck.sh"), '$PREFIX/provision', configuration])
 
-# cmocka tests not requiring a specific encironment
+# cmocka tests not requiring a specific environment
 #
 plantestsuite("samba4.dsdb.samdb.ldb_modules.unique_object_sids" , "none",
               [os.path.join(bindir(), "test_unique_object_sids")])
@@ -1122,7 +1122,15 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.encrypted_secrets", "none",
                   [os.path.join(bindir(), "test_encrypted_secrets")])
 plantestsuite("lib.audit_logging.audit_logging", "none",
                   [os.path.join(bindir(), "audit_logging_test")])
+plantestsuite("lib.audit_logging.audit_logging.errors", "none",
+                  [os.path.join(bindir(), "audit_logging_error_test")])
 plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "none",
                   [os.path.join(bindir(), "test_audit_util")])
 plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "none",
                   [os.path.join(bindir(), "test_audit_log")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log.errors", "none",
+                  [os.path.join(bindir(), "test_audit_log_errors")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit", "none",
+                  [os.path.join(bindir(), "test_group_audit")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "none",
+                  [os.path.join(bindir(), "test_group_audit_errors")])
-- 
2.7.4


From 757461c1e0650de84246b24290da8ce63c338fd9 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 17 Jul 2018 10:52:22 +1200
Subject: [PATCH 3/7] audit logging  tests: Only run JSON tests in ad dc

The audit logging tests require jansson for JSON support, this is
required for ad dc builds so the tests need to be run in a suitable
environment.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 source4/selftest/tests.py | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 4b77b7e..af5169f 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1120,17 +1120,21 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.unique_object_sids" , "none",
               [os.path.join(bindir(), "test_unique_object_sids")])
 plantestsuite("samba4.dsdb.samdb.ldb_modules.encrypted_secrets", "none",
                   [os.path.join(bindir(), "test_encrypted_secrets")])
-plantestsuite("lib.audit_logging.audit_logging", "none",
-                  [os.path.join(bindir(), "audit_logging_test")])
-plantestsuite("lib.audit_logging.audit_logging.errors", "none",
-                  [os.path.join(bindir(), "audit_logging_error_test")])
-plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "none",
-                  [os.path.join(bindir(), "test_audit_util")])
-plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "none",
-                  [os.path.join(bindir(), "test_audit_log")])
-plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log.errors", "none",
-                  [os.path.join(bindir(), "test_audit_log_errors")])
-plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit", "none",
-                  [os.path.join(bindir(), "test_group_audit")])
-plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "none",
-                  [os.path.join(bindir(), "test_group_audit_errors")])
+
+# ad dc specific cmocka tests these tests rely on the jansson library for JSON
+# logging.
+#
+plantestsuite("lib.audit_logging.audit_logging", "ad_dc:local",
+              [os.path.join(bindir(), "audit_logging_test")])
+plantestsuite("lib.audit_logging.audit_logging.errors", "ad_dc:local",
+              [os.path.join(bindir(), "audit_logging_error_test")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "ad_dc:local",
+              [os.path.join(bindir(), "test_audit_util")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "ad_dc:local",
+              [os.path.join(bindir(), "test_audit_log")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log.errors", "ad_dc:local",
+              [os.path.join(bindir(), "test_audit_log_errors")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit", "ad_dc:local",
+              [os.path.join(bindir(), "test_group_audit")])
+plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "ad_dc:local",
+              [os.path.join(bindir(), "test_group_audit_errors")])
-- 
2.7.4


From f9d28427d52b2fc55de848ffd2504a8952aee323 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 13 Jul 2018 13:33:59 +1200
Subject: [PATCH 4/7] dsdb audit logging: remove HAVE_JANSSON from audit_log

This modules is ADDC only and JANSSON is required for the ADDC builds,
so the ifdef is no longer necessary.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/audit_log.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index 81d4931..44726b6 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -163,8 +163,6 @@ static const char *get_password_action(
 	}
 }
 
-
-#ifdef HAVE_JANSSON
 /*
  * @brief generate a JSON object detailing an ldb operation.
  *
@@ -710,7 +708,6 @@ failure:
 	return wrapper;
 }
 
-#endif
 /*
  * @brief Print a human readable log line for a password change event.
  *
@@ -1132,7 +1129,6 @@ static void log_standard_operation(
 			TALLOC_FREE(entry);
 		}
 	}
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_AUDIT_JSON, OPERATION_LOG_LVL) ||
 		(audit_private->msg_ctx
 		 && audit_private->send_samdb_events)) {
@@ -1174,7 +1170,6 @@ static void log_standard_operation(
 			json_free(&json);
 		}
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
@@ -1215,7 +1210,6 @@ static void log_replicated_operation(
 			REPLICATION_LOG_LVL);
 		TALLOC_FREE(entry);
 	}
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_AUDIT_JSON, REPLICATION_LOG_LVL) ||
 		(audit_private->msg_ctx && audit_private->send_samdb_events)) {
 		struct json_object json;
@@ -1234,7 +1228,6 @@ static void log_replicated_operation(
 		}
 		json_free(&json);
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
@@ -1302,7 +1295,6 @@ static void log_transaction(
 			log_level);
 		TALLOC_FREE(entry);
 	}
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_TXN_AUDIT_JSON, log_level) ||
 		(audit_private->msg_ctx && audit_private->send_samdb_events)) {
 		struct json_object json;
@@ -1324,7 +1316,6 @@ static void log_transaction(
 		}
 		json_free(&json);
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
@@ -1372,7 +1363,6 @@ static void log_commit_failure(
 			TRANSACTION_LOG_FAILURE_LVL);
 		TALLOC_FREE(entry);
 	}
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_TXN_AUDIT_JSON, log_level) ||
 		(audit_private->msg_ctx
 		 && audit_private->send_samdb_events)) {
@@ -1396,7 +1386,6 @@ static void log_commit_failure(
 		}
 		json_free(&json);
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
-- 
2.7.4


From b97fab31ac3ab09084a447a3d01f0d94c25302ed Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 13 Jul 2018 13:34:28 +1200
Subject: [PATCH 5/7] dsdb group auditing: remove HAVE_JANSSON from group_audit

This modules is ADDC only and JANSSON is required for the ADDC builds,
so the ifdef is no longer necessary.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/group_audit.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/group_audit.c b/source4/dsdb/samdb/ldb_modules/group_audit.c
index 1166c69..1c74805 100644
--- a/source4/dsdb/samdb/ldb_modules/group_audit.c
+++ b/source4/dsdb/samdb/ldb_modules/group_audit.c
@@ -90,7 +90,6 @@ static struct GUID *get_transaction_id(
 	return &transaction_id->transaction_guid;
 }
 
-#ifdef HAVE_JANSSON
 /*
  * @brief generate a JSON log entry for a group change.
  *
@@ -208,7 +207,6 @@ failure:
 	DBG_ERR("Failed to create group change JSON log message\n");
 	return wrapper;
 }
-#endif
 
 /*
  * @brief generate a human readable log entry for a group change.
@@ -493,7 +491,6 @@ static void log_primary_group_change(
 		TALLOC_FREE(message);
 	}
 
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_GROUP_AUDIT_JSON, GROUP_LOG_LVL) ||
 		(ac->msg_ctx && ac->send_events)) {
 
@@ -519,7 +516,6 @@ static void log_primary_group_change(
 		}
 		json_free(&json);
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
@@ -569,7 +565,6 @@ static void log_membership_change(
 		TALLOC_FREE(message);
 	}
 
-#ifdef HAVE_JANSSON
 	if (CHECK_DEBUGLVLC(DBGC_DSDB_GROUP_AUDIT_JSON, GROUP_LOG_LVL) ||
 		(ac->msg_ctx && ac->send_events)) {
 		struct json_object json;
@@ -594,7 +589,6 @@ static void log_membership_change(
 		}
 		json_free(&json);
 	}
-#endif
 	TALLOC_FREE(ctx);
 }
 
-- 
2.7.4


From 85100a00ce982422d17babbb1f354a81bd66bc5b Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon, 16 Jul 2018 14:38:12 +1200
Subject: [PATCH 6/7] dsdb group_audit_test: Remove redundant mocking code

Remove a place holder test and unused mocking code.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 .../samdb/ldb_modules/tests/test_group_audit.c     | 90 ----------------------
 1 file changed, 90 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
index 2c32b65..8551dcb 100644
--- a/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_group_audit.c
@@ -64,34 +64,6 @@ int dsdb_search_one(struct ldb_context *ldb,
 }
 
 /*
- * Mocking for audit_log_hr to capture the called parameters
- */
-const char *audit_log_hr_prefix = NULL;
-const char *audit_log_hr_message = NULL;
-int audit_log_hr_debug_class = 0;
-int audit_log_hr_debug_level = 0;
-
-static void audit_log_hr_init(void)
-{
-	audit_log_hr_prefix = NULL;
-	audit_log_hr_message = NULL;
-	audit_log_hr_debug_class = 0;
-	audit_log_hr_debug_level = 0;
-}
-
-void audit_log_human_text(
-	const char *prefix,
-	const char *message,
-	int debug_class,
-	int debug_level)
-{
-	audit_log_hr_prefix = prefix;
-	audit_log_hr_message = message;
-	audit_log_hr_debug_class = debug_class;
-	audit_log_hr_debug_level = debug_level;
-}
-
-/*
  * Test helper to check ISO 8601 timestamps for validity
  */
 static void check_timestamp(time_t before, const char *timestamp)
@@ -551,60 +523,6 @@ static void test_get_primary_group_dn(void **state)
 	TALLOC_FREE(ctx);
 }
 
-/*
- * Mocking for audit_log_json to capture the called parameters
- */
-const char *audit_log_json_prefix = NULL;
-struct json_object *audit_log_json_message = NULL;
-int audit_log_json_debug_class = 0;
-int audit_log_json_debug_level = 0;
-
-static void audit_log_json_init(void)
-{
-	audit_log_json_prefix = NULL;
-	audit_log_json_message = NULL;
-	audit_log_json_debug_class = 0;
-	audit_log_json_debug_level = 0;
-}
-
-void audit_log_json(
-	const char* prefix,
-	struct json_object* message,
-	int debug_class,
-	int debug_level)
-{
-	audit_log_json_prefix = prefix;
-	audit_log_json_message = message;
-	audit_log_json_debug_class = debug_class;
-	audit_log_json_debug_level = debug_level;
-}
-
-/*
- * Mocking for audit_message_send to capture the called parameters
- */
-struct imessaging_context *audit_message_send_msg_ctx = NULL;
-const char *audit_message_send_server_name = NULL;
-uint32_t audit_message_send_message_type = 0;
-struct json_object *audit_message_send_message = NULL;
-
-static void audit_message_send_init(void) {
-	audit_message_send_msg_ctx = NULL;
-	audit_message_send_server_name = NULL;
-	audit_message_send_message_type = 0;
-	audit_message_send_message = NULL;
-}
-void audit_message_send(
-	struct imessaging_context *msg_ctx,
-	const char *server_name,
-	uint32_t message_type,
-	struct json_object *message)
-{
-	audit_message_send_msg_ctx = msg_ctx;
-	audit_message_send_server_name = server_name;
-	audit_message_send_message_type = message_type;
-	audit_message_send_message = message;
-}
-
 static void test_audit_group_json(void **state)
 {
 	struct ldb_context *ldb = NULL;
@@ -703,13 +621,6 @@ static void test_audit_group_json(void **state)
 
 }
 
-static void test_place_holder(void **state)
-{
-	audit_log_json_init();
-	audit_log_hr_init();
-	audit_message_send_init();
-}
-
 /*
  * Note: to run under valgrind us:
  *       valgrind --suppressions=test_group_audit.valgrind bin/test_group_audit
@@ -720,7 +631,6 @@ static void test_place_holder(void **state)
 int main(void) {
 	const struct CMUnitTest tests[] = {
 		cmocka_unit_test(test_audit_group_json),
-		cmocka_unit_test(test_place_holder),
 		cmocka_unit_test(test_get_transaction_id),
 		cmocka_unit_test(test_audit_group_hr),
 		cmocka_unit_test(test_get_parsed_dns),
-- 
2.7.4


From 1d655ed1ba07bb206a0891c8011c905e49bf3deb Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 24 Jul 2018 15:20:21 +1200
Subject: [PATCH 7/7] lib audit_logging: add _WARN_UNUSED_RESULT_

Have the compiler issue a warning when the return code from the API is
ignored.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 lib/audit_logging/audit_logging.h            | 76 +++++++++++++++-------------
 lib/audit_logging/tests/audit_logging_test.c | 37 +++++++++-----
 2 files changed, 66 insertions(+), 47 deletions(-)

diff --git a/lib/audit_logging/audit_logging.h b/lib/audit_logging/audit_logging.h
index 84738d2..4203743 100644
--- a/lib/audit_logging/audit_logging.h
+++ b/lib/audit_logging/audit_logging.h
@@ -21,8 +21,9 @@
 #include <talloc.h>
 #include "lib/messaging/irpc.h"
 #include "lib/tsocket/tsocket.h"
+#include "lib/util/attr.h"
 
-char* audit_get_timestamp(TALLOC_CTX *frame);
+_WARN_UNUSED_RESULT_ char *audit_get_timestamp(TALLOC_CTX *frame);
 void audit_log_human_text(const char *prefix,
 			  const char *message,
 			  int debug_class,
@@ -50,43 +51,48 @@ void audit_message_send(struct imessaging_context *msg_ctx,
 			const char *server_name,
 			uint32_t message_type,
 			struct json_object *message);
-struct json_object json_new_object(void);
-struct json_object json_new_array(void);
+_WARN_UNUSED_RESULT_ struct json_object json_new_object(void);
+_WARN_UNUSED_RESULT_ struct json_object json_new_array(void);
 void json_free(struct json_object *object);
 void json_assert_is_array(struct json_object *array);
-bool json_is_invalid(struct json_object *object);
+_WARN_UNUSED_RESULT_ bool json_is_invalid(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 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);
+_WARN_UNUSED_RESULT_ int json_add_int(struct json_object *object,
+				      const char *name,
+				      const int value);
+_WARN_UNUSED_RESULT_ int json_add_bool(struct json_object *object,
+				       const char *name,
+				       const bool value);
+_WARN_UNUSED_RESULT_ int json_add_string(struct json_object *object,
+					 const char *name,
+					 const char *value);
+_WARN_UNUSED_RESULT_ int json_add_object(struct json_object *object,
+					 const char *name,
+					 struct json_object *value);
+_WARN_UNUSED_RESULT_ int json_add_stringn(struct json_object *object,
+					  const char *name,
+					  const char *value,
+					  const size_t len);
+_WARN_UNUSED_RESULT_ int json_add_version(struct json_object *object,
+					  int major,
+					  int minor);
+_WARN_UNUSED_RESULT_ int json_add_timestamp(struct json_object *object);
+_WARN_UNUSED_RESULT_ int json_add_address(
+    struct json_object *object,
+    const char *name,
+    const struct tsocket_address *address);
+_WARN_UNUSED_RESULT_ int json_add_sid(struct json_object *object,
+				      const char *name,
+				      const struct dom_sid *sid);
+_WARN_UNUSED_RESULT_ 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);
-struct json_object json_get_object(struct json_object *object,
-				   const char* name);
-char *json_to_string(TALLOC_CTX *mem_ctx,
-		     struct json_object *object);
+_WARN_UNUSED_RESULT_ struct json_object json_get_array(
+    struct json_object *object, const char *name);
+_WARN_UNUSED_RESULT_ struct json_object json_get_object(
+    struct json_object *object, const char *name);
+_WARN_UNUSED_RESULT_ 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 07c52ea..acd2a4f 100644
--- a/lib/audit_logging/tests/audit_logging_test.c
+++ b/lib/audit_logging/tests/audit_logging_test.c
@@ -604,6 +604,7 @@ static void test_json_to_string(void **state)
 {
 	struct json_object object;
 	char *s = NULL;
+	int rc;
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
 
@@ -613,7 +614,8 @@ static void test_json_to_string(void **state)
 	assert_string_equal("{}", s);
 	TALLOC_FREE(s);
 
-	json_add_string(&object, "name", "value");
+	rc = json_add_string(&object, "name", "value");
+	assert_int_equal(0, rc);
 	s = json_to_string(ctx, &object);
 	assert_string_equal("{\"name\": \"value\"}", s);
 	TALLOC_FREE(s);
@@ -641,6 +643,7 @@ static void test_json_get_array(void **state)
 	json_t *o = NULL;
 	struct json_object o1;
 	struct json_object o2;
+	int rc;
 
 	object = json_new_object();
 
@@ -651,9 +654,12 @@ static void test_json_get_array(void **state)
 	json_free(&array);
 
 	o1 = json_new_object();
-	json_add_string(&o1, "value", "value-one");
-	json_add_object(&stored_array, NULL, &o1);
-	json_add_object(&object, "stored_array", &stored_array);
+	rc = json_add_string(&o1, "value", "value-one");
+	assert_int_equal(0, rc);
+	rc = json_add_object(&stored_array, NULL, &o1);
+	assert_int_equal(0, rc);
+	rc = json_add_object(&object, "stored_array", &stored_array);
+	assert_int_equal(0, rc);
 
 	array = json_get_array(&object, "stored_array");
 	assert_true(array.valid);
@@ -679,11 +685,14 @@ static void test_json_get_array(void **state)
 	array = json_get_array(&object, "stored_array");
 	assert_true(json_is_array(array.root));
 	o2 = json_new_object();
-	json_add_string(&o2, "value", "value-two");
+	rc = json_add_string(&o2, "value", "value-two");
+	assert_int_equal(0, rc);
 	assert_true(o2.valid);
-	json_add_object(&array, NULL, &o2);
+	rc = json_add_object(&array, NULL, &o2);
+	assert_int_equal(0, rc);
 	assert_true(json_is_array(array.root));
-	json_add_object(&object, "stored_array", &array);
+	rc = json_add_object(&object, "stored_array", &array);
+	assert_int_equal(0, rc);
 	assert_true(json_is_array(array.root));
 
 	array = json_get_array(&object, "stored_array");
@@ -728,6 +737,7 @@ static void test_json_get_object(void **state)
 	struct json_object o2;
 	struct json_object o3;
 	json_t *value = NULL;
+	int rc;
 
 	object = json_new_object();
 
@@ -738,8 +748,10 @@ static void test_json_get_object(void **state)
 	json_free(&o1);
 
 	o1 = json_new_object();
-	json_add_string(&o1, "value", "value-one");
-	json_add_object(&object, "stored_object", &o1);
+	rc = json_add_string(&o1, "value", "value-one");
+	assert_int_equal(0, rc);
+	rc = json_add_object(&object, "stored_object", &o1);
+	assert_int_equal(0, rc);
 
 	o2 = json_get_object(&object, "stored_object");
 	assert_true(o2.valid);
@@ -752,9 +764,10 @@ static void test_json_get_object(void **state)
 
 	assert_string_equal("value-one", json_string_value(value));
 
-	json_add_string(&o2, "value", "value-two");
-	json_add_object(&object, "stored_object", &o2);
-
+	rc = json_add_string(&o2, "value", "value-two");
+	assert_int_equal(0, rc);
+	rc = json_add_object(&object, "stored_object", &o2);
+	assert_int_equal(0, rc);
 
 	o3 = json_get_object(&object, "stored_object");
 	assert_true(o3.valid);
-- 
2.7.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180725/0650c60b/signature-0001.sig>


More information about the samba-technical mailing list