From: Arnout Vandecappelle via buildroot <buildroot@buildroot.org>
To: "Jan Čermák" <sairon@sairon.cz>, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/iptables: backport fix for interface comparisons in `-C` commands
Date: Fri, 2 May 2025 12:35:24 +0200 [thread overview]
Message-ID: <bbb5e327-40d9-41c3-80ae-26d8bf782a13@rnout.be> (raw)
In-Reply-To: <20250414120653.3962766-1-sairon@sairon.cz>
On 14/04/2025 14:06, Jan Čermák wrote:
> Since iptables v1.8.11, `iptables -C` commands return unexpected zero status
> even for cases when the rules don't exist. This breaks e.g. standard Docker
> operation, where checking for existing rules is used extensively when creating
> networks.
>
> The patch fixing the behavior is available upstream - apply it to v1.8.11
> before a newer version is available.
>
> Signed-off-by: Jan Čermák <sairon@sairon.cz>
Applied to 2025.02.x, thanks.
Regards,
Arnout
> ---
> ...-interface-comparisons-in-C-commands.patch | 175 ++++++++++++++++++
> 1 file changed, 175 insertions(+)
> create mode 100644 package/iptables/0001-nft-fix-interface-comparisons-in-C-commands.patch
>
> diff --git a/package/iptables/0001-nft-fix-interface-comparisons-in-C-commands.patch b/package/iptables/0001-nft-fix-interface-comparisons-in-C-commands.patch
> new file mode 100644
> index 0000000000..308f75bbd8
> --- /dev/null
> +++ b/package/iptables/0001-nft-fix-interface-comparisons-in-C-commands.patch
> @@ -0,0 +1,175 @@
> +From 1f11d1004923eb941c9b4b33c8021dd9c4a2f3da Mon Sep 17 00:00:00 2001
> +From: Jeremy Sowden <jeremy@azazel.net>
> +Date: Mon, 18 Nov 2024 13:56:50 +0000
> +Subject: [PATCH] nft: fix interface comparisons in `-C` commands
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Commit 9ccae6397475 ("nft: Leave interface masks alone when parsing from
> +kernel") removed code which explicitly set interface masks to all ones. The
> +result of this is that they are zero. However, they are used to mask interfaces
> +in `is_same_interfaces`. Consequently, the masked values are alway zero, the
> +comparisons are always true, and check commands which ought to fail succeed:
> +
> + # iptables -N test
> + # iptables -A test -i lo \! -o lo -j REJECT
> + # iptables -v -L test
> + Chain test (0 references)
> + pkts bytes target prot opt in out source destination
> + 0 0 REJECT all -- lo !lo anywhere anywhere reject-with icmp-port-unreachable
> + # iptables -v -C test -i abcdefgh \! -o abcdefgh -j REJECT
> + REJECT all opt -- in lo out !lo 0.0.0.0/0 -> 0.0.0.0/0 reject-with icmp-port-unreachable
> +
> +Remove the mask parameters from `is_same_interfaces`. Add a test-case.
> +
> +Fixes: 9ccae6397475 ("nft: Leave interface masks alone when parsing from kernel")
> +Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> +Signed-off-by: Phil Sutter <phil@nwl.cc>
> +
> +Upstream: https://git.netfilter.org/iptables/commit/?id=40406dbfaefbc204134452b2747bae4f6a122848
> +Signed-off-by: Jan Čermák <sairon@sairon.cz>
> +---
> + iptables/nft-arp.c | 10 ++----
> + iptables/nft-ipv4.c | 4 +--
> + iptables/nft-ipv6.c | 6 +---
> + iptables/nft-shared.c | 36 +++++--------------
> + iptables/nft-shared.h | 6 +---
> + .../nft-only/0020-compare-interfaces_0 | 9 +++++
> + 6 files changed, 22 insertions(+), 49 deletions(-)
> + create mode 100755 iptables/tests/shell/testcases/nft-only/0020-compare-interfaces_0
> +
> +diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
> +index 264864c3..c11d64c3 100644
> +--- a/iptables/nft-arp.c
> ++++ b/iptables/nft-arp.c
> +@@ -385,14 +385,8 @@ static bool nft_arp_is_same(const struct iptables_command_state *cs_a,
> + return false;
> + }
> +
> +- return is_same_interfaces(a->arp.iniface,
> +- a->arp.outiface,
> +- (unsigned char *)a->arp.iniface_mask,
> +- (unsigned char *)a->arp.outiface_mask,
> +- b->arp.iniface,
> +- b->arp.outiface,
> +- (unsigned char *)b->arp.iniface_mask,
> +- (unsigned char *)b->arp.outiface_mask);
> ++ return is_same_interfaces(a->arp.iniface, a->arp.outiface,
> ++ b->arp.iniface, b->arp.outiface);
> + }
> +
> + static void nft_arp_save_chain(const struct nftnl_chain *c, const char *policy)
> +diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
> +index 74092875..0c8bd291 100644
> +--- a/iptables/nft-ipv4.c
> ++++ b/iptables/nft-ipv4.c
> +@@ -113,9 +113,7 @@ static bool nft_ipv4_is_same(const struct iptables_command_state *a,
> + }
> +
> + return is_same_interfaces(a->fw.ip.iniface, a->fw.ip.outiface,
> +- a->fw.ip.iniface_mask, a->fw.ip.outiface_mask,
> +- b->fw.ip.iniface, b->fw.ip.outiface,
> +- b->fw.ip.iniface_mask, b->fw.ip.outiface_mask);
> ++ b->fw.ip.iniface, b->fw.ip.outiface);
> + }
> +
> + static void nft_ipv4_set_goto_flag(struct iptables_command_state *cs)
> +diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
> +index b184f8af..4dbb2af2 100644
> +--- a/iptables/nft-ipv6.c
> ++++ b/iptables/nft-ipv6.c
> +@@ -99,11 +99,7 @@ static bool nft_ipv6_is_same(const struct iptables_command_state *a,
> + }
> +
> + return is_same_interfaces(a->fw6.ipv6.iniface, a->fw6.ipv6.outiface,
> +- a->fw6.ipv6.iniface_mask,
> +- a->fw6.ipv6.outiface_mask,
> +- b->fw6.ipv6.iniface, b->fw6.ipv6.outiface,
> +- b->fw6.ipv6.iniface_mask,
> +- b->fw6.ipv6.outiface_mask);
> ++ b->fw6.ipv6.iniface, b->fw6.ipv6.outiface);
> + }
> +
> + static void nft_ipv6_set_goto_flag(struct iptables_command_state *cs)
> +diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> +index 6775578b..2c29e68f 100644
> +--- a/iptables/nft-shared.c
> ++++ b/iptables/nft-shared.c
> +@@ -220,36 +220,16 @@ void add_l4proto(struct nft_handle *h, struct nftnl_rule *r,
> + }
> +
> + bool is_same_interfaces(const char *a_iniface, const char *a_outiface,
> +- unsigned const char *a_iniface_mask,
> +- unsigned const char *a_outiface_mask,
> +- const char *b_iniface, const char *b_outiface,
> +- unsigned const char *b_iniface_mask,
> +- unsigned const char *b_outiface_mask)
> ++ const char *b_iniface, const char *b_outiface)
> + {
> +- int i;
> +-
> +- for (i = 0; i < IFNAMSIZ; i++) {
> +- if (a_iniface_mask[i] != b_iniface_mask[i]) {
> +- DEBUGP("different iniface mask %x, %x (%d)\n",
> +- a_iniface_mask[i] & 0xff, b_iniface_mask[i] & 0xff, i);
> +- return false;
> +- }
> +- if ((a_iniface[i] & a_iniface_mask[i])
> +- != (b_iniface[i] & b_iniface_mask[i])) {
> +- DEBUGP("different iniface\n");
> +- return false;
> +- }
> +- if (a_outiface_mask[i] != b_outiface_mask[i]) {
> +- DEBUGP("different outiface mask\n");
> +- return false;
> +- }
> +- if ((a_outiface[i] & a_outiface_mask[i])
> +- != (b_outiface[i] & b_outiface_mask[i])) {
> +- DEBUGP("different outiface\n");
> +- return false;
> +- }
> ++ if (strncmp(a_iniface, b_iniface, IFNAMSIZ)) {
> ++ DEBUGP("different iniface\n");
> ++ return false;
> ++ }
> ++ if (strncmp(a_outiface, b_outiface, IFNAMSIZ)) {
> ++ DEBUGP("different outiface\n");
> ++ return false;
> + }
> +-
> + return true;
> + }
> +
> +diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> +index 51d1e460..b57aee1f 100644
> +--- a/iptables/nft-shared.h
> ++++ b/iptables/nft-shared.h
> +@@ -105,11 +105,7 @@ void add_l4proto(struct nft_handle *h, struct nftnl_rule *r, uint8_t proto, uint
> + void add_compat(struct nftnl_rule *r, uint32_t proto, bool inv);
> +
> + bool is_same_interfaces(const char *a_iniface, const char *a_outiface,
> +- unsigned const char *a_iniface_mask,
> +- unsigned const char *a_outiface_mask,
> +- const char *b_iniface, const char *b_outiface,
> +- unsigned const char *b_iniface_mask,
> +- unsigned const char *b_outiface_mask);
> ++ const char *b_iniface, const char *b_outiface);
> +
> + void __get_cmp_data(struct nftnl_expr *e, void *data, size_t dlen, uint8_t *op);
> + void get_cmp_data(struct nftnl_expr *e, void *data, size_t dlen, bool *inv);
> +diff --git a/iptables/tests/shell/testcases/nft-only/0020-compare-interfaces_0 b/iptables/tests/shell/testcases/nft-only/0020-compare-interfaces_0
> +new file mode 100755
> +index 00000000..278cd648
> +--- /dev/null
> ++++ b/iptables/tests/shell/testcases/nft-only/0020-compare-interfaces_0
> +@@ -0,0 +1,9 @@
> ++#!/bin/bash
> ++
> ++[[ $XT_MULTI == *xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
> ++
> ++$XT_MULTI iptables -N test
> ++$XT_MULTI iptables -A test -i lo \! -o lo -j REJECT
> ++$XT_MULTI iptables -C test -i abcdefgh \! -o abcdefgh -j REJECT 2>/dev/null && exit 1
> ++
> ++exit 0
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
prev parent reply other threads:[~2025-05-02 10:35 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-14 12:06 [Buildroot] [PATCH] package/iptables: backport fix for interface comparisons in `-C` commands Jan Čermák
2025-04-19 14:10 ` Thomas Petazzoni via buildroot
2025-05-02 10:35 ` Arnout Vandecappelle via buildroot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bbb5e327-40d9-41c3-80ae-26d8bf782a13@rnout.be \
--to=buildroot@buildroot.org \
--cc=arnout@rnout.be \
--cc=sairon@sairon.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.