Getting the real task name in avc messages

Getting the real task name in avc messages

[Steve G]

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

Hello,

Attached is a patch to the kernel that puts the program's name into the avc
message. This lets you know the script that caused the problem instead of
/bin/bash. Feedback would be appreciated.

-Steve Grubb

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

[-- Attachment #2: linux-2.6.11-selinux-procname.patch --]
[-- Type: application/octet-stream, Size: 1765 bytes --]

diff -ur linux-2.6.11.orig/include/linux/audit.h linux-2.6.11/include/linux/audit.h
--- linux-2.6.11.orig/include/linux/audit.h	2005-03-07 13:45:33.000000000 -0500
+++ linux-2.6.11/include/linux/audit.h	2005-03-08 17:26:47.000000000 -0500
@@ -184,6 +184,7 @@
 					     const char *prefix,
 					     struct dentry *dentry,
 					     struct vfsmount *vfsmnt);
+extern void		    audit_log_task_info(struct audit_buffer *ab);
 extern int		    audit_set_rate_limit(int limit, uid_t loginuid);
 extern int		    audit_set_backlog_limit(int limit, uid_t loginuid);
 extern int		    audit_set_enabled(int state, uid_t loginuid);
diff -ur linux-2.6.11.orig/kernel/auditsc.c linux-2.6.11/kernel/auditsc.c
--- linux-2.6.11.orig/kernel/auditsc.c	2005-03-07 13:45:39.000000000 -0500
+++ linux-2.6.11/kernel/auditsc.c	2005-03-08 17:30:06.065237424 -0500
@@ -584,6 +584,17 @@
 		printk(KERN_ERR "audit: freed %d contexts\n", count);
 }
 
+void audit_log_task_info(struct audit_buffer *ab)
+{
+        char name[sizeof(current->comm)];
+
+        get_task_comm(name, current);
+        audit_log_format(ab, " procname=%s", name);
+        audit_log_format(ab, " syscall=%d per=%lx", 
+		current->audit_context->major,
+		current->audit_context->personality);
+}
+
 static void audit_log_exit(struct audit_context *context)
 {
 	int i;
diff -ur linux-2.6.11.orig/security/selinux/avc.c linux-2.6.11/security/selinux/avc.c
--- linux-2.6.11.orig/security/selinux/avc.c	2005-03-07 13:45:46.000000000 -0500
+++ linux-2.6.11/security/selinux/avc.c	2005-03-08 17:26:47.000000000 -0500
@@ -575,6 +575,7 @@
 						audit_log_d_path(ab, "exe=",
 							vma->vm_file->f_dentry,
 							vma->vm_file->f_vfsmnt);
+						audit_log_task_info(ab);
 						break;
 					}
 					vma = vma->vm_next;

Re: Getting the real task name in avc messages

[Stephen Smalley]

On Wed, 2005-03-30 at 11:53 -0800, Steve G wrote:
> Hello,
> 
> Attached is a patch to the kernel that puts the program's name into the avc
> message. This lets you know the script that caused the problem instead of
> /bin/bash. Feedback would be appreciated.

I don't think you want to put this in avc_audit.  Instead, the goal is
to migrate processing from avc_audit to audit_log_exit where it makes
sense to do so.  Note that anytime avc_audit generates an audit message,
audit_log_exit will be called upon syscall exit, so by adding a call to
your new function to audit_log_exit, you'll ensure that this information
is recorded for every avc denial as well as every other audit message.
Note that you don't need to repeate the syscall= and per= information,
as it will already be handled by audit_log_exit for you.  I also think
that the exe= logging should be moved to audit_log_exit, and the
existing logging of the comm should be removed from avc_audit entirely.

Note that the comm field is less complete (not a full path and may even
be truncated) and is not trustworthy (can be changed by the process to
any arbitrary string).  So you can't rely on it, but it can be useful
for debugging.

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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: Getting the real task name in avc messages

[Stephen Smalley]

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

On Wed, 2005-03-30 at 15:41 -0500, Stephen Smalley wrote:
> I don't think you want to put this in avc_audit.  Instead, the goal is
> to migrate processing from avc_audit to audit_log_exit where it makes
> sense to do so.  Note that anytime avc_audit generates an audit message,
> audit_log_exit will be called upon syscall exit, so by adding a call to
> your new function to audit_log_exit, you'll ensure that this information
> is recorded for every avc denial as well as every other audit message.
> Note that you don't need to repeate the syscall= and per= information,
> as it will already be handled by audit_log_exit for you.  I also think
> that the exe= logging should be moved to audit_log_exit, and the
> existing logging of the comm should be removed from avc_audit entirely.

To be concrete, I mean something like the following untested patch
relative to your patch, which strips the logging of task-related
information entirely from avc_audit, moves the logging of the exe to
your new function (which is simplified to reflect the fact that the
calling context for your function never holds mmap_sem), and inserts a
call to your new function into audit_log_exit just prior to logging of
the individual context items after the other basic information has been
logged.  Caveat:  Untested, not even compiled yet.  But food for
thought.  Note that this doesn't address James' concern about stack
usage.

-- 
Stephen Smalley <sds@tycho.nsa.gov>
National Security Agency

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

--- linux-2.6/kernel/auditsc.c.sgrubb	2005-03-30 16:11:07.000000000 -0500
+++ linux-2.6/kernel/auditsc.c	2005-03-30 16:11:32.000000000 -0500
@@ -580,12 +580,28 @@ static inline void audit_free_context(st
 void audit_log_task_info(struct audit_buffer *ab)
 {
         char name[sizeof(current->comm)];
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
 
         get_task_comm(name, current);
-        audit_log_format(ab, " procname=%s", name);
-        audit_log_format(ab, " syscall=%d per=%lx", 
-		current->audit_context->major,
-		current->audit_context->personality);
+        audit_log_format(ab, " comm=%s", name);
+
+	if (!mm)
+		return;
+
+	down_read(&mm->mmap_sem);
+	vma = mm->mmap;
+	while (vma) {
+		if ((vma->vm_flags & VM_EXECUTABLE) &&
+		    vma->vm_file) {
+			audit_log_d_path(ab, "exe=",
+					 vma->vm_file->f_dentry,
+					 vma->vm_file->f_vfsmnt);
+			break;
+		}
+		vma = vma->vm_next;
+	}
+	up_read(&mm->mmap_sem);
 }
 
 static void audit_log_exit(struct audit_context *context)
@@ -617,6 +633,7 @@ static void audit_log_exit(struct audit_
 		  context->gid,
 		  context->euid, context->suid, context->fsuid,
 		  context->egid, context->sgid, context->fsgid);
+	audit_log_task_info(ab);
 	audit_log_end(ab);
 	for (i = 0; i < context->name_count; i++) {
 		ab = audit_log_start(context);
--- linux-2.6/security/selinux/avc.c.sgrubb	2005-03-30 16:11:16.000000000 -0500
+++ linux-2.6/security/selinux/avc.c	2005-03-30 16:11:32.000000000 -0500
@@ -532,7 +532,6 @@ void avc_audit(u32 ssid, u32 tsid,
                u16 tclass, u32 requested,
                struct av_decision *avd, int result, struct avc_audit_data *a)
 {
-	struct task_struct *tsk = current;
 	struct inode *inode = NULL;
 	u32 denied, audited;
 	struct audit_buffer *ab;
@@ -556,40 +555,6 @@ void avc_audit(u32 ssid, u32 tsid,
 	audit_log_format(ab, "avc:  %s ", denied ? "denied" : "granted");
 	avc_dump_av(ab, tclass,audited);
 	audit_log_format(ab, " for ");
-	if (a && a->tsk)
-		tsk = a->tsk;
-	if (tsk && tsk->pid) {
-		struct mm_struct *mm;
-		struct vm_area_struct *vma;
-		audit_log_format(ab, " pid=%d", tsk->pid);
-		if (tsk == current)
-			mm = current->mm;
-		else
-			mm = get_task_mm(tsk);
-		if (mm) {
-			if (down_read_trylock(&mm->mmap_sem)) {
-				vma = mm->mmap;
-				while (vma) {
-					if ((vma->vm_flags & VM_EXECUTABLE) &&
-					    vma->vm_file) {
-						audit_log_d_path(ab, "exe=",
-							vma->vm_file->f_dentry,
-							vma->vm_file->f_vfsmnt);
-						audit_log_task_info(ab);
-						break;
-					}
-					vma = vma->vm_next;
-				}
-				up_read(&mm->mmap_sem);
-			} else {
-				audit_log_format(ab, " comm=%s", tsk->comm);
-			}
-			if (tsk != current)
-				mmput(mm);
-		} else {
-			audit_log_format(ab, " comm=%s", tsk->comm);
-		}
-	}
 	if (a) {
 		switch (a->type) {
 		case AVC_AUDIT_DATA_IPC:

Re: Getting the real task name in avc messages

[James Morris]

On Wed, 30 Mar 2005, Steve G wrote:

> Hello,
> 
> Attached is a patch to the kernel that puts the program's name into the avc
> message. This lets you know the script that caused the problem instead of
> /bin/bash. Feedback would be appreciated.

This will be looked upon dimly upstream:

+        char name[sizeof(current->comm)];

the kernel stack space is very limited.


- 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: Getting the real task name in avc messages

[James Morris]

On Wed, 30 Mar 2005, James Morris wrote:

> This will be looked upon dimly upstream:
> 
> +        char name[sizeof(current->comm)];
> 
> the kernel stack space is very limited.

Actually, current->comm is only defined by default to be 16 bytes, so it's 
ok.


- 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: Getting the real task name in avc messages

[Darrel Goeddel]

James Morris wrote:
> On Wed, 30 Mar 2005, James Morris wrote:
> 
> 
>>This will be looked upon dimly upstream:
>>
>>+        char name[sizeof(current->comm)];
>>
>>the kernel stack space is very limited.
> 
> 
> Actually, current->comm is only defined by default to be 16 bytes, so it's 
> ok.
> 
> 
> - James

You could also just do:

	task_lock(current);
	audit_log_format(ab, " comm=%s", current->comm);
	task_unlock(current);

This avoids an unnecessary copy since we have no use the data after the
audit_log_format call.

I really like the idea of moving this functionality to the standard
syscall audit record.  I'm sure that this would be nice info to have
for folks who do not run SELinux.

-- 

Darrel

--
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: Getting the real task name in avc messages

[Stephen Smalley]

On Wed, 2005-03-30 at 16:27 -0600, Darrel Goeddel wrote:
> James Morris wrote:
> You could also just do:
> 
> 	task_lock(current);
> 	audit_log_format(ab, " comm=%s", current->comm);
> 	task_unlock(current);
> 
> This avoids an unnecessary copy since we have no use the data after the
> audit_log_format call.

Is this locking truly necessary when accessing current->comm (as opposed
to accessing the comm of another task)?  Can it be set by any other
task?  We don't presently hold the lock when accessing it in avc_audit.

> I really like the idea of moving this functionality to the standard
> syscall audit record.  I'm sure that this would be nice info to have
> for folks who do not run SELinux.

Yes, I think it would be useful; Steve, want to take the updated patch
to linux-audit?

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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: Getting the real task name in avc messages

[Stephen Smalley]

On Thu, 2005-03-31 at 07:30 -0500, Stephen Smalley wrote:
> Yes, I think it would be useful; Steve, want to take the updated patch
> to linux-audit?

Note btw that with my changes to your original patch, you no longer need
to add a function prototype for audit_log_task_info to audit.h and you
can make it a static function, since it is only used internally within
auditsc.c at that point.  Full updated patch below against 2.6.11 (not
relative to your original one).  Retains the on-stack buffer since it
isn't large and the use of get_task_comm since it is consistent with
other code, although I'm not convinced it is necessary for accessing the
current->comm.

Index: linux-2.6/kernel/auditsc.c
===================================================================
RCS file: /nfshome/pal/CVS/linux-2.6/kernel/auditsc.c,v
retrieving revision 1.3
diff -u -p -r1.3 auditsc.c
--- linux-2.6/kernel/auditsc.c	2 Mar 2005 14:40:50 -0000	1.3
+++ linux-2.6/kernel/auditsc.c	31 Mar 2005 13:36:30 -0000
@@ -577,6 +577,33 @@ static inline void audit_free_context(st
 		printk(KERN_ERR "audit: freed %d contexts\n", count);
 }
 
+static void audit_log_task_info(struct audit_buffer *ab)
+{
+	char name[sizeof(current->comm)];
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+
+	get_task_comm(name, current);
+	audit_log_format(ab, " comm=%s", name);
+
+	if (!mm)
+		return;
+
+	down_read(&mm->mmap_sem);
+	vma = mm->mmap;
+	while (vma) {
+		if ((vma->vm_flags & VM_EXECUTABLE) &&
+		    vma->vm_file) {
+			audit_log_d_path(ab, "exe=",
+					 vma->vm_file->f_dentry,
+					 vma->vm_file->f_vfsmnt);
+			break;
+		}
+		vma = vma->vm_next;
+	}
+	up_read(&mm->mmap_sem);
+}
+
 static void audit_log_exit(struct audit_context *context)
 {
 	int i;
@@ -606,6 +633,7 @@ static void audit_log_exit(struct audit_
 		  context->gid,
 		  context->euid, context->suid, context->fsuid,
 		  context->egid, context->sgid, context->fsgid);
+	audit_log_task_info(ab);
 	audit_log_end(ab);
 	for (i = 0; i < context->name_count; i++) {
 		ab = audit_log_start(context);
Index: linux-2.6/security/selinux/avc.c
===================================================================
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/avc.c,v
retrieving revision 1.53
diff -u -p -r1.53 avc.c
--- linux-2.6/security/selinux/avc.c	14 Mar 2005 19:52:45 -0000	1.53
+++ linux-2.6/security/selinux/avc.c	30 Mar 2005 21:11:32 -0000
@@ -532,7 +532,6 @@ void avc_audit(u32 ssid, u32 tsid,
                u16 tclass, u32 requested,
                struct av_decision *avd, int result, struct avc_audit_data *a)
 {
-	struct task_struct *tsk = current;
 	struct inode *inode = NULL;
 	u32 denied, audited;
 	struct audit_buffer *ab;
@@ -556,39 +555,6 @@ void avc_audit(u32 ssid, u32 tsid,
 	audit_log_format(ab, "avc:  %s ", denied ? "denied" : "granted");
 	avc_dump_av(ab, tclass,audited);
 	audit_log_format(ab, " for ");
-	if (a && a->tsk)
-		tsk = a->tsk;
-	if (tsk && tsk->pid) {
-		struct mm_struct *mm;
-		struct vm_area_struct *vma;
-		audit_log_format(ab, " pid=%d", tsk->pid);
-		if (tsk == current)
-			mm = current->mm;
-		else
-			mm = get_task_mm(tsk);
-		if (mm) {
-			if (down_read_trylock(&mm->mmap_sem)) {
-				vma = mm->mmap;
-				while (vma) {
-					if ((vma->vm_flags & VM_EXECUTABLE) &&
-					    vma->vm_file) {
-						audit_log_d_path(ab, "exe=",
-							vma->vm_file->f_dentry,
-							vma->vm_file->f_vfsmnt);
-						break;
-					}
-					vma = vma->vm_next;
-				}
-				up_read(&mm->mmap_sem);
-			} else {
-				audit_log_format(ab, " comm=%s", tsk->comm);
-			}
-			if (tsk != current)
-				mmput(mm);
-		} else {
-			audit_log_format(ab, " comm=%s", tsk->comm);
-		}
-	}
 	if (a) {
 		switch (a->type) {
 		case AVC_AUDIT_DATA_IPC:

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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: Getting the real task name in avc messages

[Darrel Goeddel]

Stephen Smalley wrote:
> On Wed, 2005-03-30 at 16:27 -0600, Darrel Goeddel wrote:
>>You could also just do:
>>
>>	task_lock(current);
>>	audit_log_format(ab, " comm=%s", current->comm);
>>	task_unlock(current);
>>
>>This avoids an unnecessary copy since we have no use the data after the
>>audit_log_format call.
> 
> 
> Is this locking truly necessary when accessing current->comm (as opposed
> to accessing the comm of another task)?  Can it be set by any other
> task?  We don't presently hold the lock when accessing it in avc_audit.
> 
> 

I didn't not think so, but I wasn't 100% sure.  I only did a quick glance before 
and thought it best to be safe.  Upon further investigation, it sure looks to be 
fine without holding the task lock.

-- 

Darrel

--
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: Getting the real task name in avc messages

[Stephen Smalley]

On Thu, 2005-03-31 at 09:21 -0600, Darrel Goeddel wrote:
> I didn't not think so, but I wasn't 100% sure.  I only did a quick glance before 
> and thought it best to be safe.  Upon further investigation, it sure looks to be 
> fine without holding the task lock.

Yes, AFAICS you only need to use the helper/lock when getting the comm
of another task, not your own, as only you can change your own comm.
But for some reason the core code has been rewritten to use this helper
always (e.g. see sys_prctl), possibly they expect this to change in the
future, i.e. allow another task to set your comm?

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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: Getting the real task name in avc messages

[Steve G]

>Note that anytime avc_audit generates an audit message, audit_log_exit
>will be called upon syscall exit, so by adding a call to your new function 
>to audit_log_exit, you'll ensure that this information is recorded for 
>every avc denial as well as every other audit message.

I'm not sure we need this information for regular auditing. I haven't seen
regular audit messages that showed the interpreter instead of the program. The
problem does exist for avc denials.

>Note that the comm field is less complete (not a full path and may even
>be truncated) and is not trustworthy (can be changed by the process to
>any arbitrary string).  So you can't rely on it, but it can be useful
>for debugging.

I know about that. I know where to find the full path (example code is over in
the proc file system), but I don't have time to improve this patch right now. I
just want to get this out in the open and show that the status quo can be
improved a little.

Thanks,
-Steve Grubb

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.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: Getting the real task name in avc messages

[Stephen Smalley]

On Wed, 2005-03-30 at 13:02 -0800, Steve G wrote:
> I'm not sure we need this information for regular auditing. I haven't seen
> regular audit messages that showed the interpreter instead of the program. The
> problem does exist for avc denials.

Presently the syscall auditing (i.e. audit_log_exit) doesn't show the
program name at all, just the pid.  That's why I suggested moving this
to audit_log_exit, so that you can get more useful information.  pid is
rarely helpful except for long lived processes, whereas the exe and comm
can be helpful.

> I know about that. I know where to find the full path (example code is over in
> the proc file system), but I don't have time to improve this patch right now. I
> just want to get this out in the open and show that the status quo can be
> improved a little.

avc_audit() already does that (the exe= info).  So you just need to move
it over.  See my patch that I just sent.

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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: Getting the real task name in avc messages

[Steve G]

>Steve, want to take the updated patch to linux-audit?

Ok. Let me compile it and see what the logs look like first.

Thanks,
-Steve Grubb

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.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: Getting the real task name in avc messages

[Stephen Smalley]

On Thu, 2005-03-31 at 06:53 -0800, Steve G wrote:
> >Steve, want to take the updated patch to linux-audit?
> 
> Ok. Let me compile it and see what the logs look like first.

Ok.  I built and ran a kernel with it here, and ran some selinux tests,
and it looked good.  It has some side benefits for SELinux even beyond
the comm information, e.g. capturing the exe= upon syscall exit lets us
get it cleanly without having to worry about mmap sem locking by the
caller (which was an issue for mmap/mprotect previously) and moving the
task-related audit handling to syscall exit should avoid having bogus
information included for our networking checks that occur outside of
process context.  Tools like seaudit may need to be updated to get the
task-related info from the subsequent syscall audit record instead of
from the avc-generated record, but they can correlate it based on the
timestamp/serial.

One further change that should be made is to use something like
audit_log_untrustedstring on the comm and on the exe path.

-- 
Stephen Smalley <sds@tycho.nsa.gov>
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.