All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: [iptables PATCH] xshared: Fix response to unprivileged users
Date: Thu, 20 Jan 2022 11:16:53 +0100	[thread overview]
Message-ID: <20220120101653.28280-1-phil@nwl.cc> (raw)

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


             reply	other threads:[~2022-01-20 10:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-20 10:16 Phil Sutter [this message]
2022-01-20 10:33 ` [iptables PATCH] xshared: Fix response to unprivileged users 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220120101653.28280-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.