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

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

[Peter Moody]

This adds the ability audit the actions of a not-yet-running process,
as well as the children of a not-yet-running process.

Signed-off-by: Peter Moody <pmoody@google.com>
---
 include/linux/audit.h |    2 ++
 kernel/auditfilter.c  |    6 ++++++
 kernel/auditsc.c      |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 22f292a..5506cb1 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -260,6 +260,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 a6c3f1a..1e6c571 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -546,6 +546,12 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
 			if (f->val > AUDIT_MAX_FIELD_COMPARE)
 				goto exit_free;
 			break;
+		case AUDIT_EXE:
+		case AUDIT_EXE_CHILDREN:
+			if (f->op != Audit_equal) {
+				goto exit_free;
+			}
+			break;
 		default:
 			goto exit_free;
 		}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4b96415..9cebe95 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -46,6 +46,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>
@@ -68,6 +69,7 @@
 #include <linux/capability.h>
 #include <linux/fs_struct.h>
 #include <linux/compat.h>
+#include <linux/sched.h>
 
 #include "audit.h"
 
@@ -592,6 +594,35 @@ static int audit_field_compare(struct task_struct *tsk,
 	return 0;
 }
 
+int audit_match_exe(struct task_struct *tsk, struct audit_field *f)
+{
+	int result = 0;
+	struct mm_struct *mm;
+	struct vm_area_struct *vma;
+
+	if (!tsk)
+		goto out;
+
+	mm = tsk->mm;
+	if (!mm)
+		goto out;
+
+	down_read(&mm->mmap_sem);
+	vma = mm->mmap;
+	while (vma) {
+		if ((vma->vm_flags & VM_EXECUTABLE) &&
+		    vma->vm_file) {
+			struct inode *ino = vma->vm_file->f_path.dentry->d_inode;
+			result = audit_comparator(ino->i_ino, f->op, f->val);
+			break;
+		}
+		vma = vma->vm_next;
+	}
+	up_read(&mm->mmap_sem);
+out:
+	return result;
+}
+
 /* 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.
@@ -629,6 +660,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, f);
+			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, f)) {
+					++result;
+					break;
+				}
+			}
+		}
+			break;
 		case AUDIT_UID:
 			result = audit_comparator(cred->uid, f->op, f->val);
 			break;
-- 
1.7.7.3

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

[Peter Moody]

Hey Eric,

did you have any comments on this? Is there a better way to do this?

Cheers,
peter

On Thu, Aug 23, 2012 at 12:24 PM, Peter Moody <pmoody@google.com> wrote:
> This adds the ability audit the actions of a not-yet-running process,
> as well as the children of a not-yet-running process.
>
> Signed-off-by: Peter Moody <pmoody@google.com>
> ---
>  include/linux/audit.h |    2 ++
>  kernel/auditfilter.c  |    6 ++++++
>  kernel/auditsc.c      |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 22f292a..5506cb1 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -260,6 +260,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 a6c3f1a..1e6c571 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -546,6 +546,12 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
>                         if (f->val > AUDIT_MAX_FIELD_COMPARE)
>                                 goto exit_free;
>                         break;
> +               case AUDIT_EXE:
> +               case AUDIT_EXE_CHILDREN:
> +                       if (f->op != Audit_equal) {
> +                               goto exit_free;
> +                       }
> +                       break;
>                 default:
>                         goto exit_free;
>                 }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4b96415..9cebe95 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -46,6 +46,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>
> @@ -68,6 +69,7 @@
>  #include <linux/capability.h>
>  #include <linux/fs_struct.h>
>  #include <linux/compat.h>
> +#include <linux/sched.h>
>
>  #include "audit.h"
>
> @@ -592,6 +594,35 @@ static int audit_field_compare(struct task_struct *tsk,
>         return 0;
>  }
>
> +int audit_match_exe(struct task_struct *tsk, struct audit_field *f)
> +{
> +       int result = 0;
> +       struct mm_struct *mm;
> +       struct vm_area_struct *vma;
> +
> +       if (!tsk)
> +               goto out;
> +
> +       mm = tsk->mm;
> +       if (!mm)
> +               goto out;
> +
> +       down_read(&mm->mmap_sem);
> +       vma = mm->mmap;
> +       while (vma) {
> +               if ((vma->vm_flags & VM_EXECUTABLE) &&
> +                   vma->vm_file) {
> +                       struct inode *ino = vma->vm_file->f_path.dentry->d_inode;
> +                       result = audit_comparator(ino->i_ino, f->op, f->val);
> +                       break;
> +               }
> +               vma = vma->vm_next;
> +       }
> +       up_read(&mm->mmap_sem);
> +out:
> +       return result;
> +}
> +
>  /* 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.
> @@ -629,6 +660,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, f);
> +                       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, f)) {
> +                                       ++result;
> +                                       break;
> +                               }
> +                       }
> +               }
> +                       break;
>                 case AUDIT_UID:
>                         result = audit_comparator(cred->uid, f->op, f->val);
>                         break;
> --
> 1.7.7.3
>



-- 
Peter Moody      Google    1.650.253.7306
Security Engineer  pgp:0xC3410038

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

[Eric Paris]

Is this late enough?  I suck a LOT as a maintainer.  Anyway, I'm not in love.

inode numbers are not unique across the system.  If you had 2 binaries, completely unrelated, that just happened to have the same i_ino, we'd have false positives.  I'm not, off the top of my head, thinking of a good way to fix it.  But it does seem to me like maybe it could be something like audit watches where we give the path to the kernel and do some marking on the inode in question inside the kernel.

Possibly such that we even remark it if another binary is dropped on top of the present binary?  I'll think on it for a bit...

----- Original Message -----
> This adds the ability audit the actions of a not-yet-running process,
> as well as the children of a not-yet-running process.
> 
> Signed-off-by: Peter Moody <pmoody@google.com>
> ---
>  include/linux/audit.h |    2 ++
>  kernel/auditfilter.c  |    6 ++++++
>  kernel/auditsc.c      |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 22f292a..5506cb1 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -260,6 +260,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 a6c3f1a..1e6c571 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -546,6 +546,12 @@ static struct audit_entry *audit_data_to_entry(struct
> audit_rule_data *data,
>  			if (f->val > AUDIT_MAX_FIELD_COMPARE)
>  				goto exit_free;
>  			break;
> +		case AUDIT_EXE:
> +		case AUDIT_EXE_CHILDREN:
> +			if (f->op != Audit_equal) {
> +				goto exit_free;
> +			}
> +			break;
>  		default:
>  			goto exit_free;
>  		}
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4b96415..9cebe95 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -46,6 +46,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>
> @@ -68,6 +69,7 @@
>  #include <linux/capability.h>
>  #include <linux/fs_struct.h>
>  #include <linux/compat.h>
> +#include <linux/sched.h>
>  
>  #include "audit.h"
>  
> @@ -592,6 +594,35 @@ static int audit_field_compare(struct task_struct *tsk,
>  	return 0;
>  }
>  
> +int audit_match_exe(struct task_struct *tsk, struct audit_field *f)
> +{
> +	int result = 0;
> +	struct mm_struct *mm;
> +	struct vm_area_struct *vma;
> +
> +	if (!tsk)
> +		goto out;
> +
> +	mm = tsk->mm;
> +	if (!mm)
> +		goto out;
> +
> +	down_read(&mm->mmap_sem);
> +	vma = mm->mmap;
> +	while (vma) {
> +		if ((vma->vm_flags & VM_EXECUTABLE) &&
> +		    vma->vm_file) {
> +			struct inode *ino = vma->vm_file->f_path.dentry->d_inode;
> +			result = audit_comparator(ino->i_ino, f->op, f->val);
> +			break;
> +		}
> +		vma = vma->vm_next;
> +	}
> +	up_read(&mm->mmap_sem);
> +out:
> +	return result;
> +}
> +
>  /* 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.
> @@ -629,6 +660,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, f);
> +			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, f)) {
> +					++result;
> +					break;
> +				}
> +			}
> +		}
> +			break;
>  		case AUDIT_UID:
>  			result = audit_comparator(cred->uid, f->op, f->val);
>  			break;
> --
> 1.7.7.3
> 
> 

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

[Peter Moody]

On Thu, Apr 11, 2013 at 11:08 AM, Eric Paris <eparis@redhat.com> wrote:
> Is this late enough?  I suck a LOT as a maintainer.  Anyway, I'm not in love.

Hah. I actually figured that you hated it, so 'not in love' is a step
up from what I was expecting.

> inode numbers are not unique across the system.  If you had 2 binaries, completely unrelated, that just happened to have the same i_ino, we'd have false positives.  I'm not, off the top of my head, thinking of a good way to fix it.  But it does seem to me like maybe it could be something like audit watches where we give the path to the kernel and do some marking on the inode in question inside the kernel.

I'd been meaning to get back to this, figuring that there were some
fundamental issues with the inode approach but I had a bunch of stuff
creep onto my plate in the intervening months and this wasn't a
must-have feature anyway.

If you do think of a better way to attack this, I'd be interested to
hear it. The watch sounds promising.

> Possibly such that we even remark it if another binary is dropped on top of the present binary?  I'll think on it for a bit...

Cheers,
peter

> ----- Original Message -----
>> This adds the ability audit the actions of a not-yet-running process,
>> as well as the children of a not-yet-running process.
>>
>> Signed-off-by: Peter Moody <pmoody@google.com>
>> ---
>>  include/linux/audit.h |    2 ++
>>  kernel/auditfilter.c  |    6 ++++++
>>  kernel/auditsc.c      |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 55 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>> index 22f292a..5506cb1 100644
>> --- a/include/linux/audit.h
>> +++ b/include/linux/audit.h
>> @@ -260,6 +260,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 a6c3f1a..1e6c571 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -546,6 +546,12 @@ static struct audit_entry *audit_data_to_entry(struct
>> audit_rule_data *data,
>>                       if (f->val > AUDIT_MAX_FIELD_COMPARE)
>>                               goto exit_free;
>>                       break;
>> +             case AUDIT_EXE:
>> +             case AUDIT_EXE_CHILDREN:
>> +                     if (f->op != Audit_equal) {
>> +                             goto exit_free;
>> +                     }
>> +                     break;
>>               default:
>>                       goto exit_free;
>>               }
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index 4b96415..9cebe95 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -46,6 +46,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>
>> @@ -68,6 +69,7 @@
>>  #include <linux/capability.h>
>>  #include <linux/fs_struct.h>
>>  #include <linux/compat.h>
>> +#include <linux/sched.h>
>>
>>  #include "audit.h"
>>
>> @@ -592,6 +594,35 @@ static int audit_field_compare(struct task_struct *tsk,
>>       return 0;
>>  }
>>
>> +int audit_match_exe(struct task_struct *tsk, struct audit_field *f)
>> +{
>> +     int result = 0;
>> +     struct mm_struct *mm;
>> +     struct vm_area_struct *vma;
>> +
>> +     if (!tsk)
>> +             goto out;
>> +
>> +     mm = tsk->mm;
>> +     if (!mm)
>> +             goto out;
>> +
>> +     down_read(&mm->mmap_sem);
>> +     vma = mm->mmap;
>> +     while (vma) {
>> +             if ((vma->vm_flags & VM_EXECUTABLE) &&
>> +                 vma->vm_file) {
>> +                     struct inode *ino = vma->vm_file->f_path.dentry->d_inode;
>> +                     result = audit_comparator(ino->i_ino, f->op, f->val);
>> +                     break;
>> +             }
>> +             vma = vma->vm_next;
>> +     }
>> +     up_read(&mm->mmap_sem);
>> +out:
>> +     return result;
>> +}
>> +
>>  /* 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.
>> @@ -629,6 +660,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, f);
>> +                     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, f)) {
>> +                                     ++result;
>> +                                     break;
>> +                             }
>> +                     }
>> +             }
>> +                     break;
>>               case AUDIT_UID:
>>                       result = audit_comparator(cred->uid, f->op, f->val);
>>                       break;
>> --
>> 1.7.7.3
>>
>>



-- 
[ Peter Moody | Security Engineer | Google ]

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

[Richard Guy Briggs]

On Thu, Aug 23, 2012 at 12:24:00PM -0700, Peter Moody wrote:
> This adds the ability audit the actions of a not-yet-running process,
> as well as the children of a not-yet-running process.

Hi Peter,

I've gone back over the discussion of this feature and some of the
background in the past couple of years on this list...

We've got a kernel deadline coming up in the next month if we want to
get something included in RHEL7 if you have the interest and time to
evolve this patch (the userspace patch can follow...).

As has been discussed, passing in an inode reference is incomplete,
since it would need to be qualified by a device reference at minimum.
And even then, it isn't atomic and could change by the time the kernel
even sees this rule request.

So, the next step is to convert the path to a device/inode in the kernel.  If
this is done at the time of registering the filter rule, if/when the
rule is invalidated then the rule would be dropped, logged.  It also
means that anything else also hardlinked to it would be acted upon.

Going one step further, if instead we can arrange an fsnotify() hook on
rule registration, we could act on that path when it is executed,
renamed, unlinked (and destroyed if the refcount goes to zero), etc.

So, it should be passed as a path, logging the rule addition with path
only at first.  When the rule is triggered then log the requested path,
effective path, device/inode along with the user context.

The user, carefully crafting other rules can give other information.

A watch on the containing directory (/usr/bin) could help in case that
executable pathname disappears and re-appears since the containing
directory is less likely to go away, but it will be noisy.

Does all this make sense?

Let's deal later with namespaces, containers, mounts, chroots, bind
mounts, etc...


> Signed-off-by: Peter Moody <pmoody@google.com>
> ---
>  include/linux/audit.h |    2 ++
>  kernel/auditfilter.c  |    6 ++++++
>  kernel/auditsc.c      |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 22f292a..5506cb1 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -260,6 +260,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 a6c3f1a..1e6c571 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -546,6 +546,12 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
>  			if (f->val > AUDIT_MAX_FIELD_COMPARE)
>  				goto exit_free;
>  			break;
> +		case AUDIT_EXE:
> +		case AUDIT_EXE_CHILDREN:
> +			if (f->op != Audit_equal) {
> +				goto exit_free;
> +			}
> +			break;
>  		default:
>  			goto exit_free;
>  		}
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4b96415..9cebe95 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -46,6 +46,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>
> @@ -68,6 +69,7 @@
>  #include <linux/capability.h>
>  #include <linux/fs_struct.h>
>  #include <linux/compat.h>
> +#include <linux/sched.h>
>  
>  #include "audit.h"
>  
> @@ -592,6 +594,35 @@ static int audit_field_compare(struct task_struct *tsk,
>  	return 0;
>  }
>  
> +int audit_match_exe(struct task_struct *tsk, struct audit_field *f)
> +{
> +	int result = 0;
> +	struct mm_struct *mm;
> +	struct vm_area_struct *vma;
> +
> +	if (!tsk)
> +		goto out;
> +
> +	mm = tsk->mm;
> +	if (!mm)
> +		goto out;
> +
> +	down_read(&mm->mmap_sem);
> +	vma = mm->mmap;
> +	while (vma) {
> +		if ((vma->vm_flags & VM_EXECUTABLE) &&
> +		    vma->vm_file) {
> +			struct inode *ino = vma->vm_file->f_path.dentry->d_inode;
> +			result = audit_comparator(ino->i_ino, f->op, f->val);
> +			break;
> +		}
> +		vma = vma->vm_next;
> +	}
> +	up_read(&mm->mmap_sem);
> +out:
> +	return result;
> +}
> +
>  /* 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.
> @@ -629,6 +660,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, f);
> +			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, f)) {
> +					++result;
> +					break;
> +				}
> +			}
> +		}
> +			break;
>  		case AUDIT_UID:
>  			result = audit_comparator(cred->uid, f->op, f->val);
>  			break;
> -- 
> 1.7.7.3

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

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

[Peter Moody]

On Wed, Jul 03 2013 at 19:48, Richard Guy Briggs wrote:
> On Thu, Aug 23, 2012 at 12:24:00PM -0700, Peter Moody wrote:
>> This adds the ability audit the actions of a not-yet-running process,
>> as well as the children of a not-yet-running process.
>
> Hi Peter,
>
> I've gone back over the discussion of this feature and some of the
> background in the past couple of years on this list...
>
> We've got a kernel deadline coming up in the next month if we want to
> get something included in RHEL7 if you have the interest and time to
> evolve this patch (the userspace patch can follow...).
>
> As has been discussed, passing in an inode reference is incomplete,
> since it would need to be qualified by a device reference at minimum.
> And even then, it isn't atomic and could change by the time the kernel
> even sees this rule request.
>
> So, the next step is to convert the path to a device/inode in the kernel.  If
> this is done at the time of registering the filter rule, if/when the
> rule is invalidated then the rule would be dropped, logged.  It also
> means that anything else also hardlinked to it would be acted upon.
>
> Going one step further, if instead we can arrange an fsnotify() hook on
> rule registration, we could act on that path when it is executed,
> renamed, unlinked (and destroyed if the refcount goes to zero), etc.
>
> So, it should be passed as a path, logging the rule addition with path
> only at first.  When the rule is triggered then log the requested path,
> effective path, device/inode along with the user context.
>
> The user, carefully crafting other rules can give other information.
>
> A watch on the containing directory (/usr/bin) could help in case that
> executable pathname disappears and re-appears since the containing
> directory is less likely to go away, but it will be noisy.
>
> Does all this make sense?

Hey Richard,

Sorry for the late reply, we had a short week last week.

This makes a lot of sense, yes. Unfortunately I think it's unlikely that
I'll have a chance to work on this in time for your freeze b/c my wife
is due on Friday and as much as I'd like to thin that I'll be able to
get some free time during paternity leave to do some kernel hacking,
everyone tells me I'm crazy to think that.

I *think* I'm the only one who's been asking for this feature, so
hopefully my not getting to it won't be putting anyone out.

Cheers,
peter

> Let's deal later with namespaces, containers, mounts, chroots, bind
> mounts, etc...

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

[Richard Guy Briggs]

On Sun, Jul 07, 2013 at 03:41:41PM -0700, Peter Moody wrote:
> 
> On Wed, Jul 03 2013 at 19:48, Richard Guy Briggs wrote:
> > On Thu, Aug 23, 2012 at 12:24:00PM -0700, Peter Moody wrote:
> >> This adds the ability audit the actions of a not-yet-running process,
> >> as well as the children of a not-yet-running process.
> >
> > Hi Peter,
> >
> > I've gone back over the discussion of this feature and some of the
> > background in the past couple of years on this list...
> >
> > We've got a kernel deadline coming up in the next month if we want to
> > get something included in RHEL7 if you have the interest and time to
> > evolve this patch (the userspace patch can follow...).
> >
> > As has been discussed, passing in an inode reference is incomplete,
> > since it would need to be qualified by a device reference at minimum.
> > And even then, it isn't atomic and could change by the time the kernel
> > even sees this rule request.
> >
> > So, the next step is to convert the path to a device/inode in the kernel.  If
> > this is done at the time of registering the filter rule, if/when the
> > rule is invalidated then the rule would be dropped, logged.  It also
> > means that anything else also hardlinked to it would be acted upon.
> >
> > Going one step further, if instead we can arrange an fsnotify() hook on
> > rule registration, we could act on that path when it is executed,
> > renamed, unlinked (and destroyed if the refcount goes to zero), etc.
> >
> > So, it should be passed as a path, logging the rule addition with path
> > only at first.  When the rule is triggered then log the requested path,
> > effective path, device/inode along with the user context.
> >
> > The user, carefully crafting other rules can give other information.
> >
> > A watch on the containing directory (/usr/bin) could help in case that
> > executable pathname disappears and re-appears since the containing
> > directory is less likely to go away, but it will be noisy.
> >
> > Does all this make sense?
> 
> Hey Richard,
> 
> Sorry for the late reply, we had a short week last week.

No worries.

> This makes a lot of sense, yes. Unfortunately I think it's unlikely that
> I'll have a chance to work on this in time for your freeze b/c my wife
> is due on Friday and as much as I'd like to thin that I'll be able to
> get some free time during paternity leave to do some kernel hacking,
> everyone tells me I'm crazy to think that.

A bit delusional, yes.  First child, I gather.  Welcome to parenting.
It is quite a ride.  It can be a lot of fun.  :)

> I *think* I'm the only one who's been asking for this feature, so
> hopefully my not getting to it won't be putting anyone out.

What's your timeline and availability?

> Cheers,
> peter
> 
> > Let's deal later with namespaces, containers, mounts, chroots, bind
> > mounts, etc...

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

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

[Steve Grubb]

On Wednesday, July 03, 2013 10:48:56 PM Richard Guy Briggs wrote:
> I've gone back over the discussion of this feature and some of the
> background in the past couple of years on this list...
> 
> We've got a kernel deadline coming up in the next month if we want to
> get something included in RHEL7 if you have the interest and time to
> evolve this patch (the userspace patch can follow...).
> 
> As has been discussed, passing in an inode reference is incomplete,
> since it would need to be qualified by a device reference at minimum.
> And even then, it isn't atomic and could change by the time the kernel
> even sees this rule request.
> 
> So, the next step is to convert the path to a device/inode in the kernel. 
> If this is done at the time of registering the filter rule, if/when the
> rule is invalidated then the rule would be dropped, logged.  It also means
> that anything else also hardlinked to it would be acted upon.
> 
> Going one step further, if instead we can arrange an fsnotify() hook on
> rule registration, we could act on that path when it is executed,
> renamed, unlinked (and destroyed if the refcount goes to zero), etc.
> 
> So, it should be passed as a path, logging the rule addition with path
> only at first.  When the rule is triggered then log the requested path,
> effective path, device/inode along with the user context.
> 
> The user, carefully crafting other rules can give other information.

This sounds like how I would expect it to be. You pass a path and the kernel 
converts is very much like filesystem watches to device/inode. But the next 
step is when the execve syscall matches with the path, then the rule is 
further converted to have the pid instead of the inode/device. This is 
basically following the established pattern we already have for watching files.

 
> A watch on the containing directory (/usr/bin) could help in case that
> executable pathname disappears and re-appears since the containing
> directory is less likely to go away, but it will be noisy.

Not sure this is necessary. If this is done for file system watches, then its 
an established pattern and we should continue to follow it.

-Steve

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

[Steve Grubb]

On Sunday, July 07, 2013 15:41:41 Peter Moody wrote:
>I *think* I'm the only one who's been asking for this feature, so
>hopefully my not getting to it won't be putting anyone out.


The reason that this is needed is that what we have available for auditing 
strange problems that a particular program might have is the 
equivalent of audit by inode. You have to have the pid in order to write a 
rule. Another invocation and we need a new rule. This feature would allow you 
to do investigations like:

- give me all EPERM events generated by apache.
- give me all files opened by gnash
- give me all execve calls made by bind
- record any time sendmail fails to change uid
- exclude any opens with ENOENT by top secret processes  <- real important

-Steve

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

[Steve Grubb]

On Tuesday, July 09, 2013 03:03:59 PM Steve Grubb wrote:
> On Sunday, July 07, 2013 15:41:41 Peter Moody wrote:
> >I *think* I'm the only one who's been asking for this feature, so
> >hopefully my not getting to it won't be putting anyone out.
> 
> The reason that this is needed is that what we have available for auditing
> strange problems that a particular program might have is the
> equivalent of audit by inode. You have to have the pid in order to write a
> rule. Another invocation and we need a new rule. This feature would allow
> you to do investigations like:
> 
> - give me all EPERM events generated by apache.
> - give me all files opened by gnash
> - give me all execve calls made by bind
> - record any time sendmail fails to change uid
> - exclude any opens with ENOENT by top secret processes  <- real important

Another use case someone asked for this week:

- Give me all files transferred by scp.


-Steve

[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