xtables-restore: Unbreak *tables-restore

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>

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 57b0f9f55a921f7e01160b7d82cd80f95cf315dd..c160a2dd4e65b5147cddd572df72345b5cceb775 100644 (file)
--- 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 d7a41321760e0a6f0933c68cb1f0afb39ce3797d..544e87596e7e45787d88a3bde67ee16f6679321b 100644 (file)
--- 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':

diff --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 4e2202df986cf980cd7e6e693ecc61284c6f4607..e96140758a99d21ad3b01143c65abc5b706da2b7 100755 (executable)
--- 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 aa754d79608da27beaadb60017b49ebbfe4c8f9d..fd7d601f6136ae72b534279584398e6bd1697797 100644 (file)
--- 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 89f3271e36dd06dd89d57dbc4f45a9fd64ebac7a..8a9e0edc3bea2c57dbda067293e5f2cb986af9cc 100644 (file)
--- 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':