Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues
@ 2020-01-06 18:54 Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

There were questions about the presence and cause of unsolicited syscall events
in the logs containing NETFILTER_CFG records and sometimes unaccompanied
NETFILTER_CFG records.

During testing at least the following list of events trigger NETFILTER_CFG
records and the syscalls related (There may be more events that will trigger
this message type.):
	init_module, finit_module: modprobe
	setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
	unshare: (h?)ostnamed
	clone: libvirtd

The syscall events unsolicited by any audit rule were found to be caused by a
missing !audit_dummy_context() check before creating a NETFILTER_CFG
record and issuing the record immediately rather than saving the
information to create the record at syscall exit.
Check !audit_dummy_context() before creating the NETFILTER_CFG record.

The vast majority of 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.

A couple of other factors should help eliminate unaccompanied records
which include commit cb74ed278f80 ("audit: always enable syscall
auditing when supported and audit is enabled") which makes sure that
when audit is enabled, so automatically is syscall auditing, and ghak66
which addressed initializing audit before PID 1.

Ebtables module initialization to register tables doesn't generate records
because it was never hooked in to audit.  Recommend adding audit hooks to log
this.

Table unregistration was never logged, which is now covered.

Seemingly duplicate records are not actually exact duplicates that are caused
by netfilter table initialization in different network namespaces from the same
syscall.  Recommend adding the network namespace ID (proc inode and dev)
to the record to make this obvious (address later with ghak79 after nsid
patches).

See: https://github.com/linux-audit/audit-kernel/issues/25
See: https://github.com/linux-audit/audit-kernel/issues/35
See: https://github.com/linux-audit/audit-kernel/issues/43
See: https://github.com/linux-audit/audit-kernel/issues/44

Changelog:
v2
- Rebase (audit/next 5.5-rc1) to get audit_context access and ebt_register_table ret code
- Split x_tables and ebtables updates
- Check audit_dummy_context
- Store struct audit_nfcfg params in audit_context, abstract to audit_nf_cfg() call
- Restore back to "table, family, entries" from "family, table, entries"
- Log unregistration of tables
- Add "op=" at the end of the AUDIT_NETFILTER_CFG record
- Defer nsid patch (ghak79) to once nsid patchset upstreamed (ghak32)
- Add ghak refs
- Ditch NETFILTER_CFGSOLO record

Richard Guy Briggs (9):
  netfilter: normalize x_table function declarations
  netfilter: normalize ebtables function declarations
  netfilter: normalize ebtables function declarations II
  audit: record nfcfg params
  netfilter: x_tables audit only on syscall rule
  netfilter: ebtables audit only on syscall rule
  netfilter: ebtables audit table registration
  netfilter: add audit operation field
  netfilter: audit table unregister actions

 include/linux/audit.h           |  11 ++++
 kernel/auditsc.c                |  18 +++++
 net/bridge/netfilter/ebtables.c | 142 ++++++++++++++++++++--------------------
 net/netfilter/x_tables.c        |  56 +++++++---------
 4 files changed, 124 insertions(+), 103 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 20:23   ` Florian Westphal
  2020-01-08 16:47   ` Nicolas Dichtel
  2020-01-06 18:54 ` [PATCH ghak25 v2 2/9] netfilter: normalize ebtables " Richard Guy Briggs
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

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@redhat.com>
---
 net/netfilter/x_tables.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index ce70c2576bb2..0094853ab42a 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -77,8 +77,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;
 
@@ -88,8 +87,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;
@@ -108,8 +106,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]);
@@ -127,8 +124,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;
 
@@ -138,8 +134,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;
@@ -158,8 +153,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]);
@@ -204,8 +198,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(u8 nfproto, const char *name,
+				       u8 revision)
 {
 	struct xt_match *match;
 
@@ -391,8 +385,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",
@@ -1349,11 +1343,10 @@ struct xt_counters *xt_counters_alloc(unsigned int counters)
 }
 EXPORT_SYMBOL(xt_counters_alloc);
 
-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;
 	unsigned int cpu;
@@ -1542,7 +1535,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,
@@ -1583,7 +1576,7 @@ static void *xt_mttg_seq_next(struct seq_file *seq, void *v, loff_t *ppos,
 }
 
 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;
@@ -1692,8 +1685,8 @@ static int xt_target_seq_show(struct seq_file *seq, void *v)
  * 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.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 2/9] netfilter: normalize ebtables function declarations
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
@ 2020-01-06 18:54 ` " Richard Guy Briggs
  2020-01-06 20:30   ` Florian Westphal
  2020-01-06 18:54 ` [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II Richard Guy Briggs
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

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 ebtables function declarations so that git context diff
function labels work as expected.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 net/bridge/netfilter/ebtables.c | 100 ++++++++++++++++++++--------------------
 1 file changed, 51 insertions(+), 49 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 4096d8a74a2b..c9dff9e11ddb 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -76,9 +76,9 @@ static int ebt_standard_compat_to_user(void __user *dst, const void *src)
 #endif
 };
 
-static inline int
-ebt_do_watcher(const struct ebt_entry_watcher *w, struct sk_buff *skb,
-	       struct xt_action_param *par)
+static inline int ebt_do_watcher(const struct ebt_entry_watcher *w,
+				 struct sk_buff *skb,
+				 struct xt_action_param *par)
 {
 	par->target   = w->u.watcher;
 	par->targinfo = w->data;
@@ -87,17 +87,17 @@ static int ebt_standard_compat_to_user(void __user *dst, const void *src)
 	return 0;
 }
 
-static inline int
-ebt_do_match(struct ebt_entry_match *m, const struct sk_buff *skb,
-	     struct xt_action_param *par)
+static inline int ebt_do_match(struct ebt_entry_match *m,
+			       const struct sk_buff *skb,
+			       struct xt_action_param *par)
 {
 	par->match     = m->u.match;
 	par->matchinfo = m->data;
 	return !m->u.match->match(skb, par);
 }
 
-static inline int
-ebt_dev_check(const char *entry, const struct net_device *device)
+static inline int ebt_dev_check(const char *entry,
+				const struct net_device *device)
 {
 	int i = 0;
 	const char *devname;
@@ -114,9 +114,10 @@ static int ebt_standard_compat_to_user(void __user *dst, const void *src)
 }
 
 /* process standard matches */
-static inline int
-ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb,
-		const struct net_device *in, const struct net_device *out)
+static inline int ebt_basic_match(const struct ebt_entry *e,
+				  const struct sk_buff *skb,
+				  const struct net_device *in,
+				  const struct net_device *out)
 {
 	const struct ethhdr *h = eth_hdr(skb);
 	const struct net_bridge_port *p;
@@ -163,14 +164,12 @@ static int ebt_standard_compat_to_user(void __user *dst, const void *src)
 	return 0;
 }
 
-static inline
-struct ebt_entry *ebt_next_entry(const struct ebt_entry *entry)
+static inline struct ebt_entry *ebt_next_entry(const struct ebt_entry *entry)
 {
 	return (void *)entry + entry->next_offset;
 }
 
-static inline const struct ebt_entry_target *
-ebt_get_target_c(const struct ebt_entry *e)
+static inline const struct ebt_entry_target *ebt_get_target_c(const struct ebt_entry *e)
 {
 	return ebt_get_target((struct ebt_entry *)e);
 }
@@ -304,9 +303,9 @@ unsigned int ebt_do_table(struct sk_buff *skb,
 }
 
 /* If it succeeds, returns element and locks mutex */
-static inline void *
-find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
-			struct mutex *mutex)
+static inline void *find_inlist_lock_noload(struct list_head *head,
+					    const char *name, int *error,
+					    struct mutex *mutex)
 {
 	struct {
 		struct list_head list;
@@ -323,18 +322,18 @@ unsigned int ebt_do_table(struct sk_buff *skb,
 	return NULL;
 }
 
-static void *
-find_inlist_lock(struct list_head *head, const char *name, const char *prefix,
-		 int *error, struct mutex *mutex)
+static void *find_inlist_lock(struct list_head *head, const char *name,
+			      const char *prefix, int *error,
+			      struct mutex *mutex)
 {
 	return try_then_request_module(
 			find_inlist_lock_noload(head, name, error, mutex),
 			"%s%s", prefix, name);
 }
 
-static inline struct ebt_table *
-find_table_lock(struct net *net, const char *name, int *error,
-		struct mutex *mutex)
+static inline struct ebt_table *find_table_lock(struct net *net,
+						const char *name, int *error,
+						struct mutex *mutex)
 {
 	return find_inlist_lock(&net->xt.tables[NFPROTO_BRIDGE], name,
 				"ebtable_", error, mutex);
@@ -350,9 +349,10 @@ static inline void ebt_free_table_info(struct ebt_table_info *info)
 		vfree(info->chainstack);
 	}
 }
-static inline int
-ebt_check_match(struct ebt_entry_match *m, struct xt_mtchk_param *par,
-		unsigned int *cnt)
+
+static inline int ebt_check_match(struct ebt_entry_match *m,
+				  struct xt_mtchk_param *par,
+				  unsigned int *cnt)
 {
 	const struct ebt_entry *e = par->entryinfo;
 	struct xt_match *match;
@@ -387,9 +387,9 @@ static inline void ebt_free_table_info(struct ebt_table_info *info)
 	return 0;
 }
 
-static inline int
-ebt_check_watcher(struct ebt_entry_watcher *w, struct xt_tgchk_param *par,
-		  unsigned int *cnt)
+static inline int ebt_check_watcher(struct ebt_entry_watcher *w,
+				    struct xt_tgchk_param *par,
+				    unsigned int *cnt)
 {
 	const struct ebt_entry *e = par->entryinfo;
 	struct xt_target *watcher;
@@ -490,11 +490,12 @@ static int ebt_verify_pointers(const struct ebt_replace *repl,
 /* this one is very careful, as it is the first function
  * to parse the userspace data
  */
-static inline int
-ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
-			       const struct ebt_table_info *newinfo,
-			       unsigned int *n, unsigned int *cnt,
-			       unsigned int *totalcnt, unsigned int *udc_cnt)
+static inline int ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
+						 const struct ebt_table_info *newinfo,
+						 unsigned int *n,
+						 unsigned int *cnt,
+						 unsigned int *totalcnt,
+						 unsigned int *udc_cnt)
 {
 	int i;
 
@@ -551,9 +552,10 @@ struct ebt_cl_stack {
 /* We need these positions to check that the jumps to a different part of the
  * entries is a jump to the beginning of a new chain.
  */
-static inline int
-ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo,
-		      unsigned int *n, struct ebt_cl_stack *udc)
+static inline int ebt_get_udc_positions(struct ebt_entry *e,
+					struct ebt_table_info *newinfo,
+					unsigned int *n,
+					struct ebt_cl_stack *udc)
 {
 	int i;
 
@@ -577,8 +579,8 @@ struct ebt_cl_stack {
 	return 0;
 }
 
-static inline int
-ebt_cleanup_match(struct ebt_entry_match *m, struct net *net, unsigned int *i)
+static inline int ebt_cleanup_match(struct ebt_entry_match *m,
+				    struct net *net, unsigned int *i)
 {
 	struct xt_mtdtor_param par;
 
@@ -595,8 +597,8 @@ struct ebt_cl_stack {
 	return 0;
 }
 
-static inline int
-ebt_cleanup_watcher(struct ebt_entry_watcher *w, struct net *net, unsigned int *i)
+static inline int ebt_cleanup_watcher(struct ebt_entry_watcher *w,
+				      struct net *net, unsigned int *i)
 {
 	struct xt_tgdtor_param par;
 
@@ -613,8 +615,8 @@ struct ebt_cl_stack {
 	return 0;
 }
 
-static inline int
-ebt_cleanup_entry(struct ebt_entry *e, struct net *net, unsigned int *cnt)
+static inline int ebt_cleanup_entry(struct ebt_entry *e, struct net *net,
+				    unsigned int *cnt)
 {
 	struct xt_tgdtor_param par;
 	struct ebt_entry_target *t;
@@ -638,11 +640,11 @@ struct ebt_cl_stack {
 	return 0;
 }
 
-static inline int
-ebt_check_entry(struct ebt_entry *e, struct net *net,
-		const struct ebt_table_info *newinfo,
-		const char *name, unsigned int *cnt,
-		struct ebt_cl_stack *cl_s, unsigned int udc_cnt)
+static inline int ebt_check_entry(struct ebt_entry *e, struct net *net,
+				  const struct ebt_table_info *newinfo,
+				  const char *name, unsigned int *cnt,
+				  struct ebt_cl_stack *cl_s,
+				  unsigned int udc_cnt)
 {
 	struct ebt_entry_target *t;
 	struct xt_target *target;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 2/9] netfilter: normalize ebtables " Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 20:31   ` Florian Westphal
  2020-01-06 18:54 ` [PATCH ghak25 v2 4/9] audit: record nfcfg params Richard Guy Briggs
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Align all function declaration parameters with open parenthesis.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 net/bridge/netfilter/ebtables.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index c9dff9e11ddb..b3c784ae33a0 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1248,9 +1248,9 @@ void ebt_unregister_table(struct net *net, struct ebt_table *table,
 
 /* userspace just supplied us with counters */
 static int do_update_counters(struct net *net, const char *name,
-				struct ebt_counter __user *counters,
-				unsigned int num_counters,
-				const void __user *user, unsigned int len)
+			      struct ebt_counter __user *counters,
+			      unsigned int num_counters,
+			      const void __user *user, unsigned int len)
 {
 	int i, ret;
 	struct ebt_counter *tmp;
@@ -1294,7 +1294,7 @@ static int do_update_counters(struct net *net, const char *name,
 }
 
 static int update_counters(struct net *net, const void __user *user,
-			    unsigned int len)
+			   unsigned int len)
 {
 	struct ebt_replace hlp;
 
@@ -1457,8 +1457,8 @@ static int copy_everything_to_user(struct ebt_table *t, void __user *user,
 	   ebt_entry_to_user, entries, tmp.entries);
 }
 
-static int do_ebt_set_ctl(struct sock *sk,
-	int cmd, void __user *user, unsigned int len)
+static int do_ebt_set_ctl(struct sock *sk, int cmd, void __user *user,
+			  unsigned int len)
 {
 	int ret;
 	struct net *net = sock_net(sk);
@@ -1660,7 +1660,7 @@ static int compat_watcher_to_user(struct ebt_entry_watcher *w,
 }
 
 static int compat_copy_entry_to_user(struct ebt_entry *e, void __user **dstptr,
-				unsigned int *size)
+				     unsigned int *size)
 {
 	struct ebt_entry_target *t;
 	struct ebt_entry __user *ce;
@@ -2149,7 +2149,7 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
  * Called before validation is performed.
  */
 static int compat_copy_entries(unsigned char *data, unsigned int size_user,
-				struct ebt_entries_buf_state *state)
+			       struct ebt_entries_buf_state *state)
 {
 	unsigned int size_remaining = size_user;
 	int ret;
@@ -2167,7 +2167,8 @@ static int compat_copy_entries(unsigned char *data, unsigned int size_user,
 
 
 static int compat_copy_ebt_replace_from_user(struct ebt_replace *repl,
-					    void __user *user, unsigned int len)
+					     void __user *user,
+					     unsigned int len)
 {
 	struct compat_ebt_replace tmp;
 	int i;
@@ -2321,8 +2322,8 @@ static int compat_update_counters(struct net *net, void __user *user,
 					hlp.num_counters, user, len);
 }
 
-static int compat_do_ebt_set_ctl(struct sock *sk,
-		int cmd, void __user *user, unsigned int len)
+static int compat_do_ebt_set_ctl(struct sock *sk, int cmd, void __user *user,
+				 unsigned int len)
 {
 	int ret;
 	struct net *net = sock_net(sk);
@@ -2343,8 +2344,8 @@ static int compat_do_ebt_set_ctl(struct sock *sk,
 	return ret;
 }
 
-static int compat_do_ebt_get_ctl(struct sock *sk, int cmd,
-		void __user *user, int *len)
+static int compat_do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user,
+				 int *len)
 {
 	int ret;
 	struct compat_ebt_replace tmp;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 4/9] audit: record nfcfg params
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (2 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 5/9] netfilter: x_tables audit only on syscall rule Richard Guy Briggs
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Record the auditable parameters of any non-empty netfilter table
configuration change.

See: https://github.com/linux-audit/audit-kernel/issues/25
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h | 11 +++++++++++
 kernel/auditsc.c      | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f9ceae57ca8d..96cabb095eed 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -379,6 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 extern void __audit_fanotify(unsigned int response);
 extern void __audit_tk_injoffset(struct timespec64 offset);
 extern void __audit_ntp_log(const struct audit_ntp_data *ad);
+extern void __audit_nf_cfg(const char *name, u8 af, int nentries);
 
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 {
@@ -514,6 +515,12 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
 		__audit_ntp_log(ad);
 }
 
+static inline void audit_nf_cfg(const char *name, u8 af, int nentries)
+{
+	if (!audit_dummy_context())
+		__audit_nf_cfg(name, af, nentries);
+}
+
 extern int audit_n_rules;
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */
@@ -646,6 +653,10 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
 
 static inline void audit_ptrace(struct task_struct *t)
 { }
+
+static inline void audit_nf_cfg(const char *name, u8 af, int nentries)
+{ }
+
 #define audit_n_rules 0
 #define audit_signals 0
 #endif /* CONFIG_AUDITSYSCALL */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4effe01ebbe2..4e1df4233cd3 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2545,6 +2545,22 @@ void __audit_ntp_log(const struct audit_ntp_data *ad)
 	audit_log_ntp_val(ad, "adjust",	AUDIT_NTP_ADJUST);
 }
 
+void __audit_nf_cfg(const char *name, u8 af, int nentries)
+{
+	struct audit_buffer *ab;
+	struct audit_context *context = audit_context();
+
+	if (!nentries)
+		return;
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_NETFILTER_CFG);
+	if (!ab)
+		return;	/* audit_panic or being filtered */
+	audit_log_format(ab, "table=%s family=%u entries=%u",
+			 name, af, nentries);
+	audit_log_end(ab);
+}
+EXPORT_SYMBOL_GPL(__audit_nf_cfg);
+
 static void audit_log_task(struct audit_buffer *ab)
 {
 	kuid_t auid, uid;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 5/9] netfilter: x_tables audit only on syscall rule
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (3 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 4/9] audit: record nfcfg params Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 6/9] netfilter: ebtables " Richard Guy Briggs
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Call new audit_nf_cfg() to store table parameters for later use with
syscall records.

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@redhat.com>
---
 net/netfilter/x_tables.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 0094853ab42a..c0416ae52f7f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1401,14 +1401,8 @@ struct xt_table_info *xt_replace_table(struct xt_table *table,
 		}
 	}
 
-#ifdef CONFIG_AUDIT
-	if (audit_enabled) {
-		audit_log(audit_context(), GFP_KERNEL,
-			  AUDIT_NETFILTER_CFG,
-			  "table=%s family=%u entries=%u",
-			  table->name, table->af, private->number);
-	}
-#endif
+	if (audit_enabled)
+		audit_nf_cfg(table->name, table->af, private->number);
 
 	return private;
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 6/9] netfilter: ebtables audit only on syscall rule
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (4 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 5/9] netfilter: x_tables audit only on syscall rule Richard Guy Briggs
@ 2020-01-06 18:54 ` " Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 7/9] netfilter: ebtables audit table registration Richard Guy Briggs
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Call new audit_nf_cfg() to store table parameters for later use with
syscall records.

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@redhat.com>
---
 net/bridge/netfilter/ebtables.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index b3c784ae33a0..57dc11c0f349 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1048,14 +1048,8 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
 	vfree(table);
 	vfree(counterstmp);
 
-#ifdef CONFIG_AUDIT
-	if (audit_enabled) {
-		audit_log(audit_context(), GFP_KERNEL,
-			  AUDIT_NETFILTER_CFG,
-			  "table=%s family=%u entries=%u",
-			  repl->name, AF_BRIDGE, repl->nentries);
-	}
-#endif
+	if (audit_enabled)
+		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
 	return ret;
 
 free_unlock:
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 7/9] netfilter: ebtables audit table registration
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (5 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 6/9] netfilter: ebtables " Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 18:54 ` [PATCH ghak25 v2 8/9] netfilter: add audit operation field Richard Guy Briggs
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Generate audit NETFILTER_CFG records on ebtables table registration.

Previously this was only being done for all x_tables operations and
ebtables table replacement.

Call new audit_nf_cfg() to store table parameters for later use with
syscall records.

Here is a sample accompanied record:
  type=NETFILTER_CFG msg=audit(1494907217.558:5403): table=filter family=7 entries=0

See: https://github.com/linux-audit/audit-kernel/issues/43
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 net/bridge/netfilter/ebtables.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 57dc11c0f349..58126547b175 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1219,6 +1219,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
 		*res = NULL;
 	}
 
+	if (audit_enabled)
+		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
 	return ret;
 free_unlock:
 	mutex_unlock(&ebt_mutex);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 8/9] netfilter: add audit operation field
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (6 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 7/9] netfilter: ebtables audit table registration Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-06 20:23   ` Florian Westphal
  2020-01-06 18:54 ` [PATCH ghak25 v2 9/9] netfilter: audit table unregister actions Richard Guy Briggs
  2020-01-16 15:05 ` [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Pablo Neira Ayuso
  9 siblings, 1 reply; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Add the operation performed (register or replace) to the NETFILTER_CFG
record.

Here is the sample record:
      type=NETFILTER_CFG msg=audit(1494981627.248:9764): family=7 table=broute entries=0 op=replace

See: https://github.com/linux-audit/audit-kernel/issues/25
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h           | 8 ++++----
 kernel/auditsc.c                | 7 ++++---
 net/bridge/netfilter/ebtables.c | 4 ++--
 net/netfilter/x_tables.c        | 3 ++-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 96cabb095eed..5eab4d898c26 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -379,7 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 extern void __audit_fanotify(unsigned int response);
 extern void __audit_tk_injoffset(struct timespec64 offset);
 extern void __audit_ntp_log(const struct audit_ntp_data *ad);
-extern void __audit_nf_cfg(const char *name, u8 af, int nentries);
+extern void __audit_nf_cfg(const char *name, u8 af, int nentries, int op);
 
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 {
@@ -515,10 +515,10 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
 		__audit_ntp_log(ad);
 }
 
-static inline void audit_nf_cfg(const char *name, u8 af, int nentries)
+static inline void audit_nf_cfg(const char *name, u8 af, int nentries, int op)
 {
 	if (!audit_dummy_context())
-		__audit_nf_cfg(name, af, nentries);
+		__audit_nf_cfg(name, af, nentries, op);
 }
 
 extern int audit_n_rules;
@@ -654,7 +654,7 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
 static inline void audit_ptrace(struct task_struct *t)
 { }
 
-static inline void audit_nf_cfg(const char *name, u8 af, int nentries)
+static inline void audit_nf_cfg(const char *name, u8 af, int nentries, int op)
 { }
 
 #define audit_n_rules 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4e1df4233cd3..999ac184246b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2545,7 +2545,7 @@ void __audit_ntp_log(const struct audit_ntp_data *ad)
 	audit_log_ntp_val(ad, "adjust",	AUDIT_NTP_ADJUST);
 }
 
-void __audit_nf_cfg(const char *name, u8 af, int nentries)
+void __audit_nf_cfg(const char *name, u8 af, int nentries, int op)
 {
 	struct audit_buffer *ab;
 	struct audit_context *context = audit_context();
@@ -2555,8 +2555,9 @@ void __audit_nf_cfg(const char *name, u8 af, int nentries)
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_NETFILTER_CFG);
 	if (!ab)
 		return;	/* audit_panic or being filtered */
-	audit_log_format(ab, "table=%s family=%u entries=%u",
-			 name, af, nentries);
+	audit_log_format(ab, "table=%s family=%u entries=%u op=%s",
+			 name, af, nentries,
+			 op ? "replace" : "register");
 	audit_log_end(ab);
 }
 EXPORT_SYMBOL_GPL(__audit_nf_cfg);
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 58126547b175..baff2f05af43 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1049,7 +1049,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
 	vfree(counterstmp);
 
 	if (audit_enabled)
-		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
+		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, 1);
 	return ret;
 
 free_unlock:
@@ -1220,7 +1220,7 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	}
 
 	if (audit_enabled)
-		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
+		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, 0);
 	return ret;
 free_unlock:
 	mutex_unlock(&ebt_mutex);
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index c0416ae52f7f..4ae4f7bf8946 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1402,7 +1402,8 @@ struct xt_table_info *xt_replace_table(struct xt_table *table,
 	}
 
 	if (audit_enabled)
-		audit_nf_cfg(table->name, table->af, private->number);
+		audit_nf_cfg(table->name, table->af, private->number,
+			     private->number);
 
 	return private;
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH ghak25 v2 9/9] netfilter: audit table unregister actions
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (7 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 8/9] netfilter: add audit operation field Richard Guy Briggs
@ 2020-01-06 18:54 ` Richard Guy Briggs
  2020-01-16 15:05 ` [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Pablo Neira Ayuso
  9 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-06 18:54 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm,
	tgraf, Richard Guy Briggs

Audit the action of unregistering ebtables and x_tables.

See: https://github.com/linux-audit/audit-kernel/issues/44
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c                | 3 ++-
 net/bridge/netfilter/ebtables.c | 3 +++
 net/netfilter/x_tables.c        | 4 +++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 999ac184246b..2644130a9e66 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2557,7 +2557,8 @@ void __audit_nf_cfg(const char *name, u8 af, int nentries, int op)
 		return;	/* audit_panic or being filtered */
 	audit_log_format(ab, "table=%s family=%u entries=%u op=%s",
 			 name, af, nentries,
-			 op ? "replace" : "register");
+			 op == 1 ? "replace" :
+				   (op ? "unregister" : "register"));
 	audit_log_end(ab);
 }
 EXPORT_SYMBOL_GPL(__audit_nf_cfg);
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index baff2f05af43..3dd4eb5b13fd 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1126,6 +1126,9 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
 	mutex_lock(&ebt_mutex);
 	list_del(&table->list);
 	mutex_unlock(&ebt_mutex);
+	if (audit_enabled)
+		audit_nf_cfg(table->name, AF_BRIDGE, table->private->nentries,
+			     2);
 	EBT_ENTRY_ITERATE(table->private->entries, table->private->entries_size,
 			  ebt_cleanup_entry, net, NULL);
 	if (table->private->nentries)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 4ae4f7bf8946..e4852a0cb62f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1403,7 +1403,7 @@ struct xt_table_info *xt_replace_table(struct xt_table *table,
 
 	if (audit_enabled)
 		audit_nf_cfg(table->name, table->af, private->number,
-			     private->number);
+			     !!private->number);
 
 	return private;
 }
@@ -1466,6 +1466,8 @@ void *xt_unregister_table(struct xt_table *table)
 	private = table->private;
 	list_del(&table->list);
 	mutex_unlock(&xt[table->af].mutex);
+	if (audit_enabled)
+		audit_nf_cfg(table->name, table->af, private->number, 2);
 	kfree(table);
 
 	return private;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 8/9] netfilter: add audit operation field
  2020-01-06 18:54 ` [PATCH ghak25 v2 8/9] netfilter: add audit operation field Richard Guy Briggs
@ 2020-01-06 20:23   ` Florian Westphal
  0 siblings, 0 replies; 19+ messages in thread
From: Florian Westphal @ 2020-01-06 20:23 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

Richard Guy Briggs <rgb@redhat.com> wrote:
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 96cabb095eed..5eab4d898c26 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -379,7 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
>  extern void __audit_fanotify(unsigned int response);
>  extern void __audit_tk_injoffset(struct timespec64 offset);
>  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> -extern void __audit_nf_cfg(const char *name, u8 af, int nentries);
> +extern void __audit_nf_cfg(const char *name, u8 af, int nentries, int op);

Consider adding an enum instead of int op.

>  	if (audit_enabled)
> -		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
> +		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, 1);

audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, AUDIT_XT_OP_REPLACE);

... would be a bit more readable than '1'.

The name is just an example, you can pick something else.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations
  2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
@ 2020-01-06 20:23   ` Florian Westphal
  2020-01-08 16:47   ` Nicolas Dichtel
  1 sibling, 0 replies; 19+ messages in thread
From: Florian Westphal @ 2020-01-06 20:23 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

Richard Guy Briggs <rgb@redhat.com> wrote:
> 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.

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 2/9] netfilter: normalize ebtables function declarations
  2020-01-06 18:54 ` [PATCH ghak25 v2 2/9] netfilter: normalize ebtables " Richard Guy Briggs
@ 2020-01-06 20:30   ` Florian Westphal
  0 siblings, 0 replies; 19+ messages in thread
From: Florian Westphal @ 2020-01-06 20:30 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

Richard Guy Briggs <rgb@redhat.com> wrote:
> 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 ebtables function declarations so that git context diff
> function labels work as expected.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

I suggest that you also drop the inline keyword for all functions
that are called from control path, for example

> -static inline void *
> -find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
> -			struct mutex *mutex)
> +static inline void *find_inlist_lock_noload(struct list_head *head,
> +					    const char *name, int *error,
> +					    struct mutex *mutex)

> -static inline struct ebt_table *
> -find_table_lock(struct net *net, const char *name, int *error,
> -		struct mutex *mutex)
> +static inline struct ebt_table *find_table_lock(struct net *net,

> -static inline int
> -ebt_check_match(struct ebt_entry_match *m, struct xt_mtchk_param *par,
> -		unsigned int *cnt)
> +
> +static inline int ebt_check_match(struct ebt_entry_match *m,

> -static inline int
> -ebt_check_watcher(struct ebt_entry_watcher *w, struct xt_tgchk_param *par,
> -		  unsigned int *cnt)

> -static inline int
> -ebt_check_entry_size_and_hooks(const struct ebt_entry *e,

> -static inline int
> -ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo,
> -		      unsigned int *n, struct ebt_cl_stack *udc)

> -static inline int
> -ebt_cleanup_match(struct ebt_entry_match *m, struct net *net, unsigned int *i)

> -static inline int
> -ebt_cleanup_watcher(struct ebt_entry_watcher *w, struct net *net, unsigned int *i)

> -static inline int
> -ebt_cleanup_entry(struct ebt_entry *e, struct net *net, unsigned int *cnt)

> -static inline int
> -ebt_check_entry(struct ebt_entry *e, struct net *net,
> -		const struct ebt_table_info *newinfo,
> -		const char *name, unsigned int *cnt,
> -		struct ebt_cl_stack *cl_s, unsigned int udc_cnt)

.. can all have 'inline' removed.  Other than that this looks good to me.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II
  2020-01-06 18:54 ` [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II Richard Guy Briggs
@ 2020-01-06 20:31   ` Florian Westphal
  0 siblings, 0 replies; 19+ messages in thread
From: Florian Westphal @ 2020-01-06 20:31 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

Richard Guy Briggs <rgb@redhat.com> wrote:
> Align all function declaration parameters with open parenthesis.

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations
  2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
  2020-01-06 20:23   ` Florian Westphal
@ 2020-01-08 16:47   ` Nicolas Dichtel
  2020-01-16 21:29     ` Richard Guy Briggs
  1 sibling, 1 reply; 19+ messages in thread
From: Nicolas Dichtel @ 2020-01-08 16:47 UTC (permalink / raw)
  To: Richard Guy Briggs, Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Paul Moore, sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

Le 06/01/2020 à 19:54, Richard Guy Briggs a écrit :
> 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.
Just for my information, how do you reproduce that? With a 'git diff'?

> 
> Normalize x_table function declarations so that git context diff
> function labels work as expected.
> 
[snip]
> 
> -- 
> 1.8.3.1
git v1.8.3.1 is seven years old:
https://github.com/git/git/releases/tag/v1.8.3.1

I don't see any problems with git v2.24. Not sure that the patch brings any
helpful value except complicating backports.

Regards,
Nicolas

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues
  2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
                   ` (8 preceding siblings ...)
  2020-01-06 18:54 ` [PATCH ghak25 v2 9/9] netfilter: audit table unregister actions Richard Guy Briggs
@ 2020-01-16 15:05 ` Pablo Neira Ayuso
  2020-01-16 19:07   ` Paul Moore
  9 siblings, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2020-01-16 15:05 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

On Mon, Jan 06, 2020 at 01:54:01PM -0500, Richard Guy Briggs wrote:
> There were questions about the presence and cause of unsolicited syscall events
> in the logs containing NETFILTER_CFG records and sometimes unaccompanied
> NETFILTER_CFG records.
> 
> During testing at least the following list of events trigger NETFILTER_CFG
> records and the syscalls related (There may be more events that will trigger
> this message type.):
> 	init_module, finit_module: modprobe
> 	setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
> 	unshare: (h?)ostnamed
> 	clone: libvirtd
> 
> The syscall events unsolicited by any audit rule were found to be caused by a
> missing !audit_dummy_context() check before creating a NETFILTER_CFG
> record and issuing the record immediately rather than saving the
> information to create the record at syscall exit.
> Check !audit_dummy_context() before creating the NETFILTER_CFG record.
> 
> The vast majority of 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.
> 
> A couple of other factors should help eliminate unaccompanied records
> which include commit cb74ed278f80 ("audit: always enable syscall
> auditing when supported and audit is enabled") which makes sure that
> when audit is enabled, so automatically is syscall auditing, and ghak66
> which addressed initializing audit before PID 1.
> 
> Ebtables module initialization to register tables doesn't generate records
> because it was never hooked in to audit.  Recommend adding audit hooks to log
> this.
> 
> Table unregistration was never logged, which is now covered.
> 
> Seemingly duplicate records are not actually exact duplicates that are caused
> by netfilter table initialization in different network namespaces from the same
> syscall.  Recommend adding the network namespace ID (proc inode and dev)
> to the record to make this obvious (address later with ghak79 after nsid
> patches).
> 
> See: https://github.com/linux-audit/audit-kernel/issues/25
> See: https://github.com/linux-audit/audit-kernel/issues/35
> See: https://github.com/linux-audit/audit-kernel/issues/43
> See: https://github.com/linux-audit/audit-kernel/issues/44

What tree is this batch targeted to?

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues
  2020-01-16 15:05 ` [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Pablo Neira Ayuso
@ 2020-01-16 19:07   ` Paul Moore
  2020-01-16 21:20     ` Richard Guy Briggs
  0 siblings, 1 reply; 19+ messages in thread
From: Paul Moore @ 2020-01-16 19:07 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Richard Guy Briggs, Linux-Audit Mailing List, LKML,
	netfilter-devel, sgrubb, omosnace, fw, twoerner, Eric Paris,
	ebiederm, tgraf

On Thu, Jan 16, 2020 at 10:05 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 06, 2020 at 01:54:01PM -0500, Richard Guy Briggs wrote:
> > There were questions about the presence and cause of unsolicited syscall events
> > in the logs containing NETFILTER_CFG records and sometimes unaccompanied
> > NETFILTER_CFG records.
> >
> > During testing at least the following list of events trigger NETFILTER_CFG
> > records and the syscalls related (There may be more events that will trigger
> > this message type.):
> >       init_module, finit_module: modprobe
> >       setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
> >       unshare: (h?)ostnamed
> >       clone: libvirtd
> >
> > The syscall events unsolicited by any audit rule were found to be caused by a
> > missing !audit_dummy_context() check before creating a NETFILTER_CFG
> > record and issuing the record immediately rather than saving the
> > information to create the record at syscall exit.
> > Check !audit_dummy_context() before creating the NETFILTER_CFG record.
> >
> > The vast majority of 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.
> >
> > A couple of other factors should help eliminate unaccompanied records
> > which include commit cb74ed278f80 ("audit: always enable syscall
> > auditing when supported and audit is enabled") which makes sure that
> > when audit is enabled, so automatically is syscall auditing, and ghak66
> > which addressed initializing audit before PID 1.
> >
> > Ebtables module initialization to register tables doesn't generate records
> > because it was never hooked in to audit.  Recommend adding audit hooks to log
> > this.
> >
> > Table unregistration was never logged, which is now covered.
> >
> > Seemingly duplicate records are not actually exact duplicates that are caused
> > by netfilter table initialization in different network namespaces from the same
> > syscall.  Recommend adding the network namespace ID (proc inode and dev)
> > to the record to make this obvious (address later with ghak79 after nsid
> > patches).
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/25
> > See: https://github.com/linux-audit/audit-kernel/issues/35
> > See: https://github.com/linux-audit/audit-kernel/issues/43
> > See: https://github.com/linux-audit/audit-kernel/issues/44
>
> What tree is this batch targeted to?

I believe Richard was targeting this for the audit tree.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues
  2020-01-16 19:07   ` Paul Moore
@ 2020-01-16 21:20     ` Richard Guy Briggs
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-16 21:20 UTC (permalink / raw)
  To: Paul Moore
  Cc: Pablo Neira Ayuso, Linux-Audit Mailing List, LKML,
	netfilter-devel, sgrubb, omosnace, fw, twoerner, Eric Paris,
	ebiederm, tgraf

On 2020-01-16 14:07, Paul Moore wrote:
> On Thu, Jan 16, 2020 at 10:05 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Mon, Jan 06, 2020 at 01:54:01PM -0500, Richard Guy Briggs wrote:
> > > There were questions about the presence and cause of unsolicited syscall events
> > > in the logs containing NETFILTER_CFG records and sometimes unaccompanied
> > > NETFILTER_CFG records.
> > >
> > > During testing at least the following list of events trigger NETFILTER_CFG
> > > records and the syscalls related (There may be more events that will trigger
> > > this message type.):
> > >       init_module, finit_module: modprobe
> > >       setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
> > >       unshare: (h?)ostnamed
> > >       clone: libvirtd
> > >
> > > The syscall events unsolicited by any audit rule were found to be caused by a
> > > missing !audit_dummy_context() check before creating a NETFILTER_CFG
> > > record and issuing the record immediately rather than saving the
> > > information to create the record at syscall exit.
> > > Check !audit_dummy_context() before creating the NETFILTER_CFG record.
> > >
> > > The vast majority of 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.
> > >
> > > A couple of other factors should help eliminate unaccompanied records
> > > which include commit cb74ed278f80 ("audit: always enable syscall
> > > auditing when supported and audit is enabled") which makes sure that
> > > when audit is enabled, so automatically is syscall auditing, and ghak66
> > > which addressed initializing audit before PID 1.
> > >
> > > Ebtables module initialization to register tables doesn't generate records
> > > because it was never hooked in to audit.  Recommend adding audit hooks to log
> > > this.
> > >
> > > Table unregistration was never logged, which is now covered.
> > >
> > > Seemingly duplicate records are not actually exact duplicates that are caused
> > > by netfilter table initialization in different network namespaces from the same
> > > syscall.  Recommend adding the network namespace ID (proc inode and dev)
> > > to the record to make this obvious (address later with ghak79 after nsid
> > > patches).
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/25
> > > See: https://github.com/linux-audit/audit-kernel/issues/35
> > > See: https://github.com/linux-audit/audit-kernel/issues/43
> > > See: https://github.com/linux-audit/audit-kernel/issues/44
> >
> > What tree is this batch targeted to?
> 
> I believe Richard was targeting this for the audit tree.

Yes, sorry Pablo, it is against audit/next based on v5.5-rc1

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations
  2020-01-08 16:47   ` Nicolas Dichtel
@ 2020-01-16 21:29     ` Richard Guy Briggs
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Guy Briggs @ 2020-01-16 21:29 UTC (permalink / raw)
  To: Nicolas Dichtel
  Cc: Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore,
	sgrubb, omosnace, fw, twoerner, eparis, ebiederm, tgraf

On 2020-01-08 17:47, Nicolas Dichtel wrote:
> Le 06/01/2020 à 19:54, Richard Guy Briggs a écrit :
> > 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.
> Just for my information, how do you reproduce that? With a 'git diff'?

git format-patch is how it is presenting as a problem, which I assume
would also be git diff.

> > Normalize x_table function declarations so that git context diff
> > function labels work as expected.
> > 
> [snip]
> > 
> > -- 
> > 1.8.3.1
> git v1.8.3.1 is seven years old:
> https://github.com/git/git/releases/tag/v1.8.3.1
> 
> I don't see any problems with git v2.24. Not sure that the patch brings any
> helpful value except complicating backports.

It brings value to anyone who is on a distro that is stable and only
slightly behind.  There are other features of git 2.x that I'd like to
start using (git worktrees) but I'll have to wait until I can afford to
upgrade.

> Nicolas

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, back to index

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 18:54 [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 1/9] netfilter: normalize x_table function declarations Richard Guy Briggs
2020-01-06 20:23   ` Florian Westphal
2020-01-08 16:47   ` Nicolas Dichtel
2020-01-16 21:29     ` Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 2/9] netfilter: normalize ebtables " Richard Guy Briggs
2020-01-06 20:30   ` Florian Westphal
2020-01-06 18:54 ` [PATCH ghak25 v2 3/9] netfilter: normalize ebtables function declarations II Richard Guy Briggs
2020-01-06 20:31   ` Florian Westphal
2020-01-06 18:54 ` [PATCH ghak25 v2 4/9] audit: record nfcfg params Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 5/9] netfilter: x_tables audit only on syscall rule Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 6/9] netfilter: ebtables " Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 7/9] netfilter: ebtables audit table registration Richard Guy Briggs
2020-01-06 18:54 ` [PATCH ghak25 v2 8/9] netfilter: add audit operation field Richard Guy Briggs
2020-01-06 20:23   ` Florian Westphal
2020-01-06 18:54 ` [PATCH ghak25 v2 9/9] netfilter: audit table unregister actions Richard Guy Briggs
2020-01-16 15:05 ` [PATCH ghak25 v2 0/9] Address NETFILTER_CFG issues Pablo Neira Ayuso
2020-01-16 19:07   ` Paul Moore
2020-01-16 21:20     ` Richard Guy Briggs

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git