| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When IPv4 rule generator was changed to emit payload instead of
meta expressions for l4proto matches, the code reinserting
NFTNL_RULE_COMPAT_* attributes into rules being reused for counter
zeroing was broken by accident.
Make rule compat recovery aware of the alternative match, basically
reinstating the effect of commit 7a373f6683afb ("nft: Fix -Z for rules
with NFTA_RULE_COMPAT") but add a test case this time to make sure
things stay intact.
Fixes: 69278f9602b43 ("nft: use payload matching for layer 4 protocol")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instrument proto_to_name() to abort if given protocol number is not
among the well-known ones in xtables_chain_protos. Along with
xtables_parse_protocol() preferring said array for lookups as well, this
ensures reliable dump'n'restore regardless of /etc/protocols contents.
Another benefit is rule dump performance. A simple test-case dumping
100k rules matching on dccp protocol shows an 8s delta (2s vs. 10s for
legacy, 0.5s vs. 8s for nft) with this patch applied. For reference:
| for variant in nft legacy; do
| (
| echo "*filter"
| for ((i = 0; i < 100000; i++)); do
| echo "-A FORWARD -p dccp -j ACCEPT"
| done
| echo "COMMIT"
| ) | iptables-${variant}-restore
| time iptables-${variant}-save | wc -l
| iptables-${variant} -F
| done
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit da8ecc62dd765b15df84c3aa6b83dcb7a81d4ffa.
The patch's original intention is not entirely clear anymore. If it was
to reduce delays involved by calling getprotobynumber() though, commit
b6196c7504d4d ("xshared: Prefer xtables_chain_protos lookup over
getprotoent") avoids those if --numeric flag was given already. Also,
this numeric protocol output did not cover iptables-save which is a more
relevant candidate for such optimizations anyway.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1729
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ebtables-nft has always supported both intra- and extrapositioned
negations but defaulted to intrapositioned when printing/saving rules.
With commit 58d364c7120b5 ("ebtables: Use do_parse() from xshared")
though, it started to warn about intrapositioned negations. So change
the default to avoid mandatory warnings when e.g. loading previously
dumped rulesets.
Also adjust test cases, help texts and ebtables-nft.8 accordingly.
Cc: Jan Engelhardt <jengelh@inai.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Parameter 'wait' passed to xtables_lock() signals three modes of
operation, depending on its value:
0: --wait not specified, do not wait if lock is busy
-1: --wait specified without value, wait indefinitely until lock becomes
free
>0: Wait for 'wait' seconds for lock to become free, abort otherwise
Since fixed commit, the first two cases were treated the same apart from
calling alarm(0), but that is a nop if no alarm is pending. Fix the code
by requesting a non-blocking flock() in the second case. While at it,
restrict the alarm setup to the third case only.
Cc: Jethro Beekman <jethro@fortanix.com>
Cc: howardjohn@google.com
Cc: Antonio Ojea <antonio.ojea.garcia@gmail.com>
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1728
Fixes: 07e2107ef0cbc ("xshared: Implement xtables lock timeout using signals")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Treat it like --replace against the same rule with changed counters.
The operation is obviously not atomic, so rule counters may change in
kernel while the rule is fetched, modified and replaced.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
Report came from firwalld, but this is actually rather hard to trigger.
Since a regular chain line prevents it, typical dump/restore use-cases
are unaffected.
Fixes: 73611d5582e72 ("ebtables-nft: add broute table emulation")
Cc: Eric Garver <eric@garver.life>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Chain rename code missed to adjust the num_chains value which is used to
calculate the number of chain index buckets to allocate during an index
rebuild. So with the right number of chains present, the last chain in a
middle bucket being renamed (and ending up in another bucket) triggers
an index rebuild based on false data. The resulting NULL pointer index
bucket then causes a segfault upon reinsertion.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1713
Fixes: 64ff47cde38e4 ("libiptc: fix chain rename bug in libiptc")
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The test did not catch non-zero exit status of the spawned coprocess. To
make it happen, Drop the line killing it (it will exit anyway) and pass
its PID to 'wait'.
While being at it, put the sleep into the correct spot (otherwise the
check for chain 'foo' existence fails as it runs too early) and make
said chain existence check effective.
Fixes: 4e3c11a6f5a94 ("nft: Fix for ruleset flush while restoring")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts the following commits:
b14c971db6db0 ("tests: Test compat mode")
11c464ed015b5 ("Add --compat option to *tables-nft and *-nft-restore commands")
ca709b5784c98 ("nft: Introduce and use bool nft_handle::compat")
402b9b3c07c81 ("nft: Pass nft_handle to add_{target,action}()")
This implementation of a compatibility mode implements rules using
xtables extensions if possible and thus relies upon existence of those
in kernel space. Assuming no viable replacement for the internal
mechanics of this mode will be found in foreseeable future, it will
effectively block attempts at deprecating and removing of these xtables
extensions in favor of nftables expressions and thus hinder upstream's
future plans for iptables.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
Extend iptables-test.py by a third mode, which is using
xtables-nft-multi and passing --compat to all calls creating rules.
Also add a shell testcase asserting the effectiveness of --compat by
comparing debug (-vv) output.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
Test the last two fixes in that area.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
The old version exited unintentionally before testing ip6tables. Replace
it by a more complete variant testing for all tools, creating and
renaming of,chains with various illegal names instead of just renaming
to a clashing name.
Fixes: ed9cfe1b48526 ("tests: add initial save/restore test cases")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
If '--counters' option was not given, restore parsers would ignore
anything following the policy word. Make them more strict, rejecting
anything in that spot which does not look like counter values even if
not restoring counters.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
| |
Some versions of awk (gawk-4.2.1-4.el8 in particular) also print the
non-debug ruleset listing's empty lines, causing the diff to fail. Catch
this by exiting upon seeing the first table heading. For the sake of
comparing bytecode, the actual ruleset listing is not interesting,
anyway.
Fixes: 0f7ea0390b336 ("tests/shell: Fix nft-only/0009-needless-bitwise_0")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since 694612adf87 the "compatibility" check considers non-existent
chains as "incompatible". This broke some scripts which used calls
like `iptables -L CHAIN404` to test for chain existence and expect
"No chain/target/match by that name." in the output.
This patch changes the logic of `nft_is_table_compatible()` to
report non-existent chains as "compatible" which restores the old
behavior.
Fixes: 694612adf87 ("nft: Fix selective chain compatibility checks")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1648
Signed-off-by: Jacek Tomasiak <jtomasiak@arista.com>
Signed-off-by: Jacek Tomasiak <jacek.tomasiak@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When setting counters using ip6tables-nft -c X Y the X and Y values were
not stored.
This is a fix based on 9baf3bf0e77dab6ca4b167554ec0e57b65d0af01 but
applied to the nft variant of ipv6 not the legacy.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1647
Fixes: 0391677c1a0b2 ("xtables: add IPv6 support")
Signed-off-by: Jacek Tomasiak <jtomasiak@arista.com>
Signed-off-by: Jacek Tomasiak <jacek.tomasiak@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Rule comparison in legacy ip6tables was broken by commit eb2546a846776
("xshared: Share make_delete_mask() between ip{,6}tables"): A part of
the rules' data was masked out for comparison by accident.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use new 'meta broute set 1' to emulate -t broute. If '-t broute' is given,
automatically translate -j DROP to 'meta broute set 1 accept' internally.
Reverse translation zaps the broute and pretends verdict was DROP.
Note that BROUTING is internally handled via PREROUTING, i.e. 'redirect'
and 'nat' targets are not available, they will need to be emulated via
nft expressions.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Combining multiple corner-cases here:
* Insert a rule before another new one which is not the first. Triggers
NFTNL_RULE_ID assignment of the latter.
* Delete the referenced new rule in the same batch again. Causes
overwriting of the previously assigned RULE_ID.
Consequently, iptables-nft-restore fails during *insert*, because the
reference is dangling.
Reported-by: Eric Garver <eric@garver.life>
Fixes: 760b35b46e4cc ("nft: Fix for add and delete of same rule in single batch")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Tested-by: Eric Garver <eric@garver.life>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Unlike legacy, ebtables-nft would allow e.g.:
| -t nat -A PREROUTING --to-dst fe:ed:00:00:ba:be
While the result is correct, it may mislead users into believing
multiple targets are possible per rule. Better follow legacy's behaviour
and reject target options unless they have been "enabled" by a previous
'-j' option.
To achieve this, one needs to distinguish targets from watchers also
attached to 'xtables_targets' and otherwise behaving like regular
matches. Introduce XTABLES_EXT_WATCHER to mark the two.
The above works already, but error messages are misleading when using
the now unsupported syntax since target options have been merged
already. Solve this by not pre-loading the targets at all, code will
just fall back to loading ad '-j' parsing time as iptables does.
Note how this also fixes for 'counter' statement being in wrong position
of ebtables-translate output.
Fixes: fe97f60e5d2a9 ("ebtables-compat: add watchers support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While commit 1b8210f848631 kind of fixed the corner-case of invalid
short-options packed with others, it broke error reporting for
long-options. Revert it and deploy a proper solution:
When passing an invalid short-option, e.g. 'iptables -vaL', getopt_long
sets the variable 'optopt' to the invalid character's value. Use it for
reporting instead of optind if set.
To distinguish between invalid options and missing option arguments,
ebtables-translate optstring needs adjustment.
Fixes: 1b8210f848631 ("ebtables: Fix error message for invalid parameters")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
These were previously ignored.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Found this on disk, maybe there was a problem with this and among match
at some point? Anyway, it is relevant again since it fails recently.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
This is an odd bug: If the number of chains is right and one renames the
last one in the list, libiptc dereferences a NULL pointer. Add fix and
test case for it.
Fixes: 64ff47cde38e4 ("libiptc: fix chain rename bug in libiptc")
Reported-by: Julien Castets <castets.j@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Forgot to update shell testsuite when removing empty --log-prefix
options.
Fixes: 9cdb52d655608 ("extensions: libebt_log: Avoid empty log-prefix in output")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Forgot to update the shell testsuites when fixing for duplicate
whitespace in output.
Fixes: 11e06cbb3a877 ("extensions: libip6t_dst: Fix output for empty options")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To match on Ethernet frames using the etherproto field as length value,
ebtables accepts the special protocol name "LENGTH". Implement this in
ebtables-nft using a native match for 'ether type < 0x0600'.
Since extension 802_3 matches are valid only with such Ethernet frames,
add a local add_match() wrapper which complains if the extension is used
without '-p Length' parameter. Legacy ebtables does this within the
extension's final_check callback, but it's not possible here due for lack of
fw->bitmask field access.
While being at it, add xlate support, adjust tests and make ebtables-nft
print the case-insensitive argument with capital 'L' like legacy
ebtables does.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Feed nft-generated ruleset to iptables-nft.
At this time, this will NOT pass. because dissector can handle
meta l4proto tcp ip saddr 1.2.3.4
but not
ip saddr 1.2.3.4 meta l4proto tcp
In the latter case, iptables-nft picks up the immediate value (6) as the ip
address, because the first one (1.2.3.4) gets moved as PAYLOAD_PREV due to
missing 'removal' of the CTX_PAYLOAD flag.
This is error prone, so lets rewrite the dissector to track each
register separately and auto-clear state on writes.
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Even if iptables-nft doesn't generate them anymore, it should continue
to correctly parse them. Make sure this is tested for.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
| |
Validate that matching works as expected.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
This is much trickier than expected: On one hand, proto_to_name() is
used to lookup protocol extensions so must resolve despite FMT_NUMERIC
being set. On the other, --verbose implies --numeric but changing the
output there is probably a bad idea. Luckily the latter situation is
identified by FMT_NOTABLE bit.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Format string ensured a minimum field width of five characters, but
allowed for longer strings to eat the column delimiting white space.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Adjust captured output, ip6tables prints '--' instead of spaces since
the commit in Fixes: tag.
Fixes: 6e41c2d8747b2 ("iptables: xshared: Ouptut '--' in the opt field in ipv6's fake mode")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
Test zeroing a single rule's counters as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Call with --combine as well, even though output doesn't differ. Also
there's no need to skip for xtables-nft-multi, it provides the same
functionality.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
This increases coverage of function print_match() from 0 to 86.6%.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Some repeated calls have been reduced recently, assert this in a test
evaluating strace output.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
Treating revision 0 as compatible in EPERM case works fine as long as
there is a revision 0 of that extension defined in DSO. Fix the code for
others: Extend the EPERM handling to all revisions and keep the existing
warning for revision 0.
Fixes: 17534cb18ed0a ("Improve error messages for unsupported extensions")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
In static builds, xtables_find_match() returns a slightly different
error message if not found - make grep accept both.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
Signed-off-by: Jethro Beekman <jethro@fortanix.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
| |
The `<(cmd)` redirection is specific to Bash. Update the shebang
accordingly.
Fixes: 63ab4fe3a191 ("ebtables: Avoid dropping policy when flushing")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
| |
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.
https://bugzilla.netfilter.org/show_bug.cgi?id=1558
|
|
|
|
|
|
|
|
|
|
| |
Unexpected output from iptables commands might mess up error-checking in
scripts for instance, so do a quick test of the most common commands.
Note: Test adds two rules to make sure flush command operates on a
non-empty chain.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
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>
|