Blob Blame History Raw
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 }", "<table>", "<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 }", "<table> <chain> <priority> <args>"))
 parser_direct.add_argument("--remove-rules", nargs=3,
-                        metavar=("{ ipv4 | ipv6 | eb }", "<table> <chain> <args>"))
+                        metavar=("{ ipv4 | ipv6 | eb }", "<table> <chain>"))
 parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER,
                         metavar=("{ ipv4 | ipv6 | eb }", "<table> <chain> <priority> <args>"))
 parser_direct.add_argument("--get-rules", nargs=3,
                         metavar=("{ ipv4 | ipv6 | eb }", "<table>", "<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 } <table> <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 } <table> <chain> <priority> <args>")
@@ -824,7 +849,7 @@ if a.permanent:
             except ValueError:
                 __fail("usage: --permanent --direct --remove-rule { ipv4 | ipv6 | eb } <table> <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 } <table> <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 } <table> <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 } <table> <chain> <priority> <args>")
@@ -1068,7 +1093,7 @@ elif a.direct:
         except ValueError:
             __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb } <table> <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 } <table> <chain>")
@@ -1082,7 +1107,7 @@ elif a.direct:
         except ValueError:
             __fail("usage: --direct --query-rule { ipv4 | ipv6 | eb } <table> <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 }", "<table>", "<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 }", "<table> <chain> <priority> <args>"))
 parser_direct.add_argument("--remove-rules", nargs=3,
-                        metavar=("{ ipv4 | ipv6 | eb }", "<table> <chain> <args>"))
+                        metavar=("{ ipv4 | ipv6 | eb }", "<table> <chain>"))
 parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER,
                         metavar=("{ ipv4 | ipv6 | eb }", "<table> <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 } <table> <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 } <table> <chain> <priority> <args>")
@@ -892,7 +913,7 @@ try:
             except ValueError:
                 __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb } <table> <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 } <table> <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