]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
10 months agocache: populate chains on demand from error path
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:34:11 +0000 (12:34 +0200)] 
cache: populate chains on demand from error path

Updates on verdict maps that require many non-base chains are slowed
down due to fetching existing non-base chains into the cache.

Chains are only required for error reporting hints if kernel reports
ENOENT. Populate the cache from this error path only.

Similar approach already exists from rule ENOENT error path since:

  deb7c5927fad ("cmd: add misspelling suggestions for rule commands")

however, NFT_CACHE_CHAIN was toggled inconditionally for rule
commands, rendering this on-demand cache population useless.

before this patch, running Neels' nft_slew benchmark (peak values):

  created idx 4992 in 52587950 ns   (128 in 7122 ms)
  ...
  deleted idx  128 in 43542500 ns   (127 in 6187 ms)

after this patch:

  created idx 4992 in 11361299 ns   (128 in 1612 ms)
  ...
  deleted idx 1664 in  5239633 ns   (128 in 733 ms)

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: rule by index requires full cache
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:34:08 +0000 (12:34 +0200)] 
cache: rule by index requires full cache

In preparation for on-demand cache population with errors, set on
NFT_CACHE_FULL if rule index is used since this requires a full cache
with rules.

This is not a fix, index is already fetching a full cache before this
patch.

But follow up patches relax cache requirements, so add this patch in
first place to make sure index does not break.

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agotests: shell: add a few tests for nft -i
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:25:36 +0000 (12:25 +0200)] 
tests: shell: add a few tests for nft -i

Eric Garver recently provided a few tests for nft -i that helped
identify issues that resulted in reverting:

  e791dbe109b6 ("cache: recycle existing cache with incremental updates")

add these tests to tests/shell.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agodatatype: improve error reporting when time unit is not correct
Pablo Neira Ayuso [Wed, 14 Aug 2024 11:05:54 +0000 (13:05 +0200)] 
datatype: improve error reporting when time unit is not correct

Display:

  Wrong unit format, expecting bytes or kbytes or mbytes

instead of:

  Wrong rate format

Fixes: 6615676d825e ("src: add per-bytes limit")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agodatatype: reject rate in quota statement
Pablo Neira Ayuso [Wed, 14 Aug 2024 11:02:02 +0000 (13:02 +0200)] 
datatype: reject rate in quota statement

Bail out if rate are used:

 ruleset.nft:5:77-106: Error: Wrong rate format, expecting bytes or kbytes or mbytes
 add rule netdev firewall PROTECTED_IPS update @quota_temp_before { ip daddr quota over 45000 mbytes/second } add @quota_trigger { ip daddr }
                                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

improve error reporting while at this.

Fixes: 6615676d825e ("src: add per-bytes limit")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agotests: shell: skip vlan mangling testcase if egress is not support
Pablo Neira Ayuso [Sun, 11 Aug 2024 22:05:02 +0000 (00:05 +0200)] 
tests: shell: skip vlan mangling testcase if egress is not support

Add dependency on egress hook to skip this test in older kernels.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agodoc: add documentation about list hooks feature
Florian Westphal [Wed, 31 Jul 2024 16:51:05 +0000 (18:51 +0200)] 
doc: add documentation about list hooks feature

Add a brief segment about 'nft list hooks' and a summary
of the output format.

As nft.txt is quite large, split the additonal commands
into their own file.

The existing listing section is removed; list subcommand is
already mentioned in the relevant statement sections.

Reported-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agosrc: add egress support for 'list hooks'
Florian Westphal [Wed, 31 Jul 2024 16:51:04 +0000 (18:51 +0200)] 
src: add egress support for 'list hooks'

This was missing:  Also include the egress hooks when listing
the netdev family (or unspec).

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agosrc: drop obsolete hook argument form hook dump functions
Florian Westphal [Wed, 31 Jul 2024 16:51:03 +0000 (18:51 +0200)] 
src: drop obsolete hook argument form hook dump functions

since commit b98fee20bfe2 ("mnl: revisit hook listing"), handle.chain is
never set in this path, so 'hook' is always set to -1, so the hook arg
can be dropped.

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agosrc: mnl: make family specification more strict when listing
Florian Westphal [Wed, 31 Jul 2024 16:51:02 +0000 (18:51 +0200)] 
src: mnl: make family specification more strict when listing

make "nft list hooks <family>" more strict.

nft list hooks: query/list all NFPROTO_XXX values, i.e.
arp, bridge, ipv4, ipv6.

If a device is also given, then do include the netdev family for
the given device as well.

"nft list hooks arp" will only dump the hooks registered
for NFPROTO_ARP (or nothing at all if none are active).

"bridge", "ip", "ip6" will list the pre/in/forward/output/postrouting
hooks for these families, if any.

"inet" serves as an alias for "ip" and "ip6".

Link: https://lore.kernel.org/netfilter-devel/20240729153211.GA26048@breakpoint.cc/
Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agosrc: mnl: clean up hook listing code
Florian Westphal [Wed, 31 Jul 2024 16:51:01 +0000 (18:51 +0200)] 
src: mnl: clean up hook listing code

mnl_nft_dump_nf_hooks() can call itself for the UNSPEC case, this
avoids the second switch/case to handle printing for inet/unspec.

As for the error handling, 'nft list hooks' should not print an error,
even if nothing is printed, UNLESS there was also a lowlevel (syscall)
error from the kernel.

We don't want to indicate failure just because e.g. kernel doesn't support
NFPROTO_ARP.

This also fixes a display bug, 'nft list hooks device foo' would show hooks
registered for that device as 'bridge' family instead of the expected
'netdev' family.

This was because UNSPEC handling did not query 'netdev' family and did
pass the device name to the lowlevel function.  Add it, and pass NULL
device name for those families that don't support device attachment.

The lowelevel function currently always queries NFPROTO_NETDEV to handle
the 'inet' ingress case.

This is dubious, as 'inet ingress' is a pseudo-alias to netdev family
(inet itself is a pseudo-family that ends up registering for both ipv4
 and ipv6 hooks).

This is resolved in next patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agotests: shell: Extend table persist flag test a bit
Phil Sutter [Wed, 7 Aug 2024 19:37:39 +0000 (21:37 +0200)] 
tests: shell: Extend table persist flag test a bit

Using a co-process, assert owner flag is effective.

Signed-off-by: Phil Sutter <phil@nwl.cc>
11 months agooptimize: compare meta inner_desc pointers too
Florian Westphal [Thu, 8 Aug 2024 23:31:17 +0000 (01:31 +0200)] 
optimize: compare meta inner_desc pointers too

We can't merge if one referes inner and other outer header.
Payload checks this but meta did not.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 months agotests: shell: resolve check-tree.sh errors
Florian Westphal [Thu, 8 Aug 2024 10:25:32 +0000 (12:25 +0200)] 
tests: shell: resolve check-tree.sh errors

It prints a few errors like this:

ERR:  "tests/shell/testcases/chains/jump_to_base_chain" has no "tests/shell/testcases/chains/dumps/jump_to_base_chain.{nft,nodump}" file

For all of those, add the relevant .nft dump file.

Add a 'nodump' file in case the test doesn't print anything (e.g.
because the test checks that invalid ruleset fails validation).

Some tests have a .nft but not .json-nft, this is because json lacks
some features, in particular "typeof" and anonymous/implicit chains.

ERR:  "tests/shell/testcases/maps/delete_element_catchall" has no "tests/shell/testcases/maps/dumps/delete_element_catchall.{nft,nodump}" file
ERR:  "tests/shell/testcases/maps/dumps/delete_elem_catchall.nft" has no test "tests/shell/testcases/maps/delete_elem_catchall"

these two are related, rename the dump file to match the script name.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 months agotests: shell: move flowtable with bogus priority to correct location
Florian Westphal [Thu, 8 Aug 2024 09:24:26 +0000 (11:24 +0200)] 
tests: shell: move flowtable with bogus priority to correct location

This is an input file to be processed by "assert_failures" script.

Fixes: b40bebbcee36 ("rule: do not crash if to-be-printed flowtable lacks priority")
Signed-off-by: Florian Westphal <fw@strlen.de>
11 months agosrc: remove decnet support
Florian Westphal [Wed, 24 Jul 2024 23:48:23 +0000 (01:48 +0200)] 
src: remove decnet support

Removed two years ago with v6.1, ditch this from hook list code as well.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 months agoRevert "cache: recycle existing cache with incremental updates"
Pablo Neira Ayuso [Wed, 24 Jul 2024 07:38:33 +0000 (09:38 +0200)] 
Revert "cache: recycle existing cache with incremental updates"

This reverts commit e791dbe109b6dd891a63a4236df5dc29d7a4b863.

Eric Garver reported two issues:

- index with rule breaks, because NFT_CACHE_REFRESH is missing.
- simple set updates.

Moreover, the current process could populate the cache with objects for
listing commands (no generation ID is bumped), while another process
could update the ruleset. Leading to a inconsistent cache due to the
genid + 1 check.

This optimization needs more work and more tests for -i/--interactive,
revert it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
11 months agooptimize: skip variables in nat statements
Pablo Neira Ayuso [Thu, 18 Jul 2024 16:06:22 +0000 (18:06 +0200)] 
optimize: skip variables in nat statements

Do not hit assert():

  nft: optimize.c:486: rule_build_stmt_matrix_stmts: Assertion `k >= 0' failed.

variables are not supported by -o/--optimize at this stage.

Fixes: 9be404a153bc ("optimize: ignore existing nat mapping")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
11 months agobuild: Bump version to 1.1.0 v1.1.0
Pablo Neira Ayuso [Tue, 16 Jul 2024 20:20:19 +0000 (22:20 +0200)] 
build: Bump version to 1.1.0

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
11 months agoparser_bison: remove one more utf-8 character
Pablo Neira Ayuso [Tue, 16 Jul 2024 17:34:28 +0000 (19:34 +0200)] 
parser_bison: remove one more utf-8 character

Still one more in an error in the parser, replace it found via git grep.

Fixes: c92ec3b21979 ("src: remove utf-8 character in printf lines")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: connect chains to hook point
Florian Westphal [Wed, 10 Jul 2024 00:33:37 +0000 (02:33 +0200)] 
tests: shell: connect chains to hook point

These tests should fail because they contain a loop or exceed the jump stack.

But this depends on the kernel validating chains that are not bound to any
basechain/hook point.

Wire up the initial chain to filter type.

Without this tests will start to fail when kernel stops validating
chains that are not reachable by any base chain.

Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agotests: shell: test jump to basechain is rejected, even if there is no loop
Florian Westphal [Wed, 10 Jul 2024 21:02:08 +0000 (23:02 +0200)] 
tests: shell: test jump to basechain is rejected, even if there is no loop

Check that we can't jump to input hook from output.

Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agotests: shell: add more ruleset validation test cases
Florian Westphal [Sun, 7 Jul 2024 18:42:15 +0000 (20:42 +0200)] 
tests: shell: add more ruleset validation test cases

Passes fine on all tested kernel releases.

Same as existing tests, but try harder to fool the validation:

1. Add a ruleset where the jump that that exceeds 16 is "broken", i.e.
   c0 -> c1 ... -> c8
   c9-> c1 ... -> c16

Where c0 is a base chain, with a graph thats really a linear list
from c0 to c8 and c9 to c16 is a linear list not connected to the former
or a hook point.

Then try to link them either directly via jump/goto rule or indirectly
with a verdict map.

Try both unbound map with element doing 'goto c9' and then trying to add
vmap rule to c8 (must fail, creates link).

Then try reverse: with empty map, add vmap rule to c8 (should work, no
elements...).

Then, add map element with jump or goto to c9.  This should be rejected.

Try the same thing with a tproxy expression in a user-defined chain:
attempt to make it reachable from c0 (filter input), which is illegal.

Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agolibnftables: skip useable checks for /dev/stdin
Pablo Neira Ayuso [Tue, 9 Jul 2024 14:59:53 +0000 (16:59 +0200)] 
libnftables: skip useable checks for /dev/stdin

/dev/stdin is a placeholder, read() from STDIN_FILENO is used to fetch
the standard input into a buffer.

Since 5c2b2b0a2ba7 ("src: error reporting with -f and read from stdin")
stdin is stored in a buffer to fix error reporting.

This patch requires: ("parser_json: use stdin buffer if available")

Fixes: 149b1c95d129 ("libnftables: refuse to open onput files other than named pipes or regular files")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agoparser_json: use stdin buffer if available
Pablo Neira Ayuso [Tue, 9 Jul 2024 14:59:52 +0000 (16:59 +0200)] 
parser_json: use stdin buffer if available

Since 5c2b2b0a2ba7 ("src: error reporting with -f and read from stdin")
stdin is stored in a buffer, update json support to use it instead of
reading from /dev/stdin.

Some systems do not provide /dev/stdin symlink to /proc/self/fd/0
according to reporter (that mentions Yocto Linux as example).

Fixes: 935f82e7dd49 ("Support 'nft -f -' to read from stdin")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agolibnftables: fix crash when freeing non-malloc'd address
Florian Westphal [Wed, 10 Jul 2024 13:25:58 +0000 (15:25 +0200)] 
libnftables: fix crash when freeing non-malloc'd address

dirname may return static pointer:
munmap_chunk(): invalid pointer
20508 Aborted  nft -f test

Fixes: 6ef04f99382c ("libnftables: search for default include path last")
Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agooptimize: clone counter before insertion into set element
Pablo Neira Ayuso [Fri, 5 Jul 2024 12:03:33 +0000 (14:03 +0200)] 
optimize: clone counter before insertion into set element

The counter statement that is zapped from the rule needs to be cloned
before inserting it into each set element.

Fixes: 686ab8b6996e ("optimize: do not remove counter in verdict maps")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agoparser_bison: remove deprecated flow statement
Pablo Neira Ayuso [Thu, 4 Jul 2024 21:59:52 +0000 (23:59 +0200)] 
parser_bison: remove deprecated flow statement

Dynamic set/maps are used these days to represent what
3ed5e31f4a32 ("src: add flow statement") provides.

Unlikely meter statement, this statement was never documented
other than in the source code. Ditch it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agoparser_bison: recursive table declaration in deprecated meter statement
Pablo Neira Ayuso [Tue, 2 Jul 2024 22:08:01 +0000 (00:08 +0200)] 
parser_bison: recursive table declaration in deprecated meter statement

This is allowing for recursive table NAME declarations such as:

 ... table xyz1 table xyz2 { ... }

remove it.

Fixes: 3ed5e31f4a32 ("src: add flow statement")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: cover set element deletion in maps
Pablo Neira Ayuso [Wed, 3 Jul 2024 14:29:26 +0000 (16:29 +0200)] 
tests: shell: cover set element deletion in maps

Extend existing coverage to deal with set element deletion, including
catchall elements too.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agointervals: fix element deletions with maps
Pablo Neira Ayuso [Wed, 3 Jul 2024 14:29:26 +0000 (16:29 +0200)] 
intervals: fix element deletions with maps

Set element deletion in maps (including catchall elements) does not work.

 # nft delete element ip x m { \* }
 BUG: invalid range expression type catch-all set element
 nft: src/expression.c:1472: range_expr_value_low: Assertion `0' failed.
 Aborted

Call interval_expr_key() to fetch expr->left in the mapping but use the
expression that represents the mapping because it provides access to the
EXPR_F_REMOVE flags.

Moreover, assume maximum value for catchall expression by means of the
expr->len to reuse the existing code to check if the element to be
deleted really exists.

Fixes: 3e8d934e4f72 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agosegtree: set on EXPR_F_KERNEL flag for catchall elements in the cache
Pablo Neira Ayuso [Thu, 4 Jul 2024 12:35:07 +0000 (14:35 +0200)] 
segtree: set on EXPR_F_KERNEL flag for catchall elements in the cache

Catchall set element deletion requires this flag to be set on,
otherwise it bogusly reports that such element does not exist
in the set.

Fixes: f1cc44edb218 ("src: add EXPR_F_KERNEL to identify expression in the kernel")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agoevaluate: set on expr->len for catchall set elements
Pablo Neira Ayuso [Thu, 4 Jul 2024 14:38:22 +0000 (16:38 +0200)] 
evaluate: set on expr->len for catchall set elements

Catchall elements coming from the parser provide expr->len == 0.
However, the existing mergesort implementation requires expr->len to be
set up to the length of the set key to properly sort elements.

In particular, set element deletion leverages such list sorting to find
if elements exists in the set.

Fixes: 419d19688688 ("src: add set element catch-all support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: check for removing table via handle with incorrect family
Pablo Neira Ayuso [Thu, 27 Jun 2024 00:51:57 +0000 (02:51 +0200)] 
tests: shell: check for removing table via handle with incorrect family

This test checks for upstream commit:

f6e1532a2697 ("netfilter: nf_tables: validate family when identifying table via handle")

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agosrc: add string preprocessor and use it for log prefix string
Pablo Neira Ayuso [Tue, 18 Jun 2024 12:26:31 +0000 (14:26 +0200)] 
src: add string preprocessor and use it for log prefix string

Add a string preprocessor to identify and replace variables in a string.
Rework existing support to variables in log prefix strings to use it.

Fixes: e76bb3794018 ("src: allow for variables in the log prefix string")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: py: drop redundant JSON outputs
Pablo Neira Ayuso [Tue, 18 Jun 2024 15:31:46 +0000 (17:31 +0200)] 
tests: py: drop redundant JSON outputs

8abe71f862e6 ("tests: py: Warn if recorded JSON output matches the input")
adds a warning on duplicated JSON outputs.

Remove them when running tests with -j:

  WARNING: Recorded JSON output matches input for: icmp code { 2, 4, 54, 33, 56}

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agolibnftables: search for default include path last
Pablo Neira Ayuso [Sat, 15 Jun 2024 08:35:06 +0000 (10:35 +0200)] 
libnftables: search for default include path last

The default include path is searched for files before include paths
specified via -I/--include.

Search for default include path after user-specified include paths to
allow users for test nftables configurations spanning multiple files
without overwriting the globally installed ones.

See:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20220627222304.93139-1-dxld@darkboxed.org/

Reported-by: Daniel Gröber <dxld@darkboxed.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agolibnftables: add base directory of -f/--filename to include path
Pablo Neira Ayuso [Fri, 14 Jun 2024 09:24:19 +0000 (11:24 +0200)] 
libnftables: add base directory of -f/--filename to include path

This patch adds an include path relative to the current (the including)
file's directory.

Users of -f/--filename have to explicitly specify -I with a redundant
path to find included files in the main file, eg.

 # nft -I /path/to/files -f /path/to/files/ruleset.nft

Assuming:

 # cat /path/to/files/ruleset.nft
 include "file1.nft"
 include "file2.nft"
 include "file3.nft"

The follow up patch ("libnftables: search for default include path last")
is also required according to what it is described in the manpage update
coming with this patch.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1661
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: add test case for reset tcp warning
Florian Westphal [Tue, 4 Jun 2024 12:01:49 +0000 (14:01 +0200)] 
tests: shell: add test case for reset tcp warning

tcp reset rule + nftrace 1 triggers (harmless) splat from flow dissector:

 WARNING: CPU: 2 PID: 145809 at net/core/flow_dissector.c:1104 __skb_flow_dissect+0x19d4/0x5cc0
  __skb_get_hash+0xa8/0x220
  nft_trace_init+0x2ff/0x3b0
  nft_do_chain+0xb04/0x1370
  nft_do_chain_inet+0xc5/0x2e0
  nf_hook_slow+0xa0/0x1d0
  ip_local_out+0x14/0x90
  nf_send_reset+0x94e/0xbd0
  nft_reject_inet_eval+0x45e/0x690
  nft_do_chain+0x220/0x1370
  nf_hook_slow+0xa0/0x1d0
  ip_local_deliver+0x23f/0x2d0

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agodynset: avoid errouneous assert with ipv6 concat data
Son Dinh [Tue, 9 Apr 2024 06:23:31 +0000 (16:23 +1000)] 
dynset: avoid errouneous assert with ipv6 concat data

nft add rule ip6 table-test chain-1 update @map-X { ip6 saddr : 1000::1 . 5001 }
nft: src/netlink_linearize.c:873: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.
Aborted (core dumped)

This is because we pass the EXPR_SET_ELEM expr to the register allocation,
which will make it reserve 1 128 bit register / 16 bytes.

This happens to be enough for most cases, but its not for ipv6 concat data.
Pass the actual key and data instead: This will reserve enough space to
hold a possible concat expression.

Also add test cases.

Signed-off-by: Son Dinh <dinhtrason@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agorule: do not crash if to-be-printed flowtable lacks priority
Florian Westphal [Fri, 12 Jan 2024 12:32:17 +0000 (13:32 +0100)] 
rule: do not crash if to-be-printed flowtable lacks priority

Print an empty flowtable rather than crashing when dereferencing
flowtable->priority.expr (its NULL).

Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agotests: use common shebang in "packetpath/flowtables" test
Thomas Haller [Fri, 9 Feb 2024 12:13:04 +0000 (13:13 +0100)] 
tests: use common shebang in "packetpath/flowtables" test

"./tools/check-tree.sh" checks for a certain shebang. Either `/bin/bash` or
`/bin/bash -e`. No other are currently allowed, because it makes sense to be
strict/consistent and there is no need such flexibility.

Move the "-x" to a later command.

Note that "set -x" may not be a good choice anyway. If you want to debug
a test and see the shell commands, you could just run

  $ ./tests/shell/run-tests.sh tests/shell/testcases/packetpath/flowtables -x

That will automatically use `/bin/bash -x` as interpreter. And that
works for all tests the same. This is also the reason why
"check-tree.sh" checks for a well-known shebang. Because the "-x" option
of the test runner mangles the shebang, but for that it needs to
understand it.

Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
12 months agotests: shell: skip NFTA_RULE_POSITION_ID tests if kernel does not support it
Pablo Neira Ayuso [Wed, 12 Jun 2024 23:20:17 +0000 (01:20 +0200)] 
tests: shell: skip NFTA_RULE_POSITION_ID tests if kernel does not support it

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: skip ipsec tests if kernel does not support it
Pablo Neira Ayuso [Wed, 12 Jun 2024 22:42:41 +0000 (00:42 +0200)] 
tests: shell: skip ipsec tests if kernel does not support it

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: skip ip option tests if kernel does not support it
Pablo Neira Ayuso [Wed, 12 Jun 2024 22:37:48 +0000 (00:37 +0200)] 
tests: shell: skip ip option tests if kernel does not support it

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
12 months agotests: shell: add dependencies to skip unsupported tests in older kernels
Pablo Neira Ayuso [Wed, 12 Jun 2024 22:09:56 +0000 (00:09 +0200)] 
tests: shell: add dependencies to skip unsupported tests in older kernels

Update tests which contain unsupported features in older kernels.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agocmd: skip variable set elements when collapsing commands
Pablo Neira Ayuso [Tue, 11 Jun 2024 15:40:23 +0000 (17:40 +0200)] 
cmd: skip variable set elements when collapsing commands

ASAN reports an issue when collapsing commands that represent an element
through a variable:

include/list.h:60:13: runtime error: member access within null pointer of type 'struct list_head'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==11398==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7ffb77cf09c2 bp 0x7ffc818267c0 sp 0x7ffc818267a0 T0)
==11398==The signal is caused by a WRITE memory access.
==11398==Hint: address points to the zero page.
    #0 0x7ffb77cf09c2 in __list_add include/list.h:60
    #1 0x7ffb77cf0ad9 in list_add_tail include/list.h:87
    #2 0x7ffb77cf0e72 in list_move_tail include/list.h:169
    #3 0x7ffb77cf86ad in nft_cmd_collapse src/cmd.c:478
    #4 0x7ffb77da9f16 in nft_evaluate src/libnftables.c:531
    #5 0x7ffb77dac471 in __nft_run_cmd_from_filename src/libnftables.c:720
    #6 0x7ffb77dad703 in nft_run_cmd_from_filename src/libnftables.c:807

Skip such commands to address this issue.

This patch also extends tests/shell to cover for this bug.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1754
Fixes: 498a5f0c219d ("rule: collapse set element commands")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agocmd: provide better hint if chain is already declared with different type/hook/priority
Pablo Neira Ayuso [Mon, 10 Jun 2024 17:36:21 +0000 (19:36 +0200)] 
cmd: provide better hint if chain is already declared with different type/hook/priority

Display the following error in such case:

  ruleset.nft:7:9-52: Error: Chain "input" already exists in table ip 'filter' with different declaration
          type filter hook postrouting priority filter;
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

instead of reporting a misleading unsupported chain type when updating
an existing chain with different type/hook/priority.

Fixes: 573788e05363 ("src: improve error reporting for unsupported chain type")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agomonitor: too large shift exponent displaying payload expression
Pablo Neira Ayuso [Mon, 10 Jun 2024 17:08:20 +0000 (19:08 +0200)] 
monitor: too large shift exponent displaying payload expression

ASAN reports too large shift exponent when displaying traces for raw
payload expression:

  trace id ec23e848 ip x y packet: oif "wlan0" src/netlink.c:2100:32: runtime error: shift exponent 1431657095 is too large for 32-bit type 'int'

skip if proto_unknown_template is set on in this payload expression.

Fixes: be5d9120e81e ("nft monitor [ trace ]")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agoscanner: inet_pton() allows for broader IPv4-Mapped IPv6 addresses
Pablo Neira Ayuso [Tue, 4 Jun 2024 18:01:51 +0000 (20:01 +0200)] 
scanner: inet_pton() allows for broader IPv4-Mapped IPv6 addresses

inet_pton() allows for broader IPv4-Mapped IPv6 address syntax than
those specified by rfc4291 Sect.2.5.5. This patch extends the scanner to
support them for compatibility reasons. This allows to represent the
last 4 bytes of an IPv6 address as an IPv4 address.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1730
Fixes: fd513de78bc0 ("scanner: IPv4-Mapped IPv6 addresses support")
Fixes: 3f82ef3d0dbf ("scanner: Support rfc4291 IPv4-compatible addresses")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agocache: recycle existing cache with incremental updates
Pablo Neira Ayuso [Mon, 27 May 2024 11:26:01 +0000 (13:26 +0200)] 
cache: recycle existing cache with incremental updates

Cache tracking has improved over time by incrementally adding/deleting
objects when evaluating commands that are going to be sent to the kernel.

nft_cache_is_complete() already checks that the cache contains objects
that are required to handle this batch of commands by comparing cache
flags.

Infer from the current generation ID if no other transaction has
invalidated the existing cache, this allows to skip unnecessary cache
flush then refill situations which slow down incremental updates.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agocache: check for NFT_CACHE_REFRESH in current requested cache too
Pablo Neira Ayuso [Mon, 27 May 2024 16:12:05 +0000 (18:12 +0200)] 
cache: check for NFT_CACHE_REFRESH in current requested cache too

NFT_CACHE_REFRESH is set on inconditionally by ruleset list commands to
deal with stateful information in this ruleset. This flag results in
dropping the existing cache and fully fetching all objects from the
kernel.

Set on this flag for reset commands too, this is missing.

List/reset commands allow for filtering by specific family and object,
therefore, NFT_CACHE_REFRESH also signals that the cache is partially
populated.

Check if this flag is requested by the current list/reset command, as
well as cache->flags which represents the cache after the _previous_
list of commands.

A follow up patch allows to recycle the existing cache if the flags
report that the same objects are already available in the cache,
NFT_CACHE_REFRESH is useful to report that cache cannot be recycled.

Fixes: 407c54f71255 ("src: cache gets out of sync in interactive mode")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agoevaluate: Fix incorrect checking the `base` variable in case of IPV6
Maks Mishin [Wed, 15 May 2024 20:25:03 +0000 (23:25 +0300)] 
evaluate: Fix incorrect checking the `base` variable in case of IPV6

Found by RASU JSC.

Fixes: 2b29ea5f3c3e ("src: ct: add eval part to inject dependencies for ct saddr/daddr")
Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agotests: shell: add vlan mangling test case
Pablo Neira Ayuso [Fri, 10 May 2024 11:49:01 +0000 (13:49 +0200)] 
tests: shell: add vlan mangling test case

As a follow up for:

      74cf3d16d8e9 ("tests: shell: add vlan match test case")

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agotests: shell: add vlan double tagging match simple test case
Pablo Neira Ayuso [Fri, 10 May 2024 10:30:28 +0000 (12:30 +0200)] 
tests: shell: add vlan double tagging match simple test case

As a follow up for:

  74cf3d16d8e9 ("tests: shell: add vlan match test case")

Add basic test for q-in-q matching support.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agoevaluate: bogus protocol conflicts in vlan with implicit dependencies
Pablo Neira Ayuso [Wed, 15 May 2024 17:23:49 +0000 (19:23 +0200)] 
evaluate: bogus protocol conflicts in vlan with implicit dependencies

The following command:

 # nft add rule netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321

fails with:

Error: conflicting link layer protocols specified: ether vs. vlan
netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321
                                                    ^^^^^^^

Users can work around this issue by prepending an explicit match for
vlan ethertype field, that is:

ether type vlan ip saddr 10.1.1.1 ...
        ^-------------^

but nft should really handle this itself.

The error above is triggered by the following check in
resolve_ll_protocol_conflict():

       /* This payload and the existing context don't match, conflict. */
       if (pctx->protocol[base + 1].desc != NULL)
               return 1;

This check was added by 39f15c243912 ("nft: support listing expressions
that use non-byte header fields") and f7d5590688a6 ("tests: vlan tests")
to deal with conflicting link layer protocols, for instance:

 ether type ip vlan id 1

this is matching ethertype ip at offset 12, but then it matches for vlan
id at offset 14 which is not present given the previous check.

One possibility is to remove such check, but nft does not bail out for
the example above and it results in bytecode that never matches:

 # nft --debug=netlink netdev x y ether type ip vlan id 10
 netdev x y
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
  [ cmp eq reg 1 0x00000008 ]                     <---- ip
  [ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
  [ cmp eq reg 1 0x00000081 ]                     <---- vlan
  [ payload load 2b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
  [ cmp eq reg 1 0x00000a00 ]

This is due to resolve_ll_protocol_conflict() which deals with the
conflict by updating protocol context and emitting an implicit
dependency, but there is already an explicit match coming from the user.

This patch adds a new helper function to check if an implicit dependency
clashes with an existing statement, which results in:

# nft add rule netdev x y ether type ip vlan id 1
        Error: conflicting statements
        add rule netdev x y ether type ip vlan id 1
                            ^^^^^^^^^^^^^ ~~~~~~~

Theoretically, no duplicated implicit dependency should ever be emitted
if protocol context is correctly handled.

Only implicit payload expressions are considered at this stage for this
conflict check, this patch can be extended to deal with other dependency
types.

Fixes: 39f15c243912 ("nft: support listing expressions that use non-byte header fields")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agodoc: drop duplicate ARP HEADER EXPRESSION
谢致邦 (XIE Zhibang) [Tue, 28 May 2024 12:07:28 +0000 (12:07 +0000)] 
doc: drop duplicate ARP HEADER EXPRESSION

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
13 months agoevaluate: add support for variables in map expressions
Jeremy Sowden [Mon, 29 Apr 2024 19:27:53 +0000 (20:27 +0100)] 
evaluate: add support for variables in map expressions

It is possible to use a variable to initialize a map, which is then used
in a map statement:

  define dst_map = { ::1234 : 5678 }

  table ip6 nat {
    map dst_map {
      typeof ip6 daddr : tcp dport;
      elements = $dst_map
    }
    chain prerouting {
      ip6 nexthdr tcp redirect to ip6 daddr map @dst_map
    }
  }

However, if one tries to use the variable directly in the statement:

  define dst_map = { ::1234 : 5678 }

  table ip6 nat {
    chain prerouting {
      ip6 nexthdr tcp redirect to ip6 daddr map $dst_map
    }
  }

nft rejects it:

  /space/azazel/tmp/ruleset.1067161.nft:5:47-54: Error: invalid mapping expression variable
      ip6 nexthdr tcp redirect to ip6 daddr map $dst_map
                                  ~~~~~~~~~     ^^^^^^^^
It also rejects variables in stateful object statements:

  define quota_map = { 192.168.10.123 : "user123", 192.168.10.124 : "user124" }

  table ip nat {
    quota user123 { over 20 mbytes }
    quota user124 { over 20 mbytes }
    chain prerouting {
      quota name ip saddr map $quota_map
    }
  }

thus:

  /space/azazel/tmp/ruleset.1067161.nft:15:29-38: Error: invalid mapping expression variable
      quota name ip saddr map $quota_map
                 ~~~~~~~~     ^^^^^^^^^^

Add support for these uses together with some test-cases.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1067161
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
13 months agoevaluate: handle invalid mapping expressions in stateful object statements gracefully.
Jeremy Sowden [Mon, 29 Apr 2024 19:27:52 +0000 (20:27 +0100)] 
evaluate: handle invalid mapping expressions in stateful object statements gracefully.

Currently, they are reported as assertion failures:

  BUG: invalid mapping expression variable
  nft: src/evaluate.c:4618: stmt_evaluate_objref_map: Assertion `0' failed.
  Aborted

Instead, report them more informatively as errors:

  /space/azazel/tmp/ruleset.1067161.nft:15:29-38: Error: invalid mapping expression variable
      quota name ip saddr map $quota_map
                 ~~~~~~~~     ^^^^^^^^^^

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
14 months agotests: shell: combine dormant flag with netdevice removal
Pablo Neira Ayuso [Wed, 24 Apr 2024 18:43:58 +0000 (20:43 +0200)] 
tests: shell: combine dormant flag with netdevice removal

Exercise table is dormant and netdevice is gone combination.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
14 months agotests: packetpath: add check for drop policy
Florian Westphal [Wed, 3 Apr 2024 10:28:12 +0000 (12:28 +0200)] 
tests: packetpath: add check for drop policy

check that policy can be changed from accept to drop and that the kernel
acts on this.

Signed-off-by: Florian Westphal <fw@strlen.de>
14 months agodoc: nft.8: Highlight "hook" in flowtable description
Phil Sutter [Wed, 24 Apr 2024 21:48:56 +0000 (23:48 +0200)] 
doc: nft.8: Highlight "hook" in flowtable description

Lacking an explicit description of possible hook values, emphasising the
word in the description text should draw readers' attention in the right
direction.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agodoc: nft.8: Fix markup in ct expectation synopsis
Phil Sutter [Wed, 24 Apr 2024 21:46:11 +0000 (23:46 +0200)] 
doc: nft.8: Fix markup in ct expectation synopsis

Just a missing asterisk somewhere.

Fixes: 1dd08fcfa07a4 ("src: add ct expectations support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agotests: shell: Fix for maps/typeof_maps_add_delete with ASAN
Phil Sutter [Wed, 24 Apr 2024 21:41:59 +0000 (23:41 +0200)] 
tests: shell: Fix for maps/typeof_maps_add_delete with ASAN

With both KASAN and ASAN enabled, my VM is too slow so the ping-induced
set entry times out before the test checks its existence. Increase its
timeout to 2s, seems to do the trick.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agojson: Fix for memleak in __binop_expr_json
Phil Sutter [Wed, 24 Apr 2024 21:35:00 +0000 (23:35 +0200)] 
json: Fix for memleak in __binop_expr_json

When merging the JSON arrays generated for LHS and RHS of nested binop
expressions, the emptied array objects leak if their reference is not
decremented.

Fix this and tidy up other spots which did it right already by
introducing a json_array_extend wrapper.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 0ac39384fd9e4 ("json: Accept more than two operands in binary expressions")
Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agoAdd support for table's persist flag
Phil Sutter [Fri, 15 Dec 2023 00:10:39 +0000 (01:10 +0100)] 
Add support for table's persist flag

Bison parser lacked support for passing multiple flags, JSON parser
did not support table flags at all.

Document also 'owner' flag (and describe their relationship in nft.8.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agotests: shell: Avoid escape chars when printing to non-terminals
Phil Sutter [Sat, 23 Mar 2024 02:09:41 +0000 (03:09 +0100)] 
tests: shell: Avoid escape chars when printing to non-terminals

Print the 'EXECUTING' status line only if stdout is a terminal, the
mandatory following escape sequence to delete it messes up log file
contents.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agotests: py: Drop needless recorded JSON outputs
Phil Sutter [Fri, 22 Mar 2024 14:18:50 +0000 (15:18 +0100)] 
tests: py: Drop needless recorded JSON outputs

These match the input already, no need to track them.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agotests: py: Warn if recorded JSON output matches the input
Phil Sutter [Fri, 22 Mar 2024 14:04:40 +0000 (15:04 +0100)] 
tests: py: Warn if recorded JSON output matches the input

Actively support spring-cleaning by nagging callers.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agotests: py: Fix some JSON equivalents
Phil Sutter [Fri, 22 Mar 2024 13:00:26 +0000 (14:00 +0100)] 
tests: py: Fix some JSON equivalents

Make sure they match the standard syntax input as much as possible.

For some reason inet/tcp.t.json was using plain arrays in place of
binary OR expressions in many cases. These arrays are interpreted as
list expressions, which seems to be semantically identical but the goal
here is to present an accurate equivalent to the rule in standard
syntax.

Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agomergesort: Avoid accidental set element reordering
Phil Sutter [Fri, 22 Mar 2024 12:31:10 +0000 (13:31 +0100)] 
mergesort: Avoid accidental set element reordering

In corner cases, expr_msort_cmp() may return 0 for two non-identical
elements. An example are ORed tcp flags: 'syn' and 'syn | ack' are
considered the same value since expr_msort_value() reduces the latter to
its LHS.

Keeping the above in mind and looking at how list_expr_sort() works: The
list in 'head' is cut in half, the first half put into the temporary
list 'list' and finally 'list' is merged back into 'head' considering
each element's position. Shall expr_msort_cmp() return 0 for two
elements, the one from 'list' ends up after the one in 'head', thus
reverting their previous ordering.

The practical implication is that output never matches input for the
sample set '{ syn, syn | ack }' as the sorting after delinearization in
netlink_list_setelems() keeps swapping the elements. Out of coincidence,
the commit this fixes itself illustrates the use-case this breaks,
namely tracking a ruleset in git: Each ruleset reload will trigger an
update to the stored dump.

This change breaks interval set element deletion because __set_delete()
implicitly relies upon this reordering of duplicate entries by inserting
a clone of the one to delete into the start (via list_move()) and after
sorting assumes the clone will end up right behind the original. Fix
this by calling list_move_tail() instead.

Fixes: 14ee0a979b622 ("src: sort set elements in netlink_get_setelems()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
14 months agojson: Accept more than two operands in binary expressions
Phil Sutter [Wed, 20 Mar 2024 14:54:54 +0000 (15:54 +0100)] 
json: Accept more than two operands in binary expressions

The most common use case is ORing flags like

| syn | ack | rst

but nft seems to be fine with less intuitive stuff like

| meta mark set ip dscp << 2 << 3

so support all of them.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agodoc: nft.8: Two minor synopsis fixups
Phil Sutter [Thu, 21 Mar 2024 13:57:41 +0000 (14:57 +0100)] 
doc: nft.8: Two minor synopsis fixups

The curly braces in 'add table' are to be put literally, so need to be
bold. Also, they are optional unless either one (or both) of 'comment'
and 'flags' are specified.

The 'add chain' synopsis contained a stray tick, messing up the
following markup.

Fixes: 7fd67ce121f86 ("doc: fix synopsis of named counter, quota and ct {helper,timeout,expect}")
Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: shell: check for reset tcp options support
Pablo Neira Ayuso [Mon, 8 Apr 2024 21:05:28 +0000 (23:05 +0200)] 
tests: shell: check for reset tcp options support

Fixes: 59a33d08ab3a ("parser: tcpopt: fix tcp option parsing with NUM + length field")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: shell: maps/{vmap_unary,named_limits} require pipapo set backend
Pablo Neira Ayuso [Mon, 8 Apr 2024 20:56:00 +0000 (22:56 +0200)] 
tests: shell: maps/{vmap_unary,named_limits} require pipapo set backend

... sets/typeof_sets_concat needs it too.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: shell: chains/{netdev_netns_gone,netdev_chain_dev_gone} require inet/ingress...
Pablo Neira Ayuso [Mon, 8 Apr 2024 20:50:06 +0000 (22:50 +0200)] 
tests: shell: chains/{netdev_netns_gone,netdev_chain_dev_gone} require inet/ingress support

Fixes: 6847a7ce0fc9 ("tests: shell: cover netns removal for netdev and inet/ingress basechains")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: shell: payload matching requires egress support
Pablo Neira Ayuso [Mon, 8 Apr 2024 18:25:12 +0000 (20:25 +0200)] 
tests: shell: payload matching requires egress support

Older kernels do not support for egress hook.

Fixes: 84da729e067a ("tests: shell: add test to cover payload transport match and mangle")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: py: complete icmp and icmpv6 update
Pablo Neira Ayuso [Thu, 4 Apr 2024 11:56:39 +0000 (13:56 +0200)] 
tests: py: complete icmp and icmpv6 update

Update json update and leftover payload update to complete
5fecd2a6ef61 ("src: disentangle ICMP code types").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agosrc: disentangle ICMP code types
Pablo Neira Ayuso [Mon, 1 Apr 2024 22:28:24 +0000 (00:28 +0200)] 
src: disentangle ICMP code types

Currently, ICMP{v4,v6,inet} code datatypes only describe those that are
supported by the reject statement, but they can also be used for icmp
code matching. Moreover, ICMP code types go hand-to-hand with ICMP
types, that is, ICMP code symbols depend on the ICMP type.

Thus, the output of:

  nft describe icmp_code

look confusing because that only displays the values that are supported
by the reject statement.

Disentangle this by adding internal datatypes for the reject statement
to handle the ICMP code symbol conversion to value as well as ruleset
listing.

The existing icmp_code, icmpv6_code and icmpx_code remain in place. For
backward compatibility, a parser function is defined in case an existing
ruleset relies on these symbols.

As for the manpage, move existing ICMP code tables from the DATA TYPES
section to the REJECT STATEMENT section, where this really belongs to.
But the icmp_code and icmpv6_code table stubs remain in the DATA TYPES
section because that describe that this is an 8-bit integer field.

After this patch:

 # nft describe icmp_code
 datatype icmp_code (icmp code) (basetype integer), 8 bits
 # nft describe icmpv6_code
 datatype icmpv6_code (icmpv6 code) (basetype integer), 8 bits
 # nft describe icmpx_code
 datatype icmpx_code (icmpx code) (basetype integer), 8 bits

do not display the symbol table of the reject statement anymore.

icmpx_code_type is not used anymore, but keep it in place for backward
compatibility reasons.

And update tests/shell accordingly.

Fixes: 5fdd0b6a0600 ("nft: complete reject support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agonetlink_delinearize: unused code in reverse cross-day meta hour range
Pablo Neira Ayuso [Wed, 27 Mar 2024 17:42:53 +0000 (18:42 +0100)] 
netlink_delinearize: unused code in reverse cross-day meta hour range

f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
reverses a cross-day range expressed as "22:00"-"02:00" UTC time into
!= "02:00"-"22:00" so meta hour ranges works.

Listing is however confusing, hence, 44d144cd593e ("netlink_delinearize:
reverse cross-day meta hour range") introduces code to reverse a cross-day.

However, it also adds code to reverse a range in == to-from form
(assuming OP_IMPLICIT) which is never exercised from the listing path
because the range expression is not currently used, instead two
instructions (cmp gte and cmp lte) are used to represent the range.
Remove this branch otherwise a reversed notation will be used to display
meta hour ranges once the range instruction is to represent this.

Add test for cross-day scenario in EADT timezone.

Fixes: 44d144cd593e ("netlink_delinearize: reverse cross-day meta hour range")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: shell: packetpath/flowtables: open all temporary files in /tmp
Jeremy Sowden [Sun, 24 Mar 2024 14:59:08 +0000 (14:59 +0000)] 
tests: shell: packetpath/flowtables: open all temporary files in /tmp

The test used to do I/O over a named pipe in $PWD, until Phil changed it
to create the pipe in /tmp.  However, he missed one `socat` command.
Update that too.

Fixes: 3a9f29e21726 ("tests: shell: packetpath/flowtables: Avoid spurious EPERM")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agotests: meta_time: fix dump validation failure
Florian Westphal [Tue, 2 Apr 2024 13:31:19 +0000 (15:31 +0200)] 
tests: meta_time: fix dump validation failure

[DUMP FAIL]  1/1 tests/shell/testcases/listing/meta_time

This dump file validates only correctly for TZ=UTC-1 (i.e., CET).
Time dumps cannot be validated in a portable way, the dump depends on TZ.

As the test already does dump valiation with different TZ values, remove
the dump file again.

Signed-off-by: Florian Westphal <fw@strlen.de>
15 months agotests: shell: add regression test for double-free crash bug
Florian Westphal [Fri, 8 Mar 2024 19:57:26 +0000 (20:57 +0100)] 
tests: shell: add regression test for double-free crash bug

BUG: KASAN: slab-use-after-free in nf_tables_set_elem_destroy+0x55/0x160
Call Trace:
 nf_tables_set_elem_destroy+0x55/0x160
 nf_tables_set_elem_destroy+0x55/0x160
 nft_pipapo_destroy+0x3b4/0x5a0
 nft_set_destroy+0x118/0x3a0
 nf_tables_trans_destroy_work+0x4f2/0xa80

This is a test case for the bug fiex with kernel commit
b0e256f3dd2b ("netfilter: nft_set_pipapo: release elements in clone only from destroy path").

Reported-by: lonial con <kongln9170@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
15 months agodatatype: use DTYPE_F_PREFIX only for IP address datatype
Pablo Neira Ayuso [Thu, 21 Mar 2024 11:40:53 +0000 (12:40 +0100)] 
datatype: use DTYPE_F_PREFIX only for IP address datatype

DTYPE_F_PREFIX flag provides a hint to the netlink delinearize path to
use prefix notation.

It seems use of prefix notation in meta mark causes confusion, users
expect to see prefix in the listing only in IP address datatypes.

Untoggle this flag so (more lengthy) binop output such as:

  meta mark & 0xffffff00 == 0xffffff00

is used instead.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1739
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agoevaluate: display "Range negative size" error
Pablo Neira Ayuso [Tue, 19 Mar 2024 18:50:00 +0000 (19:50 +0100)] 
evaluate: display "Range negative size" error

zero length ranges now allowed, therefore, update error message to refer
to negative ranges which are not possible.

Fixes: 7a6e16040d65 ("evaluate: allow for zero length ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agonetlink_delinearize: reverse cross-day meta hour range
Pablo Neira Ayuso [Tue, 19 Mar 2024 18:21:04 +0000 (19:21 +0100)] 
netlink_delinearize: reverse cross-day meta hour range

f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
reverses the hour range in case that a cross-day range is used, eg.

  meta hour "03:00"-"14:00" counter accept

which results in (Sidney, Australia AEDT time):

  meta hour != "14:00"-"03:00" counter accept

kernel handles time in UTC, therefore, cross-day range may not be
obvious according to local time.

The ruleset listing above is not very intuitive to the reader depending
on their timezone, therefore, complete netlink delinearize path to
reverse the cross-day meta range.

Update manpage to recommend to use a range expression when matching meta
hour range. Recommend range expression for meta time and meta day too.

Extend testcases/listing/meta_time to cover for this scenario.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1737
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agonetlink_delinearize: restore binop syntax when listing ruleset for flags
Pablo Neira Ayuso [Mon, 18 Mar 2024 12:10:55 +0000 (13:10 +0100)] 
netlink_delinearize: restore binop syntax when listing ruleset for flags

c3d57114f119 ("parser_bison: add shortcut syntax for matching flags
without binary operations") provides a similar syntax to iptables using
a prefix representation for flag matching.

Restore original representation using binop when listing the ruleset.
The parser still accepts the prefix notation for backward compatibility.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agosrc: do not merge a set with a erroneous one
Florian Westphal [Fri, 12 Jan 2024 12:19:26 +0000 (13:19 +0100)] 
src: do not merge a set with a erroneous one

The included sample causes a crash because we attempt to
range-merge a prefix expression with a symbolic expression.

The first set is evaluated, the symbol expression evaluation fails
and nft queues an error message ("Could not resolve hostname").

However, nft continues evaluation.

nft then encounters the same set definition again and merges the
new content with the preceeding one.

But the first set structure is dodgy, it still contains the
unresolved symbolic expression.

That then makes nft crash (assert) in the set internals.

There are various different incarnations of this issue, but the low
level set processing code does not allow for any partially transformed
expressions to still remain.

Before:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
BUG: invalid range expression type binop
nft: src/expression.c:1479: range_expr_value_low: Assertion `0' failed.

After:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
invalid_range_expr_type_binop:4:18-25: Error: Could not resolve hostname: Name or service not known
elements = { 1&.141.0.1 - 192.168.0.2}
             ^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agodoc: libnftables-json: Drop invalid ops from match expression
Phil Sutter [Fri, 22 Sep 2023 16:43:11 +0000 (18:43 +0200)] 
doc: libnftables-json: Drop invalid ops from match expression

These make no sense there and are listed again in BINARY OPERATION.

Fixes: 872f373dc50f7 ("doc: Add JSON schema documentation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: shell: Fix one json-nft dump for reordered output
Phil Sutter [Wed, 20 Mar 2024 14:34:34 +0000 (15:34 +0100)] 
tests: shell: Fix one json-nft dump for reordered output

Missed this one when regenerating all dumps.

Fixes: 2a0fe52eca32a ("tests: shell: Regenerate all json-nft dumps")
Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: shell: Add missing json-nft dumps
Phil Sutter [Fri, 8 Mar 2024 23:26:07 +0000 (00:26 +0100)] 
tests: shell: Add missing json-nft dumps

Given that a bunch of issues got fixed, add some more dumps.

Also add tests/shell/testcases/owner/dumps/0002-persist.nft while at it,
even though it's really small.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agoparser: json: Support for synproxy objects
Phil Sutter [Fri, 8 Mar 2024 23:29:36 +0000 (00:29 +0100)] 
parser: json: Support for synproxy objects

Parsing code was there already, merely the entry in json_parse_cmd_add()
missing.

To support maps with synproxy target, an entry in string_to_nft_object()
is required. While being at it, add other missing entries as well.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agojson: Support maps with concatenated data
Phil Sutter [Fri, 8 Mar 2024 23:27:38 +0000 (00:27 +0100)] 
json: Support maps with concatenated data

Dump such maps with an array of types in "map" property, make the parser
aware of this.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: shell: Regenerate all json-nft dumps
Phil Sutter [Fri, 8 Mar 2024 19:22:37 +0000 (20:22 +0100)] 
tests: shell: Regenerate all json-nft dumps

Ordering of 'nft -j list ruleset' output has changed, Regenerate
existing json-nft dumps. No functional change intended, merely the
position of chain objects should have moved up in the "nftables" array.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agojson: Order output like nft_cmd_expand()
Phil Sutter [Thu, 7 Mar 2024 17:40:12 +0000 (18:40 +0100)] 
json: Order output like nft_cmd_expand()

Print empty chain add commands early in list so following verdict maps
and rules referring to them won't cause spurious errors when loading the
resulting ruleset dump.

Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: shell: packetpath/flowtables: Avoid spurious EPERM
Phil Sutter [Fri, 8 Mar 2024 14:21:30 +0000 (15:21 +0100)] 
tests: shell: packetpath/flowtables: Avoid spurious EPERM

On my system for testing, called socat is not allowed to create the pipe
file in local directory (probably due to sshfs). Specify a likely unique
path in /tmp to avoid such problems.

Fixes: 419c0199774c6 ("tests: shell: add test to cover ct offload by using nft flowtables")
Signed-off-by: Phil Sutter <phil@nwl.cc>
15 months agotests: py: move meter tests to tests/shell
Pablo Neira Ayuso [Wed, 13 Mar 2024 16:08:43 +0000 (17:08 +0100)] 
tests: py: move meter tests to tests/shell

Userspace performs an translation to dynamic set which does not fit well
into tests/py, move them to tests/shell.

Fixes: b8f8ddfff733 ("evaluate: translate meter into dynamic set")
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
15 months agosrc: remove utf-8 character in printf lines
Florian Westphal [Wed, 13 Mar 2024 13:17:55 +0000 (14:17 +0100)] 
src: remove utf-8 character in printf lines

replace "‘" (UTF-8, 0xe280 0x98) with "'" (ASCII 0x27).

Signed-off-by: Florian Westphal <fw@strlen.de>
15 months agotests: py: add payload merging test cases
Florian Westphal [Fri, 8 Mar 2024 13:40:12 +0000 (14:40 +0100)] 
tests: py: add payload merging test cases

Add a test case that would fail without preceeding fix.

Signed-off-by: Florian Westphal <fw@strlen.de>
15 months agonftables: do mot merge payloads on negation
Sriram Rajagopalan [Wed, 13 Mar 2024 08:32:42 +0000 (01:32 -0700)] 
nftables: do mot merge payloads on negation

else, a rule like
  tcp sport != 22 tcp dport != 23

will match even if the destination is 23 as long as sport is != 22.
(or vice versa).

Signed-off-by: Sriram Rajagopalan <sriramr@arista.com>
Signed-off-by: Florian Westphal <fw@strlen.de>