audit by executable name
v9 cover audit by executable name
Please see the accompanying userspace patchset:
https://www.redhat.com/archives/linux-audit/2015-July/thread.html
[[PATCH V2] 0/2] Log on the future execution of a path
The userspace interface is not expected to change appreciably unless something important has been overlooked. Setting and deleting rules works as expected.
If the path does not exist at rule creation time, it will be re-evaluated every time there is a change to the parent directory at which point the change in device and inode will be noted.
Here's a sample run:
Test for addition, trigger and deletion of tree executable rule:
# auditctl -a always,exit -S all -F dir=/tmp -F exe=/usr/bin/touch -F key=exetest_tree ---- time->Sat Jul 11 10:41:50 2015 type=CONFIG_CHANGE msg=audit(1436629310.720:44711): auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op="add_rule" key="exetest_tree" list=4 res=1 ----
# /usr/bin/touch /tmp/test ---- time->Sat Jul 11 10:41:50 2015 type=PROCTITLE msg=audit(1436629310.757:44712): proctitle=2F7573722F62696E2F746F756368002F746D702F74657374 type=PATH msg=audit(1436629310.757:44712): item=1 name="/tmp/test" inode=166932 dev=00:24 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE type=PATH msg=audit(1436629310.757:44712): item=0 name="/tmp/" inode=11525 dev=00:24 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT type=CWD msg=audit(1436629310.757:44712): cwd="/root" type=SYSCALL msg=audit(1436629310.757:44712): arch=c000003e syscall=2 success=yes exit=3 a0=7ffdee2f9e27 a1=941 a2=1b6 a3=691 items=2 ppid=17655 pid=17762 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="touch" exe="/usr/bin/touch" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="exetest_tree" ----
# auditctl -d always,exit -S all -F dir=/tmp -F exe=/usr/bin/touch -F key=exetest_tree ---- time->Sat Jul 11 10:41:50 2015 type=CONFIG_CHANGE msg=audit(1436629310.839:44713): auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op="remove_rule" key="exetest_tree" list=4 res=1 ----
Revision history:
v9: Fix a rebase manual merge error that mixed parts of two patches.
v8: Re-spin due to mods to:
"audit: save signal match info in case entry passed in is the one deleted"
v7: Add Audit Feature Bitmap macro AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH to
AUDIT_FEATURE_BITMAP_ALL.
Split out new patch to use macros for unset inode and device values. Remove BUG() usage in stubs.
Rename audit_mark_free() to audit_fsnotify_mark_free().
Remove unused audit_get_mark() and audit_put_mark() functions.
Rework ino and dev comparisons and assignments, using macros and existing
funcs and eliminating temp variables.
Move audit_update_mark() above its first usage.
Move contents of kernel/audit_exe.c to kernel/audit_watch.c.
Merge patch 3 with 1, merge patch 4 with 1 and 3 and rewrite the patch descriptions. Split out patch to audit by executable children.
v6: Explicitly declare prototypes as external.
Rename audit_dup_exe() to audit_dupe_exe() consistent with rule, watch, lsm_field. Rebased on v4.1.
Rename audit_remove_mark_rule() called from audit_mark_handle_event() to
audit_autoremove_mark_rule() to avoid confusion with audit_remove_{watch,tree}_rule() usage.
Add audit_remove_mark_rule() to provide similar interface as
audit_remove_{watch,tree}_rule().
Simplify stubs to defines.
Rename audit_free_fsnotify_mark() to audit_fsnotify_free_mark() in keeping with
the naming convention of inotify_free_mark(), dnotify_free_mark(), fanotify_free_mark(), audit_watch_free_mark().
Return -ENOMEM rather than null in case of memory allocation failure for
audit_mark in audit_alloc_mark().
Rename audit_free_mark() to audit_mark_free() to avoid association with
{i,d,fa}notify_free_mark() and audit_watch_free_mark().
Clean up exe with similar interface as watch and tree.
Clean up audit exe mark just before audit_free_rule() rather than in it to
avoid mutex in software interrupt context.
Fixed bug in audit_dupe_exe() that returned error rather than valid pointer.
v5: Revert patch "Let audit_free_rule() take care of calling
audit_remove_mark()." since it caused a group mark deadlock.
https://www.redhat.com/archives/linux-audit/2014-October/msg00024.html
v4: Re-order and squash down fixups
Fix audit_dup_exe() to copy pathname string before calling audit_alloc_mark().
https://www.redhat.com/archives/linux-audit/2014-August/msg00065.html
v3: Rationalize and rename some function names and clean up get/put and free code.
Rename several "watch" references to "mark".
Rename audit_remove_rule() to audit_remove_mark_rule().
Let audit_free_rule() take care of calling audit_remove_mark(). Put audit_alloc_mark() arguments in same order as watch, tree and inode. Move the access to the entry for audit_match_signal() to the beginning
of the function in case the entry found is the same one passed in. This will enable it to be used by audit_remove_mark_rule().
https://www.redhat.com/archives/linux-audit/2014-July/msg00000.html
v2: Misguided attempt to add in audit_exe similar to watches
https://www.redhat.com/archives/linux-audit/2014-June/msg00066.html
v1.5: eparis' switch to fsnotify
https://www.redhat.com/archives/linux-audit/2014-May/msg00046.html https://www.redhat.com/archives/linux-audit/2014-May/msg00066.html
v1: Change to path interface instead of inode
https://www.redhat.com/archives/linux-audit/2014-May/msg00017.html
v0: Peter Moodie's original patches
https://www.redhat.com/archives/linux-audit/2012-August/msg00033.html
Future step:
Get full-path notify working.
Richard Guy Briggs (3):
audit: clean simple fsnotify implementation
audit: implement audit by executable
audit: add audit by children of executable path
include/linux/audit.h | 1 + include/uapi/linux/audit.h | 6 +- kernel/Makefile | 2 +- kernel/audit.h | 18 ++++ kernel/audit_fsnotify.c | 215 ++++++++++++++++++++++++++++++++++++++++++++ kernel/audit_tree.c | 2 + kernel/audit_watch.c | 31 +++++++ kernel/auditfilter.c | 60 ++++++++++++- kernel/auditsc.c | 14 +++ 9 files changed, 345 insertions(+), 4 deletions(-) create mode 100644 kernel/audit_fsnotify.c
PATCH v9 1/3 audit: clean simple fsnotify implementation
This is to be used to audit by executable path rules, but audit watches should be able to share this code eventually.
At the moment the audit watch code is a lot more complex. That code only creates one fsnotify watch per parent directory. That 'audit_parent' in turn has a list of 'audit_watches' which contain the name, ino, dev of the specific object we care about. This just creates one fsnotify watch per object we care about. So if you watch 100 inodes in /etc this code will create 100 fsnotify watches on /etc. The audit_watch code will instead create 1 fsnotify watch on /etc (the audit_parent) and then 100 individual watches chained from that fsnotify mark.
We should be able to convert the audit_watch code to do one fsnotify mark per watch and simplify things/remove a whole lot of code. After that conversion we should be able to convert the audit_fsnotify code to support that hierarchy if the optimization is necessary.
Move the access to the entry for audit_match_signal() to the beginning of the audit_del_rule() function in case the entry found is the same one passed in. This will enable it to be used by audit_autoremove_mark_rule(), kill_rules() and audit_remove_parent_watches().
This is a heavily modified and merged version of two patches originally submitted by Eric Paris.
--- kernel/Makefile | 2 +- kernel/audit.h | 14 +++ kernel/audit_fsnotify.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++ kernel/auditfilter.c | 2 +- 4 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 kernel/audit_fsnotify.c
kernel/Makefile +1 -1
@@ -64,7 +64,7 @@ obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o obj-$(CONFIG_AUDITSYSCALL) += auditsc.o -obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_fsnotify.o obj-$(CONFIG_AUDIT_TREE) += audit_tree.o obj-$(CONFIG_GCOV_KERNEL) += gcov/ obj-$(CONFIG_KPROBES) += kprobes.o
kernel/audit.h +14 -0
@@ -50,6 +50,7 @@ enum audit_state {/* Rule lists */ struct audit_watch ; +struct audit_fsnotify_mark ;struct audit_tree ;struct audit_chunk ;
@@ -252,6 +253,7 @@ struct audit_net {extern int selinux_audit_rule_update (void );extern struct mutex audit_filter_mutex; +extern int audit_del_rule (struct audit_entry *);extern void audit_free_rule_rcu (struct rcu_head *);extern struct list_head audit_filter_list[];
@@ -269,6 +271,13 @@ extern int audit_add_watch(struct audit_krule *krule, struct list_head **list);extern void audit_remove_watch_rule (struct audit_krule *krule);extern char *audit_watch_path (struct audit_watch *watch);extern int audit_watch_compare (struct audit_watch *watch,unsigned long ino,dev_t dev); + +extern struct audit_fsnotify_mark *audit_alloc_mark (struct audit_krule *krule,char *pathname,int len); +extern char *audit_mark_path (struct audit_fsnotify_mark *mark); +extern void audit_remove_mark (struct audit_fsnotify_mark *audit_mark); +extern void audit_remove_mark_rule (struct audit_krule *krule); +extern int audit_mark_compare (struct audit_fsnotify_mark *mark,unsigned long ino,dev_t dev); +#else #define audit_put_watch (w) {}#define audit_get_watch (w) {}
@@ -278,6 +287,11 @@ extern int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev#define audit_watch_path (w) ""#define audit_watch_compare (w, i, d) 0 +#define audit_alloc_mark (k, p, l) (ERR_PTR(-EINVAL)) +#define audit_mark_path (m) "" +#define audit_remove_mark (m) +#define audit_remove_mark_rule (k) +#define audit_mark_compare (m, i, d) 0#endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE
kernel/audit_fsnotify.c new file +215 -0
215 lines not on this page · raw message
kernel/auditfilter.c +1 -1
@@ -935,7 +935,7 @@ static inline int audit_add_rule(struct audit_entry *entry) }/* Remove an existing rule from filterlist. */ -static inline int audit_del_rule (struct audit_entry *entry) +int audit_del_rule (struct audit_entry *entry) {struct audit_entry *e;struct audit_tree *tree = entry->rule.tree;
Signature · 1 line
1.7.1
Re: [PATCH V9 1/3] audit: clean simple fsnotify implementation
Quoted 28 lines · Richard Guy Briggs
This is to be used to audit by executable path rules, but audit watches should be able to share this code eventually.
At the moment the audit watch code is a lot more complex. That code only creates one fsnotify watch per parent directory. That '
audit_parent' in turn has a list of 'audit_watches' which contain the name, ino, dev of the specific object we care about. This just creates one fsnotify watch per object we care about. So if you watch 100 inodes in /etc this code will create 100 fsnotify watches on /etc. Theaudit_watchcode will instead create 1 fsnotify watch on /etc (theaudit_parent) and then 100 individual watches chained from that fsnotify mark.We should be able to convert the
audit_watchcode to do one fsnotify mark per watch and simplify things/remove a whole lot of code. After that conversion we should be able to convert theaudit_fsnotifycode to support that hierarchy if the optimization is necessary.Move the access to the entry for
audit_match_signal()to the beginning of theaudit_del_rule()function in case the entry found is the same one passed in. This will enable it to be used byaudit_autoremove_mark_rule(),kill_rules()andaudit_remove_parent_watches().This is a heavily modified and merged version of two patches originally submitted by Eric Paris.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>
Quoted 7 lines
--- kernel/Makefile | 2 +- kernel/audit.h | 14 +++ kernel/audit_fsnotify.c | 215 +++++++++++++++++++++++++++++++++++++++++++ kernel/auditfilter.c | 2 +- 4 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 kernel/audit_fsnotify.c
Merged, although I had to add a line of whitespace to keep checkpatch happy.
Quoted 295 lines
kernel/Makefile +1
-1@@ -64,7 +64,7 @@ obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o obj-$(CONFIG_AUDITSYSCALL) += auditsc.o -obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_fsnotify.o obj-$(CONFIG_AUDIT_TREE) += audit_tree.o obj-$(CONFIG_GCOV_KERNEL) += gcov/ obj-$(CONFIG_KPROBES) += kprobes.okernel/audit.h +2
-0@@ -50,6 +50,7 @@ enum audit_state {/* Rule lists */ struct audit_watch ; +struct audit_fsnotify_mark ;struct audit_tree ;struct audit_chunk ;@@ -252,6 +253,7 @@ struct audit_net {extern int selinux_audit_rule_update (void );extern struct mutex audit_filter_mutex; +extern int audit_del_rule (struct audit_entry *);extern void audit_free_rule_rcu (struct rcu_head *);extern struct list_head audit_filter_list[];@@ -269,6 +271,13 @@ extern int audit_add_watch(struct audit_krule *krule,struct
list_head**list); extern voidaudit_remove_watch_rule(struct audit_krule *krule);
extern char *audit_watch_path(structaudit_watch*watch); extern int audit_watch_compare(structaudit_watch*watch, unsigned long
ino,dev_tdev); +
+extern structaudit_fsnotify_mark*audit_alloc_mark(structaudit_krule*krule, char *pathname, int len); +extern char *audit_mark_path(structaudit_fsnotify_mark*mark);kernel/audit.h +3
-0+extern void audit_remove_mark (struct audit_fsnotify_mark *audit_mark); +extern void audit_remove_mark_rule (struct audit_krule *krule); +extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsignedlong ino,
dev_tdev); +
#else
#defineaudit_put_watch(w){}
#defineaudit_get_watch(w){}kernel/audit.h +5
-0@@ -278,6 +287,11 @@ extern int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev #define audit_watch_path(w) ""#define audit_watch_compare (w, i, d) 0 +#define audit_alloc_mark (k, p, l) (ERR_PTR(-EINVAL)) +#define audit_mark_path (m) "" +#define audit_remove_mark (m) +#define audit_remove_mark_rule (k) +#define audit_mark_compare (m, i, d) 0#endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE kernel/audit_fsnotify.c new file +47
-0@@ -0,0 +1,215 @@ +/* audit_fsnotify.c -- tracking inodes +* +* Copyright 2003-2009,2014-2015 Red Hat, Inc. +* Copyright 2005 Hewlett-Packard Development Company, L.P. +* Copyright 2005 IBM Corporation +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#include <linux/kernel.h> +#include <linux/audit.h> +#include <linux/kthread.h> +#include <linux/mutex.h> +#include <linux/fs.h> +#include <linux/fsnotify_backend.h> +#include <linux/namei.h> +#include <linux/netlink.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/security.h> +#include "audit.h" + +/* +* this mark lives on the parent directory of the inode in question. +* but dev, ino, and path are about the child +*/ +struct audit_fsnotify_mark { +dev_t dev;/* associated superblock device */ +unsigned long ino;/* associated inode number */ +char *path;/* insertion path */ +struct fsnotify_mark mark;/* fsnotify mark on the inode */ +struct audit_krule *rule; +}; + +/* fsnotify handle. */ +static struct fsnotify_group *audit_fsnotify_group; + +/* fsnotify events we care about. */ +#define AUDIT_FS_EVENTS (FS_MOVE | FS_CREATE | FS_DELETE | FS_DELETE_SELF|\ +
FS_MOVE_SELF|FS_EVENT_ON_CHILD)kernel/audit_fsnotify.c new file +2
-0+ +static void audit_fsnotify_mark_free(struct audit_fsnotify_mark
*audit_mark) +{kernel/audit_fsnotify.c new file +18
-0+kfree (audit_mark->path); +kfree (audit_mark); +} + +static void audit_fsnotify_free_mark (struct fsnotify_mark *mark) +{ +struct audit_fsnotify_mark *audit_mark; + + audit_mark = container_of(mark, struct audit_fsnotify_mark, mark); +audit_fsnotify_mark_free (audit_mark); +} + +char *audit_mark_path (struct audit_fsnotify_mark *mark) +{ +return mark->path; +} + +int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long ino,
dev_tdev) +{kernel/audit_fsnotify.c new file +13
-0+if (mark->ino ==AUDIT_INO_UNSET ) +return 0 ; +return (mark->ino == ino) && (mark->dev == dev); +} + +static void audit_update_mark (struct audit_fsnotify_mark *audit_mark, +struct inode *inode) +{ + audit_mark->dev = inode ? inode->i_sb->s_dev :AUDIT_DEV_UNSET ; + audit_mark->ino = inode ? inode->i_ino :AUDIT_INO_UNSET ; +} + +struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule *krule,char *pathname, int len) +{
kernel/audit_fsnotify.c new file +28
-0+struct audit_fsnotify_mark *audit_mark; +struct path path; +struct dentry *dentry; +struct inode *inode; +int ret; + +if (pathname[0 ] !='/' || pathname[len-1 ] =='/' ) +return ERR_PTR (-EINVAL ); + + dentry =kern_path_locked (pathname, &path); +if (IS_ERR (dentry)) +return (void *)dentry;/* returning an error */ + inode = path.dentry->d_inode; +mutex_unlock (&inode->i_mutex); + + audit_mark =kzalloc (sizeof (*audit_mark),GFP_KERNEL ); +if (unlikely (!audit_mark)) { + audit_mark =ERR_PTR (-ENOMEM ); + goto out; + } + +fsnotify_init_mark (&audit_mark->mark, audit_fsnotify_free_mark); + audit_mark->mark.mask =AUDIT_FS_EVENTS ; + audit_mark->path = pathname; +audit_update_mark (audit_mark, dentry->d_inode); + audit_mark->rule = krule; + + ret = fsnotify_add_mark(&audit_mark->mark, audit_fsnotify_group, inode,NULL, true); + if (ret < 0) {
kernel/audit_fsnotify.c new file +10
-0+audit_fsnotify_mark_free (audit_mark); + audit_mark =ERR_PTR (ret); + } +out: +dput (dentry); +path_put (&path); +return audit_mark; +} + +static void audit_mark_log_rule_change(struct audit_fsnotify_mark
*audit_mark, char *op) +{kernel/audit_fsnotify.c new file +32
-0+struct audit_buffer *ab; +struct audit_krule *rule = audit_mark->rule; +if (!audit_enabled) +return ; + ab =audit_log_start (NULL ,GFP_NOFS ,AUDIT_CONFIG_CHANGE ); +if (unlikely (!ab)) +return ; +audit_log_format (ab,"auid=%u ses=%u op=" , +from_kuid (&init_user_ns,audit_get_loginuid (current)), +audit_get_sessionid (current)); +audit_log_string (ab, op); +audit_log_format (ab," path=" ); +audit_log_untrustedstring (ab, audit_mark->path); +audit_log_key (ab, rule->filterkey); +audit_log_format (ab," list=%d res=1" , rule->listnr); +audit_log_end (ab); +} + +void audit_remove_mark (struct audit_fsnotify_mark *audit_mark) +{ +fsnotify_destroy_mark (&audit_mark->mark, audit_fsnotify_group); +fsnotify_put_mark (&audit_mark->mark); +} + +void audit_remove_mark_rule (struct audit_krule *krule) +{ +struct audit_fsnotify_mark *mark = krule->exe; + +audit_remove_mark (mark); +} + +static void audit_autoremove_mark_rule(struct audit_fsnotify_mark
*audit_mark) +{kernel/audit_fsnotify.c new file +58
-0+struct audit_krule *rule = audit_mark->rule; + struct audit_entry *entry = container_of(rule, struct audit_entry, rule); + +audit_mark_log_rule_change (audit_mark,"autoremove_rule" ); +audit_del_rule (entry); +} + +/* Update mark data in audit rules based on fsnotify events. */ +static int audit_mark_handle_event (struct fsnotify_group *group, +struct inode *to_tell, +struct fsnotify_mark *inode_mark, +struct fsnotify_mark *vfsmount_mark, +u32 mask,void *data,int data_type, +const unsigned char *dname,u32 cookie) +{ +struct audit_fsnotify_mark *audit_mark; +struct inode *inode =NULL ; + + audit_mark = container_of(inode_mark, struct audit_fsnotify_mark, mark); + +BUG_ON (group != audit_fsnotify_group); + +switch (data_type) { +case (FSNOTIFY_EVENT_PATH ): + inode = ((struct path *)data)->dentry->d_inode; +break ; +case (FSNOTIFY_EVENT_INODE ): + inode = (struct inode *)data; +break ; +default : +BUG (); +return 0 ; + }; + +if (mask & (FS_CREATE |FS_MOVED_TO |FS_DELETE |FS_MOVED_FROM )) { +if (audit_compare_dname_path (dname, audit_mark->path,AUDIT_NAME_FULL )) +return 0 ; +audit_update_mark (audit_mark, inode); + }else if (mask & (FS_DELETE_SELF |FS_UNMOUNT |FS_MOVE_SELF )) +audit_autoremove_mark_rule (audit_mark); + +return 0 ; +} + +static const struct fsnotify_ops audit_mark_fsnotify_ops = { + .handle_event = audit_mark_handle_event, +}; + +static int __init audit_fsnotify_init (void ) +{ + audit_fsnotify_group =fsnotify_alloc_group (&audit_mark_fsnotify_ops); +if (IS_ERR (audit_fsnotify_group)) { + audit_fsnotify_group =NULL ; +audit_panic ("cannot create audit fsnotify group" ); + } +return 0 ; +} +device_initcall (audit_fsnotify_init);kernel/auditfilter.c +1
-1@@ -935,7 +935,7 @@ static inline int audit_add_rule(struct audit_entry *entry) }/* Remove an existing rule from filterlist. */ -static inline int audit_del_rule (struct audit_entry *entry) +int audit_del_rule (struct audit_entry *entry) {struct audit_entry *e;struct audit_tree *tree = entry->rule.tree;
Signature · 2 lines
paul moore security @ redhat
PATCH v9 2/3 audit: implement audit by executable
In reply to Richard Guy BriggsThis adds the ability audit the actions of a not-yet-running process.
This patch implements the ability to filter on the executable path. Instead of just hard coding the ino and dev of the executable we care about at the moment the rule is inserted into the kernel, use the new audit_fsnotify infrastructure to manage this dynamically. This means that if the filename does not yet exist but the containing directory does, or if the inode in question is unlinked and creat'd (aka updated) the rule will just continue to work. If the containing directory is moved or deleted or the filesystem is unmounted, the rule is deleted automatically. A future enhancement would be to have the rule survive across directory disruptions.
This is a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
--- include/linux/audit.h | 1 + include/uapi/linux/audit.h | 5 +++- kernel/audit.h | 4 +++ kernel/audit_tree.c | 2 + kernel/audit_watch.c | 31 +++++++++++++++++++++++++ kernel/auditfilter.c | 53 +++++++++++++++++++++++++++++++++++++++++++- kernel/auditsc.c | 3 ++ 7 files changed, 97 insertions(+), 2 deletions(-)
include/linux/audit.h +1 -0
@@ -59,6 +59,7 @@ struct audit_krule {struct audit_field *inode_f;/* quick access to an inode field */ struct audit_watch *watch;/* associated watch */ struct audit_tree *tree;/* associated watched tree */ +struct audit_fsnotify_mark *exe;struct list_head rlist;/* entry in audit_{watch,tree}.rules list */ struct list_head list;/* for AUDIT_LIST* purposes only */ u64 prio;
include/uapi/linux/audit.h +4 -1
@@ -266,6 +266,7 @@#define AUDIT_OBJ_UID 109#define AUDIT_OBJ_GID 110#define AUDIT_FIELD_COMPARE 111 +#define AUDIT_EXE 112#define AUDIT_ARG0 200#define AUDIT_ARG1 (AUDIT_ARG0+1)
@@ -324,8 +325,10 @@ enum {#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002 +#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \ - AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME) + AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \ + AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH )/* deprecated: AUDIT_VERSION_* */ #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
kernel/audit.h +4 -0
@@ -277,6 +277,8 @@ extern char *audit_mark_path(struct audit_fsnotify_mark *mark);extern void audit_remove_mark (struct audit_fsnotify_mark *audit_mark);extern void audit_remove_mark_rule (struct audit_krule *krule);extern int audit_mark_compare (struct audit_fsnotify_mark *mark,unsigned long ino,dev_t dev); +extern int audit_dupe_exe (struct audit_krule *new,struct audit_krule *old); +extern int audit_exe_compare (struct task_struct *tsk,struct audit_fsnotify_mark *mark);#else #define audit_put_watch (w) {}
@@ -292,6 +294,8 @@ extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long in#define audit_remove_mark (m)#define audit_remove_mark_rule (k)#define audit_mark_compare (m, i, d) 0 +#define audit_exe_compare (t, m) (-EINVAL) +#define audit_dupe_exe (n, o) (-EINVAL)#endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE
kernel/audit_tree.c +2 -0
@@ -479,6 +479,8 @@ static void kill_rules(struct audit_tree *tree)if (rule->tree) {/* not a half-baked one */ audit_tree_log_remove_rule (rule); +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe); rule->tree =NULL ;list_del_rcu (&entry->list);list_del (&entry->rule.list);
kernel/audit_watch.c +31 -0
@@ -312,6 +312,8 @@ static void audit_update_watch(struct audit_parent *parent,list_replace (&oentry->rule.list, &nentry->rule.list); } +if (oentry->rule.exe) +audit_remove_mark (oentry->rule.exe);audit_watch_log_rule_change (r, owatch,"updated_rules" );
@@ -342,6 +344,8 @@ static void audit_remove_parent_watches(struct audit_parent *parent)list_for_each_entry_safe (r, nextr, &w->rules, rlist) { e = container_of(r, struct audit_entry, rule);audit_watch_log_rule_change (r, w,"remove_rule" ); +if (e->rule.exe) +audit_remove_mark (e->rule.exe);list_del (&r->rlist);list_del (&r->list);list_del_rcu (&e->list);
@@ -514,3 +518,30 @@ static int __init audit_watch_init(void)return 0 ; }device_initcall (audit_watch_init); + +int audit_dupe_exe (struct audit_krule *new,struct audit_krule *old) +{ +struct audit_fsnotify_mark *audit_mark; +char *pathname; + + pathname =kstrdup (audit_mark_path (old->exe),GFP_KERNEL ); +if (!pathname) +return -ENOMEM ; + + audit_mark =audit_alloc_mark (new, pathname,strlen (pathname)); +if (IS_ERR (audit_mark)) { +kfree (pathname); +return PTR_ERR (audit_mark); + } + new->exe = audit_mark; + +return 0 ; +} + +int audit_exe_compare (struct task_struct *tsk,struct audit_fsnotify_mark *mark) +{ +unsigned long ino = tsk->mm->exe_file->f_inode->i_ino; +dev_t dev = tsk->mm->exe_file->f_inode->i_sb->s_dev; + +return audit_mark_compare (mark, ino, dev); +}
kernel/auditfilter.c +52 -1
116 lines not on this page · raw message
kernel/auditsc.c +3 -0
@@ -466,6 +466,9 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_comparator (ctx->ppid, f->op, f->val); }break ; +case AUDIT_EXE : + result =audit_exe_compare (tsk, rule->exe); +break ;case AUDIT_UID : result =audit_uid_comparator (cred->uid, f->op, f->uid);break ;
Signature · 1 line
1.7.1
Re: [PATCH V9 2/3] audit: implement audit by executable
Quoted 18 lines · Richard Guy Briggs
This adds the ability audit the actions of a not-yet-running process.
This patch implements the ability to filter on the executable path. Instead of just hard coding the ino and dev of the executable we care about at the moment the rule is inserted into the kernel, use the new
audit_fsnotifyinfrastructure to manage this dynamically. This means that if the filename does not yet exist but the containing directory does, or if the inode in question is unlinked and creat'd (aka updated) the rule will just continue to work. If the containing directory is moved or deleted or the filesystem is unmounted, the rule is deleted automatically. A future enhancement would be to have the rule survive across directory disruptions.This is a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>
Quoted 9 lines
--- include/linux/audit.h | 1 + include/uapi/linux/audit.h | 5 +++- kernel/audit.h | 4 +++ kernel/audit_tree.c | 2 + kernel/audit_watch.c | 31 +++++++++++++++++++++++++ kernel/auditfilter.c | 53 ++++++++++++++++++++++++++++++++++++++++- kernel/auditsc.c | 3 ++ 7 files changed, 97 insertions(+), 2 deletions(-)
Merged, although some more minor whitespace tweaks were necessary for checkpatch. On a related note, if you're not running ./scripts/checlpatch.pl on your patches before sending them out, I would recommend it. It isn't perfect, but it can catch some silly things that we all do from time to time.
Also, one last thing. It is pretty late in the -rcX cycle to merge these two patches, but considering that we've been talking about these for a while, I'm reasonably okay merging them. In the future, if it isn't in audit#next by the time -rc5 is released, it isn't going to make the merge window.
Quoted 273 lines
include/linux/audit.h +1
-0@@ -59,6 +59,7 @@ struct audit_krule {struct audit_field *inode_f;/* quick access to an inode field */ struct audit_watch *watch;/* associated watch */ struct audit_tree *tree;/* associated watched tree */ +struct audit_fsnotify_mark *exe;struct list_head rlist;/* entry in audit_{watch,tree}.rules list */ struct list_head list;/* for AUDIT_LIST* purposes only */ u64 prio;include/uapi/linux/audit.h +4
-1@@ -266,6 +266,7 @@#define AUDIT_OBJ_UID 109#define AUDIT_OBJ_GID 110#define AUDIT_FIELD_COMPARE 111 +#define AUDIT_EXE 112#define AUDIT_ARG0 200#define AUDIT_ARG1 (AUDIT_ARG0+1)@@ -324,8 +325,10 @@ enum {#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002 +#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \ - AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME) + AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \ + AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH )/* deprecated: AUDIT_VERSION_* */ #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALLkernel/audit.h +0
-0@@ -277,6 +277,8 @@ extern char *audit_mark_path(struct audit_fsnotify_mark *mark); extern void audit_remove_mark(struct audit_fsnotify_mark *audit_mark);extern void audit_remove_mark_rule (struct audit_krule *krule);extern int audit_mark_compare (struct audit_fsnotify_mark *mark,unsigned long ino, dev_t dev); +extern int audit_dupe_exe(struct audit_krule *new,struct
audit_krule*old); +extern intaudit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark);#else
#defineaudit_put_watch(w){}kernel/audit.h +2
-0@@ -292,6 +294,8 @@ extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long in #define audit_remove_mark(m)#define audit_remove_mark_rule (k)#define audit_mark_compare (m, i, d) 0 +#define audit_exe_compare (t, m) (-EINVAL) +#define audit_dupe_exe (n, o) (-EINVAL)#endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE kernel/audit_tree.c +2
-0@@ -479,6 +479,8 @@ static void kill_rules(struct audit_tree *tree)if (rule->tree) {/* not a half-baked one */ audit_tree_log_remove_rule (rule); +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe); rule->tree =NULL ;list_del_rcu (&entry->list);list_del (&entry->rule.list);kernel/audit_watch.c +4
-0@@ -312,6 +312,8 @@ static void audit_update_watch(struct audit_parent *parent,list_replace (&oentry->rule.list, &nentry->rule.list); } +if (oentry->rule.exe) +audit_remove_mark (oentry->rule.exe);audit_watch_log_rule_change (r, owatch,"updated_rules" );@@ -342,6 +344,8 @@ static void audit_remove_parent_watches(struct audit_parent *parent) list_for_each_entry_safe(r, nextr, &w->rules, rlist) { e = container_of(r, struct audit_entry, rule);audit_watch_log_rule_change (r, w,"remove_rule" ); +if (e->rule.exe) +audit_remove_mark (e->rule.exe);list_del (&r->rlist);list_del (&r->list);kernel/audit_watch.c +21
-0list_del_rcu (&e->list);@@ -514,3 +518,30 @@ static int __init audit_watch_init(void)return 0 ; }device_initcall (audit_watch_init); + +int audit_dupe_exe (struct audit_krule *new,struct audit_krule *old) +{ +struct audit_fsnotify_mark *audit_mark; +char *pathname; + + pathname =kstrdup (audit_mark_path (old->exe),GFP_KERNEL ); +if (!pathname) +return -ENOMEM ; + + audit_mark =audit_alloc_mark (new, pathname,strlen (pathname)); +if (IS_ERR (audit_mark)) { +kfree (pathname); +return PTR_ERR (audit_mark); + } + new->exe = audit_mark; + +return 0 ; +} + +int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark*mark) +{
kernel/audit_watch.c +5
-0+unsigned long ino = tsk->mm->exe_file->f_inode->i_ino; +dev_t dev = tsk->mm->exe_file->f_inode->i_sb->s_dev; + +return audit_mark_compare (mark, ino, dev); +}kernel/auditfilter.c +6
-0@@ -405,6 +405,12 @@ 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 : +if (f->op != Audit_equal) +return -EINVAL ; +if (entry->rule.listnr !=AUDIT_FILTER_EXIT ) +return -EINVAL ; +break ; };return 0 ; }@@ -419,6 +425,7 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data*data,size_tremain = datasz -sizeof(struct audit_rule_data);kernel/auditfilter.c +21
-0int i;char *str; +struct audit_fsnotify_mark *audit_mark; entry =audit_to_entry_common (data);if (IS_ERR (entry))@@ -539,6 +546,24 @@ 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 : +if (entry->rule.exe || f->val >PATH_MAX ) + goto exit_free; + str =audit_unpack_string (&bufp, &remain, f->val); +if (IS_ERR (str)) { + err =PTR_ERR (str); + goto exit_free; + } + entry->rule.buflen += f->val; + + audit_mark =audit_alloc_mark (&entry->rule, str, f->val); +if (IS_ERR (audit_mark)) { +kfree (str); + err =PTR_ERR (audit_mark); + goto exit_free; + } + entry->rule.exe = audit_mark; +break ; } }@@ -551,6 +576,8 @@ exit_nofree: exit_free:if (entry->rule.tree)audit_put_tree (entry->rule.tree);/* that's the temporary one */ +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);/* that's the template one */ audit_free_rule (entry);return ERR_PTR (err); }@@ -615,6 +642,10 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule) data->buflen +=data->values[i] =
kernel/auditfilter.c +25
-1audit_pack_string (&bufp, krule->filterkey);break ; +case AUDIT_EXE : + data->buflen += data->values[i] = +audit_pack_string (&bufp,audit_mark_path (krule->exe)); +break ;case AUDIT_LOGINUID_SET :if (krule->pflags &AUDIT_LOGINUID_LEGACY && !f->val) { data->fields[i] =AUDIT_LOGINUID ;@@ -678,6 +709,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 : +/* both paths exist based on above type compare */ +if (strcmp (audit_mark_path (a->exe), +audit_mark_path (b->exe))) +return 1 ; +break ;case AUDIT_UID :case AUDIT_EUID :case AUDIT_SUID :@@ -799,8 +836,14 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old) err = -ENOMEM; else new->filterkey = fk; +break ; +case AUDIT_EXE : + err =audit_dupe_exe (new, old); +break ; }if (err) { +if (new->exe) +audit_remove_mark (new->exe);audit_free_rule (entry);return ERR_PTR (err); }@@ -963,6 +1006,9 @@ int audit_del_rule(struct audit_entry *entry)if (e->rule.tree)audit_remove_tree_rule (&e->rule); +if (e->rule.exe) +audit_remove_mark_rule (&e->rule); +#ifdef CONFIG_AUDITSYSCALL if (!dont_count) audit_n_rules--;@@ -1067,8 +1113,11 @@ int audit_rule_change(int type, __u32 portid, int seq, void *data,WARN_ON (1 ); } -if (err || type ==AUDIT_DEL_RULE ) +if (err || type ==AUDIT_DEL_RULE ) { +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);audit_free_rule (entry); + }return err; }@@ -1360,6 +1409,8 @@ static int update_lsm_rule(struct audit_krule *r)return 0 ; nentry =audit_dupe_rule (r); +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);if (IS_ERR (nentry)) {/* save the first error encountered for the * return value */ kernel/auditsc.c +3
-0@@ -466,6 +466,9 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_comparator (ctx->ppid, f->op, f->val); }break ; +case AUDIT_EXE : + result =audit_exe_compare (tsk, rule->exe); +break ;case AUDIT_UID : result =audit_uid_comparator (cred->uid, f->op, f->uid);break ;
Signature · 2 lines
paul moore security @ redhat
Re: [PATCH V9 2/3] audit: implement audit by executable
Quoted 29 lines · Paul Moore
Quoted 18 lines · Richard Guy Briggs
This adds the ability audit the actions of a not-yet-running process.
This patch implements the ability to filter on the executable path. Instead of just hard coding the ino and dev of the executable we care about at the moment the rule is inserted into the kernel, use the new
audit_fsnotifyinfrastructure to manage this dynamically. This means that if the filename does not yet exist but the containing directory does, or if the inode in question is unlinked and creat'd (aka updated) the rule will just continue to work. If the containing directory is moved or deleted or the filesystem is unmounted, the rule is deleted automatically. A future enhancement would be to have the rule survive across directory disruptions.This is a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>Quoted 9 lines
--- include/linux/audit.h | 1 + include/uapi/linux/audit.h | 5 +++- kernel/audit.h | 4 +++ kernel/audit_tree.c | 2 + kernel/audit_watch.c | 31 +++++++++++++++++++++++++ kernel/auditfilter.c | 53 ++++++++++++++++++++++++++++++++++++++++- kernel/auditsc.c | 3 ++ 7 files changed, 97 insertions(+), 2 deletions(-)
Quoted 4 lines
Merged, although some more minor whitespace tweaks were necessary for checkpatch. On a related note, if you're not running
./scripts/checlpatch.plon your patches before sending them out, I would recommend it. It isn't perfect, but it can catch some silly things that we all do from time to time.
No excuses... I have been running it pretty regularly and got lazy and distracted with patch revisions. I can't say I agree with the no space before closing round parenthesis due to legibility, but will comply.
Quoted 4 lines
Also, one last thing. It is pretty late in the -rcX cycle to merge these two patches, but considering that we've been talking about these for a while, I'm reasonably okay merging them. In the future, if it isn't in audit#next by the time -rc5 is released, it isn't going to make the merge window.
I've been quite aware of that looming merge window... This feature has been iterating for a while, so there are no big surprises. I was aiming for earlier. :)
Quoted 265 lines
Quoted 265 lines
include/linux/audit.h +1
-0@@ -59,6 +59,7 @@ struct audit_krule {struct audit_field *inode_f;/* quick access to an inode field */ struct audit_watch *watch;/* associated watch */ struct audit_tree *tree;/* associated watched tree */ +struct audit_fsnotify_mark *exe;struct list_head rlist;/* entry in audit_{watch,tree}.rules list */ struct list_head list;/* for AUDIT_LIST* purposes only */ u64 prio;include/uapi/linux/audit.h +4
-1@@ -266,6 +266,7 @@#define AUDIT_OBJ_UID 109#define AUDIT_OBJ_GID 110#define AUDIT_FIELD_COMPARE 111 +#define AUDIT_EXE 112#define AUDIT_ARG0 200#define AUDIT_ARG1 (AUDIT_ARG0+1)@@ -324,8 +325,10 @@ enum {#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002 +#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \ - AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME) + AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \ + AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH )/* deprecated: AUDIT_VERSION_* */ #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALLkernel/audit.h +0
-0@@ -277,6 +277,8 @@ extern char *audit_mark_path(struct audit_fsnotify_mark *mark); extern void audit_remove_mark(struct audit_fsnotify_mark *audit_mark);extern void audit_remove_mark_rule (struct audit_krule *krule);extern int audit_mark_compare (struct audit_fsnotify_mark *mark,unsigned long ino, dev_t dev); +extern int audit_dupe_exe(struct audit_krule *new,struct
audit_krule*old); +extern intaudit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark);#else
#defineaudit_put_watch(w){}kernel/audit.h +2
-0@@ -292,6 +294,8 @@ extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long in #define audit_remove_mark(m)#define audit_remove_mark_rule (k)#define audit_mark_compare (m, i, d) 0 +#define audit_exe_compare (t, m) (-EINVAL) +#define audit_dupe_exe (n, o) (-EINVAL)#endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE kernel/audit_tree.c +2
-0@@ -479,6 +479,8 @@ static void kill_rules(struct audit_tree *tree)if (rule->tree) {/* not a half-baked one */ audit_tree_log_remove_rule (rule); +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe); rule->tree =NULL ;list_del_rcu (&entry->list);list_del (&entry->rule.list);kernel/audit_watch.c +4
-0@@ -312,6 +312,8 @@ static void audit_update_watch(struct audit_parent *parent,list_replace (&oentry->rule.list, &nentry->rule.list); } +if (oentry->rule.exe) +audit_remove_mark (oentry->rule.exe);audit_watch_log_rule_change (r, owatch,"updated_rules" );@@ -342,6 +344,8 @@ static void audit_remove_parent_watches(struct audit_parent *parent) list_for_each_entry_safe(r, nextr, &w->rules, rlist) { e = container_of(r, struct audit_entry, rule);audit_watch_log_rule_change (r, w,"remove_rule" ); +if (e->rule.exe) +audit_remove_mark (e->rule.exe);list_del (&r->rlist);list_del (&r->list);kernel/audit_watch.c +21
-0list_del_rcu (&e->list);@@ -514,3 +518,30 @@ static int __init audit_watch_init(void)return 0 ; }device_initcall (audit_watch_init); + +int audit_dupe_exe (struct audit_krule *new,struct audit_krule *old) +{ +struct audit_fsnotify_mark *audit_mark; +char *pathname; + + pathname =kstrdup (audit_mark_path (old->exe),GFP_KERNEL ); +if (!pathname) +return -ENOMEM ; + + audit_mark =audit_alloc_mark (new, pathname,strlen (pathname)); +if (IS_ERR (audit_mark)) { +kfree (pathname); +return PTR_ERR (audit_mark); + } + new->exe = audit_mark; + +return 0 ; +} + +int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark*mark) +{
kernel/audit_watch.c +5
-0+unsigned long ino = tsk->mm->exe_file->f_inode->i_ino; +dev_t dev = tsk->mm->exe_file->f_inode->i_sb->s_dev; + +return audit_mark_compare (mark, ino, dev); +}kernel/auditfilter.c +6
-0@@ -405,6 +405,12 @@ 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 : +if (f->op != Audit_equal) +return -EINVAL ; +if (entry->rule.listnr !=AUDIT_FILTER_EXIT ) +return -EINVAL ; +break ; };return 0 ; }@@ -419,6 +425,7 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data*data,size_tremain = datasz -sizeof(struct audit_rule_data);kernel/auditfilter.c +21
-0int i;char *str; +struct audit_fsnotify_mark *audit_mark; entry =audit_to_entry_common (data);if (IS_ERR (entry))@@ -539,6 +546,24 @@ 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 : +if (entry->rule.exe || f->val >PATH_MAX ) + goto exit_free; + str =audit_unpack_string (&bufp, &remain, f->val); +if (IS_ERR (str)) { + err =PTR_ERR (str); + goto exit_free; + } + entry->rule.buflen += f->val; + + audit_mark =audit_alloc_mark (&entry->rule, str, f->val); +if (IS_ERR (audit_mark)) { +kfree (str); + err =PTR_ERR (audit_mark); + goto exit_free; + } + entry->rule.exe = audit_mark; +break ; } }@@ -551,6 +576,8 @@ exit_nofree: exit_free:if (entry->rule.tree)audit_put_tree (entry->rule.tree);/* that's the temporary one */ +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);/* that's the template one */ audit_free_rule (entry);return ERR_PTR (err); }@@ -615,6 +642,10 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule) data->buflen +=data->values[i] =
kernel/auditfilter.c +25
-1audit_pack_string (&bufp, krule->filterkey);break ; +case AUDIT_EXE : + data->buflen += data->values[i] = +audit_pack_string (&bufp,audit_mark_path (krule->exe)); +break ;case AUDIT_LOGINUID_SET :if (krule->pflags &AUDIT_LOGINUID_LEGACY && !f->val) { data->fields[i] =AUDIT_LOGINUID ;@@ -678,6 +709,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 : +/* both paths exist based on above type compare */ +if (strcmp (audit_mark_path (a->exe), +audit_mark_path (b->exe))) +return 1 ; +break ;case AUDIT_UID :case AUDIT_EUID :case AUDIT_SUID :@@ -799,8 +836,14 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old) err = -ENOMEM; else new->filterkey = fk; +break ; +case AUDIT_EXE : + err =audit_dupe_exe (new, old); +break ; }if (err) { +if (new->exe) +audit_remove_mark (new->exe);audit_free_rule (entry);return ERR_PTR (err); }@@ -963,6 +1006,9 @@ int audit_del_rule(struct audit_entry *entry)if (e->rule.tree)audit_remove_tree_rule (&e->rule); +if (e->rule.exe) +audit_remove_mark_rule (&e->rule); +#ifdef CONFIG_AUDITSYSCALL if (!dont_count) audit_n_rules--;@@ -1067,8 +1113,11 @@ int audit_rule_change(int type, __u32 portid, int seq, void *data,WARN_ON (1 ); } -if (err || type ==AUDIT_DEL_RULE ) +if (err || type ==AUDIT_DEL_RULE ) { +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);audit_free_rule (entry); + }return err; }@@ -1360,6 +1409,8 @@ static int update_lsm_rule(struct audit_krule *r)return 0 ; nentry =audit_dupe_rule (r); +if (entry->rule.exe) +audit_remove_mark (entry->rule.exe);if (IS_ERR (nentry)) {/* save the first error encountered for the * return value */ kernel/auditsc.c +0
-0@@ -466,6 +466,9 @@ static int audit_filter_rules(struct task_struct *tsk, result =audit_comparator (ctx->ppid, f->op, f->val);
Quoted 13 lines
Quoted 8 lines
kernel/auditsc.c +3
-0}break ; +case AUDIT_EXE : + result =audit_exe_compare (tsk, rule->exe); +break ;case AUDIT_UID : result =audit_uid_comparator (cred->uid, f->op, f->uid);break ;Signature · 2 lines
paul moore security @ redhat
- RGB
--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
Re: [PATCH V9 2/3] audit: implement audit by executable
Quoted 11 lines · Richard Guy Briggs
Quoted 5 lines · Paul Moore
Merged, although some more minor whitespace tweaks were necessary for checkpatch. On a related note, if you're not running
./scripts/checlpatch.plon your patches before sending them out, I would recommend it. It isn't perfect, but it can catch some silly things that we all do from time to time.No excuses... I have been running it pretty regularly and got lazy and distracted with patch revisions. I can't say I agree with the no space before closing round parenthesis due to legibility, but will comply.
Okay, glad to hear you run it regularly when submitting patches.
I agree that there are some things I might change about the kernel's style choices, but I think it is more important that we remain consistent with the kernel as a whole. I really like tools that enforce things like this, even if I don't agree 100% with the results.
Quoted 9 lines
Quoted 5 lines
Also, one last thing. It is pretty late in the -rcX cycle to merge these two patches, but considering that we've been talking about these for a while, I'm reasonably okay merging them. In the future, if it isn't in audit#next by the time -rc5 is released, it isn't going to make the merge window.
I've been quite aware of that looming merge window... This feature has been iterating for a while, so there are no big surprises. I was aiming for earlier. :)
Well, I think it is a little early to say there are no big surprises, we won't know that for a few more weeks, but if we make it to -rc3/4 without any problems I'll breathe a bit easier ;)
Signature · 2 lines
paul moore security @ redhat
PATCH v9 3/3 audit: add audit by children of executable path
In reply to Richard Guy BriggsThis adds the ability to audit the actions of children of a not-yet-running process.
This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
--- include/uapi/linux/audit.h | 1 + kernel/auditfilter.c | 5 +++++ kernel/auditsc.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-)
include/uapi/linux/audit.h +1 -0
@@ -267,6 +267,7 @@#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)
kernel/auditfilter.c +5 -0
@@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)return -EINVAL ;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :if (f->op != Audit_equal)return -EINVAL ;if (entry->rule.listnr !=AUDIT_FILTER_EXIT )
@@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data, entry->rule.filterkey = str;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :if (entry->rule.exe || f->val >PATH_MAX ) goto exit_free; str =audit_unpack_string (&bufp, &remain, f->val);
@@ -643,6 +645,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)audit_pack_string (&bufp, krule->filterkey);break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN : data->buflen += data->values[i] =audit_pack_string (&bufp,audit_mark_path (krule->exe));break ;
@@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)return 1 ;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :/* both paths exist based on above type compare */ if (strcmp (audit_mark_path (a->exe),audit_mark_path (b->exe)))
@@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old) new->filterkey = fk;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN : err =audit_dupe_exe (new, old);break ; }
kernel/auditsc.c +11 -0
@@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,case AUDIT_EXE : result =audit_exe_compare (tsk, rule->exe);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_exe_compare (ptsk, rule->exe)) { + ++result; +break ; + } + } + } +break ;case AUDIT_UID : result =audit_uid_comparator (cred->uid, f->op, f->uid);break ;
Signature · 1 line
1.7.1
Re: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 6 lines · Richard Guy Briggs
This adds the ability to audit the actions of children of a not-yet-running process.
This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Quoted 8 lines
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>--- include/uapi/linux/audit.h | 1 + kernel/auditfilter.c | 5 +++++ kernel/auditsc.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-)
I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it.
Quoted 69 lines
include/uapi/linux/audit.h +1
-0@@ -267,6 +267,7 @@#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)kernel/auditfilter.c +3
-0@@ -406,6 +406,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) return -EINVAL;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :if (f->op != Audit_equal)return -EINVAL ;if (entry->rule.listnr !=AUDIT_FILTER_EXIT )@@ -547,6 +548,7 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data, entry->rule.filterkey = str;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :if (entry->rule.exe || f->val >PATH_MAX ) goto exit_free; str =audit_unpack_string (&bufp, &remain, f->val);@@ -643,6 +645,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule) audit_pack_string(&bufp, krule->filterkey);break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN : data->buflen += data->values[i] =audit_pack_string (&bufp,audit_mark_path (krule->exe));kernel/auditfilter.c +2
-0break ;@@ -710,6 +713,7 @@ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b) return 1;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN :/* both paths exist based on above type compare */ if (strcmp (audit_mark_path (a->exe),audit_mark_path (b->exe)))@@ -838,6 +842,7 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old) new->filterkey = fk;break ;case AUDIT_EXE : +case AUDIT_EXE_CHILDREN : err =audit_dupe_exe (new, old);break ; }kernel/auditsc.c +4
-0@@ -469,6 +469,17 @@ static int audit_filter_rules(struct task_struct *tsk,case AUDIT_EXE : result =audit_exe_compare (tsk, rule->exe);break ; +case AUDIT_EXE_CHILDREN : + { + struct task_struct *ptsk; + for (ptsk = tsk; ptsk->parent->pid > 0; ptsk =
Quoted 1 line
find_task_by_vpid(ptsk->parent->pid)) { + if
(audit_exe_compare(ptsk,
Quoted 10 lines
rule->exe)) {kernel/auditsc.c +6
-0+ ++result; +break ; + } + } + } +break ;case AUDIT_UID : result =audit_uid_comparator (cred->uid, f->op, f->uid);break ;
Signature · 2 lines
paul moore security @ redhat
Re: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
This adds the ability to audit the actions of children of a not-yet-running
process.This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>Quoted 6 lines
---
include/uapi/linux/audit.h | 1 + kernel/auditfilter.c | 5 +++++ kernel/auditsc.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-)
Quoted 4 lines
I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it.
Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful.
-Steve
Paul Moore
maintainerRe: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 38 lines · Steve Grubb
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
This adds the ability to audit the actions of children of a not-yet-running
process.This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>Quoted 6 lines
---
include/uapi/linux/audit.h | 1 + kernel/auditfilter.c | 5 +++++ kernel/auditsc.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-)Quoted 4 lines
I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it.
Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful.
Quoted 6 lines
-Steve
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
--
paul moore
www.paul-moore.com
Re: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 47 lines · Paul Moore
Quoted 38 lines · Steve Grubb
Quoted 22 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
This adds the ability to audit the actions of children of a not-yet-running
process.This is a split-out of a heavily modified version of a patch originally submitted by Eric Paris with some ideas from Peter Moody.
Cc Peter Moody <[email protected]>Cc Eric Paris <[email protected]>Signed-off-by Richard Guy Briggs <[email protected]>Quoted 6 lines
---
include/uapi/linux/audit.h | 1 + kernel/auditfilter.c | 5 +++++ kernel/auditsc.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-)Quoted 4 lines
I'm still not really comfortable with that loop and since there hasn't been a really convincing use case I'm going to pass on this patch for right now. If someone comes up with a *really* compelling case in the future I'll reconsider it.
Its the same reason strace has a -f option. Sometimes you need to also see what the children did. For example, maybe you want to audit file access to a specific directory and several cgi-bin programs can get there. You could write a rule for apache and be done. Or maybe, you have an app that lets people have shell access and you need to see files accessed or connections opened. Or maybe its a control panel application with helper scripts and you need to see changes that its making. Or maybe you have a program that is at risk of being compromised and you want to see if someone gets a shell from it. There are a lot of cases where it could be useful.
Quoted 6 lines
-Steve
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
Quoted 5 lines
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
Can you better describe the loop that concerns you? I don't quite see it.
Quoted 1 line
paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
Paul Moore
maintainerRe: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 10 lines · Richard Guy Briggs
Quoted 5 lines · Paul Moore
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
Can you better describe the loop that concerns you? I don't quite see it.
It would be the only loop in the patch, look at the for loop in audit_filter_rules() which iterates up the process' parent chain.
Signature · 2 lines
paul moore www.paul-moore.com
Re: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 9 lines · Paul Moore
Quoted 8 lines · Richard Guy Briggs
Quoted 5 lines · Paul Moore
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
Quoted 5 lines
Quoted 2 lines
Can you better describe the loop that concerns you? I don't quite see it.
It would be the only loop in the patch, look at the for loop in
audit_filter_rules()which iterates up the process' parent chain.
Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop?
I *do* notice that find_task_by_vpid(pid_t) must be replaced with find_task_by_pid_ns(pid_t, &init_pid_ns), since task_struct->pid is always stored in the initial PID namespace.
Quoted 1 line
paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
Paul Moore
maintainerRe: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 13 lines · Richard Guy Briggs
Quoted 12 lines · Paul Moore
Quoted 10 lines · Richard Guy Briggs
Quoted 5 lines · Paul Moore
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
Can you better describe the loop that concerns you? I don't quite see it.
Quoted 7 lines
Quoted 2 lines
It would be the only loop in the patch, look at the for loop in
audit_filter_rules()which iterates up the process' parent chain.Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop?
I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first.
My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds.
I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk.
Quoted 3 lines
I *do* notice that
find_task_by_vpid(pid_t)must be replaced withfind_task_by_pid_ns(pid_t, &init_pid_ns), sincetask_struct->pidis always stored in the initial PID namespace.
Another thing that needs to be resolved.
Signature · 2 lines
paul moore www.paul-moore.com
Re: [PATCH V9 3/3] audit: add audit by children of executable path
Quoted 34 lines · Paul Moore
Quoted 13 lines · Richard Guy Briggs
Quoted 12 lines · Paul Moore
Quoted 10 lines · Richard Guy Briggs
Quoted 5 lines · Paul Moore
I guess what I'm saying is that I'm not currently convinced that there is enough value in this to offset the risk I feel the loop presents. I understand the use cases that you are mentioning, the are the same as the last time we discussed this, but I'm going to need something better than that.
Can you better describe the loop that concerns you? I don't quite see it.
Quoted 7 lines
Quoted 2 lines
It would be the only loop in the patch, look at the for loop in
audit_filter_rules()which iterates up the process' parent chain.Sorry, I should reword that... What risk do you see in that loop? It works up the task ancestry tree until it triggers, or hits init for that PID namespace that terminates the loop. Do you see a risk in the numerical pids rolling underneath the loop?
I suppose there is some risk of PID overlap, and while that is a concern, it isn't my first.
My main concern is that a malicious user could add an extra level of burden to the system by making an absurdly tall process tree and then hammer the system with trivial, short lived syscalls. Granted, there are userspace limits which would bound the impact to some extent, but there is no way to really reduce the risk. You could further put hard limits on the loop, but what good would that do? Malicious users would just know to blow past that limit before they did their Evil Deeds.
Quoted 3 lines
I'll say it again; I'm not completely opposed to something like this - perhaps in some modified form - but I have yet to see a need for this functionality that is great enough to counter the risk.
I am not going to lobby hard for it. I split this part of the patch out to avoid jeopardizing the acceptability of the rest of the patchset and to isolate it to make it easier to focus on its issues and apply it later once they are addressed.
I'll reflect on this concern and see if I can come up with any ways to minimize this danger. This issue is related to the request to list the chain of processes back to the first ancestor in each record. You can make a best effort to record or track the entire chain, but at some point need to put a limit on it to avoid a DoS, at which point there is no point in listing the information since it is incomplete. (Too many "point"s in that last sentence...)
Quoted 5 lines
Quoted 3 lines
I *do* notice that
find_task_by_vpid(pid_t)must be replaced withfind_task_by_pid_ns(pid_t, &init_pid_ns), sincetask_struct->pidis always stored in the initial PID namespace.Another thing that needs to be resolved.
I've already fixed it in my tree:
ptsk = find_task_by_pid_ns(ptsk->parent->pid) &init_pid_ns)
Quoted 1 line
paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545