[iptables PATCH v2 1/2] nft: Fix command name in ip6tables error message

[iptables PATCH v2 1/2] nft: Fix command name in ip6tables error message

[Phil Sutter]

Upon errors, ip6tables-nft would prefix its error messages with
'iptables:' instead of 'ip6tables:'. Turns out the command name was
hard-coded, use 'progname' variable instead.
While being at it, merge the two mostly identical fprintf() calls into
one.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Added this fix. Related test ignored the prefix value, so this went
  unnoticed.
---
 iptables/xtables-standalone.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
index dd6fb7919d2e1..7b71db62f1ea6 100644
--- a/iptables/xtables-standalone.c
+++ b/iptables/xtables-standalone.c
@@ -75,14 +75,10 @@ xtables_main(int family, const char *progname, int argc, char *argv[])
 	xtables_fini();
 
 	if (!ret) {
-		if (errno == EINVAL) {
-			fprintf(stderr, "iptables: %s. "
-					"Run `dmesg' for more information.\n",
-				nft_strerror(errno));
-		} else {
-			fprintf(stderr, "iptables: %s.\n",
-				nft_strerror(errno));
-		}
+		fprintf(stderr, "%s: %s.%s\n", progname, nft_strerror(errno),
+			(errno == EINVAL ?
+			 " Run `dmesg' for more information." : ""));
+
 		if (errno == EAGAIN)
 			exit(RESOURCE_PROBLEM);
 	}
-- 
2.27.0

[iptables PATCH v2 2/2] tests: shell: Merge and extend return codes test

[Phil Sutter]

Merge scripts for iptables and ip6tables, they were widely identical.
Also extend the test by one check (removing a non-existent rule with
valid chain and target) and quote the error messages where differences
are deliberately ignored.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Check error prefix value also.
---
 .../testcases/ip6tables/0004-return-codes_0   |  39 ------
 .../testcases/iptables/0004-return-codes_0    | 113 ++++++++++--------
 2 files changed, 61 insertions(+), 91 deletions(-)
 delete mode 100755 iptables/tests/shell/testcases/ip6tables/0004-return-codes_0

diff --git a/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0 b/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0
deleted file mode 100755
index c583b0ebd97c3..0000000000000
--- a/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0
+++ /dev/null
@@ -1,39 +0,0 @@
-#!/bin/sh
-
-# make sure error return codes are as expected useful cases
-# (e.g. commands to check ruleset state)
-
-global_rc=0
-
-cmd() { # (rc, cmd, [args ...])
-	rc_exp=$1; shift
-
-	$XT_MULTI "$@"
-	rc=$?
-
-	[ $rc -eq $rc_exp ] || {
-		echo "---> expected $rc_exp, got $rc for command '$@'"
-		global_rc=1
-	}
-}
-
-# test chain creation
-cmd 0 ip6tables -N foo
-cmd 1 ip6tables -N foo
-# iptables-nft allows this - bug or feature?
-#cmd 2 ip6tables -N "invalid name"
-
-# test rule adding
-cmd 0 ip6tables -A INPUT -j ACCEPT
-cmd 1 ip6tables -A noexist -j ACCEPT
-cmd 2 ip6tables -I INPUT -j foobar
-
-# test rule checking
-cmd 0 ip6tables -C INPUT -j ACCEPT
-cmd 1 ip6tables -C FORWARD -j ACCEPT
-cmd 1 ip6tables -C nonexist -j ACCEPT
-cmd 2 ip6tables -C INPUT -j foobar
-cmd 2 ip6tables -C INPUT -m foobar -j ACCEPT
-cmd 3 ip6tables -t foobar -C INPUT -j ACCEPT
-
-exit $global_rc
diff --git a/iptables/tests/shell/testcases/iptables/0004-return-codes_0 b/iptables/tests/shell/testcases/iptables/0004-return-codes_0
index f730bede1f612..dcd9dfd3c0806 100755
--- a/iptables/tests/shell/testcases/iptables/0004-return-codes_0
+++ b/iptables/tests/shell/testcases/iptables/0004-return-codes_0
@@ -13,75 +13,84 @@ cmd() { # (rc, msg, cmd, [args ...])
 		msg_exp="$1"; shift
 	}
 
-	msg="$($XT_MULTI "$@" 2>&1 >/dev/null)"
-	rc=$?
+	for ipt in iptables ip6tables; do
+		msg="$($XT_MULTI $ipt "$@" 2>&1 >/dev/null)"
+		rc=$?
 
-	[ $rc -eq $rc_exp ] || {
-		echo "---> expected return code $rc_exp, got $rc for command '$@'"
-		global_rc=1
-	}
+		[ $rc -eq $rc_exp ] || {
+			echo "---> expected return code $rc_exp, got $rc for command '$ipt $@'"
+			global_rc=1
+		}
 
-	[ -n "$msg_exp" ] || return
-	grep -q "$msg_exp" <<< $msg || {
-		echo "---> expected error message '$msg_exp', got '$msg' for command '$@'"
-		global_rc=1
-	}
+		[ -n "$msg_exp" ] || continue
+		msg_exp_full="${ipt}$msg_exp"
+		grep -q "$msg_exp_full" <<< $msg || {
+			echo "---> expected error message '$msg_exp_full', got '$msg' for command '$ipt $@'"
+			global_rc=1
+		}
+	done
 }
 
-EEXIST_F="File exists."
-EEXIST="Chain already exists."
-ENOENT="No chain/target/match by that name."
-E2BIG_I="Index of insertion too big."
-E2BIG_D="Index of deletion too big."
-E2BIG_R="Index of replacement too big."
-EBADRULE="Bad rule (does a matching rule exist in that chain?)."
-ENOTGT="Couldn't load target \`foobar':No such file or directory"
-ENOMTH="Couldn't load match \`foobar':No such file or directory"
-ENOTBL="can't initialize iptables table \`foobar': Table does not exist"
+EEXIST_F=": File exists."
+EEXIST=": Chain already exists."
+ENOENT=": No chain/target/match by that name."
+E2BIG_I=": Index of insertion too big."
+E2BIG_D=": Index of deletion too big."
+E2BIG_R=": Index of replacement too big."
+EBADRULE=": Bad rule (does a matching rule exist in that chain?)."
+#ENOTGT=" v[0-9\.]* [^ ]*: Couldn't load target \`foobar':No such file or directory"
+ENOMTH=" v[0-9\.]* [^ ]*: Couldn't load match \`foobar':No such file or directory"
+ENOTBL=": can't initialize iptables table \`foobar': Table does not exist"
 
 # test chain creation
-cmd 0 iptables -N foo
-cmd 1 "$EEXIST" iptables -N foo
+cmd 0 -N foo
+cmd 1 "$EEXIST" -N foo
 # iptables-nft allows this - bug or feature?
-#cmd 2 iptables -N "invalid name"
+#cmd 2 -N "invalid name"
 
 # test chain flushing/zeroing
-cmd 0 iptables -F foo
-cmd 0 iptables -Z foo
-cmd 1 "$ENOENT" iptables -F bar
-cmd 1 "$ENOENT" iptables -Z bar
+cmd 0 -F foo
+cmd 0 -Z foo
+cmd 1 "$ENOENT" -F bar
+cmd 1 "$ENOENT" -Z bar
 
 # test chain rename
-cmd 0 iptables -E foo bar
-cmd 1 "$EEXIST_F" iptables -E foo bar
-cmd 1 "$ENOENT" iptables -E foo bar2
-cmd 0 iptables -N foo2
-cmd 1 "$EEXIST_F" iptables -E foo2 bar
+cmd 0 -E foo bar
+cmd 1 "$EEXIST_F" -E foo bar
+cmd 1 "$ENOENT" -E foo bar2
+cmd 0 -N foo2
+cmd 1 "$EEXIST_F" -E foo2 bar
 
 # test rule adding
-cmd 0 iptables -A INPUT -j ACCEPT
-cmd 1 "$ENOENT" iptables -A noexist -j ACCEPT
-cmd 2 "" iptables -I INPUT -j foobar
-cmd 2 "" iptables -R INPUT 1 -j foobar
-cmd 2 "" iptables -D INPUT -j foobar
+cmd 0 -A INPUT -j ACCEPT
+cmd 1 "$ENOENT" -A noexist -j ACCEPT
+# next three differ:
+# legacy: Couldn't load target `foobar':No such file or directory
+# nft:    Chain 'foobar' does not exist
+cmd 2 "" -I INPUT -j foobar
+cmd 2 "" -R INPUT 1 -j foobar
+cmd 2 "" -D INPUT -j foobar
+cmd 1 "$EBADRULE" -D INPUT -p tcp --dport 22 -j ACCEPT
 
 # test rulenum commands
-cmd 1 "$E2BIG_I" iptables -I INPUT 23 -j ACCEPT
-cmd 1 "$E2BIG_D" iptables -D INPUT 23
-cmd 1 "$E2BIG_R" iptables -R INPUT 23 -j ACCEPT
-cmd 1 "$ENOENT" iptables -I nonexist 23 -j ACCEPT
-cmd 1 "$ENOENT" iptables -D nonexist 23
-cmd 1 "$ENOENT" iptables -R nonexist 23 -j ACCEPT
+cmd 1 "$E2BIG_I" -I INPUT 23 -j ACCEPT
+cmd 1 "$E2BIG_D" -D INPUT 23
+cmd 1 "$E2BIG_R" -R INPUT 23 -j ACCEPT
+cmd 1 "$ENOENT" -I nonexist 23 -j ACCEPT
+cmd 1 "$ENOENT" -D nonexist 23
+cmd 1 "$ENOENT" -R nonexist 23 -j ACCEPT
 
 # test rule checking
-cmd 0 iptables -C INPUT -j ACCEPT
-cmd 1 "$EBADRULE" iptables -C FORWARD -j ACCEPT
-cmd 1 "$BADRULE" iptables -C nonexist -j ACCEPT
-cmd 2 "$ENOMTH" iptables -C INPUT -m foobar -j ACCEPT
+cmd 0 -C INPUT -j ACCEPT
+cmd 1 "$EBADRULE" -C FORWARD -j ACCEPT
+cmd 1 "$BADRULE" -C nonexist -j ACCEPT
+cmd 2 "$ENOMTH" -C INPUT -m foobar -j ACCEPT
 # messages of those don't match, but iptables-nft ones are actually nicer.
-#cmd 2 "$ENOTGT" iptables -C INPUT -j foobar
-#cmd 3 "$ENOTBL" iptables -t foobar -C INPUT -j ACCEPT
-cmd 2 "" iptables -C INPUT -j foobar
-cmd 3 "" iptables -t foobar -C INPUT -j ACCEPT
+# legacy: Couldn't load target `foobar':No such file or directory
+# nft:    Chain 'foobar' does not exist
+cmd 2 "" -C INPUT -j foobar
+# legacy: can't initialize ip6tables table `foobar': Table does not exist (do you need to insmod?)
+# nft:    table 'foobar' does not exist
+cmd 3 "" -t foobar -C INPUT -j ACCEPT
 
 exit $global_rc
-- 
2.27.0

Re: [iptables PATCH v2 2/2] tests: shell: Merge and extend return codes test

[Pablo Neira Ayuso]

On Fri, Aug 07, 2020 at 02:02:14PM +0200, Phil Sutter wrote:
> Merge scripts for iptables and ip6tables, they were widely identical.
> Also extend the test by one check (removing a non-existent rule with
> valid chain and target) and quote the error messages where differences
> are deliberately ignored.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [iptables PATCH v2 1/2] nft: Fix command name in ip6tables error message

[Pablo Neira Ayuso]

On Fri, Aug 07, 2020 at 02:02:13PM +0200, Phil Sutter wrote:
> Upon errors, ip6tables-nft would prefix its error messages with
> 'iptables:' instead of 'ip6tables:'. Turns out the command name was
> hard-coded, use 'progname' variable instead.
> While being at it, merge the two mostly identical fprintf() calls into
> one.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>