From 9956bdb4d904a523381d3971796e829721e84e29 Mon Sep 17 00:00:00 2001

From: Jiri Popelka <jpopelka@redhat.com>

Date: Thu, 27 Feb 2014 18:38:43 +0100

Subject: [PATCH] prevent argparse from parsing iptables options

since 2f435f7a

--direct --passthrough ipv4 --table filter --delete INPUT --jump ACCEPT

has been failing with:

error: ambiguous option: --delete could match --delete-icmptype,

--delete-service, --delete-zone

---

 src/firewall-cmd               | 55 ++++++++++++++++++++++++++++++------------

 src/firewall-offline-cmd       | 41 +++++++++++++++++++++++--------

 src/firewall/functions.py      |  2 +-

 src/tests/firewall-cmd_test.sh | 10 +++++++-

 4 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/src/firewall-cmd b/src/firewall-cmd

index 029e2b7..aa7e3c0 100755

--- a/src/firewall-cmd

+++ b/src/firewall-cmd

@@ -31,7 +31,7 @@ import os

 from firewall.client import *

 from firewall.errors import *

-from firewall.functions import joinArgs

+from firewall.functions import joinArgs, splitArgs

 def __print(msg=None):

     if msg and not a.quiet:

@@ -473,7 +473,7 @@ parser_direct.add_argument("--remove-passthrough", nargs=argparse.REMAINDER,

 parser_direct.add_argument("--query-passthrough", nargs=argparse.REMAINDER,

                     metavar=("{ ipv4 | ipv6 | eb }", "<args>"))

 parser_direct.add_argument("--get-passthroughs", nargs=1,

-                    metavar=("{ ipv4 | ipv6 | eb }", "<args>"))

+                    metavar=("{ ipv4 | ipv6 | eb }"))

 parser_direct.add_argument("--get-all-passthroughs", action="store_true")

 parser_direct.add_argument("--add-chain", nargs=3,

                     metavar=("{ ipv4 | ipv6 | eb }", "", "<chain>"))

@@ -489,14 +489,39 @@ parser_direct.add_argument("--add-rule", nargs=argparse.REMAINDER,

 parser_direct.add_argument("--remove-rule", nargs=argparse.REMAINDER,

                         metavar=("{ ipv4 | ipv6 | eb }", " <chain> <priority> <args>"))

 parser_direct.add_argument("--remove-rules", nargs=3,

-                        metavar=("{ ipv4 | ipv6 | eb }", " <chain> <args>"))

+                        metavar=("{ ipv4 | ipv6 | eb }", " <chain>"))

 parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER,

                         metavar=("{ ipv4 | ipv6 | eb }", " <chain> <priority> <args>"))

 parser_direct.add_argument("--get-rules", nargs=3,

                         metavar=("{ ipv4 | ipv6 | eb }", "", "<chain>"))

 parser_direct.add_argument("--get-all-rules", action="store_true")

-a = parser.parse_args()

+i = -1

+args = sys.argv[1:]

+if '--passthrough' in args:

+  i = args.index('--passthrough') + 1

+elif '--add-passthrough' in args:

+  i = args.index('--add-passthrough') + 1

+elif '--remove-passthrough' in args:

+  i = args.index('--remove-passthrough') + 1

+elif '--query-passthrough' in args:

+  i = args.index('--query-passthrough') + 1

+elif '--add-rule' in args:

+  i = args.index('--add-rule') + 4

+elif '--remove-rule' in args:

+  i = args.index('--remove-rule') + 4

+elif '--query-rule' in args:

+  i = args.index('--query-rule') + 4

+# join <args> into one argument to prevent parser from parsing each iptables

+# option, because they can conflict with firewall-cmd options

+# # e.g. --delete (iptables) and --delete-* (firewall-cmd)

+if (i > -1) and (i < len(args) - 1):

+    aux_args = args[:]

+    args = aux_args[:i+1] # all but not <args>

+    args.append(joinArgs(aux_args[i+1:])) # add <args> as one arg

+

+a = parser.parse_args(args)

+

 options_standalone = a.help or a.version or \

     a.state or a.reload or a.complete_reload or \

@@ -756,25 +781,25 @@ if a.permanent:

             if len (a.passthrough) < 2:

                 __fail("usage: --permanent --direct --passthrough { ipv4 | ipv6 | eb } <args>")

             __print(settings.addPassthrough(_check_ipv(a.passthrough[0]),

-                                                       a.passthrough[1:]))

+                                                       splitArgs(a.passthrough[1])))

         if a.add_passthrough:

             if len (a.add_passthrough) < 2:

                 __fail("usage: --permanent --direct --add-passthrough { ipv4 | ipv6 | eb } <args>")

             __print(settings.addPassthrough(_check_ipv(a.add_passthrough[0]),

-                                            a.add_passthrough[1:]))

+                                            splitArgs(a.add_passthrough[1])))

         elif a.remove_passthrough:

             if len (a.remove_passthrough) < 2:

                 __fail("usage: --permanent --direct --remove-passthrough { ipv4 | ipv6 | eb } <args>")

             settings.removePassthrough(_check_ipv(a.remove_passthrough[0]),

-                                       a.remove_passthrough[1:])

+                                       splitArgs(a.remove_passthrough[1]))

         elif a.query_passthrough:

             if len (a.query_passthrough) < 2:

                 __fail("usage: --permanent --direct --query-passthrough { ipv4 | ipv6 | eb } <args>")

             __print_query_result(

                 settings.queryPassthrough(_check_ipv(a.query_passthrough[0]),

-                                          a.query_passthrough[1:]))

+                                          splitArgs(a.query_passthrough[1])))

             sys.exit(0)

         elif a.get_passthroughs:

             rules = settings.getPassthroughs(_check_ipv(a.get_passthroughs[0]))

@@ -815,7 +840,7 @@ if a.permanent:

             except ValueError:

                 __fail("wrong priority\nusage: --permanent --direct --add-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

             settings.addRule(_check_ipv(a.add_rule[0]), a.add_rule[1],

-                             a.add_rule[2], priority, a.add_rule[4:])

+                             a.add_rule[2], priority, splitArgs(a.add_rule[4]))

         elif a.remove_rule:

             if len (a.remove_rule) < 5:

                 __fail("usage: --permanent --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

@@ -824,7 +849,7 @@ if a.permanent:

             except ValueError:

                 __fail("usage: --permanent --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

             settings.removeRule(_check_ipv(a.remove_rule[0]), a.remove_rule[1],

-                                a.remove_rule[2], priority, a.remove_rule[4:])

+                                a.remove_rule[2], priority, splitArgs(a.remove_rule[4]))

         elif a.remove_rules:

             if len (a.remove_rules) < 3:

                 __fail("usage: --permanent --direct --remove-rules { ipv4 | ipv6 | eb }  <chain>")

@@ -840,7 +865,7 @@ if a.permanent:

             __print_query_result(

                     settings.queryRule(_check_ipv(a.query_rule[0]),

                                        a.query_rule[1], a.query_rule[2],

-                                       priority, a.query_rule[4:]))

+                                       priority, splitArgs(a.query_rule[4])))

             sys.exit(0)

         elif a.get_rules:

             rules = settings.getRules(_check_ipv(a.get_rules[0]),

@@ -1032,7 +1057,7 @@ elif a.direct:

     if a.passthrough:

         if len (a.passthrough) < 2:

             __fail("usage: --direct --passthrough { ipv4 | ipv6 | eb } <args>")

-        msg = fw.passthrough(_check_ipv(a.passthrough[0]), a.passthrough[1:])

+        msg = fw.passthrough(_check_ipv(a.passthrough[0]), splitArgs(a.passthrough[1]))

         if msg:

             print(msg)

     elif a.add_chain:

@@ -1059,7 +1084,7 @@ elif a.direct:

         except ValueError:

             __fail("wrong priority\nusage: --direct --add-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

         fw.addRule(_check_ipv(a.add_rule[0]), a.add_rule[1], a.add_rule[2],

-                   priority, a.add_rule[4:])

+                   priority, splitArgs(a.add_rule[4]))

     elif a.remove_rule:

         if len (a.remove_rule) < 5:

             __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

@@ -1068,7 +1093,7 @@ elif a.direct:

         except ValueError:

             __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

         fw.removeRule(_check_ipv(a.remove_rule[0]),

-                      a.remove_rule[1], a.remove_rule[2], priority, a.remove_rule[4:])

+                      a.remove_rule[1], a.remove_rule[2], priority, splitArgs(a.remove_rule[4]))

     elif a.remove_rules:

         if len (a.remove_rules) < 3:

             __fail("usage: --direct --remove-rules { ipv4 | ipv6 | eb }  <chain>")

@@ -1082,7 +1107,7 @@ elif a.direct:

         except ValueError:

             __fail("usage: --direct --query-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

         __print_query_result(fw.queryRule(_check_ipv(a.query_rule[0]),

-                                          a.query_rule[1], a.query_rule[2], priority, a.query_rule[4:]))

+                                          a.query_rule[1], a.query_rule[2], priority, splitArgs(a.query_rule[4])))

     elif a.get_rules:

         rules = fw.getRules(_check_ipv(a.get_rules[0]),

                             a.get_rules[1], a.get_rules[2])

diff --git a/src/firewall-offline-cmd b/src/firewall-offline-cmd

index 112aa1c..97ee213 100755

--- a/src/firewall-offline-cmd

+++ b/src/firewall-offline-cmd

@@ -33,7 +33,7 @@ import os

 from firewall.core.fw_test import Firewall_test

 from firewall.client import *

 from firewall.errors import *

-from firewall.functions import joinArgs

+from firewall.functions import joinArgs, splitArgs

 # check for root user

 if os.getuid() != 0:

@@ -495,7 +495,7 @@ parser_direct.add_argument("--remove-passthrough", nargs=argparse.REMAINDER,

 parser_direct.add_argument("--query-passthrough", nargs=argparse.REMAINDER,

                     metavar=("{ ipv4 | ipv6 | eb }", "<args>"))

 parser_direct.add_argument("--get-passthroughs", nargs=1,

-                    metavar=("{ ipv4 | ipv6 | eb }", "<args>"))

+                    metavar=("{ ipv4 | ipv6 | eb }"))

 parser_direct.add_argument("--get-all-passthroughs", action="store_true")

 parser_direct.add_argument("--add-chain", nargs=3,

                     metavar=("{ ipv4 | ipv6 | eb }", "", "<chain>"))

@@ -511,7 +511,7 @@ parser_direct.add_argument("--add-rule", nargs=argparse.REMAINDER,

 parser_direct.add_argument("--remove-rule", nargs=argparse.REMAINDER,

                         metavar=("{ ipv4 | ipv6 | eb }", " <chain> <priority> <args>"))

 parser_direct.add_argument("--remove-rules", nargs=3,

-                        metavar=("{ ipv4 | ipv6 | eb }", " <chain> <args>"))

+                        metavar=("{ ipv4 | ipv6 | eb }", " <chain>"))

 parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER,

                         metavar=("{ ipv4 | ipv6 | eb }", " <chain> <priority> <args>"))

 parser_direct.add_argument("--get-rules", nargs=3,

@@ -521,7 +521,28 @@ parser_direct.add_argument("--get-all-rules", action="store_true")

 ##############################################################################

 if len(sys.argv) > 1:

-    a = parser.parse_args()

+    i = -1

+    args = sys.argv[1:]

+    if '--add-passthrough' in args:

+        i = args.index('--add-passthrough') + 1

+    elif '--remove-passthrough' in args:

+        i = args.index('--remove-passthrough') + 1

+    elif '--query-passthrough' in args:

+        i = args.index('--query-passthrough') + 1

+    elif '--add-rule' in args:

+        i = args.index('--add-rule') + 4

+    elif '--remove-rule' in args:

+        i = args.index('--remove-rule') + 4

+    elif '--query-rule' in args:

+        i = args.index('--query-rule') + 4

+    # join <args> into one argument to prevent parser from parsing each iptables

+    # option, because they can conflict with firewall-cmd options

+    # # e.g. --delete (iptables) and --delete-* (firewall-cmd)

+    if (i > -1) and (i < len(args) - 1):

+        aux_args = args[:]

+        args = aux_args[:i+1] # all but not <args>

+        args.append(joinArgs(aux_args[i+1:])) # add <args> as one arg

+    a = parser.parse_args(args)

 else:

     # migrate configuration from /etc/sysconfig/system-config-firewall

     args = read_sysconfig_args()

@@ -826,19 +847,19 @@ try:

             if len (a.add_passthrough) < 2:

                 __fail("usage: --direct --add-passthrough { ipv4 | ipv6 | eb } <args>")

             __print(settings.add_passthrough(_check_ipv(a.add_passthrough[0]),

-                                             a.add_passthrough[1:]))

+                                             splitArgs(a.add_passthrough[1])))

         elif a.remove_passthrough:

             if len (a.remove_passthrough) < 2:

                 __fail("usage: --direct --remove-passthrough { ipv4 | ipv6 | eb } <args>")

             settings.remove_passthrough(_check_ipv(a.remove_passthrough[0]),

-                                        a.remove_passthrough[1:])

+                                        splitArgs(a.remove_passthrough[1]))

         elif a.query_passthrough:

             if len (a.query_passthrough) < 2:

                 __fail("usage: --direct --query-passthrough { ipv4 | ipv6 | eb } <args>")

             __print_query_result(

                 settings.query_passthrough(_check_ipv(a.query_passthrough[0]),

-                                           a.query_passthrough[1:]))

+                                           splitArgs(a.query_passthrough[1])))

             sys.exit(0)

         elif a.get_passthroughs:

             rules = settings.get_passthroughs(_check_ipv(a.get_passthroughs[0]))

@@ -883,7 +904,7 @@ try:

             except ValueError:

                 __fail("wrong priority\nusage: --direct --add-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

             settings.add_rule(_check_ipv(a.add_rule[0]), a.add_rule[1],

-                              a.add_rule[2], priority, a.add_rule[4:])

+                              a.add_rule[2], priority, splitArgs(a.add_rule[4]))

         elif a.remove_rule:

             if len (a.remove_rule) < 5:

                 __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

@@ -892,7 +913,7 @@ try:

             except ValueError:

                 __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }  <chain> <priority> <args>")

             settings.remove_rule(_check_ipv(a.remove_rule[0]), a.remove_rule[1],

-                                a.remove_rule[2], priority, a.remove_rule[4:])

+                                a.remove_rule[2], priority, splitArgs(a.remove_rule[4]))

         elif a.remove_rules:

             if len (a.remove_rules) < 3:

                 __fail("usage: --direct --remove-rules { ipv4 | ipv6 | eb }  <chain>")

@@ -908,7 +929,7 @@ try:

             __print_query_result(

                     settings.query_rule(_check_ipv(a.query_rule[0]),

                                         a.query_rule[1], a.query_rule[2],

-                                        priority, a.query_rule[4:]))

+                                        priority, splitArgs(a.query_rule[4])))

             sys.exit(0)

         elif a.get_rules:

             rules = settings.get_rules(_check_ipv(a.get_rules[0]),

diff --git a/src/firewall/functions.py b/src/firewall/functions.py

index 6dd4240..ccbf583 100644

--- a/src/firewall/functions.py

+++ b/src/firewall/functions.py

@@ -390,7 +390,7 @@ def joinArgs(args):

         return " ".join(pipes.quote(a) for a in args)

 def splitArgs(string):

-    if PY2:

+    if PY2 and isinstance(string, unicode):

         # Python2's shlex doesn't like unicode

         string = u2b(string)

         splits = shlex.split(string)

-- 

1.8.5.3