[SCM] Samba Shared Repository - branch master updated

Gary Lockyer gary at samba.org
Wed Jul 25 07:29:02 UTC 2018


The branch, master has been updated
       via  a5e02f7 lib audit_logging: add _WARN_UNUSED_RESULT_
       via  6a5346f dsdb group_audit_test: Remove redundant mocking code
       via  77fa340 dsdb group auditing: remove HAVE_JANSSON from group_audit
       via  a5172ee dsdb audit logging: remove HAVE_JANSSON from audit_log
       via  6f4f8c5 json: Add unit tests for error handling
       via  79f494e json: Modify API to use return codes
      from  e2ebfd8 docs/kerneloplocks: drop Irix references

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit a5e02f7264a5feb9f0a5bb3d84de0153dc0758a0
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Jul 24 15:20:21 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Gary Lockyer <gary at samba.org>
    Autobuild-Date(master): Wed Jul 25 09:28:31 CEST 2018 on sn-devel-144

commit 6a5346f166ac1a9b03bca76dc40d3645432377df
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Mon Jul 16 14:38:12 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 77fa34080c4a9fa39134ea0997870b82a49b500c
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Jul 13 13:34:28 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit a5172ee74971bda3c9f7c94bce3afbcc89ae288a
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Jul 13 13:33:59 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 6f4f8c51e0acd92aae1b57041cf787706820db77
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Jul 13 09:15:34 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 79f494e51eabb5176747fcf3b9f2efad10ec7f97
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Jul 13 09:14:09 2018 +1200

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 auth/auth_log.c                                    | 307 ++++++--
 lib/audit_logging/audit_logging.c                  | 527 ++++++++-----
 lib/audit_logging/audit_logging.h                  |  89 ++-
 lib/audit_logging/tests/audit_logging_error_test.c | 869 +++++++++++++++++++++
 lib/audit_logging/tests/audit_logging_test.c       | 289 +++++--
 lib/audit_logging/wscript_build                    |  31 +
 source4/dsdb/samdb/ldb_modules/audit_log.c         | 447 ++++++++---
 source4/dsdb/samdb/ldb_modules/audit_util.c        | 148 +++-
 source4/dsdb/samdb/ldb_modules/group_audit.c       |  97 ++-
 .../ldb_modules/tests/test_audit_log_errors.c      | 639 +++++++++++++++
 .../samdb/ldb_modules/tests/test_group_audit.c     |  90 ---
 .../ldb_modules/tests/test_group_audit_errors.c    | 265 +++++++
 .../dsdb/samdb/ldb_modules/wscript_build_server    |  38 +
 source4/selftest/tests.py                          |  10 +-
 14 files changed, 3242 insertions(+), 604 deletions(-)
 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


Changeset truncated at 500 lines:

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..ac08863 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;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list