From 84d19c668db246556fac766cff8652ea6f3a4076 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Tue, 18 Jan 2022 22:39:08 +0100 Subject: [PATCH] xshared: Fix response to unprivileged users 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 Reviewed-by: Florian Westphal (cherry picked from commit 26ecdf53960658771c0fc582f72a4025e2887f75) Conflicts: iptables/xshared.c -> Adjusted to missing commit 62c3c93d4b0f5 ("xshared: Move do_parse to shared space"). --- iptables/nft.c | 5 ++ .../testcases/iptables/0008-unprivileged_0 | 60 +++++++++++++++++++ iptables/xtables.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 ba59cfb8c47af..da9d24f5c86e2 100644 --- a/iptables/nft.c +++ b/iptables/nft.c @@ -3294,6 +3294,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/xtables.c b/iptables/xtables.c index 54f887f80497e..7ef1702a0cd50 100644 --- a/iptables/xtables.c +++ b/iptables/xtables.c @@ -637,8 +637,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[], XTF_TRY_LOAD, &cs->matches); printhelp(cs->matches); - p->command = CMD_NONE; - return; + exit(0); /* * Option selection -- 2.40.0