From 66ed4161feee196cffd87f969dc13a97218999e8 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 12 Feb 2020 19:21:32 +0100 Subject: [PATCH] iptables-1.8.4-7 - Move nft-specific extensions into iptables-nft package - Move remaining extensions into iptables-libs package - Make iptables-nft depend on iptables-libs instead of iptables - Add upstream-suggested fixes --- ...apply-Use-mktemp-instead-of-tempfile.patch | 4 +- 0002-Fix-DEBUG-build.patch | 33 ++++++++ ...ore-Fix-parser-feed-from-line-buffer.patch | 59 ++++++++++++++ ...e-Avoid-access-of-uninitialized-data.patch | 31 ++++++++ ...xtensions-time-Avoid-undefined-shift.patch | 31 ++++++++ ...nsions-cluster-Avoid-undefined-shift.patch | 31 ++++++++ ...te-Guard-strcpy-call-in-xlate_ifname.patch | 40 ++++++++++ ...extensions-among-Check-call-to-fstat.patch | 41 ++++++++++ ...tore-fix-for-noflush-and-empty-lines.patch | 78 +++++++++++++++++++ iptables.spec | 32 ++++++-- 10 files changed, 370 insertions(+), 10 deletions(-) create mode 100644 0002-Fix-DEBUG-build.patch create mode 100644 0003-xtables-restore-Fix-parser-feed-from-line-buffer.patch create mode 100644 0004-xtables-restore-Avoid-access-of-uninitialized-data.patch create mode 100644 0005-extensions-time-Avoid-undefined-shift.patch create mode 100644 0006-extensions-cluster-Avoid-undefined-shift.patch create mode 100644 0007-xtables-translate-Guard-strcpy-call-in-xlate_ifname.patch create mode 100644 0008-extensions-among-Check-call-to-fstat.patch create mode 100644 0009-xtables-restore-fix-for-noflush-and-empty-lines.patch diff --git a/0001-iptables-apply-Use-mktemp-instead-of-tempfile.patch b/0001-iptables-apply-Use-mktemp-instead-of-tempfile.patch index b6e1712..970370d 100644 --- a/0001-iptables-apply-Use-mktemp-instead-of-tempfile.patch +++ b/0001-iptables-apply-Use-mktemp-instead-of-tempfile.patch @@ -1,4 +1,4 @@ -From 1dba0d0a2c9c269dc5ed9e7d841b8ecb9dc060af Mon Sep 17 00:00:00 2001 +From 6455a8201fab45194413b326aecc1d764033db0b Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 12 Apr 2019 18:02:19 +0200 Subject: [PATCH] iptables-apply: Use mktemp instead of tempfile @@ -31,5 +31,5 @@ index 819ca4a459c42..a685b6bbcd7dc 100755 FPE USR1 SEGV USR2 PIPE ALRM TERM -- -2.21.0 +2.24.1 diff --git a/0002-Fix-DEBUG-build.patch b/0002-Fix-DEBUG-build.patch new file mode 100644 index 0000000..f95306b --- /dev/null +++ b/0002-Fix-DEBUG-build.patch @@ -0,0 +1,33 @@ +From a7eb134ce97d873c0fe5d30ac1ddce447aba576c Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Wed, 4 Dec 2019 09:36:59 +0100 +Subject: [PATCH] Fix DEBUG build + +Fixed commit missed to update this conditional call to +nft_rule_print_save(). + +Fixes: 1e8ef6a584754 ("nft: family_ops: Pass nft_handle to 'rule_to_cs' callback") +Signed-off-by: Phil Sutter +Acked-by: Pablo Neira Ayuso +(cherry picked from commit 066a19596ae3d69b49a70405e2daf75c929dcd4d) +Signed-off-by: Phil Sutter +--- + iptables/nft-shared.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c +index 78e422781723f..426765641cff6 100644 +--- a/iptables/nft-shared.c ++++ b/iptables/nft-shared.c +@@ -998,7 +998,7 @@ bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r, void *data) + + DEBUGP("comparing with... "); + #ifdef DEBUG_DEL +- nft_rule_print_save(r, NFT_RULE_APPEND, 0); ++ nft_rule_print_save(h, r, NFT_RULE_APPEND, 0); + #endif + if (!h->ops->is_same(cs, &this)) + goto out; +-- +2.24.1 + diff --git a/0003-xtables-restore-Fix-parser-feed-from-line-buffer.patch b/0003-xtables-restore-Fix-parser-feed-from-line-buffer.patch new file mode 100644 index 0000000..85a760b --- /dev/null +++ b/0003-xtables-restore-Fix-parser-feed-from-line-buffer.patch @@ -0,0 +1,59 @@ +From f587011318fd47b18d0f0174b6594485a546ca8f Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Wed, 4 Dec 2019 09:56:06 +0100 +Subject: [PATCH] xtables-restore: Fix parser feed from line buffer + +When called with --noflush, xtables-restore would trip over chain lines: +Parser uses strtok() to separate chain name, policy and counters which +inserts nul-chars into the source string. Therefore strlen() can't be +used anymore to find end of line. Fix this by caching line length before +calling xtables_restore_parse_line(). + +Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") +Signed-off-by: Phil Sutter +Acked-by: Pablo Neira Ayuso +(cherry picked from commit a103fbfadf4c17b8b12caa57eef72deaaa71a18c) +Signed-off-by: Phil Sutter +--- + .../testcases/ipt-restore/0010-noflush-new-chain_0 | 10 ++++++++++ + iptables/xtables-restore.c | 4 +++- + 2 files changed, 13 insertions(+), 1 deletion(-) + create mode 100755 iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0 + +diff --git a/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0 b/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0 +new file mode 100755 +index 0000000000000..739e684a21183 +--- /dev/null ++++ b/iptables/tests/shell/testcases/ipt-restore/0010-noflush-new-chain_0 +@@ -0,0 +1,10 @@ ++#!/bin/sh -e ++ ++# assert input feed from buffer doesn't trip over ++# added nul-chars from parsing chain line. ++ ++$XT_MULTI iptables-restore --noflush <error.lineno = ++line; + DEBUGP("%s: buffered line %d: '%s'\n", __func__, line, ptr); + xtables_restore_parse_line(h, p, &state, ptr); +- ptr += strlen(ptr) + 1; ++ ptr += len + 1; + } + if (*buffer) { + h->error.lineno = ++line; +-- +2.24.1 + diff --git a/0004-xtables-restore-Avoid-access-of-uninitialized-data.patch b/0004-xtables-restore-Avoid-access-of-uninitialized-data.patch new file mode 100644 index 0000000..d640ab6 --- /dev/null +++ b/0004-xtables-restore-Avoid-access-of-uninitialized-data.patch @@ -0,0 +1,31 @@ +From 79aea9da9108323922dce0820d362c23619371f4 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Thu, 5 Dec 2019 11:40:26 +0100 +Subject: [PATCH] xtables-restore: Avoid access of uninitialized data + +When flushing, 'buffer' is not written to prior to checking its first +byte's value. Therefore it needs to be initialized upon declaration. + +Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") +(cherry picked from commit 48be21bf39f9af35d53af0e211cbd50dcfd12d08) +Signed-off-by: Phil Sutter +--- + iptables/xtables-restore.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c +index dd907e0b8ddd5..63cc15cee9621 100644 +--- a/iptables/xtables-restore.c ++++ b/iptables/xtables-restore.c +@@ -281,7 +281,7 @@ void xtables_restore_parse(struct nft_handle *h, + const struct nft_xt_restore_parse *p) + { + struct nft_xt_restore_state state = {}; +- char preload_buffer[PREBUFSIZ] = {}, buffer[10240], *ptr; ++ char preload_buffer[PREBUFSIZ] = {}, buffer[10240] = {}, *ptr; + + if (!h->noflush) { + nft_fake_cache(h); +-- +2.24.1 + diff --git a/0005-extensions-time-Avoid-undefined-shift.patch b/0005-extensions-time-Avoid-undefined-shift.patch new file mode 100644 index 0000000..8af492d --- /dev/null +++ b/0005-extensions-time-Avoid-undefined-shift.patch @@ -0,0 +1,31 @@ +From cf6d2d3892f62d60fa029a94867a99e87e2ab175 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Thu, 5 Dec 2019 13:15:01 +0100 +Subject: [PATCH] extensions: time: Avoid undefined shift + +Value 1 is signed by default and left-shifting by 31 is undefined for +those. Fix this by marking the value as unsigned. + +Fixes: ad326ef9f734a ("Add the libxt_time iptables match") +(cherry picked from commit 98b221002960040bf3505811c06025b6b9b6984b) +Signed-off-by: Phil Sutter +--- + extensions/libxt_time.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/extensions/libxt_time.c b/extensions/libxt_time.c +index 5a8cc5de13031..d001f5b7f448f 100644 +--- a/extensions/libxt_time.c ++++ b/extensions/libxt_time.c +@@ -330,7 +330,7 @@ static void time_print_monthdays(uint32_t mask, bool human_readable) + + printf(" "); + for (i = 1; i <= 31; ++i) +- if (mask & (1 << i)) { ++ if (mask & (1u << i)) { + if (nbdays++ > 0) + printf(","); + printf("%u", i); +-- +2.24.1 + diff --git a/0006-extensions-cluster-Avoid-undefined-shift.patch b/0006-extensions-cluster-Avoid-undefined-shift.patch new file mode 100644 index 0000000..f9c429f --- /dev/null +++ b/0006-extensions-cluster-Avoid-undefined-shift.patch @@ -0,0 +1,31 @@ +From adfa4a0a95d26b7aaae0c1754f77a863bcd05120 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Thu, 5 Dec 2019 13:36:31 +0100 +Subject: [PATCH] extensions: cluster: Avoid undefined shift + +Value 1 is signed by default and left-shifting by 31 is undefined for +those. Fix this by marking the value as unsigned. + +Fixes: 64a0e09894e52 ("extensions: libxt_cluster: Add translation to nft") +(cherry picked from commit 28c16371cdad16707674450b59919e3d97185694) +Signed-off-by: Phil Sutter +--- + extensions/libxt_cluster.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/extensions/libxt_cluster.c b/extensions/libxt_cluster.c +index c9c35ee22e3df..d164bf6960166 100644 +--- a/extensions/libxt_cluster.c ++++ b/extensions/libxt_cluster.c +@@ -156,7 +156,7 @@ static int cluster_xlate(struct xt_xlate *xl, + xt_xlate_add(xl, "%s %u seed 0x%08x ", jhash_st, + info->total_nodes, info->hash_seed); + for (node = 0; node < 32; node++) { +- if (info->node_mask & (1 << node)) { ++ if (info->node_mask & (1u << node)) { + if (needs_set == 0) { + xt_xlate_add(xl, "{ "); + needs_set = 1; +-- +2.24.1 + diff --git a/0007-xtables-translate-Guard-strcpy-call-in-xlate_ifname.patch b/0007-xtables-translate-Guard-strcpy-call-in-xlate_ifname.patch new file mode 100644 index 0000000..cfc0235 --- /dev/null +++ b/0007-xtables-translate-Guard-strcpy-call-in-xlate_ifname.patch @@ -0,0 +1,40 @@ +From 24e3defb866ecd391ee92417129df96402e1867c Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Thu, 5 Dec 2019 16:01:29 +0100 +Subject: [PATCH] xtables-translate: Guard strcpy() call in xlate_ifname() + +The function potentially fed overlong strings to strcpy(). Given that +everything needed to avoid this is there, reorder code a bit to prevent +those inputs, too. + +Fixes: 0ddd663e9c167 ("iptables-translate: add in/out ifname wildcard match translation to nft") +(cherry picked from commit 2861bdbbf062071487a49103513d129ce40e2652) +Signed-off-by: Phil Sutter +--- + iptables/xtables-translate.c | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c +index a42c60a3b64c6..77a186b905d73 100644 +--- a/iptables/xtables-translate.c ++++ b/iptables/xtables-translate.c +@@ -32,14 +32,13 @@ + void xlate_ifname(struct xt_xlate *xl, const char *nftmeta, const char *ifname, + bool invert) + { ++ int ifaclen = strlen(ifname); + char iface[IFNAMSIZ]; +- int ifaclen; + +- if (ifname[0] == '\0') ++ if (ifaclen < 1 || ifaclen >= IFNAMSIZ) + return; + + strcpy(iface, ifname); +- ifaclen = strlen(iface); + if (iface[ifaclen - 1] == '+') + iface[ifaclen - 1] = '*'; + +-- +2.24.1 + diff --git a/0008-extensions-among-Check-call-to-fstat.patch b/0008-extensions-among-Check-call-to-fstat.patch new file mode 100644 index 0000000..ca84456 --- /dev/null +++ b/0008-extensions-among-Check-call-to-fstat.patch @@ -0,0 +1,41 @@ +From 10b51ba86b63a4d7afa208ea206c7c9872bc6e0a Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Thu, 5 Dec 2019 16:35:51 +0100 +Subject: [PATCH] extensions: among: Check call to fstat() + +If this fails, a bogus length value may be passed to mmap(). + +Fixes: 26753888720d8 ("nft: bridge: Rudimental among extension support") +(cherry picked from commit 25b38bcbf2fdc019f438805c7d1ecd877af9c968) +Signed-off-by: Phil Sutter +--- + extensions/libebt_among.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/extensions/libebt_among.c b/extensions/libebt_among.c +index 2e87db3bc06fa..715d559f432c2 100644 +--- a/extensions/libebt_among.c ++++ b/extensions/libebt_among.c +@@ -6,6 +6,7 @@ + * August, 2003 + */ + ++#include + #include + #include + #include +@@ -137,7 +138,10 @@ static int bramong_parse(int c, char **argv, int invert, + if ((fd = open(optarg, O_RDONLY)) == -1) + xtables_error(PARAMETER_PROBLEM, + "Couldn't open file '%s'", optarg); +- fstat(fd, &stats); ++ if (fstat(fd, &stats) < 0) ++ xtables_error(PARAMETER_PROBLEM, ++ "fstat(%s) failed: '%s'", ++ optarg, strerror(errno)); + flen = stats.st_size; + /* use mmap because the file will probably be big */ + optarg = mmap(0, flen, PROT_READ | PROT_WRITE, +-- +2.24.1 + diff --git a/0009-xtables-restore-fix-for-noflush-and-empty-lines.patch b/0009-xtables-restore-fix-for-noflush-and-empty-lines.patch new file mode 100644 index 0000000..aca04fa --- /dev/null +++ b/0009-xtables-restore-fix-for-noflush-and-empty-lines.patch @@ -0,0 +1,78 @@ +From 17b62e149147f05d419103dbbde9dca361c2bd5d Mon Sep 17 00:00:00 2001 +From: Phil Sutter +Date: Tue, 11 Feb 2020 16:52:59 +0100 +Subject: [PATCH] xtables-restore: fix for --noflush and empty lines + +Lookahead buffer used for cache requirements estimate in restore +--noflush separates individual lines with nul-chars. Two consecutive +nul-chars are interpreted as end of buffer and remaining buffer content +is skipped. + +Sadly, reading an empty line (i.e., one containing a newline character +only) caused double nul-chars to appear in buffer as well, leading to +premature stop when reading cached lines from buffer. + +To fix that, make use of xtables_restore_parse_line() skipping empty +lines without calling strtok() and just leave the newline character in +place. A more intuitive approach, namely skipping empty lines while +buffering, is deliberately not chosen as that would cause wrong values +in 'line' variable. + +Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1400 +Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") +Signed-off-by: Phil Sutter +Acked-by: Arturo Borrero Gonzalez +(cherry picked from commit 8e76391096f12212985c401ee83a67990aa27a29) +Signed-off-by: Phil Sutter +--- + .../ipt-restore/0011-noflush-empty-line_0 | 16 ++++++++++++++++ + iptables/xtables-restore.c | 8 +++++--- + 2 files changed, 21 insertions(+), 3 deletions(-) + create mode 100755 iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 + +diff --git a/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 +new file mode 100755 +index 0000000000000..bea1a690bb624 +--- /dev/null ++++ b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 +@@ -0,0 +1,16 @@ ++#!/bin/bash -e ++ ++# make sure empty lines won't break --noflush ++ ++cat <in)) { + size_t blen = strlen(buffer); + +- /* drop trailing newline; xtables_restore_parse_line() ++ /* Drop trailing newline; xtables_restore_parse_line() + * uses strtok() which replaces them by nul-characters, + * causing unpredictable string delimiting in +- * preload_buffer */ +- if (buffer[blen - 1] == '\n') ++ * preload_buffer. ++ * Unless this is an empty line which would fold into a ++ * spurious EoB indicator (double nul-char). */ ++ if (buffer[blen - 1] == '\n' && blen > 1) + buffer[blen - 1] = '\0'; + else + blen++; +-- +2.24.1 + diff --git a/iptables.spec b/iptables.spec index caa83bc..b6c3780 100644 --- a/iptables.spec +++ b/iptables.spec @@ -19,7 +19,7 @@ Name: iptables Summary: Tools for managing Linux kernel packet filtering capabilities URL: http://www.netfilter.org/projects/iptables Version: 1.8.4 -Release: 6%{?dist} +Release: 7%{?dist} Source: %{url}/files/%{name}-%{version}.tar.bz2 Source1: iptables.init Source2: iptables-config @@ -33,6 +33,15 @@ Source8: 0002-extensions-format-security-fixes-in-libip-6-t_icmp.patch %endif Patch1: 0001-iptables-apply-Use-mktemp-instead-of-tempfile.patch +Patch2: 0002-Fix-DEBUG-build.patch +Patch3: 0003-xtables-restore-Fix-parser-feed-from-line-buffer.patch +Patch4: 0004-xtables-restore-Avoid-access-of-uninitialized-data.patch +Patch5: 0005-extensions-time-Avoid-undefined-shift.patch +Patch6: 0006-extensions-cluster-Avoid-undefined-shift.patch +Patch7: 0007-xtables-translate-Guard-strcpy-call-in-xlate_ifname.patch +Patch8: 0008-extensions-among-Check-call-to-fstat.patch +Patch9: 0009-xtables-restore-fix-for-noflush-and-empty-lines.patch + # pf.os: ISC license # iptables-apply: Artistic Licence 2.0 License: GPLv2 and Artistic Licence 2.0 and ISC @@ -119,7 +128,7 @@ a bytecode generator for use with xt_bpf. %package nft Summary: nftables compatibility for iptables, arptables and ebtables -Requires: %{name} = %{version}-%{release} +Requires: %{name}-libs%{?_isa} = %{version}-%{release} Obsoletes: iptables-compat < 1.6.2-4 Provides: arptables-helper Provides: iptables @@ -350,12 +359,6 @@ fi %{_mandir}/man8/iptables* %{_mandir}/man8/ip6tables* %{_mandir}/man8/xtables-legacy* -%dir %{_libdir}/xtables -%{_libdir}/xtables/libarpt* -%{_libdir}/xtables/libebt* -%{_libdir}/xtables/libipt* -%{_libdir}/xtables/libip6t* -%{_libdir}/xtables/libxt* %ghost %{_sbindir}/iptables %ghost %{_sbindir}/iptables-restore %ghost %{_sbindir}/iptables-save @@ -370,6 +373,10 @@ fi %{_libdir}/libip{4,6}tc.so.%{ipXtc_so_ver_old}* %endif %{_libdir}/libxtables.so.12* +%dir %{_libdir}/xtables +%{_libdir}/xtables/libipt* +%{_libdir}/xtables/libip6t* +%{_libdir}/xtables/libxt* %files devel %dir %{_includedir}/iptables @@ -422,6 +429,9 @@ fi %{_sbindir}/arptables-nft* %{_sbindir}/xtables-nft-multi %{_sbindir}/xtables-monitor +%dir %{_libdir}/xtables +%{_libdir}/xtables/libarpt* +%{_libdir}/xtables/libebt* %{_libexecdir}/arptables-nft-helper %{_mandir}/man8/xtables-monitor* %{_mandir}/man8/xtables-translate* @@ -446,6 +456,12 @@ fi %changelog +* Wed Feb 12 2020 Phil Sutter - 1.8.4-7 +- Move nft-specific extensions into iptables-nft package +- Move remaining extensions into iptables-libs package +- Make iptables-nft depend on iptables-libs instead of iptables +- Add upstream-suggested fixes + * Wed Jan 29 2020 Fedora Release Engineering - 1.8.4-6 - Rebuilt for https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild