Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made. Normalize x_table function declarations so that git context diff function labels work as expected. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/netfilter/x_tables.c | 43 ++++++++++++++++++------------------------- 1 files changed, 18 insertions(+), 25 deletions(-) diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 14857af..99c27ed 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -81,8 +81,7 @@ int xt_register_target(struct xt_target *target) } EXPORT_SYMBOL(xt_register_target); -void -xt_unregister_target(struct xt_target *target) +void xt_unregister_target(struct xt_target *target) { u_int8_t af = target->family; @@ -92,8 +91,7 @@ int xt_register_target(struct xt_target *target) } EXPORT_SYMBOL(xt_unregister_target); -int -xt_register_targets(struct xt_target *target, unsigned int n) +int xt_register_targets(struct xt_target *target, unsigned int n) { unsigned int i; int err = 0; @@ -112,8 +110,7 @@ int xt_register_target(struct xt_target *target) } EXPORT_SYMBOL(xt_register_targets); -void -xt_unregister_targets(struct xt_target *target, unsigned int n) +void xt_unregister_targets(struct xt_target *target, unsigned int n) { while (n-- > 0) xt_unregister_target(&target[n]); @@ -131,8 +128,7 @@ int xt_register_match(struct xt_match *match) } EXPORT_SYMBOL(xt_register_match); -void -xt_unregister_match(struct xt_match *match) +void xt_unregister_match(struct xt_match *match) { u_int8_t af = match->family; @@ -142,8 +138,7 @@ int xt_register_match(struct xt_match *match) } EXPORT_SYMBOL(xt_unregister_match); -int -xt_register_matches(struct xt_match *match, unsigned int n) +int xt_register_matches(struct xt_match *match, unsigned int n) { unsigned int i; int err = 0; @@ -162,8 +157,7 @@ int xt_register_match(struct xt_match *match) } EXPORT_SYMBOL(xt_register_matches); -void -xt_unregister_matches(struct xt_match *match, unsigned int n) +void xt_unregister_matches(struct xt_match *match, unsigned int n) { while (n-- > 0) xt_unregister_match(&match[n]); @@ -205,8 +199,8 @@ struct xt_match *xt_find_match(u8 af, const char *name, u8 revision) } EXPORT_SYMBOL(xt_find_match); -struct xt_match * -xt_request_find_match(uint8_t nfproto, const char *name, uint8_t revision) +struct xt_match *xt_request_find_match(uint8_t nfproto, const char *name, + uint8_t revision) { struct xt_match *match; @@ -382,8 +376,8 @@ int xt_find_revision(u8 af, const char *name, u8 revision, int target, } EXPORT_SYMBOL_GPL(xt_find_revision); -static char * -textify_hooks(char *buf, size_t size, unsigned int mask, uint8_t nfproto) +static char *textify_hooks(char *buf, size_t size, unsigned int mask, + uint8_t nfproto) { static const char *const inetbr_names[] = { "PREROUTING", "INPUT", "FORWARD", @@ -1154,11 +1148,10 @@ static int xt_jumpstack_alloc(struct xt_table_info *i) return 0; } -struct xt_table_info * -xt_replace_table(struct xt_table *table, - unsigned int num_counters, - struct xt_table_info *newinfo, - int *error) +struct xt_table_info *xt_replace_table(struct xt_table *table, + unsigned int num_counters, + struct xt_table_info *newinfo, + int *error) { struct xt_table_info *private; int ret; @@ -1367,7 +1360,7 @@ enum { }; static void *xt_mttg_seq_next(struct seq_file *seq, void *v, loff_t *ppos, - bool is_target) + bool is_target) { static const uint8_t next_class[] = { [MTTG_TRAV_NFP_UNSPEC] = MTTG_TRAV_NFP_SPEC, @@ -1407,7 +1400,7 @@ enum { } static void *xt_mttg_seq_start(struct seq_file *seq, loff_t *pos, - bool is_target) + bool is_target) { struct nf_mttg_trav *trav = seq->private; unsigned int j; @@ -1553,8 +1546,8 @@ static int xt_target_open(struct inode *inode, struct file *file) * This function will create the nf_hook_ops that the x_table needs * to hand to xt_hook_link_net(). */ -struct nf_hook_ops * -xt_hook_ops_alloc(const struct xt_table *table, nf_hookfn *fn) +struct nf_hook_ops *xt_hook_ops_alloc(const struct xt_table *table, + nf_hookfn *fn) { unsigned int hook_mask = table->valid_hooks; uint8_t i, num_hooks = hweight32(hook_mask); -- 1.7.1

There were syscall events unsolicited by any audit rule caused by a missing !audit_dummy_context() check before creating an iptables/ip6tables/arptables/ebtables NETFILTER_CFG record. Check !audit_dummy_context() before creating the NETFILTER_CFG record. The vast majority of observed unaccompanied records are caused by the fedora default rule: "-a never,task" and the occasional early startup one is I believe caused by the iptables filter table module hard linked into the kernel rather than a loadable module. The !audit_dummy_context() check above should avoid them. Audit only when there is an existing syscall audit rule, otherwise issue a standalone record only on table modification rather than empty table creation. Add subject attributes to the new standalone NETFILTER_CFGSOLO record using a newly exported audit_log_task(). Since the record format will change anyways, this seemed like the right time to change the order of the fields to put the protocol family before the table name. Here is a new sample accompanied record: type=NETFILTER_CFG msg=audit(1494977049.375:9418): family=2 table=filter entries=84 and unaccompanied case: type=UNKNOWN[1331] msg=audit(1494815998.168:163): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:iptables_t:s0 pid=597 comm="iptables-restor" exe="/usr/sbin/xtables-multi" family=2 table=filter entries=4 See: https://github.com/linux-audit/audit-kernel/issues/25 See: https://github.com/linux-audit/audit-kernel/issues/35 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- include/linux/audit.h | 4 +++- include/uapi/linux/audit.h | 1 + kernel/auditsc.c | 3 ++- net/bridge/netfilter/ebtables.c | 25 +++++++++++++++++++------ net/netfilter/x_tables.c | 26 +++++++++++++++++++------- 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 2150bdc..b6fcab1 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -155,7 +155,7 @@ extern void audit_log_link_denied(const char *operation, static inline void audit_log_secctx(struct audit_buffer *ab, u32 secid) { } #endif - +extern void audit_log_task(struct audit_buffer *ab); extern int audit_log_task_context(struct audit_buffer *ab); extern void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk); @@ -205,6 +205,8 @@ static inline void audit_log_link_denied(const char *string, { } static inline void audit_log_secctx(struct audit_buffer *ab, u32 secid) { } +static inline void audit_log_task(struct audit_buffer *ab) +{ } static inline int audit_log_task_context(struct audit_buffer *ab) { return 0; diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 0714a66..8bee3f5 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -112,6 +112,7 @@ #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes */ #define AUDIT_REPLACE 1329 /* Replace auditd if this packet unanswerd */ #define AUDIT_KERN_MODULE 1330 /* Kernel Module events */ +#define AUDIT_NETFILTER_CFGSOLO 1331 /* Netfilter chain modifications standalone */ #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ diff --git a/kernel/auditsc.c b/kernel/auditsc.c index b2dcbe6..8ac38e6 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2383,7 +2383,7 @@ void __audit_log_kern_module(char *name) context->type = AUDIT_KERN_MODULE; } -static void audit_log_task(struct audit_buffer *ab) +void audit_log_task(struct audit_buffer *ab) { kuid_t auid, uid; kgid_t gid; @@ -2404,6 +2404,7 @@ static void audit_log_task(struct audit_buffer *ab) audit_log_untrustedstring(ab, get_task_comm(comm, current)); audit_log_d_path_exe(ab, current->mm); } +EXPORT_SYMBOL_GPL(audit_log_task); /** * audit_core_dumps - record information about processes that end abnormally diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 13d7fe2..743f9e6 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1071,12 +1071,25 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl, if (audit_enabled) { struct audit_buffer *ab; - ab = audit_log_start(current->audit_context, GFP_KERNEL, - AUDIT_NETFILTER_CFG); - if (ab) { - audit_log_format(ab, "table=%s family=%u entries=%u", - repl->name, AF_BRIDGE, repl->nentries); - audit_log_end(ab); + if(!audit_dummy_context()) { + ab = audit_log_start(current->audit_context, GFP_KERNEL, + AUDIT_NETFILTER_CFG); + if (ab) { + audit_log_format(ab, "family=%u table=%s entries=%u", + AF_BRIDGE, repl->name, + repl->nentries); + audit_log_end(ab); + } + } else if(repl->nentries) { + ab = audit_log_start(NULL, GFP_KERNEL, + AUDIT_NETFILTER_CFGSOLO); + if (ab) { + audit_log_task(ab); + audit_log_format(ab, " family=%u table=%s entries=%u", + AF_BRIDGE, repl->name, + repl->nentries); + audit_log_end(ab); + } } } #endif diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 99c27ed..8d28fff 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1195,13 +1195,25 @@ struct xt_table_info *xt_replace_table(struct xt_table *table, if (audit_enabled) { struct audit_buffer *ab; - ab = audit_log_start(current->audit_context, GFP_KERNEL, - AUDIT_NETFILTER_CFG); - if (ab) { - audit_log_format(ab, "table=%s family=%u entries=%u", - table->name, table->af, - private->number); - audit_log_end(ab); + if(!audit_dummy_context()) { + ab = audit_log_start(current->audit_context, GFP_KERNEL, + AUDIT_NETFILTER_CFG); + if (ab) { + audit_log_format(ab, "family=%u table=%s entries=%u", + table->af, table->name, + private->number); + audit_log_end(ab); + } + } else if(private->stacksize || private->number || private->initial_entries) { + ab = audit_log_start(NULL, GFP_KERNEL, + AUDIT_NETFILTER_CFGSOLO); + if (ab) { + audit_log_task(ab); + audit_log_format(ab, " family=%u table=%s entries=%u", + table->af, table->name, + private->number); + audit_log_end(ab); + } } } #endif -- 1.7.1

On 2017-05-24 19:36, Pablo Neira Ayuso wrote: Interesting you brought that up. I did another revision that stores this information in a struct audit_context and greatly simplifies the code in netfilter and re-uses code in audit itself, which may be a better way to go, but that idea needed to settle a bit more before seeing peer review. I'm also having doubts about two record types. > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > > index 0714a66..8bee3f5 100644 > > --- a/include/uapi/linux/audit.h > > +++ b/include/uapi/linux/audit.h > > @@ -112,6 +112,7 @@ > > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes */ > > #define AUDIT_REPLACE 1329 /* Replace auditd if this packet unanswerd */ > > #define AUDIT_KERN_MODULE 1330 /* Kernel Module events */ > > +#define AUDIT_NETFILTER_CFGSOLO 1331 /* Netfilter chain modifications standalone */ > > > > #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ > > #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index b2dcbe6..8ac38e6 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -2383,7 +2383,7 @@ void __audit_log_kern_module(char *name) > > context->type = AUDIT_KERN_MODULE; > > } > > > > -static void audit_log_task(struct audit_buffer *ab) > > +void audit_log_task(struct audit_buffer *ab) > > { > > kuid_t auid, uid; > > kgid_t gid; > > @@ -2404,6 +2404,7 @@ static void audit_log_task(struct audit_buffer *ab) > > audit_log_untrustedstring(ab, get_task_comm(comm, current)); > > audit_log_d_path_exe(ab, current->mm); > > } > > +EXPORT_SYMBOL_GPL(audit_log_task); > > > > /** > > * audit_core_dumps - record information about processes that end abnormally > > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c > > index 13d7fe2..743f9e6 100644 > > --- a/net/bridge/netfilter/ebtables.c > > +++ b/net/bridge/netfilter/ebtables.c > > @@ -1071,12 +1071,25 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl, > > if (audit_enabled) { > > struct audit_buffer *ab; > > > > - ab = audit_log_start(current->audit_context, GFP_KERNEL, > > - AUDIT_NETFILTER_CFG); > > - if (ab) { > > - audit_log_format(ab, "table=%s family=%u entries=%u", > > - repl->name, AF_BRIDGE, repl->nentries); > > - audit_log_end(ab); > > + if(!audit_dummy_context()) { > > + ab = audit_log_start(current->audit_context, GFP_KERNEL, > > + AUDIT_NETFILTER_CFG); > > + if (ab) { > > + audit_log_format(ab, "family=%u table=%s entries=%u", > > + AF_BRIDGE, repl->name, > > + repl->nentries); > > + audit_log_end(ab); > > + } > > + } else if(repl->nentries) { > > + ab = audit_log_start(NULL, GFP_KERNEL, > > + AUDIT_NETFILTER_CFGSOLO); > > + if (ab) { > > + audit_log_task(ab); > > + audit_log_format(ab, " family=%u table=%s entries=%u", > > + AF_BRIDGE, repl->name, > > + repl->nentries); > > + audit_log_end(ab); > > + } > If you have no other way, probably it is a good idea to wrap this code > around a function, eg. ebt_audit_replace_finish() ? Yes, sending the values I need into the audit subsystem to avoid exposing some bits of audit did come to mind. And it would bring 6 lines here down to one instead of adding 19. - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635

Generate audit NETFILTER_CFG records on ebtables table registration. Previously this was only being done for all x_tables operations and ebtables table replacement. Audit only when there is an existing syscall audit rule, otherwise issue a standalone record only on table modification rather than empty table creation. Include subject attributes to the new standalone NETFILTER_CFGSOLO record using audit_log_task(). Here is a sample accompanied record: type=NETFILTER_CFG msg=audit(1494907217.558:5403): family=7 table=filter entries=0 and unaccompanied case: type=UNKNOWN[1331] msg=audit(1494723394.832:111): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:iptables_t:s0 pid=556 comm="ebtables-restor" exe="/usr/sbin/ebtables-restore" family=7 table=broute entries=1 See: https://github.com/linux-audit/audit-kernel/issues/43 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/bridge/netfilter/ebtables.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 743f9e6..7499232 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1251,6 +1251,32 @@ struct ebt_table * ebt_register_table(struct net *net, } list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]); mutex_unlock(&ebt_mutex); +#ifdef CONFIG_AUDIT + if (audit_enabled) { + struct audit_buffer *ab; + + if(!audit_dummy_context()) { + ab = audit_log_start(current->audit_context, GFP_KERNEL, + AUDIT_NETFILTER_CFG); + if (ab) { + audit_log_format(ab, "family=%u table=%s entries=%u", + AF_BRIDGE, repl->name, + repl->nentries); + audit_log_end(ab); + } + } else if(repl->nentries) { + ab = audit_log_start(NULL, GFP_KERNEL, + AUDIT_NETFILTER_CFGSOLO); + if (ab) { + audit_log_task(ab); + audit_log_format(ab, " family=%u table=%s entries=%u", + AF_BRIDGE, repl->name, + repl->nentries); + audit_log_end(ab); + } + } + } +#endif return table; free_unlock: mutex_unlock(&ebt_mutex); -- 1.7.1

Add the operation performed (register or replace) to the NETFILTER_CFG and NETFILTER_CFGSOLO records. Here are sample records for accompanied: type=NETFILTER_CFG msg=audit(1494981627.248:9764): op=replace family=7 table=broute entries=0 and unaccompanied cases: type=UNKNOWN[1331] msg=audit(1494815998.178:167): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:iptables_t:s0 pid=598 comm="ip6tables-resto" exe="/usr/sbin/xtables-multi" op=replace family=10 table=filter entries=4 See: https://github.com/linux-audit/audit-kernel/issues/25 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/bridge/netfilter/ebtables.c | 8 ++++---- net/netfilter/x_tables.c | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 7499232..59b63a8 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1075,7 +1075,7 @@ 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, "family=%u table=%s entries=%u", + audit_log_format(ab, "op=replace family=%u table=%s entries=%u", AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1085,7 +1085,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(ab); - audit_log_format(ab, " family=%u table=%s entries=%u", + audit_log_format(ab, " op=replace family=%u table=%s entries=%u", AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1259,7 +1259,7 @@ struct ebt_table * ebt_register_table(struct net *net, ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_NETFILTER_CFG); if (ab) { - audit_log_format(ab, "family=%u table=%s entries=%u", + audit_log_format(ab, "op=register family=%u table=%s entries=%u", AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1269,7 +1269,7 @@ struct ebt_table * ebt_register_table(struct net *net, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(ab); - audit_log_format(ab, " family=%u table=%s entries=%u", + audit_log_format(ab, " op=register family=%u table=%s entries=%u", AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d28fff..395ebd3 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1199,7 +1199,8 @@ struct xt_table_info *xt_replace_table(struct xt_table *table, ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_NETFILTER_CFG); if (ab) { - audit_log_format(ab, "family=%u table=%s entries=%u", + audit_log_format(ab, "op=%s family=%u table=%s entries=%u", + private->number ? "replace" : "register", table->af, table->name, private->number); audit_log_end(ab); @@ -1209,7 +1210,7 @@ struct xt_table_info *xt_replace_table(struct xt_table *table, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(ab); - audit_log_format(ab, " family=%u table=%s entries=%u", + audit_log_format(ab, " op=replace family=%u table=%s entries=%u", table->af, table->name, private->number); audit_log_end(ab); -- 1.7.1

Seemingly duplicate NETFILTER_CFG records are not actually exact duplicates that are caused by netfilter table initialization in different network namespaces from the same syscall. To differentiate the NETFILTER_CFG records, the network namespace ID (proc inode) was added to the record to make this source evident. Here is a sample event with accompanied records: time->Sun May 14 22:40:26 2017 type=PROCTITLE msg=audit(1494816026.072:248): proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D0065627461626C655F6E6174 type=KERN_MODULE msg=audit(1494816026.072:248): name="ebtable_nat" type=SYSCALL msg=audit(1494816026.072:248): arch=c000003e syscall=313 success=yes exit=0 a0=0 a1=55c4648d4106 a2=0 a3=0 items=0 ppid=84 pid=431 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="modprobe" exe="/usr/bin/kmod" subj=system_u:system_r:insmod_t:s0 key="mod-load" type=NETFILTER_CFG msg=audit(1494816026.072:248): op=register net=324 family=7 table=nat entries=0 type=NETFILTER_CFG msg=audit(1494816026.072:248): op=register net=121 family=7 table=nat entries=0 and unaccompanied cases: type=UNKNOWN[1331] msg=audit(1494815998.178:167): auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:iptables_t:s0 pid=598 comm="ip6tables-resto" exe="/usr/sbin/xtables-multi" op=replace net=121 family=10 table=filter entries=4 See: https://github.com/linux-audit/audit-kernel/issues/25 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- include/linux/netfilter/x_tables.h | 1 + net/bridge/netfilter/ebtables.c | 13 +++++++++---- net/ipv4/netfilter/arp_tables.c | 2 +- net/ipv4/netfilter/ip_tables.c | 2 +- net/ipv6/netfilter/ip6_tables.c | 2 +- net/netfilter/x_tables.c | 10 +++++++--- 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index be378cf..6be4a04 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -308,6 +308,7 @@ struct xt_table *xt_register_table(struct net *net, struct xt_table_info *xt_replace_table(struct xt_table *table, unsigned int num_counters, struct xt_table_info *newinfo, + struct net *net, int *error); struct xt_match *xt_find_match(u8 af, const char *name, u8 revision); 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, AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1085,7 +1087,8 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(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, AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1259,7 +1262,8 @@ struct ebt_table * ebt_register_table(struct net *net, ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_NETFILTER_CFG); if (ab) { - audit_log_format(ab, "op=register family=%u table=%s entries=%u", + audit_log_format(ab, "op=register net=%u family=%u table=%s entries=%u", + net->ns.inum - PROC_DYNAMIC_FIRST, AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); @@ -1269,7 +1273,8 @@ struct ebt_table * ebt_register_table(struct net *net, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(ab); - audit_log_format(ab, " op=register family=%u table=%s entries=%u", + audit_log_format(ab, " op=register net=%u family=%u table=%s entries=%u", + net->ns.inum - PROC_DYNAMIC_FIRST, AF_BRIDGE, repl->name, repl->nentries); audit_log_end(ab); diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 6241a81..4933a5a 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -904,7 +904,7 @@ static int __do_replace(struct net *net, const char *name, goto put_module; } - oldinfo = xt_replace_table(t, num_counters, newinfo, &ret); + oldinfo = xt_replace_table(t, num_counters, newinfo, net, &ret); if (!oldinfo) goto put_module; diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 384b857..c638607 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1066,7 +1066,7 @@ static int get_info(struct net *net, void __user *user, goto put_module; } - oldinfo = xt_replace_table(t, num_counters, newinfo, &ret); + oldinfo = xt_replace_table(t, num_counters, newinfo, net, &ret); if (!oldinfo) goto put_module; diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 1e15c54..87ef83b 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -1096,7 +1096,7 @@ static int get_info(struct net *net, void __user *user, goto put_module; } - oldinfo = xt_replace_table(t, num_counters, newinfo, &ret); + oldinfo = xt_replace_table(t, num_counters, newinfo, net, &ret); if (!oldinfo) goto put_module; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 395ebd3..35533f1 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -26,6 +26,7 @@ #include <linux/mm.h> #include <linux/slab.h> #include <linux/audit.h> +#define PROC_DYNAMIC_FIRST 0xF0000000U #include <linux/user_namespace.h> #include <net/net_namespace.h> @@ -1151,6 +1152,7 @@ static int xt_jumpstack_alloc(struct xt_table_info *i) struct xt_table_info *xt_replace_table(struct xt_table *table, unsigned int num_counters, struct xt_table_info *newinfo, + struct net *net, int *error) { struct xt_table_info *private; @@ -1199,8 +1201,9 @@ struct xt_table_info *xt_replace_table(struct xt_table *table, ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_NETFILTER_CFG); if (ab) { - audit_log_format(ab, "op=%s family=%u table=%s entries=%u", + audit_log_format(ab, "op=%s net=%u family=%u table=%s entries=%u", private->number ? "replace" : "register", + net->ns.inum - PROC_DYNAMIC_FIRST, table->af, table->name, private->number); audit_log_end(ab); @@ -1210,7 +1213,8 @@ struct xt_table_info *xt_replace_table(struct xt_table *table, AUDIT_NETFILTER_CFGSOLO); if (ab) { audit_log_task(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, table->af, table->name, private->number); audit_log_end(ab); @@ -1251,7 +1255,7 @@ struct xt_table *xt_register_table(struct net *net, /* Simplifies replace_table code. */ table->private = bootstrap; - if (!xt_replace_table(table, 0, newinfo, &ret)) + if (!xt_replace_table(table, 0, newinfo, net, &ret)) goto unlock; private = table->private; -- 1.7.1

On 2017-05-24 19:31, Pablo Neira Ayuso wrote: 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. - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635

On Wed, May 24, 2017 at 3:44 PM, Eric W. Biederman <ebiederm(a)xmission.com&gt; wrote: Let's punt on this netfilter record specific patch until we sort out the general problem of recording namespace/container information in the audit records. -- paul moore www.paul-moore.com