linux-audit.redhat.com archive mirror
 help / color / mirror / Atom feed
* [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
@ 2023-08-29 17:51 Phil Sutter
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Phil Sutter @ 2023-08-29 17:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

Since set element reset is not integrated into nf_tables' transaction
logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
handling.

For the sake of simplicity, catchall element reset will always generate
a dedicated log entry. This relieves nf_tables_dump_set() from having to
adjust the logged element count depending on whether a catchall element
was found or not.

Cc: Richard Guy Briggs <rgb@redhat.com>
Fixes: 079cd633219d7 ("netfilter: nf_tables: Introduce NFT_MSG_GETSETELEM_RESET")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/linux/audit.h         |  1 +
 kernel/auditsc.c              |  1 +
 net/netfilter/nf_tables_api.c | 31 ++++++++++++++++++++++++++++---
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 6a3a9e122bb5e..192bf03aacc52 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -117,6 +117,7 @@ enum audit_nfcfgop {
 	AUDIT_NFT_OP_OBJ_RESET,
 	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
 	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
+	AUDIT_NFT_OP_SETELEM_RESET,
 	AUDIT_NFT_OP_INVALID,
 };
 
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index addeed3df15d3..38481e3181975 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -143,6 +143,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
 	{ 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_SETELEM_RESET,		"nft_reset_setelem"        },
 	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
 };
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 1ddbdca4e47d6..a1218ea4e0c3d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -102,6 +102,7 @@ static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
 	[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_GETSETELEM_RESET] = AUDIT_NFT_OP_SETELEM_RESET,
 };
 
 static void nft_validate_state_update(struct nft_table *table, u8 new_validate_state)
@@ -5661,13 +5662,25 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
 	return nf_tables_fill_setelem(args->skb, set, elem, args->reset);
 }
 
+static void audit_log_nft_set_reset(const struct nft_table *table,
+				    unsigned int base_seq,
+				    unsigned int nentries)
+{
+	char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, base_seq);
+
+	audit_log_nfcfg(buf, table->family, nentries,
+			AUDIT_NFT_OP_SETELEM_RESET, GFP_ATOMIC);
+	kfree(buf);
+}
+
 struct nft_set_dump_ctx {
 	const struct nft_set	*set;
 	struct nft_ctx		ctx;
 };
 
 static int nft_set_catchall_dump(struct net *net, struct sk_buff *skb,
-				 const struct nft_set *set, bool reset)
+				 const struct nft_set *set, bool reset,
+				 unsigned int base_seq)
 {
 	struct nft_set_elem_catchall *catchall;
 	u8 genmask = nft_genmask_cur(net);
@@ -5683,6 +5696,8 @@ static int nft_set_catchall_dump(struct net *net, struct sk_buff *skb,
 
 		elem.priv = catchall->elem;
 		ret = nf_tables_fill_setelem(skb, set, &elem, reset);
+		if (reset && !ret)
+			audit_log_nft_set_reset(set->table, base_seq, 1);
 		break;
 	}
 
@@ -5762,12 +5777,17 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
 	set->ops->walk(&dump_ctx->ctx, set, &args.iter);
 
 	if (!args.iter.err && args.iter.count == cb->args[0])
-		args.iter.err = nft_set_catchall_dump(net, skb, set, reset);
+		args.iter.err = nft_set_catchall_dump(net, skb, set,
+						      reset, cb->seq);
 	rcu_read_unlock();
 
 	nla_nest_end(skb, nest);
 	nlmsg_end(skb, nlh);
 
+	if (reset && args.iter.count > args.iter.skip)
+		audit_log_nft_set_reset(table, cb->seq,
+					args.iter.count - args.iter.skip);
+
 	if (args.iter.err && args.iter.err != -EMSGSIZE)
 		return args.iter.err;
 	if (args.iter.count == cb->args[0])
@@ -5992,13 +6012,13 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
 	struct netlink_ext_ack *extack = info->extack;
 	u8 genmask = nft_genmask_cur(info->net);
 	u8 family = info->nfmsg->nfgen_family;
+	int rem, err = 0, nelems = 0;
 	struct net *net = info->net;
 	struct nft_table *table;
 	struct nft_set *set;
 	struct nlattr *attr;
 	struct nft_ctx ctx;
 	bool reset = false;
-	int rem, err = 0;
 
 	table = nft_table_lookup(net, nla[NFTA_SET_ELEM_LIST_TABLE], family,
 				 genmask, 0);
@@ -6041,8 +6061,13 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
 			NL_SET_BAD_ATTR(extack, attr);
 			break;
 		}
+		nelems++;
 	}
 
+	if (reset)
+		audit_log_nft_set_reset(table, nft_pernet(net)->base_seq,
+					nelems);
+
 	return err;
 }
 
-- 
2.41.0

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


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

* [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset
  2023-08-29 17:51 [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Phil Sutter
@ 2023-08-29 17:51 ` Phil Sutter
  2023-08-29 18:02   ` Pablo Neira Ayuso
                     ` (2 more replies)
  2023-08-29 18:02 ` [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Pablo Neira Ayuso
  2023-08-30 15:46 ` Pablo Neira Ayuso
  2 siblings, 3 replies; 10+ messages in thread
From: Phil Sutter @ 2023-08-29 17:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

Resetting rules' stateful data happens outside of the transaction logic,
so 'get' and 'dump' handlers have to emit audit log entries themselves.

Cc: Richard Guy Briggs <rgb@redhat.com>
Fixes: 8daa8fde3fc3f ("netfilter: nf_tables: Introduce NFT_MSG_GETRULE_RESET")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/linux/audit.h         |  1 +
 kernel/auditsc.c              |  1 +
 net/netfilter/nf_tables_api.c | 18 ++++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 192bf03aacc52..51b1b7054a233 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -118,6 +118,7 @@ enum audit_nfcfgop {
 	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
 	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
 	AUDIT_NFT_OP_SETELEM_RESET,
+	AUDIT_NFT_OP_RULE_RESET,
 	AUDIT_NFT_OP_INVALID,
 };
 
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 38481e3181975..fc0c7c03eeabe 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -144,6 +144,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
 	{ AUDIT_NFT_OP_FLOWTABLE_REGISTER,	"nft_register_flowtable"   },
 	{ AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,	"nft_unregister_flowtable" },
 	{ AUDIT_NFT_OP_SETELEM_RESET,		"nft_reset_setelem"        },
+	{ AUDIT_NFT_OP_RULE_RESET,		"nft_reset_rule"           },
 	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
 };
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index a1218ea4e0c3d..2aa7b9a55cca4 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3432,6 +3432,18 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
 	nfnetlink_set_err(ctx->net, ctx->portid, NFNLGRP_NFTABLES, -ENOBUFS);
 }
 
+static void audit_log_rule_reset(const struct nft_table *table,
+				 unsigned int base_seq,
+				 unsigned int nentries)
+{
+	char *buf = kasprintf(GFP_ATOMIC, "%s:%u",
+			      table->name, base_seq);
+
+	audit_log_nfcfg(buf, table->family, nentries,
+			AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC);
+	kfree(buf);
+}
+
 struct nft_rule_dump_ctx {
 	char *table;
 	char *chain;
@@ -3538,6 +3550,9 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 done:
 	rcu_read_unlock();
 
+	if (reset && idx > cb->args[0])
+		audit_log_rule_reset(table, cb->seq, idx - cb->args[0]);
+
 	cb->args[0] = idx;
 	return skb->len;
 }
@@ -3645,6 +3660,9 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info,
 	if (err < 0)
 		goto err_fill_rule_info;
 
+	if (reset)
+		audit_log_rule_reset(table, nft_pernet(net)->base_seq, 1);
+
 	return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
 
 err_fill_rule_info:
-- 
2.41.0

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


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

* Re: [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
  2023-08-29 17:51 [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Phil Sutter
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
@ 2023-08-29 18:02 ` Pablo Neira Ayuso
  2023-08-29 18:38   ` Phil Sutter
  2023-08-30 15:46 ` Pablo Neira Ayuso
  2 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2023-08-29 18:02 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 07:51:57PM +0200, Phil Sutter wrote:
> Since set element reset is not integrated into nf_tables' transaction
> logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
> handling.
> 
> For the sake of simplicity, catchall element reset will always generate
> a dedicated log entry. This relieves nf_tables_dump_set() from having to
> adjust the logged element count depending on whether a catchall element
> was found or not.
> 
> Cc: Richard Guy Briggs <rgb@redhat.com>
> Fixes: 079cd633219d7 ("netfilter: nf_tables: Introduce NFT_MSG_GETSETELEM_RESET")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  include/linux/audit.h         |  1 +
>  kernel/auditsc.c              |  1 +
>  net/netfilter/nf_tables_api.c | 31 ++++++++++++++++++++++++++++---
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 6a3a9e122bb5e..192bf03aacc52 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -117,6 +117,7 @@ enum audit_nfcfgop {
>  	AUDIT_NFT_OP_OBJ_RESET,
>  	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
>  	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> +	AUDIT_NFT_OP_SETELEM_RESET,
>  	AUDIT_NFT_OP_INVALID,
>  };
>  
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index addeed3df15d3..38481e3181975 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -143,6 +143,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>  	{ 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_SETELEM_RESET,		"nft_reset_setelem"        },
>  	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
>  };
>  
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 1ddbdca4e47d6..a1218ea4e0c3d 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -102,6 +102,7 @@ static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
>  	[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_GETSETELEM_RESET] = AUDIT_NFT_OP_SETELEM_RESET,
>  };
>  
>  static void nft_validate_state_update(struct nft_table *table, u8 new_validate_state)
> @@ -5661,13 +5662,25 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
>  	return nf_tables_fill_setelem(args->skb, set, elem, args->reset);
>  }
>  
> +static void audit_log_nft_set_reset(const struct nft_table *table,
> +				    unsigned int base_seq,
> +				    unsigned int nentries)
> +{
> +	char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, base_seq);

No check for NULL?

I can see we have more like this in the tree.

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


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

* Re: [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
@ 2023-08-29 18:02   ` Pablo Neira Ayuso
  2023-08-30 15:47   ` Pablo Neira Ayuso
  2023-08-31  2:27   ` Paul Moore
  2 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2023-08-29 18:02 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 07:51:58PM +0200, Phil Sutter wrote:
> Resetting rules' stateful data happens outside of the transaction logic,
> so 'get' and 'dump' handlers have to emit audit log entries themselves.
> 
> Cc: Richard Guy Briggs <rgb@redhat.com>
> Fixes: 8daa8fde3fc3f ("netfilter: nf_tables: Introduce NFT_MSG_GETRULE_RESET")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  include/linux/audit.h         |  1 +
>  kernel/auditsc.c              |  1 +
>  net/netfilter/nf_tables_api.c | 18 ++++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 192bf03aacc52..51b1b7054a233 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -118,6 +118,7 @@ enum audit_nfcfgop {
>  	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
>  	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
>  	AUDIT_NFT_OP_SETELEM_RESET,
> +	AUDIT_NFT_OP_RULE_RESET,
>  	AUDIT_NFT_OP_INVALID,
>  };
>  
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 38481e3181975..fc0c7c03eeabe 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -144,6 +144,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>  	{ AUDIT_NFT_OP_FLOWTABLE_REGISTER,	"nft_register_flowtable"   },
>  	{ AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,	"nft_unregister_flowtable" },
>  	{ AUDIT_NFT_OP_SETELEM_RESET,		"nft_reset_setelem"        },
> +	{ AUDIT_NFT_OP_RULE_RESET,		"nft_reset_rule"           },
>  	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
>  };
>  
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index a1218ea4e0c3d..2aa7b9a55cca4 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3432,6 +3432,18 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
>  	nfnetlink_set_err(ctx->net, ctx->portid, NFNLGRP_NFTABLES, -ENOBUFS);
>  }
>  
> +static void audit_log_rule_reset(const struct nft_table *table,
> +				 unsigned int base_seq,
> +				 unsigned int nentries)
> +{
> +	char *buf = kasprintf(GFP_ATOMIC, "%s:%u",
> +			      table->name, base_seq);

No check for NULL.

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


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

* Re: [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
  2023-08-29 18:02 ` [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Pablo Neira Ayuso
@ 2023-08-29 18:38   ` Phil Sutter
  0 siblings, 0 replies; 10+ messages in thread
From: Phil Sutter @ 2023-08-29 18:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 08:02:30PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Aug 29, 2023 at 07:51:57PM +0200, Phil Sutter wrote:
> > Since set element reset is not integrated into nf_tables' transaction
> > logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
> > handling.
> > 
> > For the sake of simplicity, catchall element reset will always generate
> > a dedicated log entry. This relieves nf_tables_dump_set() from having to
> > adjust the logged element count depending on whether a catchall element
> > was found or not.
> > 
> > Cc: Richard Guy Briggs <rgb@redhat.com>
> > Fixes: 079cd633219d7 ("netfilter: nf_tables: Introduce NFT_MSG_GETSETELEM_RESET")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  include/linux/audit.h         |  1 +
> >  kernel/auditsc.c              |  1 +
> >  net/netfilter/nf_tables_api.c | 31 ++++++++++++++++++++++++++++---
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 6a3a9e122bb5e..192bf03aacc52 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -117,6 +117,7 @@ enum audit_nfcfgop {
> >  	AUDIT_NFT_OP_OBJ_RESET,
> >  	AUDIT_NFT_OP_FLOWTABLE_REGISTER,
> >  	AUDIT_NFT_OP_FLOWTABLE_UNREGISTER,
> > +	AUDIT_NFT_OP_SETELEM_RESET,
> >  	AUDIT_NFT_OP_INVALID,
> >  };
> >  
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index addeed3df15d3..38481e3181975 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -143,6 +143,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
> >  	{ 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_SETELEM_RESET,		"nft_reset_setelem"        },
> >  	{ AUDIT_NFT_OP_INVALID,			"nft_invalid"		   },
> >  };
> >  
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 1ddbdca4e47d6..a1218ea4e0c3d 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -102,6 +102,7 @@ static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
> >  	[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_GETSETELEM_RESET] = AUDIT_NFT_OP_SETELEM_RESET,
> >  };
> >  
> >  static void nft_validate_state_update(struct nft_table *table, u8 new_validate_state)
> > @@ -5661,13 +5662,25 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
> >  	return nf_tables_fill_setelem(args->skb, set, elem, args->reset);
> >  }
> >  
> > +static void audit_log_nft_set_reset(const struct nft_table *table,
> > +				    unsigned int base_seq,
> > +				    unsigned int nentries)
> > +{
> > +	char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, base_seq);
> 
> No check for NULL?
> 
> I can see we have more like this in the tree.

Well, I merely copy-pasted that bit. (No excuse, I know.) The pointer is
passed on to audit_log_format() as a string argument and will end at
vsnprintf(), which detects and sanitizes NULL string args.

Cheers, Phil


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


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

* Re: [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
  2023-08-29 17:51 [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Phil Sutter
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
  2023-08-29 18:02 ` [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Pablo Neira Ayuso
@ 2023-08-30 15:46 ` Pablo Neira Ayuso
  2023-08-30 20:54   ` Richard Guy Briggs
  2 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2023-08-30 15:46 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 07:51:57PM +0200, Phil Sutter wrote:
> Since set element reset is not integrated into nf_tables' transaction
> logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
> handling.
> 
> For the sake of simplicity, catchall element reset will always generate
> a dedicated log entry. This relieves nf_tables_dump_set() from having to
> adjust the logged element count depending on whether a catchall element
> was found or not.

Applied, thanks Phil

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


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

* Re: [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
  2023-08-29 18:02   ` Pablo Neira Ayuso
@ 2023-08-30 15:47   ` Pablo Neira Ayuso
  2023-08-31  2:27   ` Paul Moore
  2 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2023-08-30 15:47 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Richard Guy Briggs, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 07:51:58PM +0200, Phil Sutter wrote:
> Resetting rules' stateful data happens outside of the transaction logic,
> so 'get' and 'dump' handlers have to emit audit log entries themselves.

Also applied to nf, thanks

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


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

* Re: [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
  2023-08-30 15:46 ` Pablo Neira Ayuso
@ 2023-08-30 20:54   ` Richard Guy Briggs
  2023-08-31  2:26     ` Paul Moore
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Guy Briggs @ 2023-08-30 20:54 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Phil Sutter, netfilter-devel, linux-audit

On 2023-08-30 17:46, Pablo Neira Ayuso wrote:
> On Tue, Aug 29, 2023 at 07:51:57PM +0200, Phil Sutter wrote:
> > Since set element reset is not integrated into nf_tables' transaction
> > logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
> > handling.
> > 
> > For the sake of simplicity, catchall element reset will always generate
> > a dedicated log entry. This relieves nf_tables_dump_set() from having to
> > adjust the logged element count depending on whether a catchall element
> > was found or not.
> 
> Applied, thanks Phil

Thanks Phil, Pablo.  If it isn't too late, please add my
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


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

* Re: [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset
  2023-08-30 20:54   ` Richard Guy Briggs
@ 2023-08-31  2:26     ` Paul Moore
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Moore @ 2023-08-31  2:26 UTC (permalink / raw)
  To: Phil Sutter, Pablo Neira Ayuso
  Cc: Richard Guy Briggs, audit, linux-audit, netfilter-devel

On Wed, Aug 30, 2023 at 4:54 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2023-08-30 17:46, Pablo Neira Ayuso wrote:
> > On Tue, Aug 29, 2023 at 07:51:57PM +0200, Phil Sutter wrote:
> > > Since set element reset is not integrated into nf_tables' transaction
> > > logic, an explicit log call is needed, similar to NFT_MSG_GETOBJ_RESET
> > > handling.
> > >
> > > For the sake of simplicity, catchall element reset will always generate
> > > a dedicated log entry. This relieves nf_tables_dump_set() from having to
> > > adjust the logged element count depending on whether a catchall element
> > > was found or not.
> >
> > Applied, thanks Phil
>
> Thanks Phil, Pablo.  If it isn't too late, please add my
> Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

Similarly, you can add my ACK.  FWIW, if you're sending patches out
during the first few days of the merge window it might be advisable to
wait more than a day or two to give the relevant maintainers a chance
to review the patch.

Also, as a note for future submissions, we've moved the audit kernel
mailing list to audit@vger.kernel.org.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

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

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

* Re: [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset
  2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
  2023-08-29 18:02   ` Pablo Neira Ayuso
  2023-08-30 15:47   ` Pablo Neira Ayuso
@ 2023-08-31  2:27   ` Paul Moore
  2 siblings, 0 replies; 10+ messages in thread
From: Paul Moore @ 2023-08-31  2:27 UTC (permalink / raw)
  To: Phil Sutter, Pablo Neira Ayuso
  Cc: Richard Guy Briggs, audit, linux-audit, netfilter-devel

On Tue, Aug 29, 2023 at 2:24 PM Phil Sutter <phil@nwl.cc> wrote:
>
> Resetting rules' stateful data happens outside of the transaction logic,
> so 'get' and 'dump' handlers have to emit audit log entries themselves.
>
> Cc: Richard Guy Briggs <rgb@redhat.com>
> Fixes: 8daa8fde3fc3f ("netfilter: nf_tables: Introduce NFT_MSG_GETRULE_RESET")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  include/linux/audit.h         |  1 +
>  kernel/auditsc.c              |  1 +
>  net/netfilter/nf_tables_api.c | 18 ++++++++++++++++++
>  3 files changed, 20 insertions(+)

See my comments in patch 1/2.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

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

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

end of thread, other threads:[~2023-08-31  2:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-29 17:51 [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Phil Sutter
2023-08-29 17:51 ` [nf PATCH 2/2] netfilter: nf_tables: Audit log rule reset Phil Sutter
2023-08-29 18:02   ` Pablo Neira Ayuso
2023-08-30 15:47   ` Pablo Neira Ayuso
2023-08-31  2:27   ` Paul Moore
2023-08-29 18:02 ` [nf PATCH 1/2] netfilter: nf_tables: Audit log setelem reset Pablo Neira Ayuso
2023-08-29 18:38   ` Phil Sutter
2023-08-30 15:46 ` Pablo Neira Ayuso
2023-08-30 20:54   ` Richard Guy Briggs
2023-08-31  2:26     ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).