Re: [nf PATCH 3/3] netfilter: nf_tables: Audit log object reset once per table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-09-23 03:53, Phil Sutter wrote:
> When resetting multiple objects at once (via dump request), emit a log
> message per table (or filled skb) and resurrect the 'entries' parameter
> to contain the number of objects being logged for.
> 
> With the above in place, all audit logs for op=nft_register_obj have a
> predictable value in 'entries', so drop the value zeroing for them in
> audit_logread.c.
> 
> To test the skb exhaustion path, perform some bulk counter and quota
> adds in the kselftest.
> 
> Signed-off-by: Phil Sutter <phil@xxxxxx>

(Resend, forgot to include other addressees.)
Reviewed-by: Richard Guy Briggs <rgb@xxxxxxxxxx>

> ---
>  net/netfilter/nf_tables_api.c                 | 51 ++++++++++---------
>  .../testing/selftests/netfilter/nft_audit.sh  | 46 +++++++++++++++++
>  2 files changed, 74 insertions(+), 23 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 48d50df950a18..e04ef2c451be4 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -7733,6 +7733,16 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
>  	return -1;
>  }
>  
> +static void audit_log_obj_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_OBJ_RESET, GFP_ATOMIC);
> +	kfree(buf);
> +}
> +
>  struct nft_obj_dump_ctx {
>  	unsigned int	s_idx;
>  	char		*table;
> @@ -7748,8 +7758,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  	int family = nfmsg->nfgen_family;
>  	struct nftables_pernet *nft_net;
>  	const struct nft_table *table;
> +	unsigned int entries = 0;
>  	struct nft_object *obj;
>  	unsigned int idx = 0;
> +	int rc = 0;
>  
>  	rcu_read_lock();
>  	nft_net = nft_pernet(net);
> @@ -7759,6 +7771,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  		if (family != NFPROTO_UNSPEC && family != table->family)
>  			continue;
>  
> +		entries = 0;
>  		list_for_each_entry_rcu(obj, &table->objects, list) {
>  			if (!nft_is_active(net, obj))
>  				goto cont;
> @@ -7769,34 +7782,27 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  			if (ctx->type != NFT_OBJECT_UNSPEC &&
>  			    obj->ops->type->type != ctx->type)
>  				goto cont;
> -			if (ctx->reset) {
> -				char *buf = kasprintf(GFP_ATOMIC,
> -						      "%s:%u",
> -						      table->name,
> -						      nft_net->base_seq);
> -
> -				audit_log_nfcfg(buf,
> -						family,
> -						obj->handle,
> -						AUDIT_NFT_OP_OBJ_RESET,
> -						GFP_ATOMIC);
> -				kfree(buf);
> -			}
>  
> -			if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
> -						    cb->nlh->nlmsg_seq,
> -						    NFT_MSG_NEWOBJ,
> -						    NLM_F_MULTI | NLM_F_APPEND,
> -						    table->family, table,
> -						    obj, ctx->reset) < 0)
> -				goto done;
> +			rc = nf_tables_fill_obj_info(skb, net,
> +						     NETLINK_CB(cb->skb).portid,
> +						     cb->nlh->nlmsg_seq,
> +						     NFT_MSG_NEWOBJ,
> +						     NLM_F_MULTI | NLM_F_APPEND,
> +						     table->family, table,
> +						     obj, ctx->reset);
> +			if (rc < 0)
> +				break;
>  
> +			entries++;
>  			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
>  cont:
>  			idx++;
>  		}
> +		if (ctx->reset && entries)
> +			audit_log_obj_reset(table, nft_net->base_seq, entries);
> +		if (rc < 0)
> +			break;
>  	}
> -done:
>  	rcu_read_unlock();
>  
>  	ctx->s_idx = idx;
> @@ -7977,8 +7983,7 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
>  		return PTR_ERR(skb2);
>  
>  	buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, nft_net->base_seq);
> -	audit_log_nfcfg(buf, family, obj->handle,
> -			AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
> +	audit_log_nfcfg(buf, family, 1, AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
>  	kfree(buf);
>  
>  	return nfnetlink_unicast(skb2, net, portid);
> diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
> index bb34329e02a7f..e94a80859bbdb 100755
> --- a/tools/testing/selftests/netfilter/nft_audit.sh
> +++ b/tools/testing/selftests/netfilter/nft_audit.sh
> @@ -93,6 +93,12 @@ do_test 'nft add counter t1 c1' \
>  do_test 'nft add counter t2 c1; add counter t2 c2' \
>  'table=t2 family=2 entries=2 op=nft_register_obj'
>  
> +for ((i = 3; i <= 500; i++)); do
> +	echo "add counter t2 c$i"
> +done >$rulefile
> +do_test "nft -f $rulefile" \
> +'table=t2 family=2 entries=498 op=nft_register_obj'
> +
>  # adding/updating quotas
>  
>  do_test 'nft add quota t1 q1 { 10 bytes }' \
> @@ -101,6 +107,12 @@ do_test 'nft add quota t1 q1 { 10 bytes }' \
>  do_test 'nft add quota t2 q1 { 10 bytes }; add quota t2 q2 { 10 bytes }' \
>  'table=t2 family=2 entries=2 op=nft_register_obj'
>  
> +for ((i = 3; i <= 500; i++)); do
> +	echo "add quota t2 q$i { 10 bytes }"
> +done >$rulefile
> +do_test "nft -f $rulefile" \
> +'table=t2 family=2 entries=498 op=nft_register_obj'
> +
>  # changing the quota value triggers obj update path
>  do_test 'nft add quota t1 q1 { 20 bytes }' \
>  'table=t1 family=2 entries=1 op=nft_register_obj'
> @@ -150,6 +162,40 @@ done
>  do_test 'nft reset set t1 s' \
>  'table=t1 family=2 entries=3 op=nft_reset_setelem'
>  
> +# resetting counters
> +
> +do_test 'nft reset counter t1 c1' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj'
> +
> +do_test 'nft reset counters t1' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj'
> +
> +do_test 'nft reset counters t2' \
> +'table=t2 family=2 entries=342 op=nft_reset_obj
> +table=t2 family=2 entries=158 op=nft_reset_obj'
> +
> +do_test 'nft reset counters' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj
> +table=t2 family=2 entries=341 op=nft_reset_obj
> +table=t2 family=2 entries=159 op=nft_reset_obj'
> +
> +# resetting quotas
> +
> +do_test 'nft reset quota t1 q1' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj'
> +
> +do_test 'nft reset quotas t1' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj'
> +
> +do_test 'nft reset quotas t2' \
> +'table=t2 family=2 entries=315 op=nft_reset_obj
> +table=t2 family=2 entries=185 op=nft_reset_obj'
> +
> +do_test 'nft reset quotas' \
> +'table=t1 family=2 entries=1 op=nft_reset_obj
> +table=t2 family=2 entries=314 op=nft_reset_obj
> +table=t2 family=2 entries=186 op=nft_reset_obj'
> +
>  # deleting rules
>  
>  readarray -t handles < <(nft -a list chain t1 c1 | \
> -- 
> 2.41.0
> 

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
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




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux