* [PATCH iptables v2]: restore legacy behaviour of iptables-restore when rules start with -4/-6
@ 2019-07-26 7:24 Adel Belhouane
2019-07-29 0:37 ` Florian Westphal
0 siblings, 1 reply; 2+ messages in thread
From: Adel Belhouane @ 2019-07-26 7:24 UTC (permalink / raw)
To: netfilter-devel
v2: moved examples to testcase files
Legacy implementation of iptables-restore / ip6tables-restore allowed
to insert a -4 or -6 option at start of a rule line to ignore it if not
matching the command's protocol. This allowed to mix specific ipv4 and
ipv6 rules in a single file, as still described in iptables 1.8.3's man
page in options -4 and -6. The implementation over nftables doesn't behave
correctly in this case: iptables-nft-restore accepts both -4 or -6 lines
and ip6tables-nft-restore throws an error on -4.
There's a distribution bug report mentioning this problem:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925343
Restore the legacy behaviour:
- let do_parse() return and thus not add a command in those restore
special cases
- let do_commandx() ignore CMD_NONE instead of bailing out
I didn't attempt to fix all minor anomalies, but just to fix the
regression. For example in the line below, iptables should throw an error
instead of accepting -6 and then adding it as ipv4:
% iptables-nft -6 -A INPUT -p tcp -j ACCEPT
Signed-off-by: Adel Belhouane <bugs.a.b@free.fr>
diff --git a/iptables/tests/shell/testcases/ipt-restore/0005-ipt-6_0 b/iptables/tests/shell/testcases/ipt-restore/0005-ipt-6_0
new file mode 100755
index 000000000000..dd069771022c
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0005-ipt-6_0
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# Make sure iptables-restore simply ignores
+# rules starting with -6
+
+set -e
+
+# show rules, drop uninteresting policy settings
+ipt_show() {
+ $XT_MULTI iptables -S | grep -v '^-P'
+}
+
+# issue reproducer for iptables-restore
+
+$XT_MULTI iptables-restore <<EOF
+*filter
+-A FORWARD -m comment --comment any -j ACCEPT
+-4 -A FORWARD -m comment --comment ipv4 -j ACCEPT
+-6 -A FORWARD -m comment --comment ipv6 -j ACCEPT
+COMMIT
+EOF
+
+EXPECT='-A FORWARD -m comment --comment any -j ACCEPT
+-A FORWARD -m comment --comment ipv4 -j ACCEPT'
+
+diff -u -Z <(echo -e "$EXPECT") <(ipt_show)
diff --git a/iptables/tests/shell/testcases/ipt-restore/0006-ip6t-4_0 b/iptables/tests/shell/testcases/ipt-restore/0006-ip6t-4_0
new file mode 100755
index 000000000000..a37253a9d78e
--- /dev/null
+++ b/iptables/tests/shell/testcases/ipt-restore/0006-ip6t-4_0
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# Make sure ip6tables-restore simply ignores
+# rules starting with -4
+
+set -e
+
+# show rules, drop uninteresting policy settings
+ipt_show() {
+ $XT_MULTI ip6tables -S | grep -v '^-P'
+}
+
+# issue reproducer for ip6tables-restore
+
+$XT_MULTI ip6tables-restore <<EOF
+*filter
+-A FORWARD -m comment --comment any -j ACCEPT
+-4 -A FORWARD -m comment --comment ipv4 -j ACCEPT
+-6 -A FORWARD -m comment --comment ipv6 -j ACCEPT
+COMMIT
+EOF
+
+EXPECT='-A FORWARD -m comment --comment any -j ACCEPT
+-A FORWARD -m comment --comment ipv6 -j ACCEPT'
+
+diff -u -Z <(echo -e "$EXPECT") <(ipt_show)
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 93d9dcba828b..0e0cb5f53d42 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -955,6 +955,9 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
break;
case '4':
+ if (p->restore && args->family == AF_INET6)
+ return;
+
if (args->family != AF_INET)
exit_tryhelp(2);
@@ -962,6 +965,9 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
break;
case '6':
+ if (p->restore && args->family == AF_INET)
+ return;
+
args->family = AF_INET6;
xtables_set_nfproto(AF_INET6);
@@ -1174,6 +1180,9 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
case CMD_SET_POLICY:
ret = nft_chain_set(h, p.table, p.chain, p.policy, NULL);
break;
+ case CMD_NONE:
+ /* do_parse ignored the line (eg: -4 with ip6tables-restore) */
+ break;
default:
/* We should never reach this... */
exit_tryhelp(2);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH iptables v2]: restore legacy behaviour of iptables-restore when rules start with -4/-6
2019-07-26 7:24 [PATCH iptables v2]: restore legacy behaviour of iptables-restore when rules start with -4/-6 Adel Belhouane
@ 2019-07-29 0:37 ` Florian Westphal
0 siblings, 0 replies; 2+ messages in thread
From: Florian Westphal @ 2019-07-29 0:37 UTC (permalink / raw)
To: Adel Belhouane; +Cc: netfilter-devel
Adel Belhouane <bugs.a.b@free.fr> wrote:
>
> v2: moved examples to testcase files
>
> Legacy implementation of iptables-restore / ip6tables-restore allowed
> to insert a -4 or -6 option at start of a rule line to ignore it if not
> matching the command's protocol. This allowed to mix specific ipv4 and
> ipv6 rules in a single file, as still described in iptables 1.8.3's man
> page in options -4 and -6. The implementation over nftables doesn't behave
> correctly in this case: iptables-nft-restore accepts both -4 or -6 lines
> and ip6tables-nft-restore throws an error on -4.
>
> There's a distribution bug report mentioning this problem:
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925343
> Restore the legacy behaviour:
> - let do_parse() return and thus not add a command in those restore
> special cases
> - let do_commandx() ignore CMD_NONE instead of bailing out
>
> I didn't attempt to fix all minor anomalies, but just to fix the
> regression. For example in the line below, iptables should throw an error
> instead of accepting -6 and then adding it as ipv4:
>
> % iptables-nft -6 -A INPUT -p tcp -j ACCEPT
>
> Signed-off-by: Adel Belhouane <bugs.a.b@free.fr>
Applied, thank you for providing an updated patch with the test cases.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-07-29 0:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-26 7:24 [PATCH iptables v2]: restore legacy behaviour of iptables-restore when rules start with -4/-6 Adel Belhouane
2019-07-29 0:37 ` Florian Westphal
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).