[PATCH] audit validatetrans denials

[PATCH] audit validatetrans denials

[Darrel Goeddel]

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

Attached is a patch that adds the auditing of denials caused by the 
validatetrans rules in the policy.  Look good?

Thanks,

-- 

Darrel

[-- Attachment #2: validtrans_audit.patch --]
[-- Type: text/x-patch, Size: 1743 bytes --]

diff -ruNp linux-2.6-nsa-20050215/security/selinux/ss/services.c linux-2.6-20050216/security/selinux/ss/services.c
--- linux-2.6-nsa-20050215/security/selinux/ss/services.c	2005-01-27 16:48:31.000000000 -0600
+++ linux-2.6-20050216/security/selinux/ss/services.c	2005-02-16 09:36:40.612223188 -0600
@@ -38,6 +38,13 @@
 #include "services.h"
 #include "conditional.h"
 #include "mls.h"
+#ifdef CONFIG_AUDIT
+static const char *class_to_string[] = {
+#define S_(s) s,
+#include "class_to_string.h"
+#undef S_
+};
+#endif
 
 extern void selnl_notify_policyload(u32 seqno);
 unsigned int policydb_loaded_version;
@@ -417,6 +424,43 @@ int security_validate_transition(u32 old
 
 out:
 	POLICY_RDUNLOCK;
+
+	/* audit failures */
+	if (rc) {
+		int err;
+		char *context;
+		u32 context_len;
+		struct audit_buffer *ab;
+
+		ab = audit_log_start(current->audit_context);
+		if (!ab)
+			return rc; /* audit_panic has been called */
+		audit_log_format(ab, "avc:  denied validate_transition");
+ 		err = security_sid_to_context(oldsid, &context, &context_len);
+		if (err)
+			audit_log_format(ab, " oldsid=%d", oldsid);
+		else {
+			audit_log_format(ab, " oldcontext=%s", context);
+			kfree(context);
+		}
+		err = security_sid_to_context(newsid, &context, &context_len);
+		if (err)
+			audit_log_format(ab, " newsid=%d", newsid);
+		else {
+			audit_log_format(ab, " newcontext=%s", context);
+			kfree(context);
+		}
+		err = security_sid_to_context(tasksid, &context, &context_len);
+		if (err)
+			audit_log_format(ab, " tasksid=%d", tasksid);
+		else {
+			audit_log_format(ab, " taskcontext=%s", context);
+			kfree(context);
+		}
+		audit_log_format(ab, " tclass=%s", class_to_string[tclass]);
+		audit_log_end(ab);
+	}
+
 	return rc;
 }
 

Re: [PATCH] audit validatetrans denials

[James Morris]

On Wed, 16 Feb 2005, Darrel Goeddel wrote:

> Attached is a patch that adds the auditing of denials caused by the 
> validatetrans rules in the policy.  Look good?

Looks fine to me.


- James
-- 
James Morris
<jmorris@redhat.com>



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] audit validatetrans denials

[Stephen Smalley]

On Wed, 2005-02-16 at 16:54, Darrel Goeddel wrote:
> Attached is a patch that adds the auditing of denials caused by the 
> validatetrans rules in the policy.  Look good?

I'd rather do this the same way as compute_sid_handle_invalid_context(),
i.e. generate the context strings up front using
context_struct_to_string() after looking up the SIDs, use
policydb.p_class_val_to_name[tclass-1] to lookup the class name, and
perform a single audit_log() call.  Since you are already within the
security server at this point, you don't have to do it in the same
manner as the AVC.  And you already log error messages on any invalid
SIDs or classes (in which case you aren't going to be able to look them
up anyway).  So I think you just want to call a similar helper as
compute_sid_handle_invalid_context() in the case where
constraint_expr_eval() fails.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] audit validatetrans denials

[Darrel Goeddel]

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

Stephen Smalley wrote:
> I'd rather do this the same way as compute_sid_handle_invalid_context(),
> i.e. generate the context strings up front using
> context_struct_to_string() after looking up the SIDs, use
> policydb.p_class_val_to_name[tclass-1] to lookup the class name, and
> perform a single audit_log() call.  Since you are already within the
> security server at this point, you don't have to do it in the same
> manner as the AVC.  And you already log error messages on any invalid
> SIDs or classes (in which case you aren't going to be able to look them
> up anyway).  So I think you just want to call a similar helper as
> compute_sid_handle_invalid_context() in the case where
> constraint_expr_eval() fails.
> 

Yeah, I really could have made this nicer when I moved the functionality into 
security_validate_transition (I originally had it another function which was 
called after security_validate_transition)...  How about the attached patch instead.

-- 

Darrel

[-- Attachment #2: validtrans_audit.patch2 --]
[-- Type: text/plain, Size: 2026 bytes --]

diff -ruNp linux-2.6-nsa-20050215/security/selinux/ss/services.c linux-2.6-20050217/security/selinux/ss/services.c
--- linux-2.6-nsa-20050215/security/selinux/ss/services.c	2005-01-27 16:48:31.000000000 -0600
+++ linux-2.6-20050217/security/selinux/ss/services.c	2005-02-17 08:51:53.592423991 -0600
@@ -64,6 +64,10 @@ int ss_initialized = 0;
  */
 static u32 latest_granting = 0;
 
+/* Forward declarations. */
+int context_struct_to_string(struct context *context, char **scontext,
+                             u32 *scontext_len);
+
 /*
  * Return the boolean value of a constraint expression
  * when it is applied to the specified source and target
@@ -347,6 +351,31 @@ static int context_struct_compute_av(str
 	return 0;
 }
 
+static void security_validtrans_audit_fail(struct context *ocontext,
+                                           struct context *ncontext,
+                                           struct context *tcontext,
+                                           u16 tclass)
+{
+	char *o = NULL, *n = NULL, *t = NULL;
+	u32 olen, nlen, tlen;
+
+	if (context_struct_to_string(ocontext, &o, &olen) < 0)
+		goto out;
+	if (context_struct_to_string(ncontext, &n, &nlen) < 0)
+		goto out;
+	if (context_struct_to_string(tcontext, &t, &tlen) < 0)
+		goto out;
+	audit_log(current->audit_context,
+	          "security_validate_transition:  denied for"
+	          " oldcontext=%s newcontext=%s taskcontext=%s tclass=%s",
+	          o, n, t, policydb.p_class_val_to_name[tclass-1]);
+out:
+	kfree(o);
+	kfree(n);
+	kfree(t);
+	return;
+}
+
 int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
                                  u16 tclass)
 {
@@ -410,6 +439,8 @@ int security_validate_transition(u32 old
 		if (!constraint_expr_eval(ocontext, ncontext, tcontext,
 		                          constraint->expr)) {
 			rc = -EPERM;
+			security_validtrans_audit_fail(ocontext, ncontext,
+			                               tcontext, tclass);
 			goto out;
 		}
 		constraint = constraint->next;

Re: [PATCH] audit validatetrans denials

[Stephen Smalley]

On Thu, 2005-02-17 at 11:29, Darrel Goeddel wrote:
> Yeah, I really could have made this nicer when I moved the functionality into 
> security_validate_transition (I originally had it another function which was 
> called after security_validate_transition)...  How about the attached patch instead.

Looks good, thanks.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] audit validatetrans denials

[Stephen Smalley]

On Thu, 2005-02-17 at 11:29, Darrel Goeddel wrote:
> Yeah, I really could have made this nicer when I moved the functionality into 
> security_validate_transition (I originally had it another function which was 
> called after security_validate_transition)...  How about the attached patch instead.

One other question:  At present, you won't perform any such auditing
when in permissive mode, as you bail out of
security_validate_transition() immediately in that case.  Do you want
instead to handle this more like security_compute_sid(), i.e. always
auditing the denial but only returning an error if enforcing? 
Otherwise, you won't even see such denials if you are doing any kind of
policy development in permissive mode.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] audit validatetrans denials

[Darrel Goeddel]

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

Stephen Smalley wrote:
> One other question:  At present, you won't perform any such auditing
> when in permissive mode, as you bail out of
> security_validate_transition() immediately in that case.  Do you want
> instead to handle this more like security_compute_sid(), i.e. always
> auditing the denial but only returning an error if enforcing? 
> Otherwise, you won't even see such denials if you are doing any kind of
> policy development in permissive mode.
> 

Good catch.  Version 3 attached...

-- 

Darrel

[-- Attachment #2: validtrans_audit.patch3 --]
[-- Type: text/plain, Size: 2324 bytes --]

diff -ruNp linux-2.6-nsa-20050215/security/selinux/ss/services.c linux-2.6-20050217/security/selinux/ss/services.c
--- linux-2.6-nsa-20050215/security/selinux/ss/services.c	2005-01-27 16:48:31.000000000 -0600
+++ linux-2.6-20050217/security/selinux/ss/services.c	2005-02-17 12:50:22.000000000 -0600
@@ -64,6 +64,10 @@ int ss_initialized = 0;
  */
 static u32 latest_granting = 0;
 
+/* Forward declarations. */
+int context_struct_to_string(struct context *context, char **scontext,
+                             u32 *scontext_len);
+
 /*
  * Return the boolean value of a constraint expression
  * when it is applied to the specified source and target
@@ -347,6 +351,34 @@ static int context_struct_compute_av(str
 	return 0;
 }
 
+static int security_validtrans_handle_fail(struct context *ocontext,
+                                           struct context *ncontext,
+                                           struct context *tcontext,
+                                           u16 tclass)
+{
+	char *o = NULL, *n = NULL, *t = NULL;
+	u32 olen, nlen, tlen;
+
+	if (context_struct_to_string(ocontext, &o, &olen) < 0)
+		goto out;
+	if (context_struct_to_string(ncontext, &n, &nlen) < 0)
+		goto out;
+	if (context_struct_to_string(tcontext, &t, &tlen) < 0)
+		goto out;
+	audit_log(current->audit_context,
+	          "security_validate_transition:  denied for"
+	          " oldcontext=%s newcontext=%s taskcontext=%s tclass=%s",
+	          o, n, t, policydb.p_class_val_to_name[tclass-1]);
+out:
+	kfree(o);
+	kfree(n);
+	kfree(t);
+
+	if (!selinux_enforcing)
+		return 0;
+	return -EPERM;
+}
+
 int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
                                  u16 tclass)
 {
@@ -357,7 +389,7 @@ int security_validate_transition(u32 old
 	struct constraint_node *constraint;
 	int rc = 0;
 
-	if (!ss_initialized || !selinux_enforcing)
+	if (!ss_initialized)
 		return 0;
 
 	POLICY_RDLOCK;
@@ -409,7 +441,8 @@ int security_validate_transition(u32 old
 	while (constraint) {
 		if (!constraint_expr_eval(ocontext, ncontext, tcontext,
 		                          constraint->expr)) {
-			rc = -EPERM;
+			rc = security_validtrans_handle_fail(ocontext, ncontext,
+			                                     tcontext, tclass);
 			goto out;
 		}
 		constraint = constraint->next;