[PATCH] newrole auditing of failures due to user actions

[PATCH] newrole auditing of failures due to user actions

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

This patch introduces two new point in the code where audit records are 
generated for newrole. Both points are when the attempt to newrole fails.

The first point is when the default type could not be determine for the 
specified role - this is audited because, as sgrubb pointed out, it is 
currently non-tracked path to probe the policy.

The second point is when the desired context to change to is invalid.

There record format remains unchanged. Failing to validate the desired 
context will result in the old and new contexts being recorded intact to 
the log. For the default type, the old and new contexts have not yet 
been obtained, so they are recorded in the log as XXX_context=?

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>

----



[-- Attachment #2: newrole_audit_fail.patch --]
[-- Type: text/x-diff, Size: 2752 bytes --]

--- policycoreutils-1.30.29/newrole/newrole.c	2006-09-14 07:07:26.000000000 -0500
+++ policycoreutils-1.30.29.orig.dev/newrole/newrole.c	2006-09-28 14:21:27.000000000 -0500
@@ -47,7 +47,9 @@
  *
  *************************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 #include <stdio.h>
 #include <stdlib.h>		/* for malloc(), realloc(), free() */
 #include <pwd.h>		/* for getpwuid() */
@@ -394,6 +396,41 @@
 		cap_free(new_caps);
 	}
 }
+
+/* Send audit message */
+int send_audit_message(int success, security_context_t old_context,
+			security_context_t new_context, const char *ttyn)
+{
+	char *msg = NULL;
+	int rc;
+	int audit_fd = audit_open();
+
+	if (audit_fd < 0) {
+		fprintf(stderr, _("Error connecting to audit system.\n"));
+		rc = -1;
+		goto out;
+	}
+	if (asprintf(&msg, "newrole: old-context=%s new-context=%s",
+				old_context ? old_context : "?",
+				new_context ? new_context : "?") < 0) {
+		fprintf(stderr, _("Error allocating memory.\n"));
+		rc = -1;
+		goto out;
+	}
+	rc = audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
+			msg, NULL, NULL, ttyn, success);
+	if (rc <= 0) {
+		fprintf(stderr, _("Error sending audit message.\n"));
+		rc = -1;
+		goto out;
+	}
+	rc = 0;
+out:
+	free(msg);
+	close(audit_fd);
+	return rc;
+}
+
 #endif
 
 /************************************************************************
@@ -536,6 +573,9 @@
 	if (role_s && !type_s) {
 		if (get_default_type(role_s, &type_s)) {
 			fprintf(stderr, _("Couldn't get default type.\n"));
+#ifdef LOG_AUDIT_PRIV
+			send_audit_message(0, old_context, new_context, ttyn);
+#endif
 			exit(-1);
 		}
 #ifdef CANTSPELLGDB
@@ -715,6 +755,9 @@
 
 	if (security_check_context(new_context) < 0) {
 		fprintf(stderr, _("%s is not a valid context\n"), new_context);
+#ifdef LOG_AUDIT_PRIV
+		send_audit_message(0, old_context, new_context, ttyn);
+#endif
 		exit(-1);
 	}
 
@@ -874,30 +917,8 @@
 		exit(-1);
 	}
 #ifdef LOG_AUDIT_PRIV
-	/* Send audit message */
-	{
-		char *msg;
-		int rc;
-		int audit_fd = audit_open();
-		if (audit_fd < 0) {
-			fprintf(stderr,
-				_("Error connecting to audit system.\n"));
-			exit(-1);
-		}
-		if (asprintf(&msg, "newrole: old-context=%s new-context=%s",
-			     old_context, new_context) < 0) {
-			fprintf(stderr, _("Error allocating memory.\n"));
-			exit(-1);
-		}
-		rc = audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
-					    msg, NULL, NULL, ttyn, 1);
-		if (rc <= 0) {
-			fprintf(stderr, _("Error sending audit message.\n"));
-			exit(-1);
-		}
-		free(msg);
-		close(audit_fd);
-	}
+	if (send_audit_message(1, old_context, new_context, ttyn))
+		exit(-1);
 #endif
 	freecon(old_context);
 	execv(pw->pw_shell, argv + optind - 1);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] newrole auditing of failures due to user actions

[Michael C Thompson]

Michael C Thompson wrote:
> This patch introduces two new point in the code where audit records are 
> generated for newrole. Both points are when the attempt to newrole fails.

Forgot to copy the SELinux mailing list.

> The first point is when the default type could not be determine for the 
> specified role - this is audited because, as sgrubb pointed out, it is 
> currently non-tracked path to probe the policy.
> 
> The second point is when the desired context to change to is invalid.
> 
> There record format remains unchanged. Failing to validate the desired 
> context will result in the old and new contexts being recorded intact to 
> the log. For the default type, the old and new contexts have not yet 
> been obtained, so they are recorded in the log as XXX_context=?
> 
> Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>
> 
> ----
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- policycoreutils-1.30.29/newrole/newrole.c	2006-09-14 07:07:26.000000000 -0500
> +++ policycoreutils-1.30.29.orig.dev/newrole/newrole.c	2006-09-28 14:21:27.000000000 -0500
> @@ -47,7 +47,9 @@
>   *
>   *************************************************************************/
>  
> +#ifndef _GNU_SOURCE
>  #define _GNU_SOURCE
> +#endif
>  #include <stdio.h>
>  #include <stdlib.h>		/* for malloc(), realloc(), free() */
>  #include <pwd.h>		/* for getpwuid() */
> @@ -394,6 +396,41 @@
>  		cap_free(new_caps);
>  	}
>  }
> +
> +/* Send audit message */
> +int send_audit_message(int success, security_context_t old_context,
> +			security_context_t new_context, const char *ttyn)
> +{
> +	char *msg = NULL;
> +	int rc;
> +	int audit_fd = audit_open();
> +
> +	if (audit_fd < 0) {
> +		fprintf(stderr, _("Error connecting to audit system.\n"));
> +		rc = -1;
> +		goto out;
> +	}
> +	if (asprintf(&msg, "newrole: old-context=%s new-context=%s",
> +				old_context ? old_context : "?",
> +				new_context ? new_context : "?") < 0) {
> +		fprintf(stderr, _("Error allocating memory.\n"));
> +		rc = -1;
> +		goto out;
> +	}
> +	rc = audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
> +			msg, NULL, NULL, ttyn, success);
> +	if (rc <= 0) {
> +		fprintf(stderr, _("Error sending audit message.\n"));
> +		rc = -1;
> +		goto out;
> +	}
> +	rc = 0;
> +out:
> +	free(msg);
> +	close(audit_fd);
> +	return rc;
> +}
> +
>  #endif
>  
>  /************************************************************************
> @@ -536,6 +573,9 @@
>  	if (role_s && !type_s) {
>  		if (get_default_type(role_s, &type_s)) {
>  			fprintf(stderr, _("Couldn't get default type.\n"));
> +#ifdef LOG_AUDIT_PRIV
> +			send_audit_message(0, old_context, new_context, ttyn);
> +#endif
>  			exit(-1);
>  		}
>  #ifdef CANTSPELLGDB
> @@ -715,6 +755,9 @@
>  
>  	if (security_check_context(new_context) < 0) {
>  		fprintf(stderr, _("%s is not a valid context\n"), new_context);
> +#ifdef LOG_AUDIT_PRIV
> +		send_audit_message(0, old_context, new_context, ttyn);
> +#endif
>  		exit(-1);
>  	}
>  
> @@ -874,30 +917,8 @@
>  		exit(-1);
>  	}
>  #ifdef LOG_AUDIT_PRIV
> -	/* Send audit message */
> -	{
> -		char *msg;
> -		int rc;
> -		int audit_fd = audit_open();
> -		if (audit_fd < 0) {
> -			fprintf(stderr,
> -				_("Error connecting to audit system.\n"));
> -			exit(-1);
> -		}
> -		if (asprintf(&msg, "newrole: old-context=%s new-context=%s",
> -			     old_context, new_context) < 0) {
> -			fprintf(stderr, _("Error allocating memory.\n"));
> -			exit(-1);
> -		}
> -		rc = audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
> -					    msg, NULL, NULL, ttyn, 1);
> -		if (rc <= 0) {
> -			fprintf(stderr, _("Error sending audit message.\n"));
> -			exit(-1);
> -		}
> -		free(msg);
> -		close(audit_fd);
> -	}
> +	if (send_audit_message(1, old_context, new_context, ttyn))
> +		exit(-1);
>  #endif
>  	freecon(old_context);
>  	execv(pw->pw_shell, argv + optind - 1);
> 
> 
> ------------------------------------------------------------------------
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] newrole auditing of failures due to user actions

[Stephen Smalley]

On Thu, 2006-09-28 at 14:37 -0500, Michael C Thompson wrote:
> This patch introduces two new point in the code where audit records are 
> generated for newrole. Both points are when the attempt to newrole fails.
> 
> The first point is when the default type could not be determine for the 
> specified role - this is audited because, as sgrubb pointed out, it is 
> currently non-tracked path to probe the policy.
> 
> The second point is when the desired context to change to is invalid.
> 
> There record format remains unchanged. Failing to validate the desired 
> context will result in the old and new contexts being recorded intact to 
> the log. For the default type, the old and new contexts have not yet 
> been obtained, so they are recorded in the log as XXX_context=?
> 
> Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>

(note:  needs to go to selinux list if you want it merged)

--- policycoreutils-1.30.29/newrole/newrole.c	2006-09-14 07:07:26.000000000 -0500
+++ policycoreutils-1.30.29.orig.dev/newrole/newrole.c	2006-09-28 14:21:27.000000000 -0500
@@ -47,7 +47,9 @@
  *
  *************************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif

I think that the -D_GNU_SOURCE should just be removed from the Makefile.

@@ -394,6 +396,41 @@
 		cap_free(new_caps);
 	}
 }
+
+/* Send audit message */
+int send_audit_message(int success, security_context_t old_context,
+			security_context_t new_context, const char *ttyn)

You need to wrap the entire function with #ifdef LOG_AUDIT_PRIV,
and provide a trivial inline function in the #else clause, e.g.
#else
static inline int send_audit_message(int success, security_context_t old_context,
				security_context_t new_context, const char *ttyn)
{
	return 0;
}
#endif

@@ -536,6 +573,9 @@
 	if (role_s && !type_s) {
 		if (get_default_type(role_s, &type_s)) {
 			fprintf(stderr, _("Couldn't get default type.\n"));
+#ifdef LOG_AUDIT_PRIV
+			send_audit_message(0, old_context, new_context, ttyn);
+#endif

You can then drop the #ifdefs here and later, since send_audit_message() will always have a valid
definition.

-- 
Stephen Smalley
National Security Agency

Re: [PATCH] newrole auditing of failures due to user actions

[Steve Grubb]

On Thursday 28 September 2006 16:05, Michael C Thompson wrote:
> > This patch introduces two new point in the code where audit records are
> > generated for newrole. Both points are when the attempt to newrole fails.

>From an audit perspective I think it does the right thing. ACK.

-Steve

Re: [PATCH] newrole auditing of failures due to user actions

[Michael C Thompson]

Stephen Smalley wrote:
> On Thu, 2006-09-28 at 14:37 -0500, Michael C Thompson wrote:
>> This patch introduces two new point in the code where audit records are 
>> generated for newrole. Both points are when the attempt to newrole fails.
>>
>> The first point is when the default type could not be determine for the 
>> specified role - this is audited because, as sgrubb pointed out, it is 
>> currently non-tracked path to probe the policy.
>>
>> The second point is when the desired context to change to is invalid.
>>
>> There record format remains unchanged. Failing to validate the desired 
>> context will result in the old and new contexts being recorded intact to 
>> the log. For the default type, the old and new contexts have not yet 
>> been obtained, so they are recorded in the log as XXX_context=?
>>
>> Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>
> 
> (note:  needs to go to selinux list if you want it merged)
> 
> --- policycoreutils-1.30.29/newrole/newrole.c	2006-09-14 07:07:26.000000000 -0500
> +++ policycoreutils-1.30.29.orig.dev/newrole/newrole.c	2006-09-28 14:21:27.000000000 -0500
> @@ -47,7 +47,9 @@
>   *
>   *************************************************************************/
>  
> +#ifndef _GNU_SOURCE
>  #define _GNU_SOURCE
> +#endif
> 
> I think that the -D_GNU_SOURCE should just be removed from the Makefile.

Done.

> @@ -394,6 +396,41 @@
>  		cap_free(new_caps);
>  	}
>  }
> +
> +/* Send audit message */
> +int send_audit_message(int success, security_context_t old_context,
> +			security_context_t new_context, const char *ttyn)
> 
> You need to wrap the entire function with #ifdef LOG_AUDIT_PRIV,
> and provide a trivial inline function in the #else clause, e.g.
> #else
> static inline int send_audit_message(int success, security_context_t old_context,
> 				security_context_t new_context, const char *ttyn)
> {
> 	return 0;
> }
> #endif

Done.

> @@ -536,6 +573,9 @@
>  	if (role_s && !type_s) {
>  		if (get_default_type(role_s, &type_s)) {
>  			fprintf(stderr, _("Couldn't get default type.\n"));
> +#ifdef LOG_AUDIT_PRIV
> +			send_audit_message(0, old_context, new_context, ttyn);
> +#endif
> 
> You can then drop the #ifdefs here and later, since send_audit_message() will always have a valid
> definition.

Done.

Thanks,
Mike