| Message ID | dcef34f2-7271-72e3-e0c3-60844305dd62@free.fr |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [iptables,v2] : restore legacy behaviour of iptables-restore when rules start with -4/-6 | expand |
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.
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);
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>