* [iptables PATCH] xtables-restore: Unbreak *tables-restore
@ 2019-10-22 10:34 Phil Sutter
2019-10-23 8:14 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Phil Sutter @ 2019-10-22 10:34 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Commit 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
installed an error check which evaluated true in all cases as all
callers of do_command callbacks pass a pointer to a table name already.
Attached test case passed as it tested error condition only.
Fix the whole mess by introducing a boolean to indicate whether a table
parameter was seen already. Extend the test case to cover positive as
well as negative behaviour and to test ebtables-restore and
ip6tables-restore as well. Also add the required checking code to the
latter since the original commit missed it.
Fixes: 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/ip6tables.c | 6 ++++++
iptables/iptables.c | 4 +++-
| 21 +++++++++++++++++--
iptables/xtables-eb.c | 4 +++-
iptables/xtables.c | 4 +++-
5 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 57b0f9f55a921..c160a2dd4e65b 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1228,6 +1228,7 @@ int do_command6(int argc, char *argv[], char **table,
struct xtables_rule_match *matchp;
struct xtables_target *t;
unsigned long long cnt;
+ bool table_set = false;
/* re-set optind to 0 in case do_command6 gets called
* a second time */
@@ -1508,7 +1509,12 @@ int do_command6(int argc, char *argv[], char **table,
if (cs.invert)
xtables_error(PARAMETER_PROBLEM,
"unexpected ! flag before --table");
+ if (restore && table_set)
+ xtables_error(PARAMETER_PROBLEM,
+ "The -t option (seen in line %u) cannot be used in %s.\n",
+ line, xt_params->program_name);
*table = optarg;
+ table_set = true;
break;
case 'x':
diff --git a/iptables/iptables.c b/iptables/iptables.c
index d7a41321760e0..544e87596e7e4 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1217,6 +1217,7 @@ int do_command4(int argc, char *argv[], char **table,
struct xtables_rule_match *matchp;
struct xtables_target *t;
unsigned long long cnt;
+ bool table_set = false;
/* re-set optind to 0 in case do_command4 gets called
* a second time */
@@ -1494,11 +1495,12 @@ int do_command4(int argc, char *argv[], char **table,
if (cs.invert)
xtables_error(PARAMETER_PROBLEM,
"unexpected ! flag before --table");
- if (restore && *table)
+ if (restore && table_set)
xtables_error(PARAMETER_PROBLEM,
"The -t option (seen in line %u) cannot be used in %s.\n",
line, xt_params->program_name);
*table = optarg;
+ table_set = true;
break;
case 'x':
--git a/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0 b/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
index 4e2202df986cf..e96140758a99d 100755
--- a/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
+++ b/iptables/tests/shell/testcases/ipt-restore/0009-table-name-comment_0
@@ -4,10 +4,27 @@
# '-t <tablename>' so standard rule parsing routines may be used. This means
# that it has to detect and reject rules which already contain a table option.
-$XT_MULTI iptables-restore <<EOF
+families="ip ip6"
+[[ $(basename $XT_MULTI) == xtables-nft-multi ]] && families+=" eb"
+
+for fam in $families; do
+ $XT_MULTI ${fam}tables-restore <<EOF
*filter
-t nat -A FORWARD -j ACCEPT
COMMIT
EOF
+ [[ $? != 0 ]] || {
+ echo "${fam}tables-restore did not fail when it should have"
+ exit 1
+ }
-[[ $? != 0 ]] || exit 1
+ $XT_MULTI ${fam}tables-restore <<EOF
+*filter
+-A FORWARD -j ACCEPT
+COMMIT
+EOF
+ [[ $? == 0 ]] || {
+ echo "${fam}tables-restore failed when it should not have"
+ exit 1
+ }
+done
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index aa754d79608da..fd7d601f6136a 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -780,6 +780,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
int selected_chain = -1;
struct xtables_rule_match *xtrm_i;
struct ebt_match *match;
+ bool table_set = false;
/* prevent getopt to spoil our error reporting */
optind = 0;
@@ -947,7 +948,7 @@ print_zero:
break;
case 't': /* Table */
ebt_check_option2(&flags, OPT_TABLE);
- if (restore && *table)
+ if (restore && table_set)
xtables_error(PARAMETER_PROBLEM,
"The -t option (seen in line %u) cannot be used in %s.\n",
line, xt_params->program_name);
@@ -956,6 +957,7 @@ print_zero:
"Table name length cannot exceed %d characters",
EBT_TABLE_MAXNAMELEN - 1);
*table = optarg;
+ table_set = true;
break;
case 'i': /* Input interface */
case 2 : /* Logical input interface */
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 89f3271e36dd0..8a9e0edc3bea2 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -590,6 +590,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
bool wait_interval_set = false;
struct timeval wait_interval;
struct xtables_target *t;
+ bool table_set = false;
int wait = 0;
memset(cs, 0, sizeof(*cs));
@@ -879,7 +880,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
if (cs->invert)
xtables_error(PARAMETER_PROBLEM,
"unexpected ! flag before --table");
- if (p->restore && p->table)
+ if (p->restore && table_set)
xtables_error(PARAMETER_PROBLEM,
"The -t option (seen in line %u) cannot be used in %s.\n",
line, xt_params->program_name);
@@ -888,6 +889,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
"table '%s' does not exist",
optarg);
p->table = optarg;
+ table_set = true;
break;
case 'x':
--
2.23.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [iptables PATCH] xtables-restore: Unbreak *tables-restore
2019-10-22 10:34 [iptables PATCH] xtables-restore: Unbreak *tables-restore Phil Sutter
@ 2019-10-23 8:14 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2019-10-23 8:14 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Tue, Oct 22, 2019 at 12:34:46PM +0200, Phil Sutter wrote:
> Commit 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
> installed an error check which evaluated true in all cases as all
> callers of do_command callbacks pass a pointer to a table name already.
> Attached test case passed as it tested error condition only.
>
> Fix the whole mess by introducing a boolean to indicate whether a table
> parameter was seen already. Extend the test case to cover positive as
> well as negative behaviour and to test ebtables-restore and
> ip6tables-restore as well. Also add the required checking code to the
> latter since the original commit missed it.
>
> Fixes: 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-23 8:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 10:34 [iptables PATCH] xtables-restore: Unbreak *tables-restore Phil Sutter
2019-10-23 8:14 ` Pablo Neira Ayuso
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).