Linux-audit Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] audit: log nftables configuration change events once per table
@ 2021-03-18 15:39 Richard Guy Briggs
  2021-03-18 16:29 ` Pablo Neira Ayuso
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2021-03-18 15:39 UTC (permalink / raw)
  To: Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: Jones Desougi, Richard Guy Briggs, Phil Sutter, Florian Westphal,
	twoerner, Eric Paris, tgraf, dan.carpenter

Reduce logging of nftables events to a level similar to iptables.
Restore the table field to list the table, adding the generation.

Indicate the op as the most significant operation in the event.

A couple of sample events:

type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64 syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=roo
t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv6 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv4 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=inet entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld

type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64 syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=r
oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv6 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv4 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=inet entries=165 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld

The issue was originally documented in
https://github.com/linux-audit/audit-kernel/issues/124

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h         |  29 ++++++++
 net/netfilter/nf_tables_api.c | 132 +++++++++++++---------------------
 2 files changed, 78 insertions(+), 83 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 82b7c1116a85..bba6a0386742 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -118,6 +118,35 @@ enum audit_nfcfgop {
 	AUDIT_NFT_OP_INVALID,
 };
 
+static const u8 nft2audit_op[] = { // enum nf_tables_msg_types
+	/* 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);
 
 extern int __init audit_register_class(int class, unsigned *list);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8d5aa0ac45f4..ad31d8876169 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -709,17 +709,6 @@ 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,
-			event == NFT_MSG_NEWTABLE ?
-				AUDIT_NFT_OP_TABLE_REGISTER :
-				AUDIT_NFT_OP_TABLE_UNREGISTER,
-			GFP_KERNEL);
-	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -1476,18 +1465,6 @@ 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,
-			event == NFT_MSG_NEWCHAIN ?
-				AUDIT_NFT_OP_CHAIN_REGISTER :
-				AUDIT_NFT_OP_CHAIN_UNREGISTER,
-			GFP_KERNEL);
-	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -2838,18 +2815,6 @@ 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,
-			event == NFT_MSG_NEWRULE ?
-				AUDIT_NFT_OP_RULE_REGISTER :
-				AUDIT_NFT_OP_RULE_UNREGISTER,
-			GFP_KERNEL);
-	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -3882,18 +3847,6 @@ 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,
-			event == NFT_MSG_NEWSET ?
-				AUDIT_NFT_OP_SET_REGISTER :
-				AUDIT_NFT_OP_SET_UNREGISTER,
-			gfp_flags);
-	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -5067,18 +5020,6 @@ 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,
-			event == NFT_MSG_NEWSETELEM ?
-				AUDIT_NFT_OP_SETELEM_REGISTER :
-				AUDIT_NFT_OP_SETELEM_UNREGISTER,
-			GFP_KERNEL);
-	kfree(buf);
 
 	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
 		return;
@@ -6278,12 +6219,11 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 			    filter->type != NFT_OBJECT_UNSPEC &&
 			    obj->ops->type->type != filter->type)
 				goto cont;
-
 			if (reset) {
 				char *buf = kasprintf(GFP_ATOMIC,
-						      "%s:%llu;?:0",
+						      "%s:%u",
 						      table->name,
-						      table->handle);
+						      net->nft.base_seq);
 
 				audit_log_nfcfg(buf,
 						family,
@@ -6404,8 +6344,8 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
 		reset = true;
 
 	if (reset) {
-		char *buf = kasprintf(GFP_ATOMIC, "%s:%llu;?:0",
-				      table->name, table->handle);
+		char *buf = kasprintf(GFP_ATOMIC, "%s:%u",
+				      table->name, net->nft.base_seq);
 
 		audit_log_nfcfg(buf,
 				family,
@@ -6492,15 +6432,15 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
 {
 	struct sk_buff *skb;
 	int err;
-	char *buf = kasprintf(gfp, "%s:%llu;?:0",
-			      table->name, table->handle);
+	char *buf = kasprintf(gfp, "%s:%u",
+			      table->name, net->nft.base_seq);
 
 	audit_log_nfcfg(buf,
 			family,
 			obj->handle,
 			event == NFT_MSG_NEWOBJ ?
-				AUDIT_NFT_OP_OBJ_REGISTER :
-				AUDIT_NFT_OP_OBJ_UNREGISTER,
+				 AUDIT_NFT_OP_OBJ_REGISTER :
+				 AUDIT_NFT_OP_OBJ_UNREGISTER,
 			gfp);
 	kfree(buf);
 
@@ -7300,18 +7240,6 @@ 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,
-			event == NFT_MSG_NEWFLOWTABLE ?
-				AUDIT_NFT_OP_FLOWTABLE_REGISTER :
-				AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
-			GFP_KERNEL);
-	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -7432,9 +7360,6 @@ 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_NFT_OP_GEN_REGISTER, GFP_KERNEL);
-
 	if (!nlmsg_report(nlh) &&
 	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
 		return;
@@ -7979,6 +7904,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 	struct nft_trans_elem *te;
 	struct nft_chain *chain;
 	struct nft_table *table;
+	struct audit_log_nftdata {
+		struct nft_table *table;
+		int entries;
+		int op;
+		struct audit_log_nftdata *next;
+	} ad = { NULL, 0, 0, NULL }, *adp, *adnext;
+#define AUNFTABLENAMELEN (NFT_TABLE_MAXNAMELEN + 22)
+	char aubuf[AUNFTABLENAMELEN];
 	int err;
 
 	if (list_empty(&net->nft.commit_list)) {
@@ -8173,12 +8106,45 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			}
 			break;
 		}
+		adp = &ad;
+		if (!adp->table) {
+			adp->table = trans->ctx.table;
+			adp->entries = 1;
+			adp->op = trans->msg_type;
+		} else {
+			adnext = &ad;
+			do {
+				adp = adnext;
+				if (adp->table == trans->ctx.table)
+					goto found;
+				adnext = adp->next;
+			} while (adnext);
+			adp->next = kzalloc(sizeof(*adp->next), GFP_KERNEL);
+			adp = adp->next;
+			adp->table = trans->ctx.table;
+found:
+			adp->entries++;
+			if (!adp->op || adp->op > trans->msg_type)
+				adp->op = trans->msg_type;
+		}
 	}
 
 	nft_commit_notify(net, NETLINK_CB(skb).portid);
 	nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
 	nf_tables_commit_release(net);
 
+	adp = &ad;
+	while (adp && adp->table) {
+		snprintf(aubuf, AUNFTABLENAMELEN, "%s:%u", adp->table->name,
+			 net->nft.base_seq);
+		audit_log_nfcfg(aubuf, adp->table->family, adp->entries,
+				nft2audit_op[adp->op], GFP_KERNEL);
+		adnext = adp->next;
+		if (adp != &ad)
+			kfree(adp);
+		adp = adnext;
+	}
+
 	return 0;
 }
 
-- 
2.27.0

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


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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-18 15:39 [PATCH] audit: log nftables configuration change events once per table Richard Guy Briggs
@ 2021-03-18 16:29 ` Pablo Neira Ayuso
  2021-03-18 16:30 ` Phil Sutter
  2021-03-18 19:39 ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2021-03-18 16:29 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Jones Desougi, Phil Sutter, Florian Westphal, LKML,
	Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris,
	tgraf, dan.carpenter

On Thu, Mar 18, 2021 at 11:39:52AM -0400, Richard Guy Briggs wrote:
> Reduce logging of nftables events to a level similar to iptables.
> Restore the table field to list the table, adding the generation.
> 
> Indicate the op as the most significant operation in the event.
> 
> A couple of sample events:
> 
> type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64 syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=roo
> t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv6 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv4 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=inet entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> 
> type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64 syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=r
> oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv6 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv4 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=inet entries=165 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> 
> The issue was originally documented in
> https://github.com/linux-audit/audit-kernel/issues/124
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/linux/audit.h         |  29 ++++++++
>  net/netfilter/nf_tables_api.c | 132 +++++++++++++---------------------
>  2 files changed, 78 insertions(+), 83 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 82b7c1116a85..bba6a0386742 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -118,6 +118,35 @@ enum audit_nfcfgop {
>  	AUDIT_NFT_OP_INVALID,
>  };
>  
> +static const u8 nft2audit_op[] = { // enum nf_tables_msg_types
> +	/* 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);
>  
>  extern int __init audit_register_class(int class, unsigned *list);
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 8d5aa0ac45f4..ad31d8876169 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -709,17 +709,6 @@ 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,
> -			event == NFT_MSG_NEWTABLE ?
> -				AUDIT_NFT_OP_TABLE_REGISTER :
> -				AUDIT_NFT_OP_TABLE_UNREGISTER,
> -			GFP_KERNEL);
> -	kfree(buf);
>  
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -1476,18 +1465,6 @@ 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,
> -			event == NFT_MSG_NEWCHAIN ?
> -				AUDIT_NFT_OP_CHAIN_REGISTER :
> -				AUDIT_NFT_OP_CHAIN_UNREGISTER,
> -			GFP_KERNEL);
> -	kfree(buf);
>  
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -2838,18 +2815,6 @@ 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,
> -			event == NFT_MSG_NEWRULE ?
> -				AUDIT_NFT_OP_RULE_REGISTER :
> -				AUDIT_NFT_OP_RULE_UNREGISTER,
> -			GFP_KERNEL);
> -	kfree(buf);
>  
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -3882,18 +3847,6 @@ 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,
> -			event == NFT_MSG_NEWSET ?
> -				AUDIT_NFT_OP_SET_REGISTER :
> -				AUDIT_NFT_OP_SET_UNREGISTER,
> -			gfp_flags);
> -	kfree(buf);
>  
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -5067,18 +5020,6 @@ 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,
> -			event == NFT_MSG_NEWSETELEM ?
> -				AUDIT_NFT_OP_SETELEM_REGISTER :
> -				AUDIT_NFT_OP_SETELEM_UNREGISTER,
> -			GFP_KERNEL);
> -	kfree(buf);
>  
>  	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
>  		return;
> @@ -6278,12 +6219,11 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  			    filter->type != NFT_OBJECT_UNSPEC &&
>  			    obj->ops->type->type != filter->type)
>  				goto cont;
> -
>  			if (reset) {
>  				char *buf = kasprintf(GFP_ATOMIC,
> -						      "%s:%llu;?:0",
> +						      "%s:%u",
>  						      table->name,
> -						      table->handle);
> +						      net->nft.base_seq);
>  
>  				audit_log_nfcfg(buf,
>  						family,
> @@ -6404,8 +6344,8 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
>  		reset = true;
>  
>  	if (reset) {
> -		char *buf = kasprintf(GFP_ATOMIC, "%s:%llu;?:0",
> -				      table->name, table->handle);
> +		char *buf = kasprintf(GFP_ATOMIC, "%s:%u",
> +				      table->name, net->nft.base_seq);
>  
>  		audit_log_nfcfg(buf,
>  				family,
> @@ -6492,15 +6432,15 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
>  {
>  	struct sk_buff *skb;
>  	int err;
> -	char *buf = kasprintf(gfp, "%s:%llu;?:0",
> -			      table->name, table->handle);
> +	char *buf = kasprintf(gfp, "%s:%u",
> +			      table->name, net->nft.base_seq);
>  
>  	audit_log_nfcfg(buf,
>  			family,
>  			obj->handle,
>  			event == NFT_MSG_NEWOBJ ?
> -				AUDIT_NFT_OP_OBJ_REGISTER :
> -				AUDIT_NFT_OP_OBJ_UNREGISTER,
> +				 AUDIT_NFT_OP_OBJ_REGISTER :
> +				 AUDIT_NFT_OP_OBJ_UNREGISTER,
>  			gfp);
>  	kfree(buf);
>  
> @@ -7300,18 +7240,6 @@ 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,
> -			event == NFT_MSG_NEWFLOWTABLE ?
> -				AUDIT_NFT_OP_FLOWTABLE_REGISTER :
> -				AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> -			GFP_KERNEL);
> -	kfree(buf);
>  
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -7432,9 +7360,6 @@ 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_NFT_OP_GEN_REGISTER, GFP_KERNEL);
> -
>  	if (!nlmsg_report(nlh) &&
>  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
>  		return;
> @@ -7979,6 +7904,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>  	struct nft_trans_elem *te;
>  	struct nft_chain *chain;
>  	struct nft_table *table;
> +	struct audit_log_nftdata {
> +		struct nft_table *table;
> +		int entries;
> +		int op;
> +		struct audit_log_nftdata *next;

Use linux lists instead?

> +	} ad = { NULL, 0, 0, NULL }, *adp, *adnext;
> +#define AUNFTABLENAMELEN (NFT_TABLE_MAXNAMELEN + 22)
> +	char aubuf[AUNFTABLENAMELEN];
>  	int err;
>  
>  	if (list_empty(&net->nft.commit_list)) {
> @@ -8173,12 +8106,45 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>  			}
>  			break;
>  		}
> +		adp = &ad;
> +		if (!adp->table) {
> +			adp->table = trans->ctx.table;
> +			adp->entries = 1;
> +			adp->op = trans->msg_type;
> +		} else {
> +			adnext = &ad;
> +			do {
> +				adp = adnext;
> +				if (adp->table == trans->ctx.table)
> +					goto found;
> +				adnext = adp->next;
> +			} while (adnext);
> +			adp->next = kzalloc(sizeof(*adp->next), GFP_KERNEL);
> +			adp = adp->next;
> +			adp->table = trans->ctx.table;
> +found:
> +			adp->entries++;
> +			if (!adp->op || adp->op > trans->msg_type)
> +				adp->op = trans->msg_type;
> +		}

Would you wrap this code in a function?

>  	}
>  
>  	nft_commit_notify(net, NETLINK_CB(skb).portid);
>  	nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
>  	nf_tables_commit_release(net);
>  
> +	adp = &ad;
> +	while (adp && adp->table) {
> +		snprintf(aubuf, AUNFTABLENAMELEN, "%s:%u", adp->table->name,
> +			 net->nft.base_seq);
> +		audit_log_nfcfg(aubuf, adp->table->family, adp->entries,
> +				nft2audit_op[adp->op], GFP_KERNEL);
> +		adnext = adp->next;
> +		if (adp != &ad)
> +			kfree(adp);
> +		adp = adnext;
> +	}

Same thing here.

Thanks.

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


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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-18 15:39 [PATCH] audit: log nftables configuration change events once per table Richard Guy Briggs
  2021-03-18 16:29 ` Pablo Neira Ayuso
@ 2021-03-18 16:30 ` Phil Sutter
  2021-03-18 18:37   ` Richard Guy Briggs
  2021-03-18 19:39 ` kernel test robot
  2 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2021-03-18 16:30 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Jones Desougi, Florian Westphal, LKML, Linux-Audit Mailing List,
	netfilter-devel, twoerner, Eric Paris, tgraf, dan.carpenter

Hi,

On Thu, Mar 18, 2021 at 11:39:52AM -0400, Richard Guy Briggs wrote:
> Reduce logging of nftables events to a level similar to iptables.
> Restore the table field to list the table, adding the generation.

This looks much better, a few remarks below:

[...]
> +static const u8 nft2audit_op[] = { // enum nf_tables_msg_types
> +	/* 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,
> +};

NFT_MSG_MAX is itself not a valid message, it serves merely as an upper
bound for arrays, loops or sanity checks. You will never see it in
trans->msg_type.

Since enum nf_tables_msg_types contains consecutive values from 0 to
NFT_MSG_MAX, you could write the above more explicitly:

| static const u8 nft2audit_op[NFT_MSG_MAX] = {
| 	[NFT_MSG_NEWTABLE]	= AUDIT_NFT_OP_TABLE_REGISTER,
| 	[NFT_MSG_GETTABLE]	= AUDIT_NFT_OP_INVALID,
| 	[NFT_MSG_DELTABLE]	= AUDIT_NFT_OP_TABLE_UNREGISTER,
(And so forth.)

Not a must, but it clarifies the 1:1 mapping between index and said
enum. Sadly, AUDIT_NFT_OP_INVALID is non-zero. Otherwise one could skip
all uninteresting ones.

[...]
> @@ -6278,12 +6219,11 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  			    filter->type != NFT_OBJECT_UNSPEC &&
>  			    obj->ops->type->type != filter->type)
>  				goto cont;
> -
>  			if (reset) {
>  				char *buf = kasprintf(GFP_ATOMIC,
> -						      "%s:%llu;?:0",
> +						      "%s:%u",
>  						      table->name,
> -						      table->handle);
> +						      net->nft.base_seq);
>  
>  				audit_log_nfcfg(buf,
>  						family,

Why did you leave the object-related logs in place? They should reappear
at commit time just like chains and sets for instance, no?

Thanks, Phil

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


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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-18 16:30 ` Phil Sutter
@ 2021-03-18 18:37   ` Richard Guy Briggs
  2021-03-19 12:52     ` Phil Sutter
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Guy Briggs @ 2021-03-18 18:37 UTC (permalink / raw)
  To: Phil Sutter, Linux-Audit Mailing List, LKML, netfilter-devel,
	Paul Moore, Eric Paris, Steve Grubb, Florian Westphal, twoerner,
	tgraf, dan.carpenter, Jones Desougi

On 2021-03-18 17:30, Phil Sutter wrote:
> Hi,
> 
> On Thu, Mar 18, 2021 at 11:39:52AM -0400, Richard Guy Briggs wrote:
> > Reduce logging of nftables events to a level similar to iptables.
> > Restore the table field to list the table, adding the generation.
> 
> This looks much better, a few remarks below:
> 
> [...]
> > +static const u8 nft2audit_op[] = { // enum nf_tables_msg_types
> > +	/* 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,
> > +};
> 
> NFT_MSG_MAX is itself not a valid message, it serves merely as an upper
> bound for arrays, loops or sanity checks. You will never see it in
> trans->msg_type.
> 
> Since enum nf_tables_msg_types contains consecutive values from 0 to
> NFT_MSG_MAX, you could write the above more explicitly:
> 
> | static const u8 nft2audit_op[NFT_MSG_MAX] = {
> | 	[NFT_MSG_NEWTABLE]	= AUDIT_NFT_OP_TABLE_REGISTER,
> | 	[NFT_MSG_GETTABLE]	= AUDIT_NFT_OP_INVALID,
> | 	[NFT_MSG_DELTABLE]	= AUDIT_NFT_OP_TABLE_UNREGISTER,
> (And so forth.)
> 
> Not a must, but it clarifies the 1:1 mapping between index and said
> enum. Sadly, AUDIT_NFT_OP_INVALID is non-zero. Otherwise one could skip
> all uninteresting ones.

Yes, ok, I prefer your suggested way of listing them.

Yeah, the fact the values for op= already have a precedent in xtables
limits us.

> [...]
> > @@ -6278,12 +6219,11 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
> >  			    filter->type != NFT_OBJECT_UNSPEC &&
> >  			    obj->ops->type->type != filter->type)
> >  				goto cont;
> > -
> >  			if (reset) {
> >  				char *buf = kasprintf(GFP_ATOMIC,
> > -						      "%s:%llu;?:0",
> > +						      "%s:%u",
> >  						      table->name,
> > -						      table->handle);
> > +						      net->nft.base_seq);
> >  
> >  				audit_log_nfcfg(buf,
> >  						family,
> 
> Why did you leave the object-related logs in place? They should reappear
> at commit time just like chains and sets for instance, no?

There are other paths that can trigger these messages that don't go
through nf_tables_commit() that affect the configuration data.  The
counters are considered config data for auditing purposes and the act of
resetting them is audittable.  And the only time we want to emit a
record is when they are being reset.

> Thanks, Phil

- 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://listman.redhat.com/mailman/listinfo/linux-audit


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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-18 15:39 [PATCH] audit: log nftables configuration change events once per table Richard Guy Briggs
  2021-03-18 16:29 ` Pablo Neira Ayuso
  2021-03-18 16:30 ` Phil Sutter
@ 2021-03-18 19:39 ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-03-18 19:39 UTC (permalink / raw)
  To: Richard Guy Briggs, Linux-Audit Mailing List, LKML, netfilter-devel
  Cc: kbuild-all, Richard Guy Briggs, Phil Sutter, Florian Westphal,
	twoerner, Eric Paris


[-- Attachment #1: Type: text/plain, Size: 3965 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 nf/master nf-next/master linux/master linus/master v5.12-rc3 next-20210318]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-log-nftables-configuration-change-events-once-per-table/20210318-234408
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: sparc64-randconfig-p002-20210318 (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
        # https://github.com/0day-ci/linux/commit/6c3d6ac0f39a29e8474b0ff59609e3c3168339c0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Richard-Guy-Briggs/audit-log-nftables-configuration-change-events-once-per-table/20210318-234408
        git checkout 6c3d6ac0f39a29e8474b0ff59609e3c3168339c0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64 

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

All errors (new ones prefixed by >>):

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

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS || MCOUNT
   Selected by
   - LOCKDEP && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86


vim +/nft2audit_op +121 include/linux/audit.h

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

---
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: 23636 bytes --]

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

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

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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-18 18:37   ` Richard Guy Briggs
@ 2021-03-19 12:52     ` Phil Sutter
  2021-03-19 12:57       ` Richard Guy Briggs
  0 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2021-03-19 12:52 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Jones Desougi, Florian Westphal, LKML, Linux-Audit Mailing List,
	netfilter-devel, twoerner, Eric Paris, tgraf, dan.carpenter

On Thu, Mar 18, 2021 at 02:37:03PM -0400, Richard Guy Briggs wrote:
> On 2021-03-18 17:30, Phil Sutter wrote:
[...]
> > Why did you leave the object-related logs in place? They should reappear
> > at commit time just like chains and sets for instance, no?
> 
> There are other paths that can trigger these messages that don't go
> through nf_tables_commit() that affect the configuration data.  The
> counters are considered config data for auditing purposes and the act of
> resetting them is audittable.  And the only time we want to emit a
> record is when they are being reset.

Oh, I see. I wasn't aware 'nft reset' bypasses the transaction logic,
thanks for clarifying!

Cheers, Phil

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


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

* Re: [PATCH] audit: log nftables configuration change events once per table
  2021-03-19 12:52     ` Phil Sutter
@ 2021-03-19 12:57       ` Richard Guy Briggs
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2021-03-19 12:57 UTC (permalink / raw)
  To: Phil Sutter, Linux-Audit Mailing List, LKML, netfilter-devel,
	Paul Moore, Eric Paris, Steve Grubb, Florian Westphal, twoerner,
	tgraf, dan.carpenter, Jones Desougi

On 2021-03-19 13:52, Phil Sutter wrote:
> On Thu, Mar 18, 2021 at 02:37:03PM -0400, Richard Guy Briggs wrote:
> > On 2021-03-18 17:30, Phil Sutter wrote:
> [...]
> > > Why did you leave the object-related logs in place? They should reappear
> > > at commit time just like chains and sets for instance, no?
> > 
> > There are other paths that can trigger these messages that don't go
> > through nf_tables_commit() that affect the configuration data.  The
> > counters are considered config data for auditing purposes and the act of
> > resetting them is audittable.  And the only time we want to emit a
> > record is when they are being reset.
> 
> Oh, I see. I wasn't aware 'nft reset' bypasses the transaction logic,
> thanks for clarifying!

That's my current understanding.  If someone else has a better
understanding I'd be grateful if they could correct me.

> Cheers, Phil

- 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://listman.redhat.com/mailman/listinfo/linux-audit


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 15:39 [PATCH] audit: log nftables configuration change events once per table Richard Guy Briggs
2021-03-18 16:29 ` Pablo Neira Ayuso
2021-03-18 16:30 ` Phil Sutter
2021-03-18 18:37   ` Richard Guy Briggs
2021-03-19 12:52     ` Phil Sutter
2021-03-19 12:57       ` Richard Guy Briggs
2021-03-18 19:39 ` kernel test robot

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