Phil Sutter [Thu, 23 Dec 2021 18:03:37 +0000 (19:03 +0100)]
xshared: Move do_parse to shared space
Small adjustments were needed:
- Pass line variable via xt_cmd_parse, xshared.c does not have it in
namespace.
- Replace opts, prog_name and prog_vers defines by the respective
xt_params field reference.
Phil Sutter [Thu, 23 Dec 2021 17:55:53 +0000 (18:55 +0100)]
xtables: Do not pass nft_handle to do_parse()
Make it fit for sharing with legacy iptables, drop nft-specific
parameter. This requires to mirror proto_parse and post_parse callbacks
from family_ops somewhere reachable - use xt_cmd_parse, it holds other
"parser setup data" as well.
Phil Sutter [Fri, 26 Nov 2021 17:26:07 +0000 (18:26 +0100)]
xtables: Drop xtables' family on demand feature
This conditional h->family assignment was added by commit 3f7877e6be987
("xtables-restore: add -4 and -6 support") with the intention to support
something like 'xtables-restore -6 <ip6tables.dump', i.e. having
family-agnostic commands which accept flags to set the family. Yet
commit be70918eab26e ("xtables: rename xt-multi binaries to -nft,
-legacy") removed support for such command names back in 2018 and nobody
has complained so far. Therefore drop this leftover as it makes
do_parse() more generic.
Phil Sutter [Fri, 26 Nov 2021 20:45:12 +0000 (21:45 +0100)]
iptables-*-restore: Drop pointless line reference
There's no need to mention the offending line number in error message
when calling xtables_error() with a status of PARAMETER_PROBLEM as that
will cause a call to xtables_exit_tryhelp() which in turn prints "Error
occurred at line: N".
Phil Sutter [Thu, 21 Oct 2021 11:04:45 +0000 (13:04 +0200)]
xshared: Share a common printhelp function
Help texts in legacy and nft variants are supposed to be identical, but
those of iptables and ip6tables largely overlapped already. By referring
to xt_params and afinfo pointers, it is relatively trivial to craft a
suitable help text on demand, so duplicated help texts can be
eliminated.
As a side-effect, this fixes ip6tables-nft help text - it was identical
to that of iptables-nft.
Phil Sutter [Thu, 21 Oct 2021 01:00:57 +0000 (03:00 +0200)]
xshared: Share print_match_save() between legacy ip*tables
The only difference between the former two copies was the type of
ip*_entry parameter. But since it is treated opaque, just hide that
detail by casting to void.
Phil Sutter [Sat, 6 Nov 2021 00:09:37 +0000 (01:09 +0100)]
nft-shared: Drop unused function print_proto()
The last users vanished back in 2013. There is identical code in
save_rule_details(), but with only a single user there's not much point
in keeping the function.
Fixes: cdc78b1d6bd7b ("nft: convert rule into a command state structure") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 17 Nov 2020 01:49:32 +0000 (02:49 +0100)]
xshared: Share print_header() with legacy iptables
Legacy iptables fetches the relevant data via libiptc before calling the
shared routine which merely prints data as requested.
Drop the 'basechain' parameter, instead make sure a policy name is
passed only with base chains. Since the function is not shared with
ebtables (which uses a very rudimental header instead), this is safe.
In order to support legacy iptables' checking of iptc_get_references()
return code (printing an error message instead of the reference count),
make refs parameter signed and print the error message if it's negative.
Phil Sutter [Tue, 22 Oct 2019 18:06:11 +0000 (20:06 +0200)]
xshared: Share print_fragment() with legacy
Also add a fake mode to make it suitable for ip6tables. This is required
because IPT_F_FRAG value clashes with IP6T_F_PROTO, so ip6tables rules
might seem to have IPT_F_FRAG bit set.
While being at it, drop the local variable 'flags' from
print_firewall().
Phil Sutter [Fri, 5 Nov 2021 17:27:53 +0000 (18:27 +0100)]
xshared: Share save_rule_details() with legacy
The function combines printing of input and output interfaces and
protocol parameter, all being IP family independent. Extend the function
to print fragment option ('-f'), too if requested. While being at it,
drop unused iptables_command_state parameter and reorder the remaining
ones a bit.
Phil Sutter [Fri, 5 Nov 2021 17:02:13 +0000 (18:02 +0100)]
xshared: Share print_iface() function
Merge the three identical copies into one and name it 'save_iface' (as
the printed syntax is for "save"-format). Leave arptables alone for now,
its rather complicated whitespace printing doesn't allow for use of the
shared function. Also keep ebtables' custom implementation, it is used
for the --logical-in/--logical-out long-options, too. Apart from that,
ebtables-nft does not use a mask, at all.
Phil Sutter [Mon, 8 Nov 2021 16:03:21 +0000 (17:03 +0100)]
extensions: hashlimit: Fix tests with HZ=1000
In an attempt to fix for failing hashlimit tests with HZ=100, the
expected failures were changed so they are expected to pass and the
parameters changed to seemingly fix them. Yet while the new parameters
worked on HZ=100 systems, with higher tick rates they didn't so the
observed problem moved from the test failing on HZ=100 to failing on
HZ=1000 instead.
Kernel's error message "try lower: 864000000/5" turned out to be a red
herring: The burst value does not act as a dividor but a multiplier
instead, so in order to lower the overflow-checked value, a lower burst
value must be chosen. Inded, using a burst value of 1 makes the kernel
accept the rule in both HZ=100 and HZ=1000 configurations.
Fixes: bef9dc575625a ("extensions: hashlimit: Fix tests with HZ=100") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Sat, 6 Nov 2021 20:38:14 +0000 (21:38 +0100)]
Unbreak xtables-translate
Fixed commit broke xtables-translate which still relied upon do_parse()
to properly initialize the passed iptables_command_state reference. To
allow for callers to preset fields, this doesn't happen anymore so
do_command_xlate() has to initialize itself. Otherwise garbage from
stack is read leading to segfaults and program aborts.
Although init_cs callback is used by arptables only and
arptables-translate has not been implemented, do call it if set just to
avoid future issues.
Fixes: cfdda18044d81 ("nft-shared: Introduce init_cs family ops callback") Signed-off-by: Phil Sutter <phil@nwl.cc> Tested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
nft: Merge xtables-arp-standalone.c into xtables-standalone.c
By declaring the relevant family_ops callbacks for arptables, the code
becomes ready to just use do_commandx() instead of a dedicated parser.
As a side-effect, this enables a bunch of new features in arptables-nft:
* Support '-C' command
* Support '-S' command
* Support rule indexes just like xtables, e.g. in '-I' or '-R' commands
* Reject chain names starting with '!'
* Support '-c N,M' counter syntax
Since arptables still accepts intrapositioned negations, add code to
cover that but print a warning like iptables did 12 years ago prior to
removing the functionality.
Phil Sutter [Sat, 14 Nov 2020 14:22:09 +0000 (15:22 +0100)]
xtables: arptables accepts empty interface names
The empty string passed as interface name is simply ignored by legacy
arptables. Make the new common parser print a warning but accept it.
Calling xtables_parse_interface() with an empty string is safe.
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
xtables: Derive xtables_globals from family
Prepare xtables_main() for use with other families than IPV4 or IPV6
which both use the same xtables_globals object. Therefore introduce a
function to map from family value to xtables_globals object pointer.
In do_parse(), use xt_params pointer as well instead of direct
reference.
While being at it, Declare arptables_globals and ebtables_globals in
xtables_multi.h which seems to be the proper place for that.
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
arptables: Use standard data structures when parsing
Use the compound data structures introduced for dedicated parsing
routines in other families instead of the many local variables. This
allows to standardize code a bit for sharing a common parser later.
With optstring being stored in struct xtables_globals as well, it is a
natural choice to store a pointer to a help printer also which matches
the supported options.
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
xtables-standalone: Drop version number from init errors
Aside from the rather unconventional formatting, if those initialization
functions fail we've either released a completely broken iptables or
the wrong libraries are chosen by the loader. In both cases, the version
number is not really interesting.
While being at it, fix indenting of the first exit() call.
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
nft: Add family ops callbacks wrapping different nft_cmd_* functions
Commands supporting multiple source/destination addresses need to
iterate over them and call the respective nft_cmd_* function multiple
times. These loops are family-specific though as each family uses a
different data structure within struct iptables_command_state to store
the addresses.
Phil Sutter [Mon, 27 Sep 2021 14:59:49 +0000 (16:59 +0200)]
xshared: Store optstring in xtables_globals
Preparing for a common option parser, store the string of options for
each family inside the respective xtables_globals object. The
array of long option definitions sitting in there already indicates it's
the right place.
While being at it, drop '-m' support from arptables-nft.
Phil Sutter [Tue, 14 Sep 2021 10:15:29 +0000 (12:15 +0200)]
nft: Delete builtin chains compatibly
Attempting to delete all chains if --delete-chain is called without
argument has unwanted side-effects especially legacy iptables users are
not aware of and won't expect:
* Non-default policies are ignored, a previously dropping firewall may
start accepting traffic.
* The kernel refuses to remove non-empty chains, causing program abort
even if no user-defined chain exists.
Fix this by requiring a rule cache in that situation and make builtin
chain deletion depend on its policy and number of rules. Since this may
change concurrently, check again when having to refresh the transaction.
Also, hide builtin chains from verbose output - their creation is
implicit, so treat their removal as implicit, too.
When deleting a specific chain, do not allow to skip the job though.
Otherwise deleting a builtin chain which is still in use will succeed
although not executed.
Fixes: 61e85e3192dea ("iptables-nft: allow removal of empty builtin chains") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 21 Sep 2021 14:42:36 +0000 (16:42 +0200)]
nft: Check base-chain compatibility when adding to cache
With introduction of dedicated base-chain slots, a selection process was
established as no longer all base-chains ended in the same chain list
for later searching/checking but only the first one found for each hook
matching criteria is kept and the rest discarded.
A side-effect of the above is that table compatibility checking started
to omit consecutive base-chains, making iptables-nft less restrictive as
long as the expected base-chains were returned first from kernel when
populating the cache.
Make behaviour consistent and warn users about the possibly disturbing
chains found by:
* Run all base-chain checks from nft_is_chain_compatible() before
allowing a base-chain to occupy its slot.
* If an unfit base-chain was found (and discarded), flag the table's
cache as tainted and warn about it if the remaining ruleset is
otherwise compatible.
Since base-chains that remain in cache would pass
nft_is_chain_compatible() checking, remove that and reduce it to rule
inspection.
Phil Sutter [Tue, 21 Sep 2021 09:39:45 +0000 (11:39 +0200)]
nft: cache: Avoid double free of unrecognized base-chains
On error, nft_cache_add_chain() frees the allocated nft_chain object
along with the nftnl_chain it points at. Fix nftnl_chain_list_cb() to
not free the nftnl_chain again in that case.
Fixes: 176c92c26bfc9 ("nft: Introduce a dedicated base chain array") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 15 Sep 2021 15:37:51 +0000 (17:37 +0200)]
ebtables: Avoid dropping policy when flushing
Unlike nftables, ebtables' user-defined chains have policies -
ebtables-nft implements those internally as invisible last rule. In
order to recreate them after a flush command, a rule cache is needed.
Phil Sutter [Mon, 6 Sep 2021 11:07:43 +0000 (13:07 +0200)]
tests: xlate-test: Exit non-zero on error
If a test fails, return a non-zero exit code. To do so, propagate the
pass/fail statistics up to main() for evaluation. While being at it,
move the statistics printing into there as well and get rid of that
redundant assignment to 'test_passed'.
Phil Sutter [Mon, 6 Sep 2021 10:52:22 +0000 (12:52 +0200)]
tests: xlate-test: Don't skip any input after the first empty line
In conditionals, testing the empty string evaluates to false. This is
dumb but seems intentional, as readline() method returns an empty string
at EOF. This is distinct from reading an empty line as the latter
contains the newline character - unless it is stripped in between
readline() and conditional. The fixed commit introduced just that by
accident, effectively reducing any test file to the first contained
test:
Phil Sutter [Thu, 12 Aug 2021 17:11:59 +0000 (19:11 +0200)]
tests: iptables-test: Fix missing chain case
If a chain line was really missing, Python complained about reference
before assignment of 'chain_array' variable. While being at it, reuse
print_error() function for reporting and allow to continue with the next
input file instead of exiting.
Phil Sutter [Tue, 31 Aug 2021 10:29:43 +0000 (12:29 +0200)]
nft: Use xtables_{m,c}alloc() everywhere
Make use of libxtables allocators where sensible to have implicit error
checking. Leave library-internal calls in place to not create unexpected
program exit points for users, apart from xt_xlate_alloc() as that
function called xtables_error() in error case which exits by itself
already.
Phil Sutter [Mon, 9 Aug 2021 16:48:58 +0000 (18:48 +0200)]
extensions: hashlimit: Fix tests with HZ=100
With the kernel ticking at 100Hz, a limit of 1/day with burst 5 does not
overflow in kernel, making the test unstable depending on kernel config.
Change it to not overflow with 1000Hz either by increasing the burst
value by a factor of 100.
Fixes: fcf9f6f25db11 ("extensions: libxt_hashlimit: add unit test") Signed-off-by: Phil Sutter <phil@nwl.cc>
That's because nft list ruleset saves "random-fully" which is wrong
format for nft -f, right should be "fully-random".
We face this problem because we run k8s in Virtuozzo container, and k8s
creates those "random-fully" rules by iptables(nft) and then CRIU can't
restore those rules using nft.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 30 Jul 2021 10:25:10 +0000 (12:25 +0200)]
ebtables: Dump atomic waste
With ebtables-nft.8 now educating people about the missing
functionality, get rid of atomic remains in source code. This eliminates
mostly comments except for --atomic-commit which was treated as alias of
--init-table. People not using the latter are probably trying to
atomic-commit from an atomic-file which in turn is not supported, so no
point keeping it.
Erik Wilson [Tue, 13 Jul 2021 23:48:23 +0000 (16:48 -0700)]
xtables: Call init_extensions6() for static builds
Initialize extensions from libext6 for cases where xtables is built statically.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1550 Signed-off-by: Erik Wilson <Erik.E.Wilson@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de>
This patch adds a translation for connlimit matches which requires
the definition of a set and the family context (either IPv4 or IPv6)
which is required to display the netmask accordingly.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This infrastructure extends the existing xlate infrastructure:
- Extensions can define set dependencies through .xlate. The resulting
set definition can be obtained through xt_xlate_set_get().
- Add xl_xlate_set_family() and xl_xlate_get_family() to store/fetch
the family.
The first client of this new xlate API is the connlimit extension,
which is added in a follow up patch.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 2 Jun 2021 12:04:43 +0000 (14:04 +0200)]
extensions: libebt_ip6: Use xtables_ip6parse_any()
The code was almost identical and suffered from the same problem as
fixed in commit a76a5c997a235 ("libxtables: fix two off-by-one memory
corruption bugs").
The only functional change this involves is ebt_parse_ip6_address() will
now accept hostnames as well.
The call to strncpy() is actually not needed: source buffer is only
IFNAMSIZ bytes large and guaranteed to be null-terminated. Use this to
avoid compiler warnings due to size parameter matching the destination
buffer size by performing the copy using (dumb) memcpy() instead.
Phil Sutter [Tue, 4 May 2021 14:26:42 +0000 (16:26 +0200)]
extensions: sctp: Translate --chunk-types option
The translation is not fully complete as it is not possible to map 'any'
match type into nft syntax with a single rule. Also, 'only' match type
translation is a bit poor as it explicitly lists all chunk types that
are supposed to be missing.
Phil Sutter [Sat, 20 Jun 2020 08:11:52 +0000 (10:11 +0200)]
ebtables-translate: Use shared ebt_get_current_chain() function
Drop the local reimplementation. It was barely different enough to
be buggy:
| % ebtables-nft -A foo -o eth0 -j ACCEPT
| % xtables-nft-multi ebtables-translate -A foo -o eth0 -j ACCEPT
| ebtables-translate v1.8.5 (nf_tables): Use -o only in OUTPUT, FORWARD and POSTROUTING chains
| Try `ebtables-translate -h' or 'ebtables-translate --help' for more information.
Phil Sutter [Wed, 11 Nov 2020 16:16:40 +0000 (17:16 +0100)]
xshared: Eliminate iptables_command_state->invert
This field is not used by routines working with struct
iptables_command_state: It is merely a temporary flag used by parsers to
carry the '!' prefix until invflags have been populated (or error
checking done if unsupported).
Phil Sutter [Mon, 2 Nov 2020 11:05:44 +0000 (12:05 +0100)]
xtables: Make invflags 16bit wide
This is needed to merge with xtables-arp which has more builtin
options and hence needs more bits in invflags.
The only adjustment needed is the set_option() call for option '-j'
which passed a pointer to cs->fw.ip.invflags. That field can't be
changed, it belongs to uAPI. Though using args->invflags instead works
fine, aside from that '-j' doesn't support inverting so this is merely a
sanity check and no real invflag value assignment will happen.