[PATCH] audit: don't attempt to lookup PIDs when changing PID filtering audit rules

[Richard Guy Briggs]

On 14/12/15, Eric Paris wrote:
> Lets say I and in the non-init pid namespace.
> 
> I run audictl -a exit,always -S all -F pid=1

That's easy (for now).  Line 675 of kernel/audit.c in audit_netlink_ok()
called from audit_receive_msg() will prevent that with:

	if ((task_active_pid_ns(current) != &init_pid_ns))
		return -EPERM;

> Is the audit system going to show records for what I think is pid=1 or
> what the initial pid namespace thinks is pid=1 ?

Longer term this will need to be solved if we want to run
commands requiring CAP_AUDIT_CONTROL in a container.

I've still got outstanding patches to store PIDs as struct pid rather
than pid_t, so this was part of the motivation to start that in this
code.

> Which is correct? (hint, it's impossible to know pids above my
> namespace, or even to know what pid the process in question thinks it
> is, since it could be below my namespace)
> 
> I won't pretend this is easy to solve.

At the moment, this patch will solve this problem.  It is also arguably
more necessary on AUDIT_ADD_RULE than for AUDIT_DEL_RULE.

> Steve et al.  What do you think of maybe having pid= rules automatically
> removed when the pid goes away?  I can't think of another way to handle
> this (although the perf hit might be so stupidly high....)

That makes sense, as the same is done for paths that vanish.

> On Mon, 2014-12-15 at 12:14 -0500, Paul Moore wrote:
> > Commit f1dc4867 ("audit: anchor all pid references in the initial pid
> > namespace") introduced a find_vpid() call when adding/removing audit
> > rules with PID/PPID filters; unfortunately this is problematic as
> > find_vpid() only works if there is a task with the associated PID
> > alive on the system.  The following commands demonstrate a simple
> > reproducer.
> > 
> > 	# auditctl -D
> > 	# auditctl -l
> > 	# autrace /bin/true
> > 	# auditctl -l
> > 
> > This patch resolves the problem by simply using the PID provided by
> > the user without any additional validation, e.g. no calls to check to
> > see if the task/PID exists.
> > 
> > Cc: stable at vger.kernel.org # 3.15
> > Cc: Richard Guy Briggs <rgb at redhat.com>
> > Signed-off-by: Paul Moore <pmoore at redhat.com>
> > ---
> >  kernel/auditfilter.c |   13 -------------
> >  1 file changed, 13 deletions(-)
> > 
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 8e9bc9c..b2e63ba 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -433,19 +433,6 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
> >  			f->val = 0;
> >  		}
> >  
> > -		if ((f->type == AUDIT_PID) || (f->type == AUDIT_PPID)) {
> > -			struct pid *pid;
> > -			rcu_read_lock();
> > -			pid = find_vpid(f->val);
> > -			if (!pid) {
> > -				rcu_read_unlock();
> > -				err = -ESRCH;
> > -				goto exit_free;
> > -			}
> > -			f->val = pid_nr(pid);
> > -			rcu_read_unlock();
> > -		}
> > -
> >  		err = audit_field_valid(entry, f);
> >  		if (err)
> >  			goto exit_free;
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit at redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> 
> 

- RGB

--
Richard Guy Briggs <rbriggs at redhat.com>
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