All of lore.kernel.org
 help / color / mirror / Atom feed
* [iptables PATCH] xshared: Fix response to unprivileged users
@ 2022-01-20 10:16 Phil Sutter
  2022-01-20 10:33 ` Florian Westphal
  2022-01-27 16:28 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 7+ messages in thread
From: Phil Sutter @ 2022-01-20 10:16 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Expected behaviour in both variants is:

* Print help without error, append extension help if -m and/or -j
  options are present
* Indicate lack of permissions in an error message for anything else

With iptables-nft, this was broken basically from day 1. Shared use of
do_parse() then somewhat broke legacy: it started complaining about
inability to create a lock file.

Fix this by making iptables-nft assume extension revision 0 is present
if permissions don't allow to verify. This is consistent with legacy.

Second part is to exit directly after printing help - this avoids having
to make the following code "nop-aware" to prevent privileged actions.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c                                |  5 ++
 .../testcases/iptables/0008-unprivileged_0    | 60 +++++++++++++++++++
 iptables/xshared.c                            |  3 +-
 3 files changed, 66 insertions(+), 2 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/iptables/0008-unprivileged_0

diff --git a/iptables/nft.c b/iptables/nft.c
index 72f7cf1315661..b5de687c5c4cd 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -3312,6 +3312,11 @@ int nft_compatible_revision(const char *name, uint8_t rev, int opt)
 err:
 	mnl_socket_close(nl);
 
+	/* pretend revision 0 is valid if not permitted to check -
+	 * this is required for printing extension help texts as user */
+	if (ret < 0 && errno == EPERM && rev == 0)
+		return 1;
+
 	return ret < 0 ? 0 : 1;
 }
 
diff --git a/iptables/tests/shell/testcases/iptables/0008-unprivileged_0 b/iptables/tests/shell/testcases/iptables/0008-unprivileged_0
new file mode 100755
index 0000000000000..43e3bc8721dbd
--- /dev/null
+++ b/iptables/tests/shell/testcases/iptables/0008-unprivileged_0
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+# iptables may print match/target specific help texts
+# help output should work for unprivileged users
+
+run() {
+	echo "running: $*" >&2
+	runuser -u nobody -- "$@"
+}
+
+grep_or_rc() {
+	declare -g rc
+	grep -q "$*" && return 0
+	echo "missing in output: $*" >&2
+	return 1
+}
+
+out=$(run $XT_MULTI iptables --help)
+let "rc+=$?"
+grep_or_rc "iptables -h (print this help information)" <<< "$out"
+let "rc+=$?"
+
+out=$(run $XT_MULTI iptables -m limit --help)
+let "rc+=$?"
+grep_or_rc "limit match options:" <<< "$out"
+let "rc+=$?"
+
+out=$(run $XT_MULTI iptables -p tcp --help)
+let "rc+=$?"
+grep_or_rc "tcp match options:" <<< "$out"
+let "rc+=$?"
+
+out=$(run $XT_MULTI iptables -j DNAT --help)
+let "rc+=$?"
+grep_or_rc "DNAT target options:" <<< "$out"
+let "rc+=$?"
+
+out=$(run $XT_MULTI iptables -p tcp -j DNAT --help)
+let "rc+=$?"
+grep_or_rc "tcp match options:" <<< "$out"
+let "rc+=$?"
+out=$(run $XT_MULTI iptables -p tcp -j DNAT --help)
+let "rc+=$?"
+grep_or_rc "DNAT target options:" <<< "$out"
+let "rc+=$?"
+
+
+run $XT_MULTI iptables -L 2>&1 | \
+	grep_or_rc "Permission denied"
+let "rc+=$?"
+
+run $XT_MULTI iptables -A FORWARD -p tcp --dport 123 2>&1 | \
+	grep_or_rc "Permission denied"
+let "rc+=$?"
+
+run $XT_MULTI iptables -A FORWARD -j DNAT --to-destination 1.2.3.4 2>&1 | \
+	grep_or_rc "Permission denied"
+let "rc+=$?"
+
+exit $rc
diff --git a/iptables/xshared.c b/iptables/xshared.c
index c5a93290be427..1fd7acc953ed7 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1473,8 +1473,7 @@ void do_parse(int argc, char *argv[],
 					XTF_TRY_LOAD, &cs->matches);
 
 			xt_params->print_help(cs->matches);
-			p->command = CMD_NONE;
-			return;
+			exit(0);
 
 			/*
 			 * Option selection
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-27 17:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 10:16 [iptables PATCH] xshared: Fix response to unprivileged users Phil Sutter
2022-01-20 10:33 ` Florian Westphal
2022-01-27 16:28 ` Pablo Neira Ayuso
2022-01-27 17:08   ` Phil Sutter
2022-01-27 17:21     ` Pablo Neira Ayuso
2022-01-27 17:29       ` Phil Sutter
2022-01-27 17:31         ` Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.