From 0f9f220b6715c18ad71c66c3aa0a740ec4cac1c9 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 3 Mar 2022 13:28:15 +0100 Subject: [PATCH] iptables-1.8.7-16 - Improve error messages for unsupported extensions - xshared: Fix response to unprivileged users - libxtables: Register only the highest revision extension - Ignore typical 'fedpkg local' results in .gitignore --- .gitignore | 5 + ...ter-only-the-highest-revision-extens.patch | 64 +++++++++ ...d-Fix-response-to-unprivileged-users.patch | 134 ++++++++++++++++++ ...-messages-for-unsupported-extensions.patch | 84 +++++++++++ iptables.spec | 11 +- 5 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 0014-libxtables-Register-only-the-highest-revision-extens.patch create mode 100644 0015-xshared-Fix-response-to-unprivileged-users.patch create mode 100644 0016-Improve-error-messages-for-unsupported-extensions.patch diff --git a/.gitignore b/.gitignore index d5a31d9..550d2b4 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,8 @@ +/.build-*.log +/iptables-*.src.rpm +/noarch/ +/x86_64/ +/iptables.spec.orig /iptables-1.6.2.tar.bz2 /iptables-1.8.0.tar.bz2 /iptables-1.8.2.tar.bz2 diff --git a/0014-libxtables-Register-only-the-highest-revision-extens.patch b/0014-libxtables-Register-only-the-highest-revision-extens.patch new file mode 100644 index 0000000..a27d151 --- /dev/null +++ b/0014-libxtables-Register-only-the-highest-revision-extens.patch @@ -0,0 +1,64 @@ +From 3ffbffeb5193bf7259b04fcd2297a0d3e218b7a2 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Fri, 11 Feb 2022 17:39:24 +0100 +Subject: [PATCH] libxtables: Register only the highest revision extension + +When fully registering extensions, ignore all consecutive ones with same +name and family value. Since commit b3ac87038f4e4 ("libxtables: Make +sure extensions register in revision order"), one may safely assume the +list of pending extensions has highest revision numbers first. Since +iptables is only interested in the highest revision the kernel supports, +registration and compatibility checks may be skipped once the first +matching extension in pending list has validated. + +Signed-off-by: Phil Sutter +(cherry picked from commit 2dbb49d15fb44ddd521a734eca3be3f940b7c1ba) +--- + libxtables/xtables.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/libxtables/xtables.c b/libxtables/xtables.c +index 6947441fec659..b0c969676bc85 100644 +--- a/libxtables/xtables.c ++++ b/libxtables/xtables.c +@@ -668,6 +668,7 @@ xtables_find_match(const char *name, enum xtables_tryload tryload, + struct xtables_match **dptr; + struct xtables_match *ptr; + const char *icmp6 = "icmp6"; ++ bool found = false; + + if (strlen(name) >= XT_EXTENSION_MAXNAMELEN) + xtables_error(PARAMETER_PROBLEM, +@@ -686,7 +687,9 @@ xtables_find_match(const char *name, enum xtables_tryload tryload, + if (extension_cmp(name, (*dptr)->name, (*dptr)->family)) { + ptr = *dptr; + *dptr = (*dptr)->next; +- if (xtables_fully_register_pending_match(ptr, prev)) { ++ if (!found && ++ xtables_fully_register_pending_match(ptr, prev)) { ++ found = true; + prev = ptr; + continue; + } else if (prev) { +@@ -788,6 +791,7 @@ xtables_find_target(const char *name, enum xtables_tryload tryload) + struct xtables_target *prev = NULL; + struct xtables_target **dptr; + struct xtables_target *ptr; ++ bool found = false; + + /* Standard target? */ + if (strcmp(name, "") == 0 +@@ -802,7 +806,9 @@ xtables_find_target(const char *name, enum xtables_tryload tryload) + if (extension_cmp(name, (*dptr)->name, (*dptr)->family)) { + ptr = *dptr; + *dptr = (*dptr)->next; +- if (xtables_fully_register_pending_target(ptr, prev)) { ++ if (!found && ++ xtables_fully_register_pending_target(ptr, prev)) { ++ found = true; + prev = ptr; + continue; + } else if (prev) { +-- +2.34.1 + diff --git a/0015-xshared-Fix-response-to-unprivileged-users.patch b/0015-xshared-Fix-response-to-unprivileged-users.patch new file mode 100644 index 0000000..fa28fde --- /dev/null +++ b/0015-xshared-Fix-response-to-unprivileged-users.patch @@ -0,0 +1,134 @@ +From eec7fd187a9eeda1250c1a35b32c92eff074dff6 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 +-> Apply direct exit from do_parse() to xtables.c, upstream merged + do_parse() functions. +--- + iptables/nft.c | 5 ++ + .../testcases/iptables/0008-unprivileged_0 | 60 +++++++++++++++++++ + iptables/xtables.c | 2 +- + 3 files changed, 66 insertions(+), 1 deletion(-) + create mode 100755 iptables/tests/shell/testcases/iptables/0008-unprivileged_0 + +diff --git a/iptables/nft.c b/iptables/nft.c +index 795dff8605404..5aa14aebeb31e 100644 +--- a/iptables/nft.c ++++ b/iptables/nft.c +@@ -3256,6 +3256,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 9779bd83d53b3..a16bba74dc578 100644 +--- a/iptables/xtables.c ++++ b/iptables/xtables.c +@@ -645,7 +645,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[], + + printhelp(cs->matches); + p->command = CMD_NONE; +- return; ++ exit(0); + + /* + * Option selection +-- +2.34.1 + diff --git a/0016-Improve-error-messages-for-unsupported-extensions.patch b/0016-Improve-error-messages-for-unsupported-extensions.patch new file mode 100644 index 0000000..bd8282c --- /dev/null +++ b/0016-Improve-error-messages-for-unsupported-extensions.patch @@ -0,0 +1,84 @@ +From 721af7dfbfd5dc18af86e00d30de108dbf1687fb Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Fri, 11 Feb 2022 17:47:22 +0100 +Subject: [PATCH] Improve error messages for unsupported extensions + +If a given extension was not supported by the kernel, iptables would +print a rather confusing error message if extension parameters were +given: + +| # rm /lib/modules/$(uname -r)/kernel/net/netfilter/xt_LOG.ko +| # iptables -A FORWARD -j LOG --log-prefix foo +| iptables v1.8.7 (legacy): unknown option "--log-prefix" + +Avoid this by pretending extension revision 0 is always supported. It is +the same hack as used to successfully print extension help texts as +unprivileged user, extended to all error codes to serve privileged ones +as well. + +In addition, print a warning if kernel rejected revision 0 and it's not +a permissions problem. This helps users find out which extension in a +rule the kernel didn't like. + +Finally, the above commands result in these messages: + +| Warning: Extension LOG revision 0 not supported, missing kernel module? +| iptables: No chain/target/match by that name. + +Or, for iptables-nft: + +| Warning: Extension LOG revision 0 not supported, missing kernel module? +| iptables v1.8.7 (nf_tables): RULE_APPEND failed (No such file or directory): rule in chain FORWARD + +Signed-off-by: Phil Sutter +(cherry picked from commit 17534cb18ed0a5052dc45c117401251359dba6aa) +--- + iptables/nft.c | 12 +++++++++--- + libxtables/xtables.c | 7 ++++++- + 2 files changed, 15 insertions(+), 4 deletions(-) + +diff --git a/iptables/nft.c b/iptables/nft.c +index 5aa14aebeb31e..0b1759c3e35ea 100644 +--- a/iptables/nft.c ++++ b/iptables/nft.c +@@ -3256,10 +3256,16 @@ 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) ++ /* pretend revision 0 is valid - ++ * this is required for printing extension help texts as user, also ++ * helps error messaging on unavailable kernel extension */ ++ if (ret < 0 && rev == 0) { ++ if (errno != EPERM) ++ fprintf(stderr, ++ "Warning: Extension %s revision 0 not supported, missing kernel module?\n", ++ name); + return 1; ++ } + + return ret < 0 ? 0 : 1; + } +diff --git a/libxtables/xtables.c b/libxtables/xtables.c +index b0c969676bc85..10b985b7c3a79 100644 +--- a/libxtables/xtables.c ++++ b/libxtables/xtables.c +@@ -929,7 +929,12 @@ int xtables_compatible_revision(const char *name, uint8_t revision, int opt) + /* Definitely don't support this? */ + if (errno == ENOENT || errno == EPROTONOSUPPORT) { + close(sockfd); +- return 0; ++ /* Pretend revision 0 support for better error messaging */ ++ if (revision == 0) ++ fprintf(stderr, ++ "Warning: Extension %s revision 0 not supported, missing kernel module?\n", ++ name); ++ return (revision == 0); + } else if (errno == ENOPROTOOPT) { + close(sockfd); + /* Assume only revision 0 support (old kernel) */ +-- +2.34.1 + diff --git a/iptables.spec b/iptables.spec index f5f0e31..d8c8823 100644 --- a/iptables.spec +++ b/iptables.spec @@ -11,7 +11,7 @@ Name: iptables Summary: Tools for managing Linux kernel packet filtering capabilities URL: https://www.netfilter.org/projects/iptables Version: 1.8.7 -Release: 15%{?dist} +Release: 16%{?dist} Source: %{url}/files/%{name}-%{version}.tar.bz2 Source1: iptables.init Source2: iptables-config @@ -33,6 +33,9 @@ Patch10: 0010-nft-cache-Sort-chains-on-demand-only.patch Patch11: 0011-nft-Increase-BATCH_PAGE_SIZE-to-support-huge-ruleset.patch Patch12: 0012-doc-ebtables-nft.8-Adjust-for-missing-atomic-options.patch Patch13: 0013-nft-Fix-for-non-verbose-check-command.patch +Patch14: 0014-libxtables-Register-only-the-highest-revision-extens.patch +Patch15: 0015-xshared-Fix-response-to-unprivileged-users.patch +Patch16: 0016-Improve-error-messages-for-unsupported-extensions.patch # pf.os: ISC license # iptables-apply: Artistic Licence 2.0 @@ -431,6 +434,12 @@ fi %changelog +* Thu Mar 03 2022 Phil Sutter - 1.8.7-16 +- Improve error messages for unsupported extensions +- xshared: Fix response to unprivileged users +- libxtables: Register only the highest revision extension +- Ignore typical 'fedpkg local' results in .gitignore + * Thu Jan 20 2022 Fedora Release Engineering - 1.8.7-15 - Rebuilt for https://fedoraproject.org/wiki/Fedora_36_Mass_Rebuild