]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
11 hours agotests: shell: use binary defined by run-tests.sh master
Zhongqiu Duan [Thu, 3 Jul 2025 13:57:17 +0000 (13:57 +0000)] 
tests: shell: use binary defined by run-tests.sh

Remove hardcoded binary in testcases/transactions/handle_bad_family.

Fixes: aa44b61a560d ("tests: shell: check for removing table via handle with incorrect family")
Signed-off-by: Zhongqiu Duan <dzq.aishenghu0@gmail.com>
13 hours agodoc: Clarify cgroup meta variable
Michal Koutný [Mon, 30 Jun 2025 14:15:26 +0000 (16:15 +0200)] 
doc: Clarify cgroup meta variable

The documentation mentions control group id where the meaning is a class
id associated to the cgroup of a socket. This used to be fine until
there came cgroup v2 that use similar terminolgy (cgroup id) for very
different thing -- a numeric identifier of a particular (v2) cgroup.

This contemporary cgroup id isn't exposed by netfilter (v2 matching is
based on paths externally). Fix the docs and decrease confusion by more
precise description of the metavariable.

[ Added comment in description to refer to socket cgroupv2 --pablo ]

Signed-off-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 days agoMerge branch 'tests_shell_check_tree_fixes'
Florian Westphal [Mon, 30 Jun 2025 12:39:22 +0000 (14:39 +0200)] 
Merge branch 'tests_shell_check_tree_fixes'

Add many more json dump files, now that json input parser handles
'typeof', add dump files for all of them.

Also some tests lacked a .nft dump file too, add them.

Other tests can't have dump files because they produce unstable output
(e.g. due to timeouts or because test is randomized).

Add a 'nodump' files for those so tools/check-tree.sh won't complain
about them.

Furthermore check-tree.sh should not report the json bogon inputs in
tests/shell/testcases/bogons/nft-j-f as "Unexpected files".

Finally, two bogon inputs were in the wrong directory (and thus were
not used as test inputs) move them to the right location.

After his merge, only 10 check-tree.sh warnings remain and all errors
are gone.

There are still missing json test files, this is mostly due to missing
anonymous (implicit) chain support in the json parser.

Leave the warnings as-is so it can be properly resolved (add support)
at a later time.

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add json dump files
Florian Westphal [Sun, 29 Jun 2025 10:39:14 +0000 (12:39 +0200)] 
tests: shell: add json dump files

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: move bogons to correct directory
Florian Westphal [Sun, 29 Jun 2025 10:13:38 +0000 (12:13 +0200)] 
tests: shell: move bogons to correct directory

These two bogons were never loaded, they have to placed in the "nft-f"
subdir.

Also add the "nft-j-f" bogon input dir to the ignored files list so their
existence is not reported as an error by check-tree.sh.

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add a few nodump files
Florian Westphal [Sun, 29 Jun 2025 10:09:56 +0000 (12:09 +0200)] 
tests: shell: add a few nodump files

These tests produce no rules.

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add include dumps
Florian Westphal [Sun, 29 Jun 2025 10:07:00 +0000 (12:07 +0200)] 
tests: shell: add include dumps

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add maps dumps
Florian Westphal [Sun, 29 Jun 2025 09:11:25 +0000 (11:11 +0200)] 
tests: shell: add maps dumps

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add nft-i dumps
Florian Westphal [Sun, 29 Jun 2025 08:52:53 +0000 (10:52 +0200)] 
tests: shell: add nft-i dumps

Signed-off-by: Florian Westphal <fw@strlen.de>
3 days agotests: shell: add sets dumps
Florian Westphal [Sun, 29 Jun 2025 08:50:01 +0000 (10:50 +0200)] 
tests: shell: add sets dumps

add nodump file for inerval_size_random test, it has no stable output.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 days agotests: shell: add optimize dump files
Florian Westphal [Sun, 29 Jun 2025 08:30:41 +0000 (10:30 +0200)] 
tests: shell: add optimize dump files

nomerge_vmap gains a nodump file, the test uses --check.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 days agotests: shell: add bitwise json dump files
Florian Westphal [Sun, 29 Jun 2025 08:15:03 +0000 (10:15 +0200)] 
tests: shell: add bitwise json dump files

Signed-off-by: Florian Westphal <fw@strlen.de>
7 days agofib: allow to use it in set statements
Pablo Neira Ayuso [Tue, 24 Jun 2025 16:11:10 +0000 (18:11 +0200)] 
fib: allow to use it in set statements

Allow to use fib expression in set statements, eg.

 meta mark set ip saddr . fib daddr check map { 1.2.3.4 . exists : 0x00000001 }

Fixes: 4a75ed32132d ("src: add fib expression")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 days agofib: allow to check if route exists in maps
Pablo Neira Ayuso [Tue, 24 Jun 2025 16:11:06 +0000 (18:11 +0200)] 
fib: allow to check if route exists in maps

f686a17eafa0 ("fib: Support existence check") adds EXPR_F_BOOLEAN as a
workaround to infer from the rhs of the relational expression if the fib
lookup wants to check for a specific output interface or, instead,
simply check for existence. This, however, does not work with maps.

The NFT_FIB_F_PRESENT flag can be used both with NFT_FIB_RESULT_OIF and
NFT_FIB_RESULT_OFINAME, my understanding is that they serve the same
purpose which is to check if a route exists, so they are redundant.

Add a 'check' fib result to check for routes while still keeping the
inference workaround for backward compatibility, but prefer the new
syntax in the listing.

Update man nft(8) and tests/py.

Fixes: f686a17eafa0 ("fib: Support existence check")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 days agotests: shell: Fix ifname_based_hooks feature check
Phil Sutter [Wed, 25 Jun 2025 16:53:36 +0000 (18:53 +0200)] 
tests: shell: Fix ifname_based_hooks feature check

The test was technically incorrect: Instead of detecting whether
interface hooks are name-based or not, it actually tested whether
netdev-family chains are removed along with their last hook.

Since the latter behaviour is established in kernel commit fc0133428e7a
("netfilter: nf_tables: Tolerate chains with no remaining hooks") and
thus independent from the name-based hooks change, treating both as the
same kernel feature is not acceptable.

Fix this by detecting whether a netdev-family chain may be added despite
specifying a non-existent interface to hook into. Keep the old check
around with a better name, although unused for now.

Reported-by: Florian Westphal <fw@strlen.de>
Fixes: f27e5abd81f29 ("tests: shell: Adjust to ifname-based hooks")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
7 days agoevaluate: prevent merge of sets with incompatible keys
Florian Westphal [Thu, 26 Jun 2025 00:52:48 +0000 (02:52 +0200)] 
evaluate: prevent merge of sets with incompatible keys

Its not enough to check for interval flag, this would assert in interval
code due to concat being passed to the interval code:
BUG: unhandled key type 13

After fix:
same_set_name_but_different_keys_assert:8:6-7: Error: set already exists with
different datatype (concatenation of (IPv4 address, network interface index) vs
network interface index)
        set s4 {
            ^^

This also improves error verbosity when mixing datamap and objref maps:

invalid_transcation_merge_map_and_objref_map:9:13-13:
Error: map already exists with different datatype (IPv4 address vs string)

.. instead of 'Cannot merge map with incompatible existing map of same name'.
The 'Cannot merge map with incompatible existing map of same name' check
is kept in place to catch when ruleset contains a set and map with same name
and same key definition.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 days agoevaluate: check that set type is identical before merging
Florian Westphal [Mon, 23 Jun 2025 19:37:31 +0000 (21:37 +0200)] 
evaluate: check that set type is identical before merging

Reject maps and sets of the same name:
 BUG: invalid range expression type catch-all set element
 nft: src/expression.c:1704: range_expr_value_low: Assertion `0' failed.

After:
Error: Cannot merge set with existing datamap of same name
  set z {
      ^

v2:
Pablo points out that we shouldn't merge datamaps (plain value) and objref
maps either, catch this too and add another test:

nft --check -f invalid_transcation_merge_map_and_objref_map
invalid_transcation_merge_map_and_objref_map:9:13-13: Error: Cannot merge map with incompatible existing map of same name

We should also make sure that both data (for map case) and
set keys are identical, this is added in a followup patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
8 days agoevaluate: avoid double-free on error handling of bogus objref maps
Florian Westphal [Tue, 24 Jun 2025 21:20:58 +0000 (23:20 +0200)] 
evaluate: avoid double-free on error handling of bogus objref maps

commit 98c51aaac42b ("evaluate: bail out if anonymous concat set defines a non concat expression")
clears set->init to avoid a double-free.

Extend this to also handle object maps.
The included bogon triggers a double-free of set->init expression:

Error: unqualified type invalid specified in map definition. Try "typeof expression" instead of "type datatype".
ct helper set ct  saddr map { 1c3:: : "p", dead::beef : "myftp" }
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This might not crash, depending on libc/malloc, but ASAN reports this:
==17728==ERROR: AddressSanitizer: heap-use-after-free on address 0x50b0000005e8 at ..
READ of size 4 at 0x50b0000005e8 thread T0
    #0 0x7f1be3cb7526 in expr_free src/expression.c:87
    #1 0x7f1be3cbdf29 in map_expr_destroy src/expression.c:1488
    #2 0x7f1be3cb74d5 in expr_destroy src/expression.c:80
    #3 0x7f1be3cb75c6 in expr_free src/expression.c:96
    #4 0x7f1be3d5925e in objref_stmt_destroy src/statement.c:331
    #5 0x7f1be3d5831f in stmt_free src/statement.c:56
    #6 0x7f1be3d583c2 in stmt_list_free src/statement.c:66
    #7 0x7f1be3d42805 in rule_free src/rule.c:495
    #8 0x7f1be3d48329 in cmd_free src/rule.c:1417
    #9 0x7f1be3cd2c7c in __nft_run_cmd_from_filename src/libnftables.c:759
    #10 0x7f1be3cd340c in nft_run_cmd_from_filename src/libnftables.c:847
    #11 0x55dcde0440be in main src/main.c:535

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 days agoevaluate: make sure chain jump name comes with a null byte
Florian Westphal [Tue, 24 Jun 2025 21:01:13 +0000 (23:01 +0200)] 
evaluate: make sure chain jump name comes with a null byte

There is a stack oob read access in netlink_gen_chain():

mpz_export_data(chain, expr->chain->value,
BYTEORDER_HOST_ENDIAN, len);
snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain);

There is no guarantee that chain[] is null terminated, so snprintf
can read past chain[] array.  ASAN report is:

AddressSanitizer: stack-buffer-overflow on address 0x7ffff5f00520 at ..
READ of size 257 at 0x7ffff5f00520 thread T0
    #0 0x00000032ffb6 in printf_common(void*, char const*, __va_list_tag*) (src/nft+0x32ffb6)
    #1 0x00000033055d in vsnprintf (src/nft+0x33055d)
    #2 0x000000332071 in snprintf (src/nft+0x332071)
    #3 0x0000004eef03 in netlink_gen_chain src/netlink.c:454:2
    #4 0x0000004eef03 in netlink_gen_verdict src/netlink.c:467:4

Reject chain jumps that exceed 255 characters, which matches the netlink
policy on the kernel side.

The included reproducer fails without asan too because the kernel will
reject the too-long chain name. But that happens after the asan detected
bogus read.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 days agojson: reject too long interface names
Florian Westphal [Tue, 24 Jun 2025 21:46:59 +0000 (23:46 +0200)] 
json: reject too long interface names

Blamed commit added a length check on ifnames to the bison parser.
Unfortunately that wasn't enough, json parser has the same issue.

Bogon results in:
BUG: Interface length 44 exceeds limit
nft: src/mnl.c:742: nft_dev_add: Assertion `0' failed.

After patch, included bogon results in:
Error: Invalid device at index 0. name d2345678999999999999999999999999999999012345 too long

I intentionally did not extend evaluate.c to catch this, past sentiment
was that frontends should not send garbage.

I'll send a followup patch to also catch this from eval stage in case there
are further reports for frontends passing in such long names.

Fixes: fa52bc225806 ("parser: reject zero-length interface names")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 days agotests/py: clean up set backend support fallout
Florian Westphal [Tue, 24 Jun 2025 19:39:46 +0000 (21:39 +0200)] 
tests/py: clean up set backend support fallout

Pablo reports failing py tests woth recent kernel and userland:
 any/objects.t: OK
WARNING: line 3: 'add rule ip6 test-ip6 input ..
mismatches 'family 2 __set0 test-ip4 3 backend nft_set_bitmap_type [nf_tables] count 7'

When nf_tables is built as a module, the set backend name coming
from kernel contains the module name ([nf_tables]), this makes the
test script treat it as part of the pseudo instructions.

Skip this line explicitly to avoid these warnings.

Fixes: 7cec20e45a75 ("tests/py: prepare for set debug change")
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Tested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
10 days agosrc: use EXPR_RANGE_VALUE in interval maps
Pablo Neira Ayuso [Mon, 16 Jun 2025 20:48:04 +0000 (22:48 +0200)] 
src: use EXPR_RANGE_VALUE in interval maps

Remove the restriction on maps to use EXPR_RANGE_VALUE to reduce
memory consumption.

With 100k map with concatenation:

  table inet x {
         map y {
                    typeof ip saddr . tcp dport :  ip saddr
                    flags interval
                    elements = {
                        1.0.2.0-1.0.2.240 . 0-2 : 1.0.2.10,
...
 }
  }

Before: 153.6 Mbytes
After: 108.9 Mbytes (-29.11%)

With 100k map without concatenation:

  table inet x {
         map y {
                    typeof ip saddr :  ip saddr
                    flags interval
                    elements = {
                        1.0.2.0-1.0.2.240 : 1.0.2.10,
...
 }
  }

Before: 74.36 Mbytes
After: 62.39 Mbytes (-16.10%)

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agoexpression: constant range is not a singleton
Pablo Neira Ayuso [Mon, 16 Jun 2025 20:48:02 +0000 (22:48 +0200)] 
expression: constant range is not a singleton

Remove the EXPR_F_SINGLETON flag in EXPR_RANGE_VALUE so it can be used
in maps.

expr_evaluate_set() does not toggle NFT_SET_INTERVAL for anonymous sets
because a singleton is assumed to be place, leading to this BUG:

 BUG: invalid data expression type range_value
 nft: src/netlink.c:577: netlink_gen_key: Assertion `0' failed.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agosrc: use constant range expression for interval+concatenation sets
Pablo Neira Ayuso [Mon, 16 Jun 2025 20:47:57 +0000 (22:47 +0200)] 
src: use constant range expression for interval+concatenation sets

Expand 347039f64509 ("src: add symbol range expression to further
compact intervals") to use constant range expression for elements with
concatenation of intervals.

Ruleset with 100k elements of this type:

 table inet x {
        set y {
                typeof ip saddr . tcp dport
                flags interval
                elements = {
0.1.2.0-0.1.2.240 . 0-1,
...
}
}
 }

Memory consumption for this set:

Before: 123.80 Mbytes
After:   80.19 Mbytes (-35.23%)

This patch keeps the workaround 2fbade3cd990 ("netlink: bogus
concatenated set ranges with netlink message overrun") in place.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agoparser_bison: allow delete command with map via handle
Pablo Neira Ayuso [Sun, 15 Jun 2025 09:36:28 +0000 (11:36 +0200)] 
parser_bison: allow delete command with map via handle

For consistency with sets, allow delete via handle for maps too.

Fixes: f4a34d25f6d5 ("src: list set handle and delete set via set handle")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agoparser_bison: only reset by name is supported by now
Pablo Neira Ayuso [Sun, 15 Jun 2025 09:34:11 +0000 (11:34 +0200)] 
parser_bison: only reset by name is supported by now

NFT_MSG_GETSET does not support for handle lookup yet, restrict this to
reset by name by now.

Add a bogon test reported by Florian Westphal.

Fixes: 83e0f4402fb7 ("Implement 'reset {set,map,element}' commands")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agocache: pass name to cache_add()
Pablo Neira Ayuso [Sun, 15 Jun 2025 09:34:04 +0000 (11:34 +0200)] 
cache: pass name to cache_add()

Consolidate the name hash in the cache_add() function.

No functional changes are intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agocache: assert name is non-nul when looking up
Pablo Neira Ayuso [Sun, 15 Jun 2025 09:33:49 +0000 (11:33 +0200)] 
cache: assert name is non-nul when looking up

{table,chain,set,obj,flowtable}_cache_find() should not be called when
handles are used

Fixes: 5ec5c706d993 ("cache: add hashtable cache for table")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 days agorule: skip fuzzy lookup if object name is not available
Pablo Neira Ayuso [Sun, 15 Jun 2025 09:33:42 +0000 (11:33 +0200)] 
rule: skip fuzzy lookup if object name is not available

Skip fuzzy lookup for suggestions when handles are used.

Note that 4cf97abfee61 ("rule: Avoid segfault with anonymous chains")
already skips it for chain.

Fixes: 285bb67a11ad ("src: introduce simple hints on incorrect set")
Fixes: 9f7817a4e022 ("src: introduce simple hints on incorrect chain")
Fixes: d7476ddd5f7d ("src: introduce simple hints on incorrect table")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
11 days agotests: shell: add feature check for count output change
Florian Westphal [Tue, 8 Apr 2025 14:21:32 +0000 (16:21 +0200)] 
tests: shell: add feature check for count output change

New kernels with latest nft release will print the number
of set elements allocated on the kernel side.

This causes shell test dump validation to fail in several
places.  We can't just update the affected dump files
because the test cases are also supposed to pass on current
-stable releases.

Add a feature check for this.  Dump failure can then use
sed to postprocess the stored dump file and can then call

diff a second time.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agosrc: print count variable in normal set listings
Florian Westphal [Tue, 8 Apr 2025 14:21:31 +0000 (16:21 +0200)] 
src: print count variable in normal set listings

Also print the number of allocated set elements if the set provided
an upper size limit and there is at least one element.

Example:

table ip t {
   set s {
       type ipv4_addr
       size 65535      # count 1
       flags dynamic
       counter
       elements = { 1.1.1.1 counter packets 1 bytes 11 }
   }
   ...

JSON output is unchanged as this only has informational purposes.

This change breaks tests, followup patch addresses this.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agodebug: include kernel set information on cache fill
Florian Westphal [Tue, 8 Apr 2025 14:21:30 +0000 (16:21 +0200)] 
debug: include kernel set information on cache fill

Honor --debug=netlink flag also when doing initial set dump
from the kernel.

With recent libnftnl update this will include the chosen
set backend name that is used by the kernel.

Because set names are scoped by table and protocol family,
also include the family protocol number.

Dumping this information breaks tests/py as the recorded
debug output no longer matches, this is fixed in previous
change.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotests/py: prepare for set debug change
Florian Westphal [Tue, 8 Apr 2025 14:21:29 +0000 (16:21 +0200)] 
tests/py: prepare for set debug change

Next patch will make initial set dump from kernel emit set debug
information, so the obtained netlink debug file won't match what is
recorded in tests/py.

Furthermore, as the python add rules for each of the family the test is
for, subsequent dump will include debug information of the other/previous
families.

Change the script to skip all unrelated information to only compare the
relevant set element information and the generated expressions.

This change still finds changes in [ expr ... ] and set elem debug output.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agosrc: BASECHAIN flag no longer implies presence of priority expression
Florian Westphal [Thu, 12 Jun 2025 18:17:15 +0000 (20:17 +0200)] 
src: BASECHAIN flag no longer implies presence of priority expression

The included bogon will crash nft because print side assumes that BASECHAIN
flag presence also means that priority expression is available.

Make the print side conditional.

Fixes: a66b5ad9540d ("src: allow for updating devices on existing netdev chain")
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agoevaluate: restrict allowed subtypes of concatenations
Florian Westphal [Fri, 6 Jun 2025 12:12:37 +0000 (14:12 +0200)] 
evaluate: restrict allowed subtypes of concatenations

We need to restrict this, included bogon asserts with:
BUG: unknown expression type prefix
nft: src/netlink_linearize.c:940: netlink_gen_expr: Assertion `0' failed.

Prefix expressions are only allowed if the concatenation is used within
a set element, not when specifying the lookup key.

For the former, anything that represents a value is allowed.
For the latter, only what will generate data (fill a register) is
permitted.

At this time we do not have an annotation that tells if the expression
is on the left hand side (lookup key) or right hand side (set element).

Add a new list recursion counter for this. If its 0 then we're building
the lookup key, if its the latter the concatenation is the RHS part
of a relational expression and prefix, ranges and so on are allowed.

IOW, we don't really need a recursion counter, another type of annotation
that would tell if the expression is placed on the left or right hand side
of another expression would work too.

v2: explicitly list all 'illegal' expression types instead of
using a default label for them.

This will raise a compiler warning to remind us to adjust the case
labels in case a new expression type gets added in the future.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agoevaluate: rename recursion counter to recursion.binop
Florian Westphal [Fri, 6 Jun 2025 12:12:36 +0000 (14:12 +0200)] 
evaluate: rename recursion counter to recursion.binop

The existing recursion counter is used by the binop expression to detect
if we've completely followed all the binops.

We can only chain up to NFT_MAX_EXPR_RECURSION binops, but the evaluation
step can perform constant-folding, so we must recurse until we found the
rightmost (last) binop in the chain.

Then we can check the post-eval chain to see if it is something that can
be serialized later (i.e., if we are within the NFT_MAX_EXPR_RECURSION
after constant folding) or not.

Thus we can't reuse the existing ctx->recursion counter for other
expressions; entering the initial expr_evaluate_binop with
ctx->recursion > 0 would break things.

Therefore rename this to an embedded structure.
This allows us to add a new recursion counter in a followup patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotest: shell: Add rate_limit test case for 'limit statement'.
Yi Chen [Sun, 22 Jun 2025 12:55:54 +0000 (20:55 +0800)] 
test: shell: Add rate_limit test case for 'limit statement'.

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotest: shell: Add wait_local_port_listen() helper to lib.sh
Yi Chen [Sun, 22 Jun 2025 12:55:53 +0000 (20:55 +0800)] 
test: shell: Add wait_local_port_listen() helper to lib.sh

Introduce a new helper function wait_local_port_listen() in helpers/lib.sh.
Update the flowtables and nat_ftp test cases to use this helper.

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotest: shell: Introduce $NFT_TEST_LIBRARY_FILE, helper/lib.sh
Yi Chen [Sun, 22 Jun 2025 12:55:52 +0000 (20:55 +0800)] 
test: shell: Introduce $NFT_TEST_LIBRARY_FILE, helper/lib.sh

Consolidate frequently used functions in helper/lib.sh
switch nat_ftp and flowtables to use it.

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotest: shell: nat_ftp: test files must be world-readable
Florian Westphal [Sun, 22 Jun 2025 13:19:39 +0000 (15:19 +0200)] 
test: shell: nat_ftp: test files must be world-readable

Directory and test need to be readable, with a default umask of 077
this test fails because vsftp can't open the curl-requested file.

Signed-off-by: Florian Westphal <fw@strlen.de>
11 days agotest: shell: Don't use system nft binary
Yi Chen [Sun, 22 Jun 2025 12:55:51 +0000 (20:55 +0800)] 
test: shell: Don't use system nft binary

Use the defined $NFT variable instead of calling the system nft binary directly.
Add a nat_ftp.nodump file to avoid the following check-tree.sh error:
ERR: "tests/shell/testcases/packetpath/nat_ftp" has no "tests/shell/testcases/packetpath/dumps/nat_ftp.{nft,nodump}" file.

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
2 weeks agoevaluate: don't BUG on unexpected base datatype
Florian Westphal [Fri, 13 Jun 2025 14:46:06 +0000 (16:46 +0200)] 
evaluate: don't BUG on unexpected base datatype

Included bogon will cause a crash but this is the evaluation stage where
we can just emit an error instead.

Signed-off-by: Florian Westphal <fw@strlen.de>
3 weeks agonetlink: Avoid crash upon missing NFTNL_OBJ_CT_TIMEOUT_ARRAY attribute
Phil Sutter [Thu, 12 Jun 2025 18:17:22 +0000 (20:17 +0200)] 
netlink: Avoid crash upon missing NFTNL_OBJ_CT_TIMEOUT_ARRAY attribute

If missing, the memcpy call ends up reading from address zero.

Fixes: c7c94802679cd ("src: add ct timeout support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agotests: py: Properly fix JSON equivalents for netdev/reject.t
Phil Sutter [Thu, 12 Jun 2025 10:59:29 +0000 (12:59 +0200)] 
tests: py: Properly fix JSON equivalents for netdev/reject.t

Revert commit d1a7b9e19fe65 ("tests: py: update netdev reject test
file"), the stored JSON equivalents were correct in that they matched
the standard syntax input.

In fact, we missed a .json.output file recording the expected deviation
in JSON output.

Fixes: d1a7b9e19fe65 ("tests: py: update netdev reject test file")
Fixes: 7ca3368cd7575 ("reject: Unify inet, netdev and bridge delinearization")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agotests: shell: Adjust to ifname-based hooks
Phil Sutter [Tue, 6 May 2025 22:06:20 +0000 (00:06 +0200)] 
tests: shell: Adjust to ifname-based hooks

Interface specs won't disappear anymore upon device removal. Drop them
manually if kernel has ifname-based hooks.

Skip transactions/0050rule_1 if kernel has name-based hooks: The test
relies upon the ruleset being rejected for non-existent interfaces,
which obviously won't happen then.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agotests: monitor: Fix for single flag array avoidance
Phil Sutter [Wed, 11 Jun 2025 15:15:22 +0000 (17:15 +0200)] 
tests: monitor: Fix for single flag array avoidance

Missed to update the JSON monitor expected output.

Fixes: 6bedb12af1658 ("json: Print single set flag as non-array")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agojson: Dump flowtable hook spec only if present
Phil Sutter [Wed, 11 Jun 2025 14:45:48 +0000 (16:45 +0200)] 
json: Dump flowtable hook spec only if present

If there is no priority.expr set, assume hook.num is bogus, too.

While this is fixing JSON output, it's hard to tell what commit this is
actually fixing: Before commit 627c451b23513 ("src: allow variables in
the chain priority specification"), there was no way to detect
flowtables missing hook specs (e.g. when printing flowtable delete
monitor event).

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agomonitor: Correctly print flowtable updates
Phil Sutter [Wed, 11 Jun 2025 12:24:37 +0000 (14:24 +0200)] 
monitor: Correctly print flowtable updates

An update deleting a hook from a flowtable was indistinguishable from a
flowtable deletion.

Fixes: 73a8adfc2432e ("monitor: Recognize flowtable add/del events")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agonetlink: Do not allocate a bogus flowtable priority expr
Phil Sutter [Wed, 11 Jun 2025 12:15:38 +0000 (14:15 +0200)] 
netlink: Do not allocate a bogus flowtable priority expr

Code accidentally treats missing NFTNL_FLOWTABLE_PRIO attribute as zero
prio value which may not be correct.

Fixes: db0697ce7f602 ("src: support for flowtable listing")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agonetlink: Fix for potential crash parsing a flowtable
Phil Sutter [Wed, 11 Jun 2025 11:12:56 +0000 (13:12 +0200)] 
netlink: Fix for potential crash parsing a flowtable

Kernel's flowtable message might not contain the
NFTA_FLOWTABLE_HOOK_DEVS attribute. In that case, nftnl_flowtable_get()
will return NULL for the respective nftnl attribute.

Fixes: db0697ce7f602 ("src: support for flowtable listing")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agomnl: catch bogus expressions before crashing
Florian Westphal [Thu, 5 Jun 2025 22:20:28 +0000 (00:20 +0200)] 
mnl: catch bogus expressions before crashing

We can't recover from errors here, but we can abort with a more
precise reason than 'segmentation fault', or stack corruptions
that get caught way later, or not at all.

expr->value is going to be read, we can't cope with other expression
types here.

We will copy to stack buffer of IFNAMSIZ size, abort if we would
overflow.

Check there is a NUL byte present too.
This is a preemptive patch, I've seen one crash in this area but
no reproducer yet.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 weeks agodoc: Basic documentation of anonymous chains
Phil Sutter [Wed, 4 Jun 2025 17:53:47 +0000 (19:53 +0200)] 
doc: Basic documentation of anonymous chains

Joint work with Folsk Pratima.

Signed-off-by: Folsk Pratima <folsk0pratima@cock.li>
Signed-off-by: Phil Sutter <phil@nwl.cc>
3 weeks agotests: shell: Add a test case for FTP helper combined with NAT.
Yi Chen [Mon, 9 Jun 2025 08:14:28 +0000 (16:14 +0800)] 
tests: shell: Add a test case for FTP helper combined with NAT.

This test verifies functionality of the FTP helper,
for both passive, active FTP modes,
and the functionality of the nf_nat_ftp module.

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
3 weeks agotests: helpers: suppress mount error messages
Florian Westphal [Fri, 6 Jun 2025 12:26:14 +0000 (14:26 +0200)] 
tests: helpers: suppress mount error messages

Prevent repeated error messages from spamming the console.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 weeks agojson: work around fuzzer-induced assert crashes
Florian Westphal [Mon, 31 Mar 2025 14:47:11 +0000 (16:47 +0200)] 
json: work around fuzzer-induced assert crashes

fuzzer can cause assert failures due to json_pack() returning a NULL
value and therefore triggering the assert(out) in __json_pack macro.

All instances I saw are due to invalid UTF-8 strings, i.e., table/chain
names with non-text characters in them.

Work around this for now, replace the assert with a plaintext error
message and return NULL instead of abort().

Signed-off-by: Florian Westphal <fw@strlen.de>
4 weeks agojson: prevent null deref if chain->policy is not set
Florian Westphal [Mon, 2 Jun 2025 12:22:33 +0000 (14:22 +0200)] 
json: prevent null deref if chain->policy is not set

The two commits mentioned below resolved null dererence crashes when the
policy resp. priority keyword was missing in the chain/flowtable
specification.

Same issue exists in the json output path, so apply similar fix there
and extend the existing test cases.

Fixes: 5b37479b42b3 ("nftables: don't crash in 'list ruleset' if policy is not set")
Fixes: b40bebbcee36 ("rule: do not crash if to-be-printed flowtable lacks priority")
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Phil Sutter <phil@nwl.cc>
4 weeks agotests: py: fix json single-flag output for fib & synproxy
Florian Westphal [Mon, 2 Jun 2025 12:12:16 +0000 (14:12 +0200)] 
tests: py: fix json single-flag output for fib & synproxy

Blamed commits change output format but did not adjust existing tests:
  inet/fib.t: WARNING: line 16: '{"nftables": ..

Fixes: 38f99ee84fe6 ("json: Print single synproxy flags as non-array")
Fixes: dbe5c44f2b89 ("json: Print single fib flag as non-array")
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Phil Sutter <phil@nwl.cc>
5 weeks agotests: shell: check for features not available in 5.4
Pablo Neira Ayuso [Thu, 29 May 2025 09:49:18 +0000 (11:49 +0200)] 
tests: shell: check for features not available in 5.4

5.4 -stable kernels report failures in these tests, this kernel version
is lacking these feature.

The bitshift requirement is needed by this ruleset:

 table ip x {
        set s13 {
                typeof tcp option mptcp subtype
                elements = { mp-join, dss }
        }

        chain y {
                tcp option mptcp subtype @s13 accept
        }
 }

which uses bitshift in its bytecode.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 weeks agocache: Tolerate object deserialization failures
Phil Sutter [Fri, 16 May 2025 17:36:37 +0000 (19:36 +0200)] 
cache: Tolerate object deserialization failures

If netlink_delinearize_obj() fails, it will print an error message. Skip
this object and keep going.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 weeks agonetlink: Keep going after set element parsing failures
Phil Sutter [Fri, 16 May 2025 17:33:25 +0000 (19:33 +0200)] 
netlink: Keep going after set element parsing failures

Print an error message and try to deserialize the remaining elements
instead of calling BUG().

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 weeks agonetlink: Pass netlink_ctx to netlink_delinearize_setelem()
Phil Sutter [Fri, 16 May 2025 17:17:00 +0000 (19:17 +0200)] 
netlink: Pass netlink_ctx to netlink_delinearize_setelem()

Prepare for calling netlink_io_error() which needs the context pointer.
Trade this in for the cache pointer since no caller uses a special one.
No functional change intended.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 weeks agonetlink_delinearize: Replace some BUG()s by error messages
Phil Sutter [Fri, 16 May 2025 11:28:19 +0000 (13:28 +0200)] 
netlink_delinearize: Replace some BUG()s by error messages

Netlink parser tries to keep going despite errors. Faced with an
incompatible ruleset, this is much more user-friendly than exiting the
program upon the first obstacle. This patch fixes three more spots to
support this.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 weeks agotests: shell: check if kernel supports for cgroupsv2 matching
Pablo Neira Ayuso [Mon, 19 May 2025 23:59:06 +0000 (01:59 +0200)] 
tests: shell: check if kernel supports for cgroupsv2 matching

Update packetpath/cgroupv2 to skip it if kernel does not support it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 weeks agotests: shell: skip egress in netdev chain release path test
Pablo Neira Ayuso [Mon, 19 May 2025 23:17:39 +0000 (01:17 +0200)] 
tests: shell: skip egress in netdev chain release path test

Update test to skip egress coverage if kernel does not support it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 weeks agonetlink: Catch unknown types when deserializing objects
Phil Sutter [Fri, 16 May 2025 17:41:19 +0000 (19:41 +0200)] 
netlink: Catch unknown types when deserializing objects

Print an error message and discard the object instead of returning it to
the caller. At least when trying to print it, we would hit an assert()
in obj_type_name() anyway.

Fixes: 4756d92e517ae ("src: listing of stateful objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 weeks agonetlink: Avoid potential NULL-ptr deref parsing set elem expressions
Phil Sutter [Fri, 16 May 2025 18:08:05 +0000 (20:08 +0200)] 
netlink: Avoid potential NULL-ptr deref parsing set elem expressions

Since netlink_parse_set_expr() may return NULL, the following deref must
be guarded.

Fixes: e6d1d0d611958 ("src: add set element multi-statement support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 weeks agotests: shell: Include kernel taint value in warning
Phil Sutter [Fri, 9 May 2025 12:46:50 +0000 (14:46 +0200)] 
tests: shell: Include kernel taint value in warning

If kernel is already tainted, not all tests yield usable results.
Printing the taint cause might help users tracking down the external
cause.

If a test taints the kernel, the value is stored in rc-failed-tainted
file already.

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agotests: shell: Add test case for JSON 'flags' arrays
Phil Sutter [Thu, 8 May 2025 15:34:18 +0000 (17:34 +0200)] 
tests: shell: Add test case for JSON 'flags' arrays

Ensure these arrays are reduced if containing just a single item and
parser interprets them correctly in any case.

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agojson: Introduce json_add_array_new()
Phil Sutter [Thu, 8 May 2025 15:28:02 +0000 (17:28 +0200)] 
json: Introduce json_add_array_new()

Propagate nat_stmt_add_array() to a generic helper for use in all spots
adding an array property which may reduce to a single item or even not
exist at all.

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agojson: Print single synproxy flags as non-array
Phil Sutter [Thu, 8 May 2025 14:44:39 +0000 (16:44 +0200)] 
json: Print single synproxy flags as non-array

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agojson: Print single fib flag as non-array
Phil Sutter [Thu, 8 May 2025 14:40:41 +0000 (16:40 +0200)] 
json: Print single fib flag as non-array

Check array size and reduce the array if possible.

The zero array length check is dead code here due to the surrounding 'if
(flags)' block, but it's a common idiom one could replace by a shared
routine later.

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agojson: Print single set flag as non-array
Phil Sutter [Thu, 8 May 2025 14:39:24 +0000 (16:39 +0200)] 
json: Print single set flag as non-array

The code obviously intended to do this already but got the array length
check wrong.

Fixes: e70354f53e9f6 ("libnftables: Implement JSON output support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agodoc: Fix typo in nat statement 'prefix' description
Phil Sutter [Thu, 8 May 2025 14:35:47 +0000 (16:35 +0200)] 
doc: Fix typo in nat statement 'prefix' description

No point in repeating 'to map' here.

Fixes: 19d73ccdd39fa ("doc: add nat examples")
Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agoparser_json: Introduce parse_flags_array()
Phil Sutter [Thu, 10 Apr 2025 14:42:42 +0000 (16:42 +0200)] 
parser_json: Introduce parse_flags_array()

Various objects support a 'flags' property with value usually being an
array of strings. There is a special case, when merely a single flag is
set: The value may be a string representing this flag.

Introduce a function assisting in parsing this polymorphic value. Have
callers pass a parser callback translating a single flag name into a
corresponding value. Luckily, these single flag parsers are very common
already.

As a side-effect, enable the single flag spec for set flags as well and
update the documentation accordingly.

Signed-off-by: Phil Sutter <phil@nwl.cc>
7 weeks agosrc: netlink: fix crash when ops doesn't support udata
Florian Westphal [Thu, 8 May 2025 14:29:04 +0000 (16:29 +0200)] 
src: netlink: fix crash when ops doesn't support udata

Whenever a new version adds udata support to an expression, then old
versions of nft will crash when trying to list such a ruleset generated
by a more recent version of nftables.

Fix this by falling back to 'type' format.

Fixes: 6e48df5329ea ('src: add "typeof" build/parse/print support')
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 weeks agotests/shell: Skip netdev_chain_dev_addremove on tainted kernels
Phil Sutter [Thu, 8 May 2025 10:08:39 +0000 (12:08 +0200)] 
tests/shell: Skip netdev_chain_dev_addremove on tainted kernels

The test checks taint state to indicate success or failure. Since this
won't work if the kernel is already tainted at start, skip the test
instead of failing it.

Fixes: 02dbf86f39410 ("tests: shell: add a test case for netdev ruleset flush + parallel link down")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agosrc: remove bogus empty file
Florian Westphal [Fri, 25 Apr 2025 06:20:19 +0000 (08:20 +0200)] 
src: remove bogus empty file

No idea how this happened, remove it.

Reported-by: Sunny73Cr <Sunny73Cr@protonmail.com>
Fixes: 058246016188 ("src: allow to map key to nfqueue number")
Signed-off-by: Florian Westphal <fw@strlen.de>
2 months agotests: shell: Update packetpath/flowtables
Yi Chen [Wed, 16 Apr 2025 15:53:20 +0000 (23:53 +0800)] 
tests: shell: Update packetpath/flowtables

1. The socat receiver should not use the pipfile as output where the sender
   reads data from, this could create an infinite data loop.
2. Sending a packet right after establishing the connection helped uncover
   a new bug (see kernel commit
   d2d31ea8cd80, "netfilter: conntrack: fix erronous removal of offload bit").
3. Optimize test log output

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
2 months agobuild: Bump version to 1.1.3 v1.1.3
Pablo Neira Ayuso [Tue, 22 Apr 2025 09:47:38 +0000 (11:47 +0200)] 
build: Bump version to 1.1.3

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agonetlink: bogus concatenated set ranges with netlink message overrun
Pablo Neira Ayuso [Thu, 17 Apr 2025 19:40:23 +0000 (21:40 +0200)] 
netlink: bogus concatenated set ranges with netlink message overrun

When building each component of the set element key, a late byteorder
switch is performed to ensure that all components in the interval are
represented in big endian, as required by the pipapo backend.

In case that the set element does not fit into the netlink message, the
byteorder switch happens twice, leading to inserting an element with a
bogus component with large sets, so instead:

      "lo" . 00:11:22:33:44:55 . 10.1.2.3 comment "123456789012345678901234567890"

listing reports:

  16777216 . 00:11:22:33:44:55 . 10.1.2.3 comment "123456789012345678901234567890"

Note that 16777216 is 0x1000000, which should instead be 0x00000001 to
represent "lo" as u32.

Fix this by switching the value in a temporary variable and use it to
set the set element key attribute in the netlink message.

Later, revisit this to perform this byteorder switch from evaluation
step.

Add tests/shell unit to cover for this bug.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1792
Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agoevalute: make vlan pcp updates work
Florian Westphal [Sat, 19 Apr 2025 11:44:39 +0000 (13:44 +0200)] 
evalute: make vlan pcp updates work

On kernel side, nft_payload_set_vlan() requires a 2 or 4 byte
write to the vlan header.

As-is, nft emits a 1 byte write:
  [ payload load 1b @ link header + 14 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000001f ) ^ 0x00000020 ]

... which the kernel doesn't support.  Expand all vlan header updates to
a 2 or 4 byte write and update the existing vlan id test case.

Reported-by: Kevin Vigouroux <ke.vigouroux@laposte.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agoRevert "intervals: do not merge intervals with different timeout"
Pablo Neira Ayuso [Thu, 17 Apr 2025 11:23:10 +0000 (13:23 +0200)] 
Revert "intervals: do not merge intervals with different timeout"

This reverts commit da0bac050c8b2588242727f9915a1ea8bc48ceb2.

This results in an error when adding an interval that overlaps an
existing interval in the kernel, this defeats the purpose of the
auto-merge feature.

Reported-by: Slavko <linux@slavino.sk>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agobuild: Bump version to 1.1.2 v1.1.2
Pablo Neira Ayuso [Mon, 14 Apr 2025 16:55:15 +0000 (18:55 +0200)] 
build: Bump version to 1.1.2

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agoparser_bison: add selector_expr rule to restrict typeof_expr
Pablo Neira Ayuso [Thu, 10 Apr 2025 21:23:58 +0000 (23:23 +0200)] 
parser_bison: add selector_expr rule to restrict typeof_expr

typeof_expr allows for symbol, constant and bitwise expressions,
restrict it to selector expressions.

After this patch, input generated by fuzzer is rejected upfront:

 # nft -f test.nft
 test.nft:3:53-53: Error: syntax error, unexpected number
               typeof numgen inc mod 2 : ip daddr . 0
                                                    ^
 test.nft:2:12-13: Error: set definition does not specify key
       map t2 {
           ^^
 test.nft:8:65-67: Error: No such file or directory
               meta l4proto tcp dnat ip to numgen inc mod 2 map @t2
                                                                ^^^
 test.nft:8:65-67: Error: No such file or directory
               meta l4proto tcp dnat ip to numgen inc mod 2 map @t2
                                                                ^^^

Revisit 4ab1e5e60779 ("src: allow use of 'verdict' in typeof
definitions") to handle verdict as string, later a token can be added
to the scanner and enable it via flex start conditions.

Fixes: 14357cff40ed ("parser: add typeof keyword for declarations")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agooptimize: invalidate merge in case of duplicated key in set/map
Pablo Neira Ayuso [Wed, 9 Apr 2025 09:38:17 +0000 (11:38 +0200)] 
optimize: invalidate merge in case of duplicated key in set/map

-o/--optimize results in EEXIST error when merging two rules that lead
to ambiguous set/map, for instance:

 table ip x {
        chain v4icmp {}
        chain v4icmpc {}

        chain y {
                ip protocol icmp jump v4icmp
                ip protocol icmp goto v4icmpc
        }
 }

which is not possible because duplicated keys are not possible in
set/map. This is how it shows when running a test:

 Merging:
 testcases/sets/dumps/sets_with_ifnames.nft:56:3-30:            ip protocol icmp jump v4icmp
 testcases/sets/dumps/sets_with_ifnames.nft:57:3-31:            ip protocol icmp goto v4icmpc
 into:
       ip protocol vmap { icmp : jump v4icmp, icmp : goto v4icmpc }
 internal:0:0-0: Error: Could not process rule: File exists

Add a new step to compare rules that are candidate to be merged to
detect colissions in set/map keys in order to skip them in the next
final merging step.

Add tests/shell unit to improve coverage.

Fixes: fb298877ece2 ("src: add ruleset optimization infrastructure")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
2 months agoevaluate: bail out if ct saddr/daddr dependency cannot be inserted
Florian Westphal [Wed, 2 Apr 2025 23:09:22 +0000 (01:09 +0200)] 
evaluate: bail out if ct saddr/daddr dependency cannot be inserted

If we have an incomplete rule like "ct original saddr" in inet
family, this function generates an error because it can't determine the required protocol
dependency, hinting at missing ip/ip6 keyword.

We should not go on in this case to avoid a redundant followup error:

nft add rule inet f c ct original saddr 1.2.3.4
Error: cannot determine ip protocol version, use "ip saddr" or "ip6 saddr" instead
add rule inet f c ct original saddr 1.2.3.4
                  ^^^^^^^^^^^^^^^^^
Error: Could not parse symbolic invalid expression
add rule inet f c ct original saddr 1.2.3.4

After this change only the first error is shown.

Fixes: 2b29ea5f3c3e ("src: ct: add eval part to inject dependencies for ct saddr/daddr")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoparser_json: only allow concatenations with 2 or more expressions
Florian Westphal [Wed, 2 Apr 2025 05:18:18 +0000 (07:18 +0200)] 
parser_json: only allow concatenations with 2 or more expressions

The bison parser enforces this implicitly by grammar rules.
Because subkeys have to be conatenated via ".", notation, e.g.
"mark . ip saddr", all concatenation expressions always consist of at
least two elements.

But this doesn't apply to the json frontend which just uses an array:
it can be empty or only contain one element.

The included reproducer makes the eval stage set the "concatenation" flag
on the interval set.  This prevents the needed conversion code to turn the
element values into ranges from getting run.

The reproducer asserts with:
nft: src/intervals.c:786: setelem_to_interval: Assertion `key->etype == EXPR_RANGE_VALUE' failed.

Convert the assertion to BUG() so we can see what element type got passed
to the set interval code in case we have further issues in this area.

Reject 0-or-1-element concatenations from the json parser.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoevaluate: fix crash when generating reject statement error
Florian Westphal [Mon, 31 Mar 2025 12:43:34 +0000 (14:43 +0200)] 
evaluate: fix crash when generating reject statement error

After patch, this gets rejected with:
internal:0:0-0: Error: conflicting protocols specified: ip vs ip6

Without patch, we crash with a NULL dereference: we cannot use
reject.expr->location unconditionally.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoevaluate: reject: remove unused expr function argument
Florian Westphal [Mon, 31 Mar 2025 12:43:33 +0000 (14:43 +0200)] 
evaluate: reject: remove unused expr function argument

stmt_evaluate_reject passes cmd->expr argument but its never used.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agooptimize: expand expression list when merging into concatenation
Pablo Neira Ayuso [Tue, 1 Apr 2025 16:11:45 +0000 (18:11 +0200)] 
optimize: expand expression list when merging into concatenation

The following rules:

    udp dport 137 ct state new,untracked accept
    udp dport 138 ct state new,untracked accept

results in:

  nft: src/optimize.c:670: __merge_concat: Assertion `0' failed.

The logic to expand to the new,untracked list in the concatenation is
missing.

Fixes: 187c6d01d357 ("optimize: expand implicit set element when merging into concatenation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agocache: prevent possible crash rule filter is NULL
Pablo Neira Ayuso [Tue, 1 Apr 2025 15:36:48 +0000 (17:36 +0200)] 
cache: prevent possible crash rule filter is NULL

Similar to 3f0a47f9f00c ("cache: don't crash when filter is NULL").

No real crash observed but it is good to tigthen this.

Fixes: dbff26bfba83 ("cache: consolidate reset command")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoparser_json: bail out on malformed statement in set
Pablo Neira Ayuso [Tue, 1 Apr 2025 07:57:59 +0000 (09:57 +0200)] 
parser_json: bail out on malformed statement in set

Propagate error to caller so it bails out on malformed set statements.

Fixes: 07958ec53830 ("json: add set statement list support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoparser_json: allow statement stateful statement only in set elements
Pablo Neira Ayuso [Tue, 1 Apr 2025 07:49:48 +0000 (09:49 +0200)] 
parser_json: allow statement stateful statement only in set elements

Upfront reject of non stateful statements in set elements.

Fixes: 07958ec53830 ("json: add set statement list support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoparser_json: reject empty jump/goto chain
Pablo Neira Ayuso [Mon, 31 Mar 2025 15:55:45 +0000 (17:55 +0200)] 
parser_json: reject empty jump/goto chain

When parsing a verdict map json where element jumps to chain represented
as empty string.

internal:0:0-0: Error: Parsing list expression item at index 0 failed.
internal:0:0-0: Error: Invalid set elem at index 0.
internal:0:0-0: Error: Invalid set elem expression.
internal:0:0-0: Error: Parsing command array at index 2 failed.

Fixes: 586ad210368b ("libnftables: Implement JSON parser")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agocache: don't crash when filter is NULL
Florian Westphal [Tue, 1 Apr 2025 14:29:14 +0000 (16:29 +0200)] 
cache: don't crash when filter is NULL

a delete request will cause a crash in obj_cache_dump, move the deref
into the filter block.

Fixes: dbff26bfba83 ("cache: consolidate reset command")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoexpression: incorrect assert() list_expr_to_binop
Pablo Neira Ayuso [Mon, 31 Mar 2025 22:36:27 +0000 (00:36 +0200)] 
expression: incorrect assert() list_expr_to_binop

assert() logic is reversed, all expressions in the list are handled,
including the first.

  src/expression.c:1285: list_expr_to_binop: Assertion `first' failed.

Fixes: 53d6bb992445 ("expression: initialize list of expression to silence gcc compile warning")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
3 months agoevaluate: only allow stateful statements in set and map definitions
Florian Westphal [Mon, 31 Mar 2025 15:23:20 +0000 (17:23 +0200)] 
evaluate: only allow stateful statements in set and map definitions

The bison parser doesn't allow this to happen due to grammar
restrictions, but the json input has no such issues.

The bogon input assigns 'notrack' which triggers:
BUG: unknown stateful statement type 19
nft: src/netlink_linearize.c:1061: netlink_gen_stmt_stateful: Assertion `0' failed.

After patch, we get:
Error: map statement must be stateful

Fixes: 07958ec53830 ("json: add set statement list support")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoevaluate: compact STMT_F_STATEFUL checks
Florian Westphal [Mon, 31 Mar 2025 15:23:19 +0000 (17:23 +0200)] 
evaluate: compact STMT_F_STATEFUL checks

We'll gain another F_STATEFUL check in a followup patch,
so lets condense the pattern into a helper to reduce copypaste.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoexpression: don't try to import empty string
Florian Westphal [Thu, 27 Mar 2025 15:17:11 +0000 (16:17 +0100)] 
expression: don't try to import empty string

The bogon will trigger the assertion in mpz_import_data:
src/expression.c:418: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
3 months agoexpression: initialize list of expression to silence gcc compile warning
Pablo Neira Ayuso [Mon, 31 Mar 2025 15:15:39 +0000 (17:15 +0200)] 
expression: initialize list of expression to silence gcc compile warning

The helper function to translate flagcmp expression to binop expression
results in the following compile warning.

  src/expression.c: In function 'list_expr_to_binop':
  src/expression.c:1286:16: warning: 'last' may be used uninitialized [-Wmaybe-uninitialized]
  1286 |         return last;

While at it, add assert() to validate the premises where this function
can be called.

Fixes: 4d5990c92c83 ("src: transform flag match expression to binop expression from parser")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>