Phil Sutter [Tue, 1 Mar 2022 22:05:29 +0000 (23:05 +0100)]
xshared: Prefer xtables_chain_protos lookup over getprotoent
When dumping a large ruleset, common protocol matches such as for TCP
port number significantly slow down rule printing due to repeated calls
for getprotobynumber(). The latter does not involve any caching, so
/etc/protocols is consulted over and over again.
As a simple countermeasure, make functions converting between proto
number and name prefer the built-in list of "well-known" protocols. This
is not a perfect solution, repeated rules for protocol names libxtables
does not cache (e.g. igmp or dccp) will still be slow. Implementing
getprotoent() result caching could solve this.
As a side-effect, explicit check for pseudo-protocol "all" may be
dropped as it is contained in the built-in list and therefore immutable.
Also update xtables_chain_protos entries a bit to align with typical
/etc/protocols contents. The testsuite assumes those names, so the
preferred ones prior to this patch are indeed uncommon nowadays.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 1 Mar 2022 17:59:31 +0000 (18:59 +0100)]
nft: Simplify immediate parsing
Implementations of parse_immediate callback are mostly trivial, the only
relevant part is access to family-specific parts of struct
iptables_command_state when setting goto flag for iptables and
ip6tables. Refactor them into simple set_goto_flag callbacks.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
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
Phil Sutter [Fri, 11 Feb 2022 16:39:24 +0000 (17:39 +0100)]
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.
Jethro Beekman [Mon, 14 Feb 2022 09:35:56 +0000 (10:35 +0100)]
xshared: Implement xtables lock timeout using signals
Previously, if a lock timeout is specified using `-wN `, flock() is
called using LOCK_NB in a loop with a sleep. This results in two issues.
The first issue is that the process may wait longer than necessary when
the lock becomes available. For this the `-W` option was added, but this
requires fine-tuning.
The second issue is that if lock contention is high, invocations using
`-w` (without a timeout) will always win lock acquisition from
invocations that use `-w N`. This is because invocations using `-w` are
actively waiting on the lock whereas those using `-w N` only check from
time to time whether the lock is free, which will never be the case.
This patch removes the sleep loop and deprecates the `-W` option (making
it non-functional). Instead, flock() is always called in a blocking
fashion, but the alarm() function is used with a non-SA_RESTART signal
handler to cancel the system call.
Jeremy Sowden [Sat, 12 Feb 2022 16:58:32 +0000 (16:58 +0000)]
tests: NFLOG: enable `--nflog-range` tests
iptables-legacy and iptable-nft have different results for these tests.
Now that it is possible to specify the expected results correctly, we
can enable the tests.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Sat, 12 Feb 2022 16:58:30 +0000 (16:58 +0000)]
tests: add `NOMATCH` test result
Currently, there are two supported test results: `OK` and `FAIL`. It is
expected that either the iptables command fails, or it succeeds and
dumping the rule has the correct output. However, it is possible that
the command may succeed but the output may not be correct. Add a
`NOMATCH` result to cover this outcome.
Make a few white-space improvements at the same time.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 4 Feb 2022 17:45:22 +0000 (18:45 +0100)]
tests: iptables-test: Support variant deviation
Some test results are not consistent between variants:
* CLUSTERIP is not supported with nft_compat, so all related tests fail
with iptables-nft.
* iptables-legacy mandates TCPMSS be combined with SYN flag match,
iptables-nft does not care. (Or precisely, xt_TCPMSS.ko can't validate
match presence.)
Introduce an optional fourth test spec field to specify the variant it
applies to. Consequently, the opposite result is expected with the other
variant.
Phil Sutter [Fri, 4 Feb 2022 13:44:26 +0000 (14:44 +0100)]
nft: cache: Dump rules if debugging
If verbose flag was given twice, dump rules while populating the cache.
This not only applies to list commands, but all requiring a rule cache -
e.g. insert with position.
Phil Sutter [Fri, 4 Feb 2022 12:27:51 +0000 (13:27 +0100)]
ebtables: Support verbose mode
Accept '-v' flag in both ebtables-nft and ebtables-nft-restore. Mostly
interesting because it allows for netlink debug output when specified
multiple times.
Phil Sutter [Fri, 28 Jan 2022 14:32:10 +0000 (15:32 +0100)]
iptables-restore: Support for extra debug output
Treat --verbose just like iptables itself, increasing debug level with
number of invocations.
To propagate the level into do_command() callback, insert virtual '-v'
flags into rule lines.
The only downside of this is that simple verbose output is changed and
now also prints the rules as they are added - which would be useful if
the lines contained the chain they apply to.
Phil Sutter [Fri, 28 Jan 2022 18:35:54 +0000 (19:35 +0100)]
nft: Use verbose flag to toggle debug output
Copy legacy iptables' behaviour, printing debug output if verbose flag
is given more than once.
Since nft debug output applies to netlink messages which are not created
until nft_action() phase, carrying verbose value is non-trivial -
introduce a field in struct nft_handle for that.
Florian Westphal [Tue, 25 Jan 2022 16:52:59 +0000 (17:52 +0100)]
nft: prefer native expressions instead of tcp match
Instead of using nft_compat+xtables tcp match, prefer to
emit payload+cmp or payload+range expression.
Unlike udp, tcp has flag bits that can be matched too but
we have to fall back to the xt expression for now.
We also don't support tcp option match, but thats a rarely
used feature anyway.
Delinearization support for ports was added in previous patches.
Phil Sutter [Tue, 18 Jan 2022 21:39:08 +0000 (22:39 +0100)]
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 <phil@nwl.cc> Reviewed-by: Florian Westphal <fw@strlen.de>
nft has no equivalent to `--nflog-range`, so we cannot emulate it and
the Python unit-tests for it fail. However, since `--nflog-range` is
broken and doesn't do anything, the tests are not testing anything
useful.
Signed-off-by: Kyle Bowman <kbowman@cloudflare.com> Signed-off-by: Alex Forster <aforster@cloudflare.com> Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Kyle Bowman [Fri, 1 Oct 2021 17:41:37 +0000 (18:41 +0100)]
extensions: libxt_NFLOG: don't truncate log prefix on print/save
When parsing the rule, use a struct with a layout compatible to that of
struct xt_nflog_info, but with a buffer large enough to contain the
whole 128-character nft prefix.
We always send the nflog-group to the kernel since, for nft, log and
nflog targets are handled by the same kernel module, and are
distinguished by whether they define an nflog-group. Therefore, we must
send the group even if it is zero, or the kernel will configure the
target as a log, not an nflog.
Changes to nft_is_expr_compatible were made since only targets which
have an `nflog-group` are compatible. Since nflog targets are
distinguished by having an nflog-group, we ignore targets without one.
We also set the copy-len flag if the snap-len is set since without this,
iptables will mistake `nflog-size` for `nflog-range`.
Signed-off-by: Kyle Bowman <kbowman@cloudflare.com> Signed-off-by: Alex Forster <aforster@cloudflare.com> Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Kyle Bowman [Fri, 1 Oct 2021 17:41:36 +0000 (18:41 +0100)]
extensions: libxt_NFLOG: use nft built-in logging instead of xt_NFLOG
Replaces the use of xt_NFLOG with the nft built-in log statement.
This additionally adds support for using longer log prefixes of 128
characters in size. Until now NFLOG has truncated the log-prefix to the
64-character limit supported by iptables-legacy. We now use the struct
xtables_target's udata member to store the longer 128-character prefix
supported by iptables-nft.
Signed-off-by: Kyle Bowman <kbowman@cloudflare.com> Signed-off-by: Alex Forster <aforster@cloudflare.com> Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
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.
projects / thirdparty / iptables.git / log
