Phil Sutter [Fri, 28 Feb 2020 19:32:13 +0000 (20:32 +0100)]
nft: cache: Fix nft_release_cache() under stress
iptables-nft-restore calls nft_action(h, NFT_COMPAT_COMMIT) for each
COMMIT line in input. When restoring a dump containing multiple large
tables, chances are nft_rebuild_cache() has to run multiple times.
If the above happens, consecutive table contents are added to __cache[1]
which nft_rebuild_cache() then frees, so next commit attempt accesses
invalid memory.
Fix this by making nft_release_cache() (called after each successful
commit) return things into pre-rebuild state again, but keeping the
fresh cache copy.
Fixes: f6ad231d698c7 ("nft: keep original cache in case of ERESTART") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 4 Mar 2020 01:43:27 +0000 (02:43 +0100)]
connlabel: Allow numeric labels even if connlabel.conf exists
Existing code is a bit quirky: If no connlabel.conf was found, the local
function connlabel_value_parse() is called which tries to interpret
given label as a number. If the config exists though,
nfct_labelmap_get_bit() is called instead which doesn't care about
"undefined" connlabel names. So unless installed connlabel.conf contains
entries for all possible numeric labels, rules added by users may stop
working if a connlabel.conf is created.
Related man page snippet states: "Using a number always overrides
connlabel.conf", so try numeric parsing and fall back to nfct only if
that failed.
Fixes: 51340f7b6a110 ("extensions: libxt_connlabel: use libnetfilter_conntrack") Fixes: 3a3bb480a738a ("extensions: connlabel: Fallback on missing connlabel.conf") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 21 Feb 2020 13:55:52 +0000 (14:55 +0100)]
xtables: Review nft_init()
Move common code into nft_init(), such as:
* initial zeroing nft_handle fields
* family ops lookup and assignment to 'ops' field
* setting of 'family' field
This requires minor adjustments in xtables_restore_main() so extra field
initialization doesn't happen before nft_init() call.
As a side-effect, this fixes segfaulting xtables-monitor binary when
printing rules for trace event as in that code-path 'ops' field wasn't
initialized.
Phil Sutter [Fri, 21 Feb 2020 12:18:32 +0000 (13:18 +0100)]
xtables: Align effect of -4/-6 options with legacy
Legacy iptables doesn't accept -4 or -6 if they don't match the
symlink's native family. The only exception to that is iptables-restore
which simply ignores the lines introduced by non-matching options, which
is useful to create combined dump files for feeding into both
iptables-restore and ip6tables-restore.
Phil Sutter [Tue, 18 Feb 2020 15:43:16 +0000 (16:43 +0100)]
iptables-test.py: Fix --host mode
In some cases, the script still called repo binaries. Avoid this when in
--host mode to allow testing without the need to compile sources in
beforehand.
Fixes: 1b5d762c1865e ("iptables-test: Support testing host binaries") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 13 Feb 2020 16:49:53 +0000 (17:49 +0100)]
ebtables: among: Support mixed MAC and MAC/IP entries
Powered by Stefano's support for concatenated ranges, a full among match
replacement can be implemented. The trick is to add MAC-only elements as
a concatenation of MAC and zero-length prefix, i.e. a range from
0.0.0.0 till 255.255.255.255.
Although not quite needed, detection of pure MAC-only matches is left in
place. For those, no implicit 'meta protocol' match is added (which is
required otherwise at least to keep nft output correct) and no concat
type is used for the set.
Phil Sutter [Thu, 13 Feb 2020 13:01:50 +0000 (14:01 +0100)]
xtables-translate: Fix for iface++
In legacy iptables, only the last plus sign remains special, any
previous ones are taken literally. Therefore xtables-translate must not
replace all of them with asterisk but just the last one.
Fixes: e179e87a1179e ("xtables-translate: Fix for interface name corner-cases") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 12 Feb 2020 20:26:06 +0000 (21:26 +0100)]
tests: shell: Fix skip checks with --host mode
When testing host binaries, XT_MULTI variable contains just the program
name without path component which most skip checks didn't expect. Fix
them, and while being at it also reduce indenting level in two scripts
by moving the skip check up front with an early exit call.
Fixes: 416898e335322 ("tests/shell: Support testing host binaries") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 11 Feb 2020 15:52:59 +0000 (16:52 +0100)]
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 <phil@nwl.cc> Acked-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
Phil Sutter [Thu, 6 Feb 2020 14:08:41 +0000 (15:08 +0100)]
xtables-translate: Fix for interface name corner-cases
There are two special situations xlate_ifname() didn't cover for:
* Interface name containing '*': This went unchanged, creating a command
nft wouldn't accept. Instead translate into '\*' which doesn't change
semantics.
* Interface name being '+': Can't translate into nft wildcard character
as nft doesn't accept asterisk-only interface names. Instead decide
what to do based on 'invert' value: Skip match creation if false,
match against an invalid interface name if true.
Also add a test to make sure future changes to this behaviour are
noticed.
Jeremy Sowden [Fri, 20 Dec 2019 19:54:50 +0000 (19:54 +0000)]
extensions: AUDIT: fix man-page typo.
A recent commit fixed uses of "allows to" in man-pages. There was one
instance where the "to" was removed but the "allows" was left behind.
Remove that as well.
Fixes: 3b9b515618c6 ("iptables: cleanup "allows to" usage") Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 5 Dec 2019 15:01:29 +0000 (16:01 +0100)]
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")
This is GW's update to iptables-apply. It does a code cleanup and adds two
options: one runs a command and the other writes the sucessful rules file.
I modified the script to use mktemp instead of tempfile. I also fixed a couple
of hyphens in the man page addition.
Arturo says:
I'm not a strong supporter of this script, but there are many users of it, so
better do things right and add this patch that should produce no harm anyway.
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: GW <gw.2010@tnode.com> Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Arturo says:
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Add iptables-apply(8) to the SEE ALSO section of *-save(8) and *-restore(8).
Arturo says:
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Arturo says:
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Cleanup, scape hyphens so they are not interpreted by the manpage generator.
Arturo says:
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Arturo says:
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
iptables: install iptables-apply script and manpage
We have the iptables-apply script in the tree (and in the release tarball), but
is not being installed anywhere. Same for the manpage.
Arturo says:
I'm not a strong supporter of this script, but there are many users of it, so
better do things right and do a proper installation.
This patch is forwarded from the iptables Debian package, where it has been
around for many years now.
Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 4 Dec 2019 08:56:06 +0000 (09:56 +0100)]
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 <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 22:42:13 +0000 (00:42 +0200)]
nft: bridge: Rudimental among extension support
Support among match as far as possible given the limitations of nftables
sets, namely limited to homogeneous MAC address only or MAC and IP
address only matches.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 20:09:04 +0000 (22:09 +0200)]
nft: Bore up nft_parse_payload()
Allow for closer inspection by storing payload expression's base and
length values. Also facilitate for two consecutive payload expressions
as LHS of a (cmp/lookup) statement as used with concatenations.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 09:21:42 +0000 (11:21 +0200)]
nft: Introduce NFT_CL_SETS cache level
In order to support anonymous sets, introduce an intermediate cache
level between NFT_CL_CHAINS and NFT_CL_RULES. Actually chains are not
needed to fetch sets, but given that sets are only needed for rules, put
it late to not slow down fetching chains.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 21 Aug 2019 08:42:19 +0000 (10:42 +0200)]
nft: Eliminate pointless calls to nft_family_ops_lookup()
If nft_handle is available, use its 'ops' field instead of performing a
new lookup. For the same reason, there is no need to pass ops pointer to
__nft_print_header().
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 22:19:25 +0000 (00:19 +0200)]
nft: Keep nft_handle pointer in nft_xt_ctx
Instead of carrying the family value, carry the handle (which contains
the family value) and relieve expression parsers from having to call
nft_family_ops_lookup().
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 16:20:53 +0000 (18:20 +0200)]
nft: family_ops: Pass nft_handle to 'rule_find' callback
In order to prepare for rules containing set references, nft handle has
to be passed to nft_rule_to_iptables_command_state() in order to let it
access the set in cache.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 20 Aug 2019 13:15:19 +0000 (15:15 +0200)]
nft: family_ops: Pass nft_handle to 'add' callback
In order for add_match() to create anonymous sets when converting
xtables matches it needs access to nft handle. So pass it along from
callers of family ops' add callback.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 15 Nov 2019 09:47:25 +0000 (10:47 +0100)]
nft: Fix -Z for rules with NFTA_RULE_COMPAT
The special nested attribute NFTA_RULE_COMPAT holds information about
any present l4proto match (given via '-p' parameter) in input. The match
is contained as meta expression as well, but some xtables extensions
explicitly check it's value (see e.g. xt_TPROXY).
This nested attribute is input only, the information is lost after
parsing (and initialization of compat extensions). So in order to feed a
rule back to kernel with zeroed counters, the attribute has to be
reconstructed based on the rule's expressions.
Other code paths are not affected since rule_to_cs() callback will
populate respective fields in struct iptables_command_state and 'add'
callback (which is the inverse to rule_to_cs()) calls add_compat() in
any case.
Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 17 Sep 2019 18:27:27 +0000 (20:27 +0200)]
xtables-restore: Improve performance of --noflush operation
The reason for that full cache fetching when called with --noflush even
before looking at any input data was that there might be a command
requiring a rule cache following some rule add/insert ones which don't.
At that point one needs to fetch rules from kernel and try to insert the
local ones at the right spot which is non-trivial.
At the same time there is a performance-critical use-case for --noflush,
namely fast insertion of a bunch of rules in one go, avoiding the
process spawn overhead.
Optimize for this use-case by preloading input into a 64KB buffer to see
if it fits. If so, search for commands requiring a rule cache. If there
are none, skip initial full cache fetching.
The above algorithm may abort at any point, so actual input parsing must
happen in three stages:
1) parse all preloaded lines from 64KB buffer
2) parse any leftover line in line buffer (happens if input exceeds
the preload buffer size)
3) parse remaining input from input file pointer
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 18 Sep 2019 13:39:44 +0000 (15:39 +0200)]
xtables-restore: Allow lines without trailing newline character
Old code in add_param_to_argv() assumed the input line would always end
with a newline character. Without it, the last word of input wasn't
recognized. Fix this by adding a final check for param.len (indicating
leftover data in buffer).
In line parsing code itself, only COMMIT line check required presence of
trailing newline. The replaced conditional is not 100% accurate as it
allows for characters after newline to be present, but since fgets() is
used this shouldn't happen anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 17 Sep 2019 14:15:23 +0000 (16:15 +0200)]
xtables-restore: Introduce line parsing function
Move the loop code parsing a distinct line of input into a dedicated
function as a preparation for changing input sources. Since loop code
either calls continue or exit() directly, there is no need for a return
code to indicate failure.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
This data structure holds parser state information. A follow-up patch
will extract line parsing code into a separate function which will need
a place to persistently store this info in between calls.
While being at it, make 'in_table' variable boolean and drop some extra
braces in conditionals checking its value.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 28 Oct 2019 10:46:04 +0000 (11:46 +0100)]
nft-arp: Use xtables_print_mac_and_mask()
This libxtables function does exactly what the local implementation did.
The only noteworthy difference is that it assumes MAC/mask lengths, but
the local implementation was passed ETH_ALEN in each invocation, so no
practical difference.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 26 Oct 2019 19:37:48 +0000 (21:37 +0200)]
xtables-arp: Use xtables_parse_interface()
The local implementation differs just slightly but libxtables version
seems more correct (no needless memsetting of mask, more relevant
illegal character checking) so use that one.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 25 Oct 2019 15:21:13 +0000 (17:21 +0200)]
xtables-arp: Integrate OPT_* defines into xshared.h
These defines are internal use only, so their actual value doesn't
matter as long as they're unique and inverse_for_options array items
match:
When negating a given option, the corresponding OPT_* value's bit is
used as an index into inverse_for_options to retrieve the corresponding
invflag. If zero, either negating or the option itself is not supported.
(In practice, a lookup for unsupported option won't happen as those are
caught by getopt_long()).
Since xtables-arp's OPT_* values change, adjust the local
inverse_for_options array accordingly.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 13 May 2019 13:32:01 +0000 (15:32 +0200)]
Merge CMD_* defines
They are mostly identical, just xtables-arp ones differ slightly. Though
since they are internal use only and their actual value doesn't matter
(as long as it's a distinct bit), they can be merged anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 17 Oct 2019 21:36:47 +0000 (23:36 +0200)]
xshared: Introduce struct argv_store
The use of global variables in code around add_argv() is error-prone and
hard to follow. Replace them by a struct which functions will modify
instead of causing side-effects.
Given the lack of static variables, this effectively makes argv
construction code reentrant.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 17 Oct 2019 23:30:22 +0000 (01:30 +0200)]
iptables-xml: Use add_param_to_argv()
Extend the shared argv parser by storing whether a given argument was
quoted or not, then use it in iptables-xml. One remaining extra bit is
extraction of chain name in -A commands, do that afterwards in a loop.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 22 Oct 2019 10:25:28 +0000 (12:25 +0200)]
xtables-restore: Unbreak *tables-restore
Commit 3dc433b55bbfa ("xtables-restore: Fix --table parameter check")
installed an error check which evaluated true in all cases as all
callers of do_command callbacks pass a pointer to a table name already.
Attached test case passed as it tested error condition only.
Fix the whole mess by introducing a boolean to indicate whether a table
parameter was seen already. Extend the test case to cover positive as
well as negative behaviour and to test ebtables-restore and
ip6tables-restore as well. Also add the required checking code to the
latter since the original commit missed it.
Fixes: 3dc433b55bbfa ("xtables-restore: Fix --table parameter check") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 20 Sep 2019 15:31:58 +0000 (17:31 +0200)]
xtables-restore: Fix --table parameter check
Xtables-restore tries to reject rule commands in input which contain a
--table parameter (since it is adding this itself based on the previous
table line). The manual check was not perfect though as it caught any
parameter starting with a dash and containing a 't' somewhere, even in
rule comments:
| *filter
| -A FORWARD -m comment --comment "- allow this one" -j ACCEPT
| COMMIT
Instead of error-prone manual checking, go a much simpler route: All
do_command callbacks are passed a boolean indicating they're called from
*tables-restore. React upon this when handling a table parameter and
error out if it's not the first one.
Fixes: f8e5ebc5986bf ("iptables: Fix crash on malformed iptables-restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 17 Oct 2019 22:03:00 +0000 (00:03 +0200)]
xtables-restore: Drop local xtc_ops instance
It is merely used to hold nft_strerror() pointer but using that function
in turn does not provide any benefit as it falls back to plain
strerror() if nft_fn is not initialized.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 17 Sep 2019 14:45:20 +0000 (16:45 +0200)]
xtables-restore: Use xt_params->program_name
Instead of setting newargv[0] to argv[0]'s value, just use whatever
xt_params->program_name contains. The latter is arbitrarily defined, but
may still be more correct than real argv[0] which may simply be for
instance xtables-nft-multi. Either way, there is no practical
significance since newargv[0] is used exclusively in debug output.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 28 Aug 2019 10:33:55 +0000 (12:33 +0200)]
nft: Optimize flushing all chains of a table
Leverage nftables' support for flushing all chains of a table by
omitting NFTNL_RULE_CHAIN attribute in NFT_MSG_DELRULE payload.
The only caveat is with verbose output, as that still requires to have a
list of (existing) chains to iterate over. Apart from that, implementing
this shortcut is pretty straightforward: Don't retrieve a chain list and
just call __nft_rule_flush() directly which doesn't set above attribute
if chain name pointer is NULL.
A bigger deal is keeping rule cache consistent: Instead of just clearing
rule list for each flushed chain, flush_rule_cache() is updated to
iterate over all cached chains of the given table, clearing their rule
lists if not called for a specific chain.
While being at it, sort local variable declarations in nft_rule_flush()
from longest to shortest and drop the loop-local 'chain_name' variable
(but instead use 'chain' function parameter which is not used at that
point).
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 25 Sep 2019 16:48:07 +0000 (18:48 +0200)]
nft: Support nft_is_table_compatible() per chain
When operating on a single chain only, compatibility checking causes
unwanted overhead by checking all chains of the current table. Avoid
this by accepting the current chain name as parameter and pass it along
to nft_chain_list_get().
While being at it, introduce nft_assert_table_compatible() which
calls xtables_error() in case compatibility check fails. If a chain name
was given, include that in error message.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 2 Oct 2019 19:13:47 +0000 (21:13 +0200)]
nft-cache: Support partial rule cache per chain
Accept an additional chain name pointer in __nft_build_cache() and pass
it along to fetch only that specific chain and its rules.
Enhance nft_build_cache() to take an optional nftnl_chain pointer to
fetch rules for.
Enhance nft_chain_list_get() to take an optional chain name. If cache
level doesn't include chains already, it will fetch only the specified
chain from kernel (if existing) and add that to table's chain list which
is returned. This keeps operations for all chains of a table or a
specific one within the same code path in nft.c.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 25 Sep 2019 11:49:19 +0000 (13:49 +0200)]
nft-cache: Support partial cache per table
Accept a builtin_table pointer in __nft_build_cache() and pass it along
when fetching chains and rules to operate on that table only (unless the
pointer is NULL).
Make use of it in nft_chain_list_get() since that accepts a table name
and performs a builtin table lookup internally already.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 7 Oct 2019 16:40:40 +0000 (18:40 +0200)]
nft-cache: Cover for multiple fetcher invocation
Preparing for partial caches, it is necessary to make sure these
functions don't cause harm if called repeatedly.
* Use h->cache->tables pointer as indicator for existing table cache,
return immediately from fetch_table_cache() if non-NULL.
* Initialize table's chain list only if non-NULL.
* Search for chain in table's chain list before adding it.
* Don't fetch rules for a chain if it has any rules already. With rule
list being embedded in struct nftnl_chain, this is the best way left
to check if rules have been fetched already or not. It will fail for
empty chains, but causes no harm in that case, either.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 1 Oct 2019 14:23:24 +0000 (16:23 +0200)]
nft-cache: Introduce cache levels
Replace the simple have_cache boolean by a cache level indicator
defining how complete the cache is. Since have_cache indicated full
cache (including rules), make code depending on it check for cache level
NFT_CL_RULES.
Core cache fetching routine __nft_build_cache() accepts a new level via
parameter and raises cache completeness to that level.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 7 Oct 2019 10:35:21 +0000 (12:35 +0200)]
nft: Avoid nested cache fetching
Don't call fetch_table_cache() from within fetch_chain_cache() but
instead from __nft_build_cache(). Since that is the only caller of
fetch_chain_cache(), this change should not have any effect in practice.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 1 Oct 2019 13:14:48 +0000 (15:14 +0200)]
nft: Pass nft_handle to flush_cache()
This allows to call nft_table_builtin_find() and hence removes the only
real user of __nft_table_builtin_find(). Consequently remove the latter
by integrating it into its sole caller.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 25 Sep 2019 09:29:59 +0000 (11:29 +0200)]
xtables-restore: Minimize caching when flushing
Unless --noflush was given, xtables-restore merely needs the list of
tables to decide whether to delete it or not. Introduce nft_fake_cache()
function which populates table list, initializes chain lists (so
nft_chain_list_get() returns an empty list instead of NULL) and sets
'have_cache' to turn any later calls to nft_build_cache() into nops.
If --noflush was given, call nft_build_cache() just once instead of for
each table line in input.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 3 Sep 2019 16:10:55 +0000 (18:10 +0200)]
nft: Fix for add and delete of same rule in single batch
Another corner-case found when extending restore ordering test: If a
delete command in a dump referenced a rule added earlier within the same
dump, kernel would reject the resulting NFT_MSG_DELRULE command.
Catch this by assigning the rule to delete a RULE_ID value if it doesn't
have a handle yet. Since __nft_rule_del() does not duplicate the
nftnl_rule object when creating the NFT_COMPAT_RULE_DELETE command, this
RULE_ID value is added to both NEWRULE and DELRULE commands - exactly
what is needed to establish the reference.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 25 Sep 2019 10:54:55 +0000 (12:54 +0200)]
tests: shell: Support running for legacy/nft only
After some changes, one might want to test a single variant only. Allow
this by supporting -n/--nft and -l/--legacy parameters, each disabling
the other variant.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 3 Sep 2019 13:22:39 +0000 (15:22 +0200)]
tests/shell: Speed up ipt-restore/0004-restore-race_0
This test tended to cause quite excessive load on my system, sometimes
taking longer than all other tests combined. Even with the reduced
numbers, it still fails reliably after reverting commit 58d7de0181f61
("xtables: handle concurrent ruleset modifications").
Fixes: 4000b4cf2ea38 ("tests: add test script for race-free restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 17 Sep 2019 15:53:31 +0000 (17:53 +0200)]
xtables_error() does not return
It's a define which resolves into a callback which in turn is declared
with noreturn attribute. It will never return, therefore drop all
explicit exit() calls or other dead code immediately following it.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 20 Sep 2019 09:19:15 +0000 (11:19 +0200)]
nft: Fix add_bitwise_u16() on Big Endian
Type used for 'mask' and 'xor' parameters was wrong, 'int' is four bytes
on 32 or 64 bit architectures. After casting a uint16_t to int, on Big
Endian the first two bytes of data are (the leading) zero which libnftnl
then copies instead of the actual value.
This problem was noticed when using '--fragment' option:
| # iptables-nft -A FORWARD --fragment -j ACCEPT
| # nft list ruleset | grep frag-off
| ip frag-off & 0 != 0 counter packets 0 bytes 0 accept
With this fix in place, the resulting nft rule is correct:
projects / thirdparty / iptables.git / log
