From patchwork Fri Sep 8 00:22:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 13376935 X-Patchwork-Delegate: paul@paul-moore.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6FBCBEC8747 for ; Fri, 8 Sep 2023 00:22:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234596AbjIHAWj (ORCPT ); Thu, 7 Sep 2023 20:22:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42832 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231712AbjIHAWj (ORCPT ); Thu, 7 Sep 2023 20:22:39 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D1A8C1BD6; Thu, 7 Sep 2023 17:22:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=B8UPWVAUsOxHKGzz5IoIPAiukKHDgBS0xJWJd0P3HZk=; b=B917humhATU4HzHCWGuLJI2Xjq 6GhgqGbA4fXas8hH6buaVPrbzXjDw9ohNf6UgxN24f8YWAqoAuyrzBoyLhkyETQ65mOqOpXaPrBg3 pbJEjS5xV9G+xG/gGtXn6MFNe57j50ypV0yYik+95jCsp9AewtHUUmWRxu19hpibCwxNZ6JmVLwQk WmnZrxh+q03NBAuDzrHoKBLRpL4ThZxsYC7D0TyR+BjX2XOOnHMi+1800+UtNDPFhgrnhZ4vXpeue 1OhIqSqp+MZicKcYtINQIkr1OfKrkvMSWkoBK8CNhmCY5gi18huUO2c55uXCsiIlX1A2jIwUtiBnM TVtdotFQ==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qePGa-000050-K3; Fri, 08 Sep 2023 02:22:32 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org, Florian Westphal , audit@vger.kernel.org Subject: [nf PATCH 1/2] netfilter: nf_tables: Fix entries val in rule reset audit log Date: Fri, 8 Sep 2023 02:22:28 +0200 Message-ID: <20230908002229.1409-2-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230908002229.1409-1-phil@nwl.cc> References: <20230908002229.1409-1-phil@nwl.cc> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: audit@vger.kernel.org The value in idx and the number of rules handled in that particular __nf_tables_dump_rules() call is not identical. The former is a cursor to pick up from if multiple netlink messages are needed, so its value is ever increasing. Fixing this is not just a matter of subtracting s_idx from it, though: When resetting rules in multiple chains, __nf_tables_dump_rules() is called for each and cb->args[0] is not adjusted in between. The audit notification in __nf_tables_dump_rules() had another problem: If nf_tables_fill_rule_info() failed (e.g. due to buffer exhaustion), no notification was sent - despite the rules having been reset already. To catch all the above and return to a single (if possible) notification per table again, move audit logging back into the caller but into the table loop instead of past it to avoid the potential null-pointer deref. This requires to trigger the notification in two spots. Care has to be taken in the second case as cb->args[0] is also not updated in between tables. This requires a helper variable as either it is the first table (with potential non-zero cb->args[0] cursor) or a consecutive one (with idx holding the current cursor already). Fixes: 9b5ba5c9c5109 ("netfilter: nf_tables: Unbreak audit log reset") Signed-off-by: Phil Sutter --- net/netfilter/nf_tables_api.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index e429ebba74b3d..d429270676131 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3481,9 +3481,6 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, (*idx)++; } - if (reset && *idx) - audit_log_rule_reset(table, cb->seq, *idx); - return 0; } @@ -3494,11 +3491,12 @@ static int nf_tables_dump_rules(struct sk_buff *skb, const struct nft_rule_dump_ctx *ctx = cb->data; struct nft_table *table; const struct nft_chain *chain; - unsigned int idx = 0; + unsigned int idx = 0, s_idx; struct net *net = sock_net(skb->sk); int family = nfmsg->nfgen_family; struct nftables_pernet *nft_net; bool reset = false; + int ret; if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) reset = true; @@ -3529,16 +3527,23 @@ static int nf_tables_dump_rules(struct sk_buff *skb, cb, table, chain, reset); break; } + if (reset && idx > cb->args[0]) + audit_log_rule_reset(table, cb->seq, + idx - cb->args[0]); goto done; } + s_idx = max(idx, cb->args[0]); list_for_each_entry_rcu(chain, &table->chains, list) { - if (__nf_tables_dump_rules(skb, &idx, - cb, table, chain, reset)) - goto done; + ret = __nf_tables_dump_rules(skb, &idx, + cb, table, chain, reset); + if (ret) + break; } + if (reset && idx > s_idx) + audit_log_rule_reset(table, cb->seq, idx - s_idx); - if (ctx && ctx->table) + if ((ctx && ctx->table) || ret) break; } done: From patchwork Fri Sep 8 00:22:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 13376936 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E462DEB8FD7 for ; Fri, 8 Sep 2023 00:22:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231712AbjIHAWk (ORCPT ); Thu, 7 Sep 2023 20:22:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42846 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233455AbjIHAWj (ORCPT ); Thu, 7 Sep 2023 20:22:39 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D06991BD3; Thu, 7 Sep 2023 17:22:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=G9mXEfJSBER4EnSxCqgA1rbfEQRhxdCUIx4WvO2AO80=; b=kfgB5GXs5R7imCUK3JVH2ZLwfv AR3QLJ5uIsoOsJFDrnH9wzDBl3twfBmAF4lBM4U7CrRCsdAKCAvjht8mDblM7dtZ3Yw0GyPHwBca3 3aAPx98KEy+2P3ptxgCdeTdTUdWiQt6ZoEDzHL56uNHHXN3GrH2BPPN/rB+YmQUjWjDibOKWKWIKp +PJ2ByZve5b2NzMuR/CMzN8r3r7niIsHG2zlY/6LizOLdIrln69MMOLUvGqyJZUv+Uv3GfmmojFvR 0iC6I9Zs01FquYVu1468FLpecASxgw0oeUhS6D8OwYkQFwGWzeXEaGh5bAfR4NlXPUjGPOhQUGVJF 3RvD7yPw==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qePGb-000058-7X; Fri, 08 Sep 2023 02:22:33 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org, Florian Westphal , audit@vger.kernel.org Subject: [nf-next RFC 2/2] selftests: netfilter: Test nf_tables audit logging Date: Fri, 8 Sep 2023 02:22:29 +0200 Message-ID: <20230908002229.1409-3-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230908002229.1409-1-phil@nwl.cc> References: <20230908002229.1409-1-phil@nwl.cc> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: audit@vger.kernel.org Perform ruleset modifications and compare the NETFILTER_CFG type notifications emitted by auditd match expectations. Signed-off-by: Phil Sutter --- Calling auditd means enabling audit logging in kernel for the remaining uptime. So this test will slow down following ones or even cause spurious failures due to unexpected kernel log entries, timeouts, etc. Is there a way to test this in a less intrusive way? Maybe fence this test so it does not run automatically (is it any good having it in kernel then)? --- .../testing/selftests/netfilter/nft_audit.sh | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 tools/testing/selftests/netfilter/nft_audit.sh diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh new file mode 100755 index 0000000000000..55c750720137f --- /dev/null +++ b/tools/testing/selftests/netfilter/nft_audit.sh @@ -0,0 +1,75 @@ +#!/bin/bash + +SKIP_RC=4 +RC=0 + +nft --version >/dev/null 2>&1 || { + echo "SKIP: missing nft tool" + exit $SKIP_RC +} + +auditd --help >/dev/null 2>&1 +[ $? -eq 2 ] || { + echo "SKIP: missing auditd tool" + exit $SKIP_RC +} + +tmpdir=$(mktemp -d) +audit_log="$tmpdir/audit.log" +cat >"$tmpdir/auditd.conf" <"$audit_log" & +audit_pid=$! +trap 'kill $audit_pid; rm -rf $tmpdir' EXIT +sleep 1 + +logread() { + grep 'type=NETFILTER_CFG' "$audit_log" | \ + sed -e 's/\(type\|msg\|pid\)=[^ ]* //g' \ + -e 's/\(table=[^:]*\):[0-9]*/\1/' +} + +do_test() { # (cmd, log) + echo -n "testing for cmd: $1 ... " + echo >"$audit_log" + $1 >/dev/null || exit 1 + diff -q <(echo "$2") <(logread) >/dev/null && { echo "OK"; return; } + echo "FAIL" + diff -u <(echo "$2") <(logread) + ((RC++)) +} + +nft flush ruleset + +for table in t1 t2; do + echo "add table $table" + for chain in c1 c2 c3; do + echo "add chain $table $chain" + echo "add rule $table $chain counter accept" + echo "add rule $table $chain counter accept" + echo "add rule $table $chain counter accept" + done +done | nft -f - || exit 1 + +do_test 'nft reset rules t1 c2' \ + 'table=t1 family=2 entries=3 op=nft_reset_rule subj=kernel comm="nft"' + +do_test 'nft reset rules table t1' \ + 'table=t1 family=2 entries=9 op=nft_reset_rule subj=kernel comm="nft"' + +do_test 'nft reset rules' \ + 'table=t1 family=2 entries=9 op=nft_reset_rule subj=kernel comm="nft" +table=t2 family=2 entries=9 op=nft_reset_rule subj=kernel comm="nft"' + +for ((i = 0; i < 500; i++)); do + echo "add rule t2 c3 counter accept comment \"rule $i\"" +done | nft -f - || exit 1 + +do_test 'nft reset rules t2 c3' \ + 'table=t2 family=2 entries=189 op=nft_reset_rule subj=kernel comm="nft" +table=t2 family=2 entries=188 op=nft_reset_rule subj=kernel comm="nft" +table=t2 family=2 entries=126 op=nft_reset_rule subj=kernel comm="nft"' + +exit $RC