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.
Phil Sutter [Thu, 29 Apr 2021 13:28:59 +0000 (15:28 +0200)]
extensions: SECMARK: Implement revision 1
The changed data structure for communication with kernel allows to
exclude the field 'secid' which is populated on kernel side. Thus
this fixes the formerly always failing extension comparison breaking
rule check and rule delete by content.
Phil Sutter [Tue, 6 Apr 2021 08:51:20 +0000 (10:51 +0200)]
nft: Increase BATCH_PAGE_SIZE to support huge rulesets
In order to support the same ruleset sizes as legacy iptables, the
kernel's limit of 1024 iovecs has to be overcome. Therefore increase
each iovec's size from 128KB to 2MB.
While being at it, add a log message for failing sendmsg() call. This is
not supposed to happen, even if the transaction fails. Yet if it does,
users are left with only a "line XXX failed" message (with line number
being the COMMIT line).
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 25 Mar 2021 15:24:39 +0000 (16:24 +0100)]
nft: cache: Sort chains on demand only
Mandatory sorted insert of chains into cache significantly slows down
restoring of large rulesets. Since the sorted list of user-defined
chains is needed for listing and verbose output only, introduce
nft_cache_sort_chains() and call it where needed.
extensions: libxt_conntrack: use bitops for status negation
At the moment, status_xlate_print function prints statusmask as comma-separated
sequence of enabled statusmask flags. But if we have inverted conntrack ctstatus
condition then we have to use more complex expression (if more than one flag enabled)
because nft not supports syntax like "ct status != expected,assured".
Examples:
! --ctstatus CONFIRMED,ASSURED
should be translated as
ct status & (assured|confirmed) == 0
! --ctstatus CONFIRMED
can be translated as
ct status & confirmed == 0
See also netfilter/xt_conntrack.c (conntrack_mt() function as a reference).
Reproducer:
$ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstatus expected,assured -j DROP
$ nft list ruleset
...
meta l4proto tcp ip daddr 127.0.0.1 ct status != expected,assured counter packets 0 bytes 0 drop
...
it will fail if we try to load this rule:
$ nft -f nft_test
../nft_test:6:97-97: Error: syntax error, unexpected comma, expecting newline or semicolon
extensions: libxt_conntrack: use bitops for state negation
Currently, state_xlate_print function prints statemask as comma-separated sequence of enabled
statemask flags. But if we have inverted conntrack ctstate condition then we have to use more
complex expression because nft not supports syntax like "ct state != related,established".
Reproducer:
$ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstate RELATED,ESTABLISHED -j DROP
$ nft list ruleset
...
meta l4proto tcp ip daddr 127.0.0.1 ct state != related,established counter packets 0 bytes 0 drop
...
it will fail if we try to load this rule:
$ nft -f nft_test
../nft_test:6:97-97: Error: syntax error, unexpected comma, expecting newline or semicolon
Phil Sutter [Tue, 2 Mar 2021 13:50:07 +0000 (14:50 +0100)]
xtables-translate: Fix translation of odd netmasks
Iptables supports netmasks which are not prefixes to match on (or
ignore) arbitrary bits in an address. Yet nftables' prefix notation is
available for real prefixes only, so translation is not as trivial -
print bitmask syntax for those cases.
Phil Sutter [Fri, 19 Feb 2021 15:54:57 +0000 (16:54 +0100)]
nft: Fix bitwise expression avoidance detection
Byte-boundary prefix detection was too sloppy: Any data following the
first zero-byte was ignored. Add a follow-up loop making sure there are
no stray bits in the designated host part.
Fixes: 323259001d617 ("nft: Optimize class-based IP prefix matches") Signed-off-by: Phil Sutter <phil@nwl.cc>
Florian Westphal [Wed, 24 Feb 2021 10:08:02 +0000 (11:08 +0100)]
iptables-nft: fix -Z option
it zeroes the rule counters, so it needs fully populated cache.
Add a test case to cover this.
Fixes: 9d07514ac5c7a ("nft: calculate cache requirements from list of commands") Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Phil Sutter <phil@nwl.cc>
Avoid this by checking table existence just like iptables-nft does upon
parsing '-t' optarg. Since the list of tables is known and fixed,
checking the given name's length is pointless. So just drop that check
in return.
With this patch in place, output looks much better:
| # ebtables-nft -t dummy -P INPUT ACCEPT
| ebtables v1.8.7 (nf_tables): table 'dummy' does not exist
| Perhaps iptables or your kernel needs to be upgraded.
Phil Sutter [Fri, 15 Jan 2021 20:58:48 +0000 (21:58 +0100)]
tests/shell: Fix nft-only/0009-needless-bitwise_0
For whatever reason, stored expected output contains false handles. To
overcome this, filter the rule data lines from both expected and stored
output before comparing.
Fixes: 81a2e12851283 ("tests/shell: Add test for bitwise avoidance fixes") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 10 Jul 2020 16:23:50 +0000 (18:23 +0200)]
nft: Avoid pointless table/chain creation
Accept a chain name in nft_xt_builtin_init() to limit the base chain
creation to that specific chain only.
Introduce nft_xt_builtin_table_init() to create just the table for
situations where no builtin chains are needed but the command may still
succeed in an empty ruleset, particularly when creating a custom chain,
restoring base chains or adding a set for ebtables among match.
Introduce nft_xt_fake_builtin_chains(), a function to call after cache
has been populated to fill empty base chain slots. This keeps ruleset
listing output intact if some base chains do not exist (or even the
whole ruleset is completely empty).
Phil Sutter [Thu, 30 Jul 2020 08:24:10 +0000 (10:24 +0200)]
nft: cache: Sort custom chains by name
With base chains no longer residing in the tables' chain lists, they can
easily be sorted upon insertion. This on one hand aligns custom chain
ordering with legacy iptables and on the other makes it predictable,
which is very helpful when manually comparing ruleset dumps for
instance.
Adjust the one ebtables-nft test case this change breaks (as wrong
ordering is expected in there). The manual output sorting done for tests
which apply to legacy as well as nft is removed in a separate patch.
Phil Sutter [Tue, 7 Jul 2020 09:43:26 +0000 (11:43 +0200)]
nft: Introduce a dedicated base chain array
Preparing for sorted chain output, introduce a per-table array holding
base chains indexed by nf_inet_hooks value. Since the latter is ordered
correctly, iterating over the array will return base chains in expected
order.
Phil Sutter [Wed, 29 Jul 2020 12:33:33 +0000 (14:33 +0200)]
nft: Introduce struct nft_chain
Preparing for ordered output of user-defined chains, introduce a local
datatype wrapping nftnl_chain. In order to maintain the chain name hash
table, introduce nft_chain_list as well and use it instead of
nftnl_chain_list.
Phil Sutter [Wed, 8 Jul 2020 14:09:52 +0000 (16:09 +0200)]
nft: Implement nft_chain_foreach()
This is just a fancy wrapper around nftnl_chain_list_foreach() with the
added benefit of detecting invalid table names or uninitialized chain
lists. This in turn allows to drop the checks in flush_rule_cache() and
ignore the return code of nft_chain_foreach() as it fails only if the
dropped checks had failed, too.
Since this wrapper does the chain list lookup by itself, use of
nft_chain_list_get() shrinks down to a single place, namely inside
nft_chain_find(). Therefore fold it into the latter.
Phil Sutter [Wed, 23 Sep 2020 17:13:45 +0000 (19:13 +0200)]
nft: Fix selective chain compatibility checks
Since commit 80251bc2a56ed ("nft: remove cache build calls"), 'chain'
parameter passed to nft_chain_list_get() is no longer effective.
Before, it was used to fetch only that single chain from kernel when
populating the cache. So the returned list of chains for which
compatibility checks are done would contain only that single chain.
Re-establish the single chain compat checking by introducing a dedicated
code path to nft_is_chain_compatible() doing so.