[PATCH] audit: log on the future execution of a path

[PATCH] audit: log on the future execution of a path

[Richard Guy Briggs]

Here is another attempt at getting closer to auditing on the future execution
of a path.

Storing the dev/inode of the path in the rule won't help us because the path
may not exist yet.

Please see the accompanying userspace patch.  I don't expect the userspace
interface to change appreciably unless I've overlooked something important.  I
am able to set and get rules as expected.

It will be slow because it has to do a string compare on every sys_execve()
invocation.  The compare function uses the process' struct filename *.  I'm
guessing a hash of the string could speed that up.

Only problem is, it doesn't work.  What assumptions am I making that aren't
valid about the approach in this kernel code?

I also considered adding the path string pointer to the struct audit_field.

Any suggestions?

See: (I'd use the redhat.com/archives/linux-audit links, but they don't link across months.)
"auditing syscalls made 'by' an inode?"
        http://comments.gmane.org/gmane.linux.redhat.security.audit/4255
"audit: audit on the future execution of a binary."
        http://comments.gmane.org/gmane.linux.redhat.security.audit/4388
"Support for auditing on the actions of a not-yet-executed process."
        http://comments.gmane.org/gmane.linux.redhat.security.audit/4389
"Excluding events by command"
        http://comments.gmane.org/gmane.linux.redhat.security.audit/4428


Richard Guy Briggs (1):
  audit: audit on the future execution of a binary.

 include/linux/audit.h      |    1 +
 include/uapi/linux/audit.h |    2 ++
 kernel/auditfilter.c       |   35 +++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           |   35 +++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 0 deletions(-)

[PATCH] audit: audit on the future execution of a binary.

[Richard Guy Briggs]

Enable creation of rules to monitor for the execution of a future path.

This adds the ability to audit the actions of a not-yet-running process,
possibly not-yet-existing path, as well as the children of a process/path.

A path is supplied and stored with the rule, which is subsequently compared
with the path stored by sys_execve() when called.

Based-on-code-by: Peter Moody <pmoody@google.com>
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h      |    1 +
 include/uapi/linux/audit.h |    2 ++
 kernel/auditfilter.c       |   35 +++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           |   35 +++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 7c42075..293759e 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -62,6 +62,7 @@ struct audit_krule {
 	struct list_head	rlist;	/* entry in audit_{watch,tree}.rules list */
 	struct list_head	list;	/* for AUDIT_LIST* purposes only */
 	u64			prio;
+	char			*path; /* associated path */
 };
 
 struct audit_field {
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 573dc36..f4a72b9 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -266,6 +266,8 @@
 #define AUDIT_OBJ_UID	109
 #define AUDIT_OBJ_GID	110
 #define AUDIT_FIELD_COMPARE	111
+#define AUDIT_EXE	112
+#define AUDIT_EXE_CHILDREN	113
 
 #define AUDIT_ARG0      200
 #define AUDIT_ARG1      (AUDIT_ARG0+1)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 6daea0a..3309943 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -87,6 +87,7 @@ static inline void audit_free_rule(struct audit_entry *e)
 		}
 	kfree(erule->fields);
 	kfree(erule->filterkey);
+	kfree(erule->path);
 	kfree(e);
 }
 
@@ -390,6 +391,11 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
 		if (f->val > AUDIT_MAX_FIELD_COMPARE)
 			return -EINVAL;
 		break;
+	case AUDIT_EXE:
+	case AUDIT_EXE_CHILDREN:
+		if (f->op != Audit_equal)
+			return -EINVAL;
+		break;
 	};
 	return 0;
 }
@@ -539,6 +545,16 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
 			entry->rule.buflen += f->val;
 			entry->rule.filterkey = str;
 			break;
+		case AUDIT_EXE:
+		case AUDIT_EXE_CHILDREN:
+			if (entry->rule.path || f->val > PATH_MAX)
+				goto exit_free;
+			str = audit_unpack_string(&bufp, &remain, f->val);
+			if (IS_ERR(str))
+				goto exit_free;
+			entry->rule.buflen += f->val;
+			entry->rule.path = str;
+			break;
 		}
 	}
 
@@ -617,6 +633,11 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
 			data->buflen += data->values[i] =
 				audit_pack_string(&bufp, krule->filterkey);
 			break;
+		case AUDIT_EXE:
+		case AUDIT_EXE_CHILDREN:
+			data->buflen += data->values[i] =
+				audit_pack_string(&bufp, krule->path);
+			break;
 		default:
 			data->values[i] = f->val;
 		}
@@ -672,6 +693,12 @@ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
 			if (strcmp(a->filterkey, b->filterkey))
 				return 1;
 			break;
+		case AUDIT_EXE:
+		case AUDIT_EXE_CHILDREN:
+			/* both paths exist based on above type compare */
+			if (strcmp(a->path, b->path))
+				return 1;
+			break;
 		case AUDIT_UID:
 		case AUDIT_EUID:
 		case AUDIT_SUID:
@@ -742,6 +769,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old)
 	struct audit_entry *entry;
 	struct audit_krule *new;
 	char *fk;
+	char *path;
 	int i, err = 0;
 
 	entry = audit_init_entry(fcount);
@@ -793,6 +821,13 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old)
 				err = -ENOMEM;
 			else
 				new->filterkey = fk;
+		case AUDIT_EXE:
+		case AUDIT_EXE_CHILDREN:
+			path = kstrdup(old->path, GFP_KERNEL);
+			if (unlikely(!path))
+				err = -ENOMEM;
+			else
+				new->path = path;
 		}
 		if (err) {
 			audit_free_rule(entry);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f251a5e..6c6963d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -48,6 +48,7 @@
 #include <asm/types.h>
 #include <linux/atomic.h>
 #include <linux/fs.h>
+#include <linux/dcache.h>
 #include <linux/namei.h>
 #include <linux/mm.h>
 #include <linux/export.h>
@@ -70,6 +71,7 @@
 #include <linux/capability.h>
 #include <linux/fs_struct.h>
 #include <linux/compat.h>
+#include <linux/sched.h>
 #include <linux/ctype.h>
 
 #include "audit.h"
@@ -432,6 +434,23 @@ static int audit_field_compare(struct task_struct *tsk,
 	return 0;
 }
 
+int audit_match_exe(struct task_struct *tsk, struct audit_context *ctx, struct audit_names *name, unsigned char *path)
+{
+	struct audit_names *n;
+
+	if ((!tsk || !tsk->audit_context) && !ctx && !name)
+		return 0;
+
+	if (name)
+		if (!strcmp(name->name->name, path))
+			return 1;
+		
+	list_for_each_entry(n, &(ctx ?: tsk->audit_context)->names_list, list)
+		if (n->name && !strcmp(n->name->name, path))
+			return 1;
+	return 0;
+}
+
 /* Determine if any context name data matches a rule's watch data */
 /* Compare a task_struct with an audit_rule.  Return 1 on match, 0
  * otherwise.
@@ -471,6 +490,22 @@ static int audit_filter_rules(struct task_struct *tsk,
 				result = audit_comparator(ctx->ppid, f->op, f->val);
 			}
 			break;
+		case AUDIT_EXE:
+			result = audit_match_exe(tsk, ctx, name, rule->path);
+			break;
+		case AUDIT_EXE_CHILDREN:
+		{
+			struct task_struct *ptsk;
+			for (ptsk = tsk;
+			     ptsk->parent->pid > 0;
+			     ptsk = find_task_by_vpid(ptsk->parent->pid)) {
+				if (audit_match_exe(ptsk, ctx, name, rule->path)) {
+					++result;
+					break;
+				}
+			}
+		}
+			break;
 		case AUDIT_UID:
 			result = audit_uid_comparator(cred->uid, f->op, f->uid);
 			break;
-- 
1.7.1

Re: [PATCH] audit: log on the future execution of a path

[Steve Grubb]

On Mon,  5 May 2014 16:41:53 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:

> Only problem is, it doesn't work.  What assumptions am I making that
> aren't valid about the approach in this kernel code?
> 
> I also considered adding the path string pointer to the struct
> audit_field.
> 
> Any suggestions?

What I was thinking about is that it should work a lot like a watch for
execution except when the watch triggers, it actually fills in a pid
field for a syscall rule and loads it instead of emitting an event.

For example, suppose you had this rule:
-a exit,always -F arch=b64 -S socket -F 'a0!=1' -F exe=/bin/bash -F
success=1

It could be started as this:
-a exit,always -F path=/bin/bash -F perm=x

Then when it triggers, it loads this:
-a exit,always -F arch=b64 -S socket -F 'a0!=1' -F success=1 -F pid=##

Where ## is the pid known to the kernel. Then when the program exits for
any reason, the rules it created for that pid are all removed. 

It would also need to handle execve/clone/fork/vfork sanely once a
rule was created.

auditctl -l should only show the rule that was loaded from user space
and not any helpers that might be created dynamically. Deleting the
rule should get rid of any helpers.

-Steve

Re: [PATCH] audit: log on the future execution of a path

[Eric Paris]

On Mon, 2014-05-05 at 17:10 -0400, Steve Grubb wrote:
> On Mon,  5 May 2014 16:41:53 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > Only problem is, it doesn't work.  What assumptions am I making that
> > aren't valid about the approach in this kernel code?
> > 
> > I also considered adding the path string pointer to the struct
> > audit_field.
> > 
> > Any suggestions?
> 
> What I was thinking about is that it should work a lot like a watch for

We agree up to this point.

> execution except when the watch triggers, it actually fills in a pid
> field for a syscall rule and loads it instead of emitting an event.

And now we disagree.

> For example, suppose you had this rule:
> -a exit,always -F arch=b64 -S socket -F 'a0!=1' -F exe=/bin/bash -F
> success=1
> 
> It could be started as this:
> -a exit,always -F path=/bin/bash -F perm=x
> 
> Then when it triggers, it loads this:
> -a exit,always -F arch=b64 -S socket -F 'a0!=1' -F success=1 -F pid=##
> 
> Where ## is the pid known to the kernel. Then when the program exits for
> any reason, the rules it created for that pid are all removed. 
> 
> It would also need to handle execve/clone/fork/vfork sanely once a
> rule was created.

Please no.  Do not dynamically create/delete rules.  That's not how the
kernel filter architecture is built and we don't have a need to change
that now.

Instead we want to do what watches do.  Insert an fsnotify watch on the
parent directory.  If the executable exists, we add the device+inode to
a list of 'watched inodes'.  At filter time we'd match similar to
audit_watch_compare using current->mm->exe_file->f_inode

If the executable does not exist when the rule is added, add the
fsnotify watch and do nothing else.  Every time you get notification of
the watch check to see if it was about the right file being created and
add the new executable to the 'watched inodes' list.

Obviously you need to watch the fsnotify for the execuable being deleted
and remove it from the 'watched inodes'.

Exactly the same as watches, if the parent directory is removed, we log
that the rule is lost.

Maybe i'll get a couple of minutes this afternoon to code it up...

-Eric

Re: [PATCH] audit: log on the future execution of a path

[Steve Grubb]

On Tue, 06 May 2014 10:57:30 -0400
Eric Paris <eparis@redhat.com> wrote:

> On Mon, 2014-05-05 at 17:10 -0400, Steve Grubb wrote:
> > On Mon,  5 May 2014 16:41:53 -0400
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> > 
> > > Only problem is, it doesn't work.  What assumptions am I making
> > > that aren't valid about the approach in this kernel code?
> > > 
> > > I also considered adding the path string pointer to the struct
> > > audit_field.
> > > 
> > > Any suggestions?
> > 
> > What I was thinking about is that it should work a lot like a watch
> > for
> 
> We agree up to this point.
> 
> > execution except when the watch triggers, it actually fills in a pid
> > field for a syscall rule and loads it instead of emitting an event.
> 
> And now we disagree.

That's fine. It was only a suggestion. As long as the effect is the
same, I don't care how its implemented. :-)

-Steve