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