Re: [PATCH] audit: add nspid and nsppid in audit_log_task_info

Re: [PATCH] audit: add nspid and nsppid in audit_log_task_info

[Paul Moore]

On Sunday, November 23, 2014 09:58:48 AM Eric Paris wrote:
> [forwarding to 2 people looking at audit now, do you mind resending to
> linux-audit@redhat.com and inluding them both?]

I'm also adding the linux-audit list to the CC line.

I know Richard has been working on namespaces/audit, I'd like to hear his 
comments on this patch.

> On Sat, 2014-11-22 at 13:53 -0500, Mark Ellzey wrote:
> > If the current task being sent to audit_log_task_info() is not within
> > the root namespace, add two new fields "nspid=X nsppid=Y".
> > 
> > This allows a user to map the real pid/ppid to a namespaced pid/ppid.
> > ---
> > 
> >  kernel/audit.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index cebb11d..5439f66 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1853,6 +1853,7 @@ void audit_log_task_info(struct audit_buffer
> > *ab, struct task_struct *tsk)
> > 
> >   char comm[sizeof(tsk->comm)];
> >   struct mm_struct *mm = tsk->mm;
> >   char *tty;
> > 
> > + struct pid_namespace * pns;
> > 
> >   if (!ab)
> >   return;
> > 
> > @@ -1865,8 +1866,19 @@ void audit_log_task_info(struct audit_buffer
> > *ab, struct task_struct *tsk)
> > 
> >   tty = tsk->signal->tty->name;
> >   else
> >   tty = "(none)";
> > 
> > +
> > 
> >   spin_unlock_irq(&tsk->sighand->siglock);
> > 
> > + if ((pns = task_active_pid_ns(tsk)) != &init_pid_ns) {
> > +    pid_t nsppid = 0;
> > +    pid_t nspid  = 0;
> > +
> > +    nsppid = task_ppid_nr_ns(tsk, pns);
> > +    nspid  = task_pid_nr_ns(tsk, pns);
> > +
> > +    audit_log_format(ab, " nsppid=%d nspid=%d", nsppid, nspid);
> > + }
> > +
> > 
> >   audit_log_format(ab,
> >   " ppid=%d pid=%d auid=%u uid=%u gid=%u"
> >   " euid=%u suid=%u fsuid=%u"
> > 
> > --
> > 1.9.1

-- 
paul moore
security and virtualization @ redhat

Re: [PATCH] audit: add nspid and nsppid in audit_log_task_info

[Richard Guy Briggs]

On 14/12/03, Paul Moore wrote:
> On Sunday, November 23, 2014 09:58:48 AM Eric Paris wrote:
> > [forwarding to 2 people looking at audit now, do you mind resending to
> > linux-audit@redhat.com and inluding them both?]
> 
> I'm also adding the linux-audit list to the CC line.
> 
> I know Richard has been working on namespaces/audit, I'd like to hear his 
> comments on this patch.

At first when I saw this, I wondered if it was even necessary, thinking
that information should either be irrelevant, or available elsewhere.

Given that it could be several nested pid namespaces, it may even be
incomplete.

The most obvious one is that of vanishing fields in audit log messages
which concerns Steve Grubb.  If we fixed the ordering issue, vanishing
fields should no longer be a concern.

> > On Sat, 2014-11-22 at 13:53 -0500, Mark Ellzey wrote:
> > > If the current task being sent to audit_log_task_info() is not within
> > > the root namespace, add two new fields "nspid=X nsppid=Y".
> > > 
> > > This allows a user to map the real pid/ppid to a namespaced pid/ppid.
> > > ---
> > > 
> > >  kernel/audit.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index cebb11d..5439f66 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1853,6 +1853,7 @@ void audit_log_task_info(struct audit_buffer
> > > *ab, struct task_struct *tsk)
> > > 
> > >   char comm[sizeof(tsk->comm)];
> > >   struct mm_struct *mm = tsk->mm;
> > >   char *tty;
> > > 
> > > + struct pid_namespace * pns;
> > > 
> > >   if (!ab)
> > >   return;
> > > 
> > > @@ -1865,8 +1866,19 @@ void audit_log_task_info(struct audit_buffer
> > > *ab, struct task_struct *tsk)
> > > 
> > >   tty = tsk->signal->tty->name;
> > >   else
> > >   tty = "(none)";
> > > 
> > > +
> > > 
> > >   spin_unlock_irq(&tsk->sighand->siglock);
> > > 
> > > + if ((pns = task_active_pid_ns(tsk)) != &init_pid_ns) {
> > > +    pid_t nsppid = 0;
> > > +    pid_t nspid  = 0;
> > > +
> > > +    nsppid = task_ppid_nr_ns(tsk, pns);
> > > +    nspid  = task_pid_nr_ns(tsk, pns);
> > > +
> > > +    audit_log_format(ab, " nsppid=%d nspid=%d", nsppid, nspid);
> > > + }
> > > +
> > > 
> > >   audit_log_format(ab,
> > >   " ppid=%d pid=%d auid=%u uid=%u gid=%u"
> > >   " euid=%u suid=%u fsuid=%u"
> > > 
> > > --
> > > 1.9.1
> 
> paul moore

- RGB

--
Richard Guy Briggs <rbriggs@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

Re: [PATCH] audit: add nspid and nsppid in audit_log_task_info

[Paul Moore]

On Wednesday, December 03, 2014 10:14:32 PM Richard Guy Briggs wrote:
> On 14/12/03, Paul Moore wrote:
> > On Sunday, November 23, 2014 09:58:48 AM Eric Paris wrote:
> > > [forwarding to 2 people looking at audit now, do you mind resending to
> > > linux-audit@redhat.com and inluding them both?]
> > 
> > I'm also adding the linux-audit list to the CC line.
> > 
> > I know Richard has been working on namespaces/audit, I'd like to hear his
> > comments on this patch.
> 
> At first when I saw this, I wondered if it was even necessary, thinking
> that information should either be irrelevant, or available elsewhere.
> 
> Given that it could be several nested pid namespaces, it may even be
> incomplete.

Okay, thanks for the input.  It doesn't look like this is something we want to 
merge at this point.
 
> The most obvious one is that of vanishing fields in audit log messages
> which concerns Steve Grubb.  If we fixed the ordering issue, vanishing
> fields should no longer be a concern.

Yes, this is just one more reason why we need to rework the audit record 
format.  I've got more ideas on this since we last talked on-list, but I've 
had to shelve things a bit to deal with the audit bugs.

However, make no mistake, the audit record format will be changing, this fixed 
string format is garbage.

-- 
paul moore
security and virtualization @ redhat