Re: [PATCH 6/6 RFC] netfilter: add audit netns ID

[ebiederm@xmission.com (Eric W. Biederman)]

Richard Guy Briggs <rgb@redhat.com> writes:

> On 2017-05-24 19:31, Pablo Neira Ayuso wrote:
>> Cc'ing Eric Biederman.
>> 
>> On Thu, May 18, 2017 at 01:21:52PM -0400, Richard Guy Briggs wrote:
>> > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
>> > index 59b63a8..0f77b2a 100644
>> > --- a/net/bridge/netfilter/ebtables.c
>> > +++ b/net/bridge/netfilter/ebtables.c
>> > @@ -27,6 +27,7 @@
>> >  #include <linux/smp.h>
>> >  #include <linux/cpumask.h>
>> >  #include <linux/audit.h>
>> > +#define PROC_DYNAMIC_FIRST 0xF0000000U
>> >  #include <net/sock.h>
>> >  /* needed for logical [in,out]-dev filtering */
>> >  #include "../br_private.h"
>> > @@ -1075,7 +1076,8 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
>> >  			ab = audit_log_start(current->audit_context, GFP_KERNEL,
>> >  					     AUDIT_NETFILTER_CFG);
>> >  			if (ab) {
>> > -				audit_log_format(ab, "op=replace family=%u table=%s entries=%u",
>> > +				audit_log_format(ab, "op=replace net=%u family=%u table=%s entries=%u",
>> > +						 net->ns.inum - PROC_DYNAMIC_FIRST,
>> 
>> IIRC, there was a discussion on exposing netns i-node number to
>> userspace time ago on netdev and Eric Biederman was not happy about
>> this?
>
> He was not happy about it being exposed in the /proc filesystem.  We've
> been talking since then and while we've not come to a definitive
> conclusion there is a communication channel open.
>
> This is more of an RFC patch than the rest of this set and I didn't
> seriously expect this one to be accepted, I did want to present the idea
> to see if there were concerns or better ideas generated how to
> differentiate this record from a seemingly identical one.  The only
> other ID would be the network namespace' struct pointer.
>
> At this stage, one thing that is missing is a device number to qualify
> this namespace ID.
>
> Once I started printing the namespace proc inode number (minus the
> starting offset) in decimal, it was very clear what was happenning and
> seemed worth sharing that debugging tool patch.

If the appropriate device number and full inode number is included I
don't have any deep problems with the idea.  I don't like the bare inode
number as we have had in the past and may in the future have these inode
numbers in multiple filesystems so the inode number by itself is not
unique.

Eric