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