Patch Fix JSON API

Jeremy Allison jra at samba.org
Tue Jul 24 20:30:12 UTC 2018


On Wed, Jul 25, 2018 at 06:59:00AM +1200, Gary Lockyer wrote:
> Hopefully the final patch set, have added _WARN_UNUSED_RESULT_ to the
> function prototypes.
> 
> Could I please get a review on this.

Will do asap Gary - thanks !

Jeremy.

> 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
> >>

> 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
> 







More information about the samba-technical mailing list