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.
Phil Sutter [Wed, 2 Dec 2020 12:37:06 +0000 (13:37 +0100)]
extensions: dccp: Fix for DCCP type 'INVALID'
Support for matching on invalid DCCP type field values was pretty
broken: While RFC4340 declares any type value from 10 to 15 invalid, the
extension's type name 'INVALID' mapped to type value 10 only. Fix this
by introduction of INVALID_OTHER_TYPE_MASK which has the remaining
invalid type's bits set and apply it if bit 10 is set after parsing the
type list. When printing, stop searching type names after printing
'INVALID' - unless numeric output was requested. The latter prints all
actual type values. Since parsing types in numeric form is not
supported, changing the output should not break existing scripts.
When translating into nftables syntax, the code returned prematurely if
'INVALID' was among the list of types - thereby emitting invalid syntax.
Instead print a real match for invalid types by use of a range
expression.
While being at it, fix syntax of translator output: If only
'--dccp-types' was translated, the output contained an extra 'dccp'. On
the other hand, if '--sport' and '--dport' was present, a required
'dccp' between the translations of both was missing.
Fixes: e40b11d7ef827 ("add support for new 'dccp' protocol match") Fixes: c94a998724143 ("extensions: libxt_dccp: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 4 Dec 2020 16:44:51 +0000 (17:44 +0100)]
tests/shell: Test for fixed extension registration
Use strace to look at iptables-restore behaviour with typically
problematic input (conntrack revision 0 is no longer supported by
current kernels) to make sure the fix in commit a1eaaceb0460b
("libxtables: Simplify pending extension registration") is still
effective.
Phil Sutter [Fri, 30 Oct 2020 11:42:57 +0000 (12:42 +0100)]
xshared: Merge some command option-related code
Add OPT_FRAGMENT define into the enum of other OPT_* defines at the
right position and adjust the arptables-specific ones that follow
accordingly. Appropriately adjust inverse_for_options array in
xtables-arp.c.
Extend optflags from iptables.c by the arptables values for the sake of
completeness, then move it to xshared.h along with NUMBER_OF_OPT
definition. As a side-effect, this fixes for wrong ordering of entries
in arptables' 'optflags' copy.
Add arptables-specific bits to commands_v_options table (the speicific
options are matches on ARP header fields, just treat them like '-s'
option. This is also just a cosmetic change, arptables doesn't have a
generic_opt_check() implementation and hence doesn't use such a table.
With things potentially ready for common use, move commands_v_options
table along with generic_opt_check() and opt2char() into xshared.c and
drop the local (identical) implementations from iptables.c, ip6tables.c
xtables.c and xtables-arp.c. While doing so, fix ordering of entries in
that table: the row for CMD_ZERO_NUM was in the wrong position. Since
all moved rows though are identical, this had no effect in practice.
Fixes: d960a991350ca ("xtables-arp: Integrate OPT_* defines into xshared.h") Fixes: 384958620abab ("use nf_tables and nf_tables compatibility interface") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 3 Nov 2020 11:21:29 +0000 (12:21 +0100)]
xtables-arp: Don't use ARPT_INV_*
Arptables invflags are partly identical to IPT_INV_* ones but the bits
are differently assigned. Eliminate this incompatibility by definition
of the unique invflags in nft-arp.h on bits that don't collide with
IPT_INV_* ones, then use those in combination with IPT_INV_* ones in
arptables-specific code.
Note that ARPT_INV_ARPPRO is replaced by IPT_INV_PROTO although these
are in fact different options - yet since '-p' option is not supported
by arptables, this does not lead to a collision.
Phil Sutter [Tue, 3 Nov 2020 11:08:39 +0000 (12:08 +0100)]
libxtables: Extend MAC address printing/parsing support
Adding a parser which supports common names for special MAC/mask
combinations and a print routine detecting those special addresses and
printing the respective name allows to consolidate all the various
duplicated implementations.
The side-effects of this change are manageable:
* arptables now accepts "BGA" as alias for the bridge group address
* "mac" match now prints MAC addresses in lower-case which is consistent
with the remaining code at least
Phil Sutter [Tue, 17 Nov 2020 18:00:45 +0000 (19:00 +0100)]
iptables-test.py: Try to unshare netns by default
If user did not explicitly requst to "test netnamespace path", try an
import of 'unshare' module and call unshare() to avoid killing the local
host's network by accident.
Phil Sutter [Tue, 17 Nov 2020 10:38:27 +0000 (11:38 +0100)]
ebtables: Fix for broken chain renaming
Loading extensions pollutes 'errno' value, hence before using it to
indicate failure it should be sanitized. This was done by the called
function before the parsing/netlink split and not migrated by accident.
Move it into calling code to clarify the connection.
Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands") Signed-off-by: Phil Sutter <phil@nwl.cc>
tests: shell: update format of registers in bitwise payloads.
libnftnl has been changed to bring the format of registers in bitwise
dumps in line with those in other types of expression. Update the
expected output of Python test-cases.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 10 Nov 2020 13:50:46 +0000 (14:50 +0100)]
tests/shell: Add test for bitwise avoidance fixes
Masked address matching was recently improved to avoid bitwise
expression if the given mask covers full bytes. Make use of nft netlink
debug output to assert iptables-nft generates the right bytecode for
each situation.
Phil Sutter [Mon, 26 Oct 2020 16:25:03 +0000 (17:25 +0100)]
tests: shell: Improve concurrent noflush restore test a bit
The described issue happens only if chain FOO does not exist at program
start so flush the ruleset after each iteration to make sure this is the
case. Sadly the bug is still not 100% reproducible on my testing VM.
While being at it, add a paragraph describing what exact situation the
test is trying to provoke.
Fixes: dac904bdcd9a1 ("nft: Fix for concurrent noflush restore calls") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Mon, 5 Oct 2020 14:06:49 +0000 (16:06 +0200)]
nft: Fix for concurrent noflush restore calls
Transaction refresh was broken with regards to nft_chain_restore(): It
created a rule flush batch object only if the chain was found in cache
and a chain add object only if the chain was not found. Yet with
concurrent ruleset updates, one has to expect both situations:
* If a chain vanishes, the rule flush job must be skipped and instead
the chain add job become active.
* If a chain appears, the chain add job must be skipped and instead
rules flushed.
Change the code accordingly: Create both batch objects and set their
'skip' field depending on the situation in cache and adjust both in
nft_refresh_transaction().
As a side-effect, the implicit rule flush becomes explicit and all
handling of implicit batch jobs is dropped along with the related field
indicating such.
Reuse the 'implicit' parameter of __nft_rule_flush() to control the
initial 'skip' field value instead.
A subtle caveat is vanishing of existing chains: Creating the chain add
job based on the chain in cache causes a netlink message containing that
chain's handle which the kernel dislikes. Therefore unset the chain's
handle in that case.
Phil Sutter [Thu, 8 Oct 2020 12:51:52 +0000 (14:51 +0200)]
libiptc: Avoid gcc-10 zero-length array warning
Gcc-10 doesn't like the use of zero-length arrays as last struct member
to denote variable sized objects. The suggested alternative, namely to
use a flexible array member as defined by C99, is problematic as that
doesn't allow for said struct to be embedded into others. With the
relevant structs being part of kernel UAPI, this can't be precluded
though.
The call to memcpy() which triggers the warning copies data from one
struct xt_counters to another. Since this struct is flat and merely
contains two u64 fields, One can use direct assignment instead which
avoids the warning.
Previous to this patch, the basechain policy could not be properly
configured if it wasn't explictly set when loading the ruleset, leading
to iptables-nft-restore (and ip6tables-nft-restore) trying to send an
invalid ruleset to the kernel.
Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 5 Oct 2020 13:54:35 +0000 (15:54 +0200)]
nft: Fix error reporting for refreshed transactions
When preparing a batch from the list of batch objects in nft_action(),
the sequence number used for each object is stored within that object
for later matching against returned error messages. Though if the
transaction has to be refreshed, some of those objects may be skipped,
other objects take over their sequence number and errors are matched to
skipped objects. Avoid this by resetting the skipped object's sequence
number to zero.
Phil Sutter [Tue, 22 Sep 2020 18:01:15 +0000 (20:01 +0200)]
libxtables: Register multiple extensions in ascending order
The newly introduced ordered insert algorithm in
xtables_register_{match,target}() works best if extensions of same name
are passed in ascending revisions. Since this is the case in about all
extensions' arrays, iterate over them from beginning to end.
Assuming that pending extensions are sorted by first name and family,
then descending revision, the decision where to insert a newly
registered extension may be simplified by memorizing the previous
registration (which obviously is of same name and family and higher
revision).
As a side-effect, fix for unsupported old extension revisions lingering
in pending extension list forever and being retried with every use of
the given extension. Any revision being rejected by the kernel may
safely be dropped iff a previous (read: higher) revision was accepted
already.
Yet another side-effect of this change is the removal of an unwanted
recursion by xtables_fully_register_pending_*() into itself via
xtables_find_*().
Phil Sutter [Mon, 21 Sep 2020 11:42:06 +0000 (13:42 +0200)]
libxtables: Make sure extensions register in revision order
Insert extensions into pending lists in ordered fashion: Group by
extension name (and, for matches, family) and order groups by descending
revision number.
This allows to simplify the later full registration considerably. Since
that involves kernel compatibility checks, the extra cycles here pay off
eventually.
Phil Sutter [Tue, 6 Oct 2020 17:07:19 +0000 (19:07 +0200)]
extensions: libipt_icmp: Fix translation of type 'any'
By itself, '-m icmp --icmp-type any' is a noop, it matches any icmp
types. Yet nft_ipv4_xlate() does not emit an 'ip protocol' match if
there's an extension with same name present in the rule. Luckily, legacy
iptables demands icmp match to be prepended by '-p icmp', so we can
assume this is present and just emit the 'ip protocol' match from icmp
xlate callback.
Fixes: aa158ca0fda65 ("extensions: libipt_icmp: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 28 Sep 2020 16:57:18 +0000 (18:57 +0200)]
nft: Fix for broken address mask match detection
Trying to decide whether a bitwise expression is needed to match parts
of a source or destination address only, add_addr() checks if all bytes
in 'mask' are 0xff or not. The check is apparently broken though as each
byte in 'mask' is cast to a signed char before comparing against 0xff,
therefore the bitwise is always added:
Phil Sutter [Wed, 8 Jul 2020 21:03:12 +0000 (23:03 +0200)]
nft: Fold nftnl_rule_list_chain_save() into caller
Existence of this function was mostly code-duplication: Caller already
branches depending on whether 'chain' is NULL or not and even does the
chain list lookup.
While being at it, simplify __nftnl_rule_list_chain_save function name a
bit now that the non-prefixed name is gone.
Phil Sutter [Fri, 31 Jul 2020 16:20:17 +0000 (18:20 +0200)]
nft: Fix for ruleset flush while restoring
If ruleset is flushed while an instance of iptables-nft-restore is
running and has seen a COMMIT line once, it doesn't notice the
disappeared table while handling the next COMMIT. This is due to table
existence being tracked via 'initialized' boolean which is only reset
by nft_table_flush().
To fix this, drop the dedicated 'initialized' boolean and switch users
to the recently introduced 'exists' one.
As a side-effect, this causes base chain existence being checked for
each command calling nft_xt_builtin_init() as the old 'initialized' bit
was used to track if that function has been called before or not.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 7 Aug 2020 14:42:07 +0000 (16:42 +0200)]
xtables-monitor: Fix ip6tables rule printing
When printing an ip6tables rule event, false family ops are used as they
are initially looked up for AF_INET and reused no matter the current
rule's family. In practice, this means that nft_rule_print_save() calls
the wrong rule_to_cs, save_rule and clear_cs callbacks. Therefore, if a
rule specifies a source or destination address, the address is not
printed.
Fix this by performing a family lookup each time rule_cb is called.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 6 Aug 2020 16:52:34 +0000 (18:52 +0200)]
tests: shell: Merge and extend return codes test
Merge scripts for iptables and ip6tables, they were widely identical.
Also extend the test by one check (removing a non-existent rule with
valid chain and target) and quote the error messages where differences
are deliberately ignored.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 7 Aug 2020 11:48:28 +0000 (13:48 +0200)]
nft: Fix command name in ip6tables error message
Upon errors, ip6tables-nft would prefix its error messages with
'iptables:' instead of 'ip6tables:'. Turns out the command name was
hard-coded, use 'progname' variable instead.
While being at it, merge the two mostly identical fprintf() calls into
one.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 30 Jul 2020 09:54:36 +0000 (11:54 +0200)]
nft: Eliminate table list from cache
The full list of tables in kernel is not relevant, only those used by
iptables-nft and for those, knowing if they exist or not is sufficient.
For holding that information, the already existing 'table' array in
nft_cache suits well.
Consequently, nft_table_find() merely checks if the new 'exists' boolean
is true or not and nft_for_each_table() iterates over the builtin_table
array in nft_handle, additionally checking the boolean in cache for
whether to skip the entry or not.
iptables: replace libnftnl table list by linux list
This patch removes the libnftnl table list by linux list. This comes
with an extra memory allocation to store the nft_table object. Probably,
there is no need to cache the entire nftnl_table in the near future.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 7 Jul 2020 16:40:11 +0000 (18:40 +0200)]
nft: Reorder enum nft_table_type
This list of table types is used internally only, the actual values
don't matter that much. Reorder them to match the order in which
iptables-legacy-save prints them (if present). As a consequence, entries
in builtin_table array 'xtables_ipv4' are correctly sorted as well.
Phil Sutter [Wed, 8 Jul 2020 13:18:48 +0000 (15:18 +0200)]
nft: Turn nft_chain_save() into a foreach-callback
Let nftnl_chain_list_foreach() do the chain list iterating instead of
open-coding it. While being at it, simplify the policy value selection
code as well.
Phil Sutter [Fri, 10 Jul 2020 19:53:08 +0000 (21:53 +0200)]
nft: cache: Drop duplicate chain check
When fetching chains from kernel, checking for duplicate chain names is
not needed: Nftables doesn't support them in the first place. This is
merely a leftover from when multiple cache fetches could happen and so a
bit of sanity checking was in order.
Phil Sutter [Fri, 10 Jul 2020 18:08:35 +0000 (20:08 +0200)]
nft: Make table creation purely implicit
While asserting a required builtin chain exists, its table is created
implicitly if missing. Exploit this from xtables-restore, too: The only
actions which need adjustment are chain_new and chain_restore, i.e. when
restoring (either builtin or custom) chains.
Note: The call to nft_table_builtin_add() wasn't sufficient as it
doesn't set the table as initialized and therefore a following call to
nft_xt_builtin_init() would override non-default base chain policies.
Note2: The 'table_new' callback in 'nft_xt_restore_cb' is left in place
as xtables-translate uses it to print an explicit 'add table' command.
Note3: nft_table_new() function was already unused since a7f1e208cdf9c
("nft: split parsing from netlink commands").
allow users to override at runtime the lock file to use through the
XTABLES_LOCKFILE environment variable.
It allows to use iptables when the user has granted enough
capabilities (e.g. a user+network namespace) to configure the network
but that lacks access to the XT_LOCK_NAME (by default placed under
/run).
libxtables: compiler warning fixes for NO_SHARED_LIBS
Fixes two issues with NO_SHARED_LIBS:
- #include <dlfcn.h> is ifdef'ed out and thus dlclose()
triggers an undeclared function compiler warning
- dlreg_add() is unused and thus triggers an unused
function warning
Test: builds without warnings Signed-off-by: Maciej Żenczykowski <maze@google.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: d4409d449c10fa ("nft: Don't exit early after printing help texts") Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
Phil Sutter [Tue, 12 May 2020 10:59:42 +0000 (12:59 +0200)]
xtables-restore: Fix verbose mode table flushing
When called with --verbose mode, iptables-nft-restore did not print
anything when flushing the table. Fix this by adding a "manual" mode to
nft_cmd_table_flush(), turning it into a wrapper around '-F' and '-X'
commands, which is exactly what iptables-legacy-restore does to flush a
table. This though requires a real cache, so don't set NFT_CL_FAKE then.
Phil Sutter [Wed, 3 Jun 2020 09:41:55 +0000 (11:41 +0200)]
build: bump dependency on libnftnl
Recently added full among match support depends on concatenated ranges
in nftables sets, a feature which was not available in libnftnl before
version 1.1.6.
Fixes: c33bae9c6c7a4 ("ebtables: among: Support mixed MAC and MAC/IP entries") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 5 Dec 2019 12:35:25 +0000 (13:35 +0100)]
include: Avoid undefined left-shift in xt_sctp.h
Pull the fix in kernel commit 164166558aace ("netfilter: uapi: Avoid
undefined left-shift in xt_sctp.h") into iptables repository. The
original description is:
With 'bytes(__u32)' being 32, a left-shift of 31 may happen which is
undefined for the signed 32-bit value 1. Avoid this by declaring 1 as
unsigned.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 19 May 2020 23:00:57 +0000 (01:00 +0200)]
doc: libxt_MARK: OUTPUT chain is fine, too
In order to route packets originating from the host itself based on
fwmark, mangle table's OUTPUT chain must be used. Mention this chain as
alternative to PREROUTING.
Fixes: c9be7f153f7bf ("doc: libxt_MARK: no longer restricted to mangle table") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 7 May 2020 16:53:47 +0000 (18:53 +0200)]
nft: Merge nft_*_rule_find() functions
Both ebtables and arptables are fine with using nft_ipv46_rule_find()
instead of their own implementations. Take the chance and move the
former into nft.c as a static helper since it is used in a single place,
only. Then get rid of the callback from family_ops.
Phil Sutter [Sat, 9 May 2020 11:42:56 +0000 (13:42 +0200)]
nfnl_osf: Improve error handling
For some error cases, no log message was created - hence apart from the
return code there was no indication of failing execution.
If a line load fails, don't abort but continue with the remaining
file contents. The current pf.os file in this repository serves as
proof-of-concept:
Lines 700 and 701: Duplicates of lines 698 and 699 because 'W*' and 'W0'
parse into the same data.
Line 704: Duplicate of line 702 because apart from 'W*' and 'W0', only
the first three fields on right-hand side are sent to the kernel.
When loading, these dups are ignored (they would bounce if NLM_F_EXCL
was given). Upon deletion, they cause ENOENT response from kernel. In
order to align duplicate-tolerance in both modes, just ignore that
ENOENT.
Phil Sutter [Sat, 9 May 2020 11:36:49 +0000 (13:36 +0200)]
nfnl_osf: Fix broken conversion to nfnl_query()
Due to missing NLM_F_ACK flag in request, nfnetlink code in kernel
didn't create an own ACK message but left it upon subsystem to ACK or
not. Since nfnetlink_osf doesn't ACK by itself, nfnl_query() got stuck
waiting for a reply.
Whoever did the conversion from deprecated nfnl_talk() obviously didn't
even test basic functionality of the tool.
Fixes: 52aa15098ebd6 ("nfnl_osf: Replace deprecated nfnl_talk() by nfnl_query()") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 8 May 2020 12:57:36 +0000 (14:57 +0200)]
iptables-test: Don't choke on empty lines
The script code wasn't expecting empty lines:
| Traceback (most recent call last):
| File "./iptables-test.py", line 380, in <module>
| main()
| File "./iptables-test.py", line 370, in main
| file_tests, file_passed = run_test_file(filename, args.netns)
| File "./iptables-test.py", line 265, in run_test_file
| if item[1] == "=":
| IndexError: list index out of range
Fix this by ignoring empty lines or those consisting of whitespace only.
While being at it, remove the empty line from libxt_IDLETIMER.t which
exposed the problem.
Phil Sutter [Wed, 6 May 2020 11:33:20 +0000 (13:33 +0200)]
nft: Fix leak when replacing a rule
If nft_rule_append() is called with a reference rule, it is supposed to
insert the new rule at the reference position and then remove the
reference from cache. Instead, it removed the new rule from cache again
right after inserting it. Also, it missed to free the removed rule.
Fixes: 5ca9acf51adf9 ("xtables: Fix position of replaced rules in cache") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 5 May 2020 17:36:13 +0000 (19:36 +0200)]
nft: Use clear_cs() instead of open coding
In a few places, initialized struct iptables_command_state was not fully
deinitialized. Change them to call nft_clear_iptables_command_state()
which does it properly.
Phil Sutter [Tue, 5 May 2020 11:56:11 +0000 (13:56 +0200)]
libxtables: Introduce xtables_fini()
Record handles of loaded shared objects in a linked list and dlclose()
them from the newly introduced function. While functionally not
necessary, this clears up valgrind's memcheck output when also
displaying reachable memory.
Since this is an extra function that doesn't change the existing API,
increment both current and age.
Phil Sutter [Tue, 5 May 2020 11:45:06 +0000 (13:45 +0200)]
ebtables: Free statically loaded extensions again
All ebtables extensions are loaded upon program start as due to the lack
of '-m' parameters, loading on demand is not possible. Introduce
nft_fini_eb() to counteract nft_init_eb() and free dynamic memory in
matches and targets from there.
Phil Sutter [Mon, 4 May 2020 17:30:29 +0000 (19:30 +0200)]
nft: Fix leaks in ebt_add_policy_rule()
The function leaked memory allocated in temporary struct
iptables_command_state, clean it immediately after use.
In any of the udata-related error cases, allocated nftnl_rule would
leak, fix this by introducing a common error path to goto.
In regular code path, the allocated nftnl_rule would still leak:
batch_obj_del() does not free rules in NFT_COMPAT_RULE_APPEND jobs, as
they typically sit in cache as well. Policy rules in turn weren't added
to cache: They are created immediately before commit and never
referenced from other rules. Add them now so they are freed just like
regular rules.
Fixes: aff1162b3e4b7 ("ebtables-nft: Support user-defined chain policies") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Mon, 4 May 2020 17:20:52 +0000 (19:20 +0200)]
nft: Clear all lists in nft_fini()
Remove and free any pending entries in obj_list and err_list as well. To
get by without having to declare list-specific cursors, use generic
list_head types and call list_entry() explicitly.
Phil Sutter [Fri, 1 May 2020 05:59:36 +0000 (07:59 +0200)]
nft: cache: Re-establish cache consistency check
Restore code ensuring __nft_build_cache() returns a consistent cache in
which all ruleset elements belong to the same generation.
This check was removed by commit 200bc39965149 ("nft: cache: Fix
iptables-save segfault under stress") as it could lead to segfaults if a
partial cache fetch was done while cache's chain list was traversed.
With the new cache fetch logic, __nft_build_cache() is never called
while holding references to cache entries.
Phil Sutter [Tue, 5 May 2020 16:37:49 +0000 (18:37 +0200)]
tests: shell: Implement --valgrind mode
Wrap every call to $XT_MULTI with valgrind, or actually a wrapper script
which does the valgrind wrap and stores the log if it contains something
relevant.
Carefully name the wrapper script(s) so that test cases' checks on
$XT_MULTI name stay intact.
This mode slows down testsuite execution horribly. Luckily, it's not
meant for constant use, though.
For now, ignore commands with non-zero exit status - error paths
typically hit direct exit() calls and therefore leave reachable memory
in place.
Phil Sutter [Fri, 24 Apr 2020 09:32:08 +0000 (11:32 +0200)]
nft: Fix for '-F' in iptables dumps
When restoring a dump which contains an explicit flush command,
previously added rules are removed from cache and the following commit
will try to create netlink messages based on freed memory.
Fix this by weeding any rule-based commands from obj_list if they
address the same chain.
Phil Sutter [Mon, 27 Apr 2020 10:08:59 +0000 (12:08 +0200)]
nft: cache: Optimize caching for flush command
When flushing all chains and verbose mode is not enabled,
nft_rule_flush() uses a shortcut: It doesn't specify a chain name for
NFT_MSG_DELRULE, so the kernel will flush all existing chains without
user space needing to know which they are.
The above allows to avoid a chain cache, but there's a caveat:
nft_xt_builtin_init() will create base chains as it assumes they are
missing and thereby possibly overrides any non-default chain policies.
Solve this by making nft_xt_builtin_init() cache-aware: If a command
doesn't need a chain cache, there's no need to bother with creating any
non-existing builtin chains, either. For the sake of completeness, also
do nothing if cache is not initialized (although that shouldn't happen).
Phil Sutter [Thu, 19 Mar 2020 17:58:29 +0000 (18:58 +0100)]
nft: cache: Fetch cache for specific chains
Iterate over command list and collect chains to cache. Insert them into
a sorted list to pass to __nft_build_cache().
If a command is interested in all chains (e.g., --list), cmd->chain
remains unset. To record this case reliably, use a boolean
('all_chains'). Otherwise, it is hard to distinguish between first call
to nft_cache_level_set() and previous command with NULL cmd->chain
value.
When caching only specific chains, manually add builtin ones for the
given table as well - otherwise nft_xt_builtin_init() will act as if
they don't exist and possibly override non-default chain policies.
Phil Sutter [Wed, 18 Mar 2020 16:08:31 +0000 (17:08 +0100)]
nft-cache: Fetch cache per table
Restore per-table operation of cache routines as initially implemented
in commit e2883c5531e6e ("nft-cache: Support partial cache per table").
As before, this doesn't limit fetching of tables (their number is
supposed to be low) but instead limits fetching of sets, chains and
rules to the specified table.
For this to behave correctly when restoring without flushing over
multiple tables, cache must be freed fully after each commit - otherwise
the previous table's cache level is reused for the current one. The
exception being fake cache, used for flushing restore: NFT_CL_FAKE is
set just once at program startup, so it must stay set otherwise
consecutive tables cause pointless cache fetching.
The sole use-case requiring a multi-table cache, iptables-save, is
indicated by req->table being NULL. Therefore, req->table assignment is
a bit sloppy: All calls to nft_cache_level_set() are assumed to set the
same table value, collision detection exists merely to catch programming
mistakes.
Make nft_fini() call nft_release_cache() instead of flush_chain_cache(),
the former does a full cache deinit including cache_req contents.
Phil Sutter [Tue, 7 Apr 2020 11:47:54 +0000 (13:47 +0200)]
nft: cache: Improve fake cache integration
With NFT_CL_FAKE being highest cache level while at the same time
__nft_build_cache() treating it equal to NFT_CL_TABLES, no special
handling for fake cache is required anymore.
Phil Sutter [Thu, 26 Mar 2020 03:47:11 +0000 (04:47 +0100)]
nft: cache: Simplify rule and set fetchers
Since no incremental cache fetching happens anymore, code fetching rules
for chains or elements for sets may safely assume that whatever is in
cache also didn't get populated with rules or elements before.
Therefore no (optional) chain name needs to be passed on to
fetch_rule_cache() and fetch_set_cache() doesn't have to select for
which sets in a table to call set_fetch_elem_cb().
The cache requirements are now calculated once from the parsing phase.
There is no need to call __nft_build_cache() from several spots in the
codepath anymore.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Phil Sutter <phil@nwl.cc>
projects / thirdparty / iptables.git / log
