Linux-audit Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH ghak124 v2] audit: log nftables configuration change events
@ 2020-05-29  1:44 Richard Guy Briggs
  2020-05-31 23:38 ` kbuild test robot
  2020-06-01 16:10 ` Paul Moore
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2020-05-29  1:44 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Richard Guy Briggs, fw, twoerner, eparis, tgraf

iptables, ip6tables, arptables and ebtables table registration,
replacement and unregistration configuration events are logged for the
native (legacy) iptables setsockopt api, but not for the
nftables netlink api which is used by the nft-variant of iptables in
addition to nftables itself.

Add calls to log the configuration actions in the nftables netlink api.

This uses the same NETFILTER_CFG record format but overloads the table
field.

  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
  ...
  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
  ...
  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
  ...
  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
  ...
  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
  ...
  type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld

For further information please see issue
https://github.com/linux-audit/audit-kernel/issues/124

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
Changelog:
v2:
- differentiate between xtables and nftables
- add set, setelem, obj, flowtable, gen
- use nentries field as appropriate per type
- overload the "tables" field with table handle and chain/set/flowtable

 include/linux/audit.h         | 52 +++++++++++++++++++++++++
 kernel/auditsc.c              | 24 ++++++++++--
 net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 3 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 3fcd9ee49734..d79866a38505 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -12,6 +12,7 @@
 #include <linux/sched.h>
 #include <linux/ptrace.h>
 #include <uapi/linux/audit.h>
+#include <uapi/linux/netfilter/nf_tables.h>
 
 #define AUDIT_INO_UNSET ((unsigned long)-1)
 #define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -98,6 +99,57 @@ enum audit_nfcfgop {
 	AUDIT_XT_OP_REGISTER,
 	AUDIT_XT_OP_REPLACE,
 	AUDIT_XT_OP_UNREGISTER,
+	AUDIT_NFT_OP_TABLE_REGISTER,
+	AUDIT_NFT_OP_TABLE_UNREGISTER,
+	AUDIT_NFT_OP_CHAIN_REGISTER,
+	AUDIT_NFT_OP_CHAIN_UNREGISTER,
+	AUDIT_NFT_OP_RULE_REGISTER,
+	AUDIT_NFT_OP_RULE_UNREGISTER,
+	AUDIT_NFT_OP_SET_REGISTER,
+	AUDIT_NFT_OP_SET_UNREGISTER,
+	AUDIT_NFT_OP_SETELEM_REGISTER,
+	AUDIT_NFT_OP_SETELEM_UNREGISTER,
+	AUDIT_NFT_OP_GEN_REGISTER,
+	AUDIT_NFT_OP_OBJ_REGISTER,
+	AUDIT_NFT_OP_OBJ_UNREGISTER,
+	AUDIT_NFT_OP_OBJ_RESET,
+	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
+	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
+	AUDIT_NFT_OP_INVALID,
+};
+
+struct audit_nftcfgop_tab {
+	enum nf_tables_msg_types	nftop;
+	enum audit_nfcfgop		op;
+};
+
+static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
+	{ NFT_MSG_NEWTABLE,	AUDIT_NFT_OP_TABLE_REGISTER		},
+	{ NFT_MSG_GETTABLE,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELTABLE,	AUDIT_NFT_OP_TABLE_UNREGISTER		},
+	{ NFT_MSG_NEWCHAIN,	AUDIT_NFT_OP_CHAIN_REGISTER		},
+	{ NFT_MSG_GETCHAIN,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELCHAIN,	AUDIT_NFT_OP_CHAIN_UNREGISTER		},
+	{ NFT_MSG_NEWRULE,	AUDIT_NFT_OP_RULE_REGISTER		},
+	{ NFT_MSG_GETRULE,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELRULE,	AUDIT_NFT_OP_RULE_UNREGISTER		},
+	{ NFT_MSG_NEWSET,	AUDIT_NFT_OP_SET_REGISTER		},
+	{ NFT_MSG_GETSET,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELSET,	AUDIT_NFT_OP_SET_UNREGISTER		},
+	{ NFT_MSG_NEWSETELEM,	AUDIT_NFT_OP_SETELEM_REGISTER		},
+	{ NFT_MSG_GETSETELEM,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELSETELEM,	AUDIT_NFT_OP_SETELEM_UNREGISTER		},
+	{ NFT_MSG_NEWGEN,	AUDIT_NFT_OP_GEN_REGISTER		},
+	{ NFT_MSG_GETGEN,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_TRACE,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_NEWOBJ,	AUDIT_NFT_OP_OBJ_REGISTER		},
+	{ NFT_MSG_GETOBJ,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELOBJ,	AUDIT_NFT_OP_OBJ_UNREGISTER		},
+	{ NFT_MSG_GETOBJ_RESET,	AUDIT_NFT_OP_OBJ_RESET			},
+	{ NFT_MSG_NEWFLOWTABLE,	AUDIT_NFT_OP_FLOWTABLE_REGISTER		},
+	{ NFT_MSG_GETFLOWTABLE,	AUDIT_NFT_OP_INVALID			},
+	{ NFT_MSG_DELFLOWTABLE,	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER	},
+	{ NFT_MSG_MAX,		AUDIT_NFT_OP_INVALID			},
 };
 
 extern int is_audit_feature_set(int which);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 468a23390457..3a9100e95fda 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
 #include <linux/uaccess.h>
 #include <linux/fsnotify_backend.h>
 #include <uapi/linux/limits.h>
+#include <uapi/linux/netfilter/nf_tables.h>
 
 #include "audit.h"
 
@@ -136,9 +137,26 @@ struct audit_nfcfgop_tab {
 };
 
 static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
-	{ AUDIT_XT_OP_REGISTER,		"register"	},
-	{ AUDIT_XT_OP_REPLACE,		"replace"	},
-	{ AUDIT_XT_OP_UNREGISTER,	"unregister"	},
+	{ AUDIT_XT_OP_REGISTER,			"xt_register"		   },
+	{ AUDIT_XT_OP_REPLACE,			"xt_replace"		   },
+	{ AUDIT_XT_OP_UNREGISTER,		"xt_unregister"		   },
+	{ AUDIT_NFT_OP_TABLE_REGISTER,		"nft_register_table"	   },
+	{ AUDIT_NFT_OP_TABLE_UNREGISTER,	"nft_unregister_table"	   },
+	{ AUDIT_NFT_OP_CHAIN_REGISTER,		"nft_register_chain"	   },
+	{ AUDIT_NFT_OP_CHAIN_UNREGISTER,	"nft_unregister_chain"	   },
+	{ AUDIT_NFT_OP_RULE_REGISTER,		"nft_register_rule"	   },
+	{ AUDIT_NFT_OP_RULE_UNREGISTER,		"nft_unregister_rule"	   },
+	{ AUDIT_NFT_OP_SET_REGISTER,		"nft_register_set"	   },
+	{ AUDIT_NFT_OP_SET_UNREGISTER,		"nft_unregister_set"	   },
+	{ AUDIT_NFT_OP_SETELEM_REGISTER,	"nft_register_setelem"	   },
+	{ AUDIT_NFT_OP_SETELEM_UNREGISTER,	"nft_unregister_setelem"   },
+	{ AUDIT_NFT_OP_GEN_REGISTER,		"nft_register_gen"	   },
+	{ AUDIT_NFT_OP_OBJ_REGISTER,		"nft_register_obj"	   },
+	{ AUDIT_NFT_OP_OBJ_UNREGISTER,		"nft_unregister_obj"	   },
+	{ AUDIT_NFT_OP_OBJ_RESET,		"nft_reset_obj"		   },
+	{ AUDIT_NFT_OP_FLOWTABLE_REGISTER,	"nft_register_flowtable"   },
+	{ AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,	"nft_unregister_flowtable" },
+	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
 };
 
 static int audit_match_perm(struct audit_context *ctx, int mask)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 4471393da6d8..7a386eca6e04 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -12,6 +12,7 @@
 #include <linux/netlink.h>
 #include <linux/vmalloc.h>
 #include <linux/rhashtable.h>
+#include <linux/audit.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nf_tables.h>
@@ -693,6 +694,14 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event)
 {
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
+			      ctx->table->name, ctx->table->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			ctx->table->use,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -1428,6 +1437,15 @@ static void nf_tables_chain_notify(const struct nft_ctx *ctx, int event)
 {
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu",
+			      ctx->table->name, ctx->table->handle,
+			      ctx->chain->name, ctx->chain->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			ctx->chain->use,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -2691,6 +2709,15 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
 {
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu",
+			      ctx->table->name, ctx->table->handle,
+			      ctx->chain->name, ctx->chain->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			rule->handle,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -3692,6 +3719,15 @@ static void nf_tables_set_notify(const struct nft_ctx *ctx,
 	struct sk_buff *skb;
 	u32 portid = ctx->portid;
 	int err;
+	char *buf = kasprintf(gfp_flags, "%s:%llu;%s:%llu",
+			      ctx->table->name, ctx->table->handle,
+			      set->name, set->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			set->field_count,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -4789,6 +4825,15 @@ static void nf_tables_setelem_notify(const struct nft_ctx *ctx,
 	u32 portid = ctx->portid;
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu",
+			      ctx->table->name, ctx->table->handle,
+			      set->name, set->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			set->handle,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
 		return;
@@ -5875,6 +5920,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 			    obj->ops->type->type != filter->type)
 				goto cont;
 
+			if (reset) {
+				char *buf = kasprintf(GFP_KERNEL,
+						      "%s:%llu;?:0",
+						      table->name,
+						      table->handle);
+
+				audit_log_nfcfg(buf,
+						family,
+						obj->handle,
+						audit_nftcfgs[NFT_MSG_GETOBJ_RESET].op);
+				kfree(buf);
+			}
+
 			if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
 						    cb->nlh->nlmsg_seq,
 						    NFT_MSG_NEWOBJ,
@@ -5985,6 +6043,17 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
 	if (NFNL_MSG_TYPE(nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET)
 		reset = true;
 
+	if (reset) {
+		char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
+				      table->name, table->handle);
+
+		audit_log_nfcfg(buf,
+				family,
+				obj->handle,
+				audit_nftcfgs[NFT_MSG_GETOBJ_RESET].op);
+		kfree(buf);
+	}
+
 	err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid,
 				      nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0,
 				      family, table, obj, reset);
@@ -6060,6 +6129,14 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
 {
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
+			      table->name, table->handle);
+
+	audit_log_nfcfg(buf,
+			family,
+			obj->handle,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (!report &&
 	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
@@ -6686,6 +6763,15 @@ static void nf_tables_flowtable_notify(struct nft_ctx *ctx,
 {
 	struct sk_buff *skb;
 	int err;
+	char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu",
+			      flowtable->table->name, flowtable->table->handle,
+			      flowtable->name, flowtable->handle);
+
+	audit_log_nfcfg(buf,
+			ctx->family,
+			flowtable->hooknum,
+			audit_nftcfgs[event].op);
+	kfree(buf);
 
 	if (ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -6807,6 +6893,9 @@ static void nf_tables_gen_notify(struct net *net, struct sk_buff *skb,
 	struct sk_buff *skb2;
 	int err;
 
+	audit_log_nfcfg("?:0;?:0", 0, net->nft.base_seq,
+			audit_nftcfgs[event].op);
+
 	if (nlmsg_report(nlh) &&
 	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
 		return;
-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

* Re: [PATCH ghak124 v2] audit: log nftables configuration change events
  2020-05-29  1:44 [PATCH ghak124 v2] audit: log nftables configuration change events Richard Guy Briggs
@ 2020-05-31 23:38 ` kbuild test robot
  2020-06-01 16:10 ` Paul Moore
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2020-05-31 23:38 UTC (permalink / raw)
  To: Richard Guy Briggs, Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: kbuild-all, fw, twoerner, eparis, tgraf


[-- Attachment #1: Type: text/plain, Size: 3343 bytes --]

Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on next-20200529]
[cannot apply to nf/master nf-next/master linus/master linux/master v5.7-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-log-nftables-configuration-change-events/20200531-043244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

In file included from arch/sparc/kernel/ptrace_64.c:25:
>> include/linux/audit.h:126:40: error: 'audit_nftcfgs' defined but not used [-Werror=unused-const-variable=]
126 | static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
|                                        ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

vim +/audit_nftcfgs +126 include/linux/audit.h

   125	
 > 126	static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
   127		{ NFT_MSG_NEWTABLE,	AUDIT_NFT_OP_TABLE_REGISTER		},
   128		{ NFT_MSG_GETTABLE,	AUDIT_NFT_OP_INVALID			},
   129		{ NFT_MSG_DELTABLE,	AUDIT_NFT_OP_TABLE_UNREGISTER		},
   130		{ NFT_MSG_NEWCHAIN,	AUDIT_NFT_OP_CHAIN_REGISTER		},
   131		{ NFT_MSG_GETCHAIN,	AUDIT_NFT_OP_INVALID			},
   132		{ NFT_MSG_DELCHAIN,	AUDIT_NFT_OP_CHAIN_UNREGISTER		},
   133		{ NFT_MSG_NEWRULE,	AUDIT_NFT_OP_RULE_REGISTER		},
   134		{ NFT_MSG_GETRULE,	AUDIT_NFT_OP_INVALID			},
   135		{ NFT_MSG_DELRULE,	AUDIT_NFT_OP_RULE_UNREGISTER		},
   136		{ NFT_MSG_NEWSET,	AUDIT_NFT_OP_SET_REGISTER		},
   137		{ NFT_MSG_GETSET,	AUDIT_NFT_OP_INVALID			},
   138		{ NFT_MSG_DELSET,	AUDIT_NFT_OP_SET_UNREGISTER		},
   139		{ NFT_MSG_NEWSETELEM,	AUDIT_NFT_OP_SETELEM_REGISTER		},
   140		{ NFT_MSG_GETSETELEM,	AUDIT_NFT_OP_INVALID			},
   141		{ NFT_MSG_DELSETELEM,	AUDIT_NFT_OP_SETELEM_UNREGISTER		},
   142		{ NFT_MSG_NEWGEN,	AUDIT_NFT_OP_GEN_REGISTER		},
   143		{ NFT_MSG_GETGEN,	AUDIT_NFT_OP_INVALID			},
   144		{ NFT_MSG_TRACE,	AUDIT_NFT_OP_INVALID			},
   145		{ NFT_MSG_NEWOBJ,	AUDIT_NFT_OP_OBJ_REGISTER		},
   146		{ NFT_MSG_GETOBJ,	AUDIT_NFT_OP_INVALID			},
   147		{ NFT_MSG_DELOBJ,	AUDIT_NFT_OP_OBJ_UNREGISTER		},
   148		{ NFT_MSG_GETOBJ_RESET,	AUDIT_NFT_OP_OBJ_RESET			},
   149		{ NFT_MSG_NEWFLOWTABLE,	AUDIT_NFT_OP_FLOWTABLE_REGISTER		},
   150		{ NFT_MSG_GETFLOWTABLE,	AUDIT_NFT_OP_INVALID			},
   151		{ NFT_MSG_DELFLOWTABLE,	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER	},
   152		{ NFT_MSG_MAX,		AUDIT_NFT_OP_INVALID			},
   153	};
   154	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62442 bytes --]

[-- Attachment #3: Type: text/plain, Size: 102 bytes --]

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

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

* Re: [PATCH ghak124 v2] audit: log nftables configuration change events
  2020-05-29  1:44 [PATCH ghak124 v2] audit: log nftables configuration change events Richard Guy Briggs
  2020-05-31 23:38 ` kbuild test robot
@ 2020-06-01 16:10 ` Paul Moore
  2020-06-01 22:58   ` Richard Guy Briggs
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Moore @ 2020-06-01 16:10 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	Eric Paris, tgraf

On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> iptables, ip6tables, arptables and ebtables table registration,
> replacement and unregistration configuration events are logged for the
> native (legacy) iptables setsockopt api, but not for the
> nftables netlink api which is used by the nft-variant of iptables in
> addition to nftables itself.
>
> Add calls to log the configuration actions in the nftables netlink api.
>
> This uses the same NETFILTER_CFG record format but overloads the table
> field.
>
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>   ...
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>   ...
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>   ...
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>   ...
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>   ...
>   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
>
> For further information please see issue
> https://github.com/linux-audit/audit-kernel/issues/124
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> Changelog:
> v2:
> - differentiate between xtables and nftables
> - add set, setelem, obj, flowtable, gen
> - use nentries field as appropriate per type
> - overload the "tables" field with table handle and chain/set/flowtable
>
>  include/linux/audit.h         | 52 +++++++++++++++++++++++++
>  kernel/auditsc.c              | 24 ++++++++++--
>  net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 162 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 3fcd9ee49734..d79866a38505 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -12,6 +12,7 @@
>  #include <linux/sched.h>
>  #include <linux/ptrace.h>
>  #include <uapi/linux/audit.h>
> +#include <uapi/linux/netfilter/nf_tables.h>
>
>  #define AUDIT_INO_UNSET ((unsigned long)-1)
>  #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -98,6 +99,57 @@ enum audit_nfcfgop {
>         AUDIT_XT_OP_REGISTER,
>         AUDIT_XT_OP_REPLACE,
>         AUDIT_XT_OP_UNREGISTER,
> +       AUDIT_NFT_OP_TABLE_REGISTER,
> +       AUDIT_NFT_OP_TABLE_UNREGISTER,
> +       AUDIT_NFT_OP_CHAIN_REGISTER,
> +       AUDIT_NFT_OP_CHAIN_UNREGISTER,
> +       AUDIT_NFT_OP_RULE_REGISTER,
> +       AUDIT_NFT_OP_RULE_UNREGISTER,
> +       AUDIT_NFT_OP_SET_REGISTER,
> +       AUDIT_NFT_OP_SET_UNREGISTER,
> +       AUDIT_NFT_OP_SETELEM_REGISTER,
> +       AUDIT_NFT_OP_SETELEM_UNREGISTER,
> +       AUDIT_NFT_OP_GEN_REGISTER,
> +       AUDIT_NFT_OP_OBJ_REGISTER,
> +       AUDIT_NFT_OP_OBJ_UNREGISTER,
> +       AUDIT_NFT_OP_OBJ_RESET,
> +       AUDIT_NFT_OP_FLOWTABLE_REGISTER,
> +       AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> +       AUDIT_NFT_OP_INVALID,
> +};
> +
> +struct audit_nftcfgop_tab {
> +       enum nf_tables_msg_types        nftop;
> +       enum audit_nfcfgop              op;
> +};
> +
> +static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
> +       { NFT_MSG_NEWTABLE,     AUDIT_NFT_OP_TABLE_REGISTER             },
> +       { NFT_MSG_GETTABLE,     AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELTABLE,     AUDIT_NFT_OP_TABLE_UNREGISTER           },
> +       { NFT_MSG_NEWCHAIN,     AUDIT_NFT_OP_CHAIN_REGISTER             },
> +       { NFT_MSG_GETCHAIN,     AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELCHAIN,     AUDIT_NFT_OP_CHAIN_UNREGISTER           },
> +       { NFT_MSG_NEWRULE,      AUDIT_NFT_OP_RULE_REGISTER              },
> +       { NFT_MSG_GETRULE,      AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELRULE,      AUDIT_NFT_OP_RULE_UNREGISTER            },
> +       { NFT_MSG_NEWSET,       AUDIT_NFT_OP_SET_REGISTER               },
> +       { NFT_MSG_GETSET,       AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELSET,       AUDIT_NFT_OP_SET_UNREGISTER             },
> +       { NFT_MSG_NEWSETELEM,   AUDIT_NFT_OP_SETELEM_REGISTER           },
> +       { NFT_MSG_GETSETELEM,   AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELSETELEM,   AUDIT_NFT_OP_SETELEM_UNREGISTER         },
> +       { NFT_MSG_NEWGEN,       AUDIT_NFT_OP_GEN_REGISTER               },
> +       { NFT_MSG_GETGEN,       AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_TRACE,        AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_NEWOBJ,       AUDIT_NFT_OP_OBJ_REGISTER               },
> +       { NFT_MSG_GETOBJ,       AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELOBJ,       AUDIT_NFT_OP_OBJ_UNREGISTER             },
> +       { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET                  },
> +       { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER         },
> +       { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID                    },
> +       { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER       },
> +       { NFT_MSG_MAX,          AUDIT_NFT_OP_INVALID                    },
>  };

I didn't check every "op" defined above to match with the changes in
nf_tables_api.c, but is there a reason why we can't simply hardcode
the AUDIT_NFT_OP_* values in the audit_log_nfcfg() calls in
nf_tables_api.c?  If we can, let's do that.

If we can't do that, we need to add some build-time protection to
catch if NFT_MSG_MAX increases without this table being updated.

>  static int audit_match_perm(struct audit_context *ctx, int mask)
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 4471393da6d8..7a386eca6e04 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -12,6 +12,7 @@
>  #include <linux/netlink.h>
>  #include <linux/vmalloc.h>
>  #include <linux/rhashtable.h>
> +#include <linux/audit.h>
>  #include <linux/netfilter.h>
>  #include <linux/netfilter/nfnetlink.h>
>  #include <linux/netfilter/nf_tables.h>
> @@ -693,6 +694,14 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event)
>  {
>         struct sk_buff *skb;
>         int err;
> +       char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
> +                             ctx->table->name, ctx->table->handle);
> +
> +       audit_log_nfcfg(buf,
> +                       ctx->family,
> +                       ctx->table->use,
> +                       audit_nftcfgs[event].op);

As an example, the below would work, yes?

audit_log_nfcfg(...,
 (event == NFT_MSG_NEWTABLE ?
  AUDIT_NFT_OP_TABLE_REGISTER :
  AUDIT_NFT_OP_TABLE_UNREGISTER)

> +       kfree(buf);
>
>         if (!ctx->report &&
>             !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

* Re: [PATCH ghak124 v2] audit: log nftables configuration change events
  2020-06-01 16:10 ` Paul Moore
@ 2020-06-01 22:58   ` Richard Guy Briggs
  2020-06-02  0:12     ` Paul Moore
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Guy Briggs @ 2020-06-01 22:58 UTC (permalink / raw)
  To: Paul Moore
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	Eric Paris, tgraf

On 2020-06-01 12:10, Paul Moore wrote:
> On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > iptables, ip6tables, arptables and ebtables table registration,
> > replacement and unregistration configuration events are logged for the
> > native (legacy) iptables setsockopt api, but not for the
> > nftables netlink api which is used by the nft-variant of iptables in
> > addition to nftables itself.
> >
> > Add calls to log the configuration actions in the nftables netlink api.
> >
> > This uses the same NETFILTER_CFG record format but overloads the table
> > field.
> >
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >   ...
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >   ...
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >   ...
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >   ...
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >   ...
> >   type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >
> > For further information please see issue
> > https://github.com/linux-audit/audit-kernel/issues/124
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > Changelog:
> > v2:
> > - differentiate between xtables and nftables
> > - add set, setelem, obj, flowtable, gen
> > - use nentries field as appropriate per type
> > - overload the "tables" field with table handle and chain/set/flowtable
> >
> >  include/linux/audit.h         | 52 +++++++++++++++++++++++++
> >  kernel/auditsc.c              | 24 ++++++++++--
> >  net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 162 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 3fcd9ee49734..d79866a38505 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -12,6 +12,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/ptrace.h>
> >  #include <uapi/linux/audit.h>
> > +#include <uapi/linux/netfilter/nf_tables.h>
> >
> >  #define AUDIT_INO_UNSET ((unsigned long)-1)
> >  #define AUDIT_DEV_UNSET ((dev_t)-1)
> > @@ -98,6 +99,57 @@ enum audit_nfcfgop {
> >         AUDIT_XT_OP_REGISTER,
> >         AUDIT_XT_OP_REPLACE,
> >         AUDIT_XT_OP_UNREGISTER,
> > +       AUDIT_NFT_OP_TABLE_REGISTER,
> > +       AUDIT_NFT_OP_TABLE_UNREGISTER,
> > +       AUDIT_NFT_OP_CHAIN_REGISTER,
> > +       AUDIT_NFT_OP_CHAIN_UNREGISTER,
> > +       AUDIT_NFT_OP_RULE_REGISTER,
> > +       AUDIT_NFT_OP_RULE_UNREGISTER,
> > +       AUDIT_NFT_OP_SET_REGISTER,
> > +       AUDIT_NFT_OP_SET_UNREGISTER,
> > +       AUDIT_NFT_OP_SETELEM_REGISTER,
> > +       AUDIT_NFT_OP_SETELEM_UNREGISTER,
> > +       AUDIT_NFT_OP_GEN_REGISTER,
> > +       AUDIT_NFT_OP_OBJ_REGISTER,
> > +       AUDIT_NFT_OP_OBJ_UNREGISTER,
> > +       AUDIT_NFT_OP_OBJ_RESET,
> > +       AUDIT_NFT_OP_FLOWTABLE_REGISTER,
> > +       AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> > +       AUDIT_NFT_OP_INVALID,
> > +};
> > +
> > +struct audit_nftcfgop_tab {
> > +       enum nf_tables_msg_types        nftop;
> > +       enum audit_nfcfgop              op;
> > +};
> > +
> > +static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
> > +       { NFT_MSG_NEWTABLE,     AUDIT_NFT_OP_TABLE_REGISTER             },
> > +       { NFT_MSG_GETTABLE,     AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELTABLE,     AUDIT_NFT_OP_TABLE_UNREGISTER           },
> > +       { NFT_MSG_NEWCHAIN,     AUDIT_NFT_OP_CHAIN_REGISTER             },
> > +       { NFT_MSG_GETCHAIN,     AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELCHAIN,     AUDIT_NFT_OP_CHAIN_UNREGISTER           },
> > +       { NFT_MSG_NEWRULE,      AUDIT_NFT_OP_RULE_REGISTER              },
> > +       { NFT_MSG_GETRULE,      AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELRULE,      AUDIT_NFT_OP_RULE_UNREGISTER            },
> > +       { NFT_MSG_NEWSET,       AUDIT_NFT_OP_SET_REGISTER               },
> > +       { NFT_MSG_GETSET,       AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELSET,       AUDIT_NFT_OP_SET_UNREGISTER             },
> > +       { NFT_MSG_NEWSETELEM,   AUDIT_NFT_OP_SETELEM_REGISTER           },
> > +       { NFT_MSG_GETSETELEM,   AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELSETELEM,   AUDIT_NFT_OP_SETELEM_UNREGISTER         },
> > +       { NFT_MSG_NEWGEN,       AUDIT_NFT_OP_GEN_REGISTER               },
> > +       { NFT_MSG_GETGEN,       AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_TRACE,        AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_NEWOBJ,       AUDIT_NFT_OP_OBJ_REGISTER               },
> > +       { NFT_MSG_GETOBJ,       AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELOBJ,       AUDIT_NFT_OP_OBJ_UNREGISTER             },
> > +       { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET                  },
> > +       { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER         },
> > +       { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID                    },
> > +       { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER       },
> > +       { NFT_MSG_MAX,          AUDIT_NFT_OP_INVALID                    },
> >  };
> 
> I didn't check every "op" defined above to match with the changes in
> nf_tables_api.c, but is there a reason why we can't simply hardcode
> the AUDIT_NFT_OP_* values in the audit_log_nfcfg() calls in
> nf_tables_api.c?  If we can, let's do that.

Yes, to distinguish between x/ebtables and nftables.  The first three
overlap (as suggested by the first comment in the changelog above).

> If we can't do that, we need to add some build-time protection to
> catch if NFT_MSG_MAX increases without this table being updated.

I would have liked to do that, but it would then have meant making a new
function name, say audit_log_nftcfg vs the original audit_log_nfcfg so
that the overlapping values could be translated appropriately with the
correct prefix.

Well, considering the buildbot error, I had to restructure it so that
the table was moved from include/linux/audit.h to
net/netfilter/nf_tables_api.c where it would be a bit more evident.

At first I thought the bot was complaining about the first field not
being used, but it turned out it was ptrace that was including it and
not using it.

> >  static int audit_match_perm(struct audit_context *ctx, int mask)
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 4471393da6d8..7a386eca6e04 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/netlink.h>
> >  #include <linux/vmalloc.h>
> >  #include <linux/rhashtable.h>
> > +#include <linux/audit.h>
> >  #include <linux/netfilter.h>
> >  #include <linux/netfilter/nfnetlink.h>
> >  #include <linux/netfilter/nf_tables.h>
> > @@ -693,6 +694,14 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event)
> >  {
> >         struct sk_buff *skb;
> >         int err;
> > +       char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
> > +                             ctx->table->name, ctx->table->handle);
> > +
> > +       audit_log_nfcfg(buf,
> > +                       ctx->family,
> > +                       ctx->table->use,
> > +                       audit_nftcfgs[event].op);
> 
> As an example, the below would work, yes?
> 
> audit_log_nfcfg(...,
>  (event == NFT_MSG_NEWTABLE ?
>   AUDIT_NFT_OP_TABLE_REGISTER :
>   AUDIT_NFT_OP_TABLE_UNREGISTER)

Ok, I see what you are getting at now...  Yes, it could be done this
way, but it seems noisier to me.

> > +       kfree(buf);
> >
> >         if (!ctx->report &&
> >             !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> 
> 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

* Re: [PATCH ghak124 v2] audit: log nftables configuration change events
  2020-06-01 22:58   ` Richard Guy Briggs
@ 2020-06-02  0:12     ` Paul Moore
  2020-06-02 11:34       ` Richard Guy Briggs
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Moore @ 2020-06-02  0:12 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	Eric Paris, tgraf

On Mon, Jun 1, 2020 at 6:58 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-06-01 12:10, Paul Moore wrote:
> > On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <rgb@redhat.com> wrote:

...

> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 4471393da6d8..7a386eca6e04 100644
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/netlink.h>
> > >  #include <linux/vmalloc.h>
> > >  #include <linux/rhashtable.h>
> > > +#include <linux/audit.h>
> > >  #include <linux/netfilter.h>
> > >  #include <linux/netfilter/nfnetlink.h>
> > >  #include <linux/netfilter/nf_tables.h>
> > > @@ -693,6 +694,14 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event)
> > >  {
> > >         struct sk_buff *skb;
> > >         int err;
> > > +       char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
> > > +                             ctx->table->name, ctx->table->handle);
> > > +
> > > +       audit_log_nfcfg(buf,
> > > +                       ctx->family,
> > > +                       ctx->table->use,
> > > +                       audit_nftcfgs[event].op);
> >
> > As an example, the below would work, yes?
> >
> > audit_log_nfcfg(...,
> >  (event == NFT_MSG_NEWTABLE ?
> >   AUDIT_NFT_OP_TABLE_REGISTER :
> >   AUDIT_NFT_OP_TABLE_UNREGISTER)
>
> Ok, I see what you are getting at now...  Yes, it could be done this
> way, but it seems noisier to me.

I'll admit it is not as clean, but it doesn't hide the mapping between
the netfilter operation and the audit operation which hopefully makes
it clear to those modifying the netfilter/nf_tables/etc. code that
there is an audit impact.  I'm basically trying to make sure the code
is as robust as possible in the face of subsystem changes beyond the
audit subsystem.

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

* Re: [PATCH ghak124 v2] audit: log nftables configuration change events
  2020-06-02  0:12     ` Paul Moore
@ 2020-06-02 11:34       ` Richard Guy Briggs
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2020-06-02 11:34 UTC (permalink / raw)
  To: Paul Moore
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	Eric Paris, tgraf

On 2020-06-01 20:12, Paul Moore wrote:
> On Mon, Jun 1, 2020 at 6:58 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2020-06-01 12:10, Paul Moore wrote:
> > > On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> ...
> 
> > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > > index 4471393da6d8..7a386eca6e04 100644
> > > > --- a/net/netfilter/nf_tables_api.c
> > > > +++ b/net/netfilter/nf_tables_api.c
> > > > @@ -12,6 +12,7 @@
> > > >  #include <linux/netlink.h>
> > > >  #include <linux/vmalloc.h>
> > > >  #include <linux/rhashtable.h>
> > > > +#include <linux/audit.h>
> > > >  #include <linux/netfilter.h>
> > > >  #include <linux/netfilter/nfnetlink.h>
> > > >  #include <linux/netfilter/nf_tables.h>
> > > > @@ -693,6 +694,14 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event)
> > > >  {
> > > >         struct sk_buff *skb;
> > > >         int err;
> > > > +       char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0",
> > > > +                             ctx->table->name, ctx->table->handle);
> > > > +
> > > > +       audit_log_nfcfg(buf,
> > > > +                       ctx->family,
> > > > +                       ctx->table->use,
> > > > +                       audit_nftcfgs[event].op);
> > >
> > > As an example, the below would work, yes?
> > >
> > > audit_log_nfcfg(...,
> > >  (event == NFT_MSG_NEWTABLE ?
> > >   AUDIT_NFT_OP_TABLE_REGISTER :
> > >   AUDIT_NFT_OP_TABLE_UNREGISTER)
> >
> > Ok, I see what you are getting at now...  Yes, it could be done this
> > way, but it seems noisier to me.
> 
> I'll admit it is not as clean, but it doesn't hide the mapping between
> the netfilter operation and the audit operation which hopefully makes
> it clear to those modifying the netfilter/nf_tables/etc. code that
> there is an audit impact.  I'm basically trying to make sure the code
> is as robust as possible in the face of subsystem changes beyond the
> audit subsystem.

Yup, I agree, a compile time check to make sure they aren't out of sync.

> 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  1:44 [PATCH ghak124 v2] audit: log nftables configuration change events Richard Guy Briggs
2020-05-31 23:38 ` kbuild test robot
2020-06-01 16:10 ` Paul Moore
2020-06-01 22:58   ` Richard Guy Briggs
2020-06-02  0:12     ` Paul Moore
2020-06-02 11:34       ` Richard Guy Briggs

Linux-audit Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-audit/0 linux-audit/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 linux-audit linux-audit/ https://lore.kernel.org/linux-audit \
		linux-audit@redhat.com
	public-inbox-index linux-audit

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.redhat.linux-audit


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