From 9956bdb4d904a523381d3971796e829721e84e29 Mon Sep 17 00:00:00 2001 From: Jiri Popelka 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 }", "")) parser_direct.add_argument("--get-passthroughs", nargs=1, - metavar=("{ ipv4 | ipv6 | eb }", "")) + 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 }", "", "")) @@ -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 }", "
")) parser_direct.add_argument("--remove-rules", nargs=3, - metavar=("{ ipv4 | ipv6 | eb }", "
")) + metavar=("{ ipv4 | ipv6 | eb }", "
")) parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER, metavar=("{ ipv4 | ipv6 | eb }", "
")) parser_direct.add_argument("--get-rules", nargs=3, metavar=("{ ipv4 | ipv6 | eb }", "
", "")) 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 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.append(joinArgs(aux_args[i+1:])) # add 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 } ") __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 } ") __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 } ") 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 } ") __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 }
") 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 }
") @@ -824,7 +849,7 @@ if a.permanent: except ValueError: __fail("usage: --permanent --direct --remove-rule { ipv4 | ipv6 | eb }
") 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 }
") @@ -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 } ") - 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 }
") 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 }
") @@ -1068,7 +1093,7 @@ elif a.direct: except ValueError: __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }
") 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 }
") @@ -1082,7 +1107,7 @@ elif a.direct: except ValueError: __fail("usage: --direct --query-rule { ipv4 | ipv6 | eb }
") __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 }", "")) parser_direct.add_argument("--get-passthroughs", nargs=1, - metavar=("{ ipv4 | ipv6 | eb }", "")) + 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 }", "
", "")) @@ -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 }", "
")) parser_direct.add_argument("--remove-rules", nargs=3, - metavar=("{ ipv4 | ipv6 | eb }", "
")) + metavar=("{ ipv4 | ipv6 | eb }", "
")) parser_direct.add_argument("--query-rule", nargs=argparse.REMAINDER, metavar=("{ ipv4 | ipv6 | eb }", "
")) 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 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.append(joinArgs(aux_args[i+1:])) # add 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 } ") __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 } ") 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 } ") __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 }
") 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 }
") @@ -892,7 +913,7 @@ try: except ValueError: __fail("usage: --direct --remove-rule { ipv4 | ipv6 | eb }
") 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 }
") @@ -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