Linux-audit Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH ghak124 v3] audit: log nftables configuration change events
@ 2020-06-04 13:20 Richard Guy Briggs
  2020-06-04 17:03 ` Steve Grubb
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Richard Guy Briggs @ 2020-06-04 13:20 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:
v3:
- inline message type rather than table

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         |  18 ++++++++
 kernel/auditsc.c              |  24 ++++++++--
 net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 3 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 3fcd9ee49734..604ede630580 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,23 @@ 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,
 };
 
 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 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -1428,6 +1439,17 @@ 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);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -2691,6 +2713,17 @@ 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);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -3693,6 +3726,17 @@ 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);
+	kfree(buf);
 
 	if (!ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -4809,6 +4853,17 @@ 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);
+	kfree(buf);
 
 	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
 		return;
@@ -5890,6 +5945,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_NFT_OP_OBJ_RESET);
+				kfree(buf);
+			}
+
 			if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
 						    cb->nlh->nlmsg_seq,
 						    NFT_MSG_NEWOBJ,
@@ -6000,6 +6068,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_NFT_OP_OBJ_RESET);
+		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);
@@ -6075,6 +6154,16 @@ 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,
+			event == NFT_MSG_NEWOBJ ?
+				AUDIT_NFT_OP_OBJ_REGISTER :
+				AUDIT_NFT_OP_OBJ_UNREGISTER);
+	kfree(buf);
 
 	if (!report &&
 	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
@@ -6701,6 +6790,17 @@ 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);
+	kfree(buf);
 
 	if (ctx->report &&
 	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -6822,6 +6922,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_NFT_OP_GEN_REGISTER);
+
 	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] 9+ messages in thread

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs
@ 2020-06-04 17:03 ` Steve Grubb
  2020-06-04 17:57   ` Richard Guy Briggs
  2020-06-24  0:34 ` Paul Moore
  2020-06-24 10:03 ` Pablo Neira Ayuso
  2 siblings, 1 reply; 9+ messages in thread
From: Steve Grubb @ 2020-06-04 17:03 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On Thursday, June 4, 2020 9:20:49 AM EDT Richard Guy Briggs 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:
> v3:
> - inline message type rather than table
> 
> 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         |  18 ++++++++
>  kernel/auditsc.c              |  24 ++++++++--
>  net/netfilter/nf_tables_api.c | 103
> ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142
> insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 3fcd9ee49734..604ede630580 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,23 @@ 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,
>  };
> 
>  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"		   
},
>  };

I still don't like the event format because it doesn't give complete subject 
information. However, I thought I'd comment on this string table. Usually 
it's sufficient to log the number and then have the string table in user space 
which looks it up during interpretation.

-Steve

>  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 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> +	kfree(buf);
> 
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -1428,6 +1439,17 @@ 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);
> +	kfree(buf);
> 
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -2691,6 +2713,17 @@ 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);
> +	kfree(buf);
> 
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -3693,6 +3726,17 @@ 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);
> +	kfree(buf);
> 
>  	if (!ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -4809,6 +4853,17 @@ 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);
> +	kfree(buf);
> 
>  	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
>  		return;
> @@ -5890,6 +5945,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_NFT_OP_OBJ_RESET);
> +				kfree(buf);
> +			}
> +
>  			if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb-
>skb).portid,
>  						    cb->nlh->nlmsg_seq,
>  						    NFT_MSG_NEWOBJ,
> @@ -6000,6 +6068,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_NFT_OP_OBJ_RESET);
> +		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);
> @@ -6075,6 +6154,16 @@ 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,
> +			event == NFT_MSG_NEWOBJ ?
> +				AUDIT_NFT_OP_OBJ_REGISTER :
> +				AUDIT_NFT_OP_OBJ_UNREGISTER);
> +	kfree(buf);
> 
>  	if (!report &&
>  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> @@ -6701,6 +6790,17 @@ 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);
> +	kfree(buf);
> 
>  	if (ctx->report &&
>  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> @@ -6822,6 +6922,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_NFT_OP_GEN_REGISTER);
> +
>  	if (nlmsg_report(nlh) &&
>  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
>  		return;




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


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

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-04 17:03 ` Steve Grubb
@ 2020-06-04 17:57   ` Richard Guy Briggs
  2020-06-04 18:51     ` Steve Grubb
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Guy Briggs @ 2020-06-04 17:57 UTC (permalink / raw)
  To: Steve Grubb
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On 2020-06-04 13:03, Steve Grubb wrote:
> On Thursday, June 4, 2020 9:20:49 AM EDT Richard Guy Briggs 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:
> > v3:
> > - inline message type rather than table
> > 
> > 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         |  18 ++++++++
> >  kernel/auditsc.c              |  24 ++++++++--
> >  net/netfilter/nf_tables_api.c | 103
> > ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142
> > insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 3fcd9ee49734..604ede630580 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,23 @@ 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,
> >  };
> > 
> >  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"		   
> },
> >  };
> 
> I still don't like the event format because it doesn't give complete subject 
> information. However, I thought I'd comment on this string table. Usually 
> it's sufficient to log the number and then have the string table in user space 
> which looks it up during interpretation.

That is a good idea that would help reduce kernel cycles and netlink
bandwidth, but the format was set in 2011 so it is a bit late to change
that now:
	fbabf31e4d48 ("netfilter: create audit records for x_tables replaces")

> -Steve
> 
> >  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 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (!ctx->report &&
> >  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> > @@ -1428,6 +1439,17 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (!ctx->report &&
> >  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> > @@ -2691,6 +2713,17 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (!ctx->report &&
> >  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> > @@ -3693,6 +3726,17 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (!ctx->report &&
> >  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> > @@ -4809,6 +4853,17 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> >  		return;
> > @@ -5890,6 +5945,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_NFT_OP_OBJ_RESET);
> > +				kfree(buf);
> > +			}
> > +
> >  			if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb-
> >skb).portid,
> >  						    cb->nlh->nlmsg_seq,
> >  						    NFT_MSG_NEWOBJ,
> > @@ -6000,6 +6068,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_NFT_OP_OBJ_RESET);
> > +		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);
> > @@ -6075,6 +6154,16 @@ 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,
> > +			event == NFT_MSG_NEWOBJ ?
> > +				AUDIT_NFT_OP_OBJ_REGISTER :
> > +				AUDIT_NFT_OP_OBJ_UNREGISTER);
> > +	kfree(buf);
> > 
> >  	if (!report &&
> >  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> > @@ -6701,6 +6790,17 @@ 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);
> > +	kfree(buf);
> > 
> >  	if (ctx->report &&
> >  	    !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
> > @@ -6822,6 +6922,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_NFT_OP_GEN_REGISTER);
> > +
> >  	if (nlmsg_report(nlh) &&
> >  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> >  		return;
> 
> 
> 
> 

- 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] 9+ messages in thread

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-04 17:57   ` Richard Guy Briggs
@ 2020-06-04 18:51     ` Steve Grubb
  0 siblings, 0 replies; 9+ messages in thread
From: Steve Grubb @ 2020-06-04 18:51 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On Thursday, June 4, 2020 1:57:56 PM EDT Richard Guy Briggs wrote:
> > > 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"
> > 
> > },
> > 
> > > };
> > 
> > I still don't like the event format because it doesn't give complete
> > subject information. However, I thought I'd comment on this string
> > table. Usually it's sufficient to log the number and then have the
> > string table in user space which looks it up during interpretation.
> 
> That is a good idea that would help reduce kernel cycles and netlink
> bandwidth, but the format was set in 2011 so it is a bit late to change
> that now:
>         fbabf31e4d48 ("netfilter: create audit records for x_tables
> replaces")

Nothing searches/interprets that field name. So, you can redefine it by 
renaming it. Or just go with what you have. My preference is push that to 
user space. But not a showstopper "as is".

-Steve


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


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

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs
  2020-06-04 17:03 ` Steve Grubb
@ 2020-06-24  0:34 ` Paul Moore
  2020-06-24 10:03 ` Pablo Neira Ayuso
  2 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2020-06-24  0:34 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	Eric Paris, tgraf

On Thu, Jun 4, 2020 at 9:21 AM 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:
> v3:
> - inline message type rather than table
>
> 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         |  18 ++++++++
>  kernel/auditsc.c              |  24 ++++++++--
>  net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 142 insertions(+), 3 deletions(-)

I'm not seeing any additional comments from the netfilter folks so
I've gone ahead and merged this into audit/next.  Thanks Richard.

-- 
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] 9+ messages in thread

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs
  2020-06-04 17:03 ` Steve Grubb
  2020-06-24  0:34 ` Paul Moore
@ 2020-06-24 10:03 ` Pablo Neira Ayuso
  2020-06-24 12:34   ` Richard Guy Briggs
  2 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2020-06-24 10:03 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs 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:
> v3:
> - inline message type rather than table
> 
> 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         |  18 ++++++++
>  kernel/auditsc.c              |  24 ++++++++--
>  net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 142 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 3fcd9ee49734..604ede630580 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,23 @@ 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,
>  };
>  
>  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 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> +	kfree(buf);

As a follow up: Would you wrap this code into a function?

        nft_table_audit()

Same thing for other pieces of code below.

Thanks.

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


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

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-24 10:03 ` Pablo Neira Ayuso
@ 2020-06-24 12:34   ` Richard Guy Briggs
  2020-06-24 13:03     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Guy Briggs @ 2020-06-24 12:34 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On 2020-06-24 12:03, Pablo Neira Ayuso wrote:
> On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs 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:
> > v3:
> > - inline message type rather than table
> > 
> > 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         |  18 ++++++++
> >  kernel/auditsc.c              |  24 ++++++++--
> >  net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 142 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 3fcd9ee49734..604ede630580 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,23 @@ 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,
> >  };
> >  
> >  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 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> > +	kfree(buf);
> 
> As a follow up: Would you wrap this code into a function?
> 
>         nft_table_audit()
> 
> Same thing for other pieces of code below.

If I'm guessing right, you are asking for a supplementary follow-up
cleanup patch to this one (or are you nacking this patch)?

Also, I gather you would like to see the kasprintf and kfree hidden in
nft_table_audit(), handing this function at least 8 parameters?  This
sounds pretty messy given the format of the table field.

> Thanks.

- 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] 9+ messages in thread

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-24 12:34   ` Richard Guy Briggs
@ 2020-06-24 13:03     ` Pablo Neira Ayuso
  2020-06-24 13:26       ` Richard Guy Briggs
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2020-06-24 13:03 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On Wed, Jun 24, 2020 at 08:34:23AM -0400, Richard Guy Briggs wrote:
> On 2020-06-24 12:03, Pablo Neira Ayuso wrote:
> > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote:
[...]
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> > > +	kfree(buf);
> > 
> > As a follow up: Would you wrap this code into a function?
> > 
> >         nft_table_audit()
> > 
> > Same thing for other pieces of code below.
> 
> If I'm guessing right, you are asking for a supplementary follow-up
> cleanup patch to this one (or are you nacking this patch)?

No nack, it's just that I'd prefer to see this wrapped in a function.
I think your patch is already in the audit tree.

> Also, I gather you would like to see the kasprintf and kfree hidden in
> nft_table_audit(), handing this function at least 8 parameters?  This
> sounds pretty messy given the format of the table field.

I think you can pass ctx and the specific object, e.g. table, in most
cases? There is also event and the gfp_flags. That counts 4 here, but
maybe I'm overlooking something.

Thanks.

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


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

* Re: [PATCH ghak124 v3] audit: log nftables configuration change events
  2020-06-24 13:03     ` Pablo Neira Ayuso
@ 2020-06-24 13:26       ` Richard Guy Briggs
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Guy Briggs @ 2020-06-24 13:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner,
	eparis, tgraf

On 2020-06-24 15:03, Pablo Neira Ayuso wrote:
> On Wed, Jun 24, 2020 at 08:34:23AM -0400, Richard Guy Briggs wrote:
> > On 2020-06-24 12:03, Pablo Neira Ayuso wrote:
> > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote:
> [...]
> > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > > index 3558e76e2733..b9e7440cc87d 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,16 @@ 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);
> > > > +	kfree(buf);
> > > 
> > > As a follow up: Would you wrap this code into a function?
> > > 
> > >         nft_table_audit()
> > > 
> > > Same thing for other pieces of code below.
> > 
> > If I'm guessing right, you are asking for a supplementary follow-up
> > cleanup patch to this one (or are you nacking this patch)?
> 
> No nack, it's just that I'd prefer to see this wrapped in a function.
> I think your patch is already in the audit tree.
> 
> > Also, I gather you would like to see the kasprintf and kfree hidden in
> > nft_table_audit(), handing this function at least 8 parameters?  This
> > sounds pretty messy given the format of the table field.
> 
> I think you can pass ctx and the specific object, e.g. table, in most
> cases? There is also event and the gfp_flags. That counts 4 here, but
> maybe I'm overlooking something.

Since every event is sufficiently different, it isn't as simple as
passing ctx, unfortunately, and the table field I've overloaded with 4
bits of information for tracking the chain as well, some of which are ?
that would need an in-band representation (such as -1? that might
already be valid).  So 4 right there, family, nentries, event, gfp for 8.

I did try in the first patch to make it just one call keyed on event,
but there was enough variety of information available for each message
type that it became necessary to break it out.

> Thanks.

- 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] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs
2020-06-04 17:03 ` Steve Grubb
2020-06-04 17:57   ` Richard Guy Briggs
2020-06-04 18:51     ` Steve Grubb
2020-06-24  0:34 ` Paul Moore
2020-06-24 10:03 ` Pablo Neira Ayuso
2020-06-24 12:34   ` Richard Guy Briggs
2020-06-24 13:03     ` Pablo Neira Ayuso
2020-06-24 13:26       ` 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