]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
5 months agosrc: rework singleton interval transformation to reduce memory consumption
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:41:06 +0000 (17:41 +0100)] 
src: rework singleton interval transformation to reduce memory consumption

set_to_intervals() expands range expressions into a list of singleton
elements before building the netlink message that is sent to userspace.
This is because the kernel expects this list of singleton elements where
EXPR_F_INTERVAL_END denotes a closing interval. This expansion
significantly increases memory consumption in userspace.

This patch updates the logic to transform the range expression up to two
temporary singleton element expressions through setelem_to_interval().
Then, these two elements are used to allocate the nftnl_set_elem objects
through alloc_nftnl_setelem_interval() to build the netlink message,
finally all these temporary objects are released. For anonymous sets,
when adjacent ranges are found, the end element is not added to the set
to pack the set representation as in the original set_to_intervals()
routine.

After this update, set_to_intervals() only deals with adding the
non-matching all zero element to the interval set when it is not there
as the kernel expects.

In combination with the new EXPR_RANGE_VALUE expression, this shrinks
runtime userspace memory consumption from 70.50 Mbytes to 43.38 Mbytes
for a 100k intervals set sample.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agomnl: do not send set size when set is constant set
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:41:01 +0000 (17:41 +0100)] 
mnl: do not send set size when set is constant set

When turning element range into the interval representation based on
singleton elements for the rbtree tree set backend, userspace adjusts
the size to the internal kernel implementation.

For constant sets, this is leaking an internal kernel implementation
detail that is fixed by kernel patch ("netfilter: nf_tables: fix set
size with rbtree backend"). For non-constant sets, set size is just
broken.

This patch is required by the follow up patch ("src: rework singleton
interval transformation to reduce memory consumption").

On top of this, constant sets cannot be updated once they are bound, set
size is not useful in this case. Remove this implicit set size for
constant sets.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agomnl: rename list of expression in mnl_nft_setelem_batch()
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:40:58 +0000 (17:40 +0100)] 
mnl: rename list of expression in mnl_nft_setelem_batch()

Rename set to init to prepare to pass struct set to this function in
the follow up patch. No functional changes are intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agorule: constify set_is_non_concat_range()
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:40:56 +0000 (17:40 +0100)] 
rule: constify set_is_non_concat_range()

This is read-only, constify it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agosrc: add EXPR_RANGE_VALUE expression and use it
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:40:54 +0000 (17:40 +0100)] 
src: add EXPR_RANGE_VALUE expression and use it

set element with range takes 4 instances of struct expr:

  EXPR_SET_ELEM -> EXPR_RANGE -> (2) EXPR_VALUE

where EXPR_RANGE represents two references to struct expr with constant
value.

This new EXPR_RANGE_VALUE trims it down to two expressions:

  EXPR_SET_ELEM -> EXPR_RANGE_VALUE

with two direct low and high values that represent the range:

  struct {
      mpz_t low;
      mpz_t high;
  };

this two new direct values in struct expr do not modify its size.

setelem_expr_to_range() translates EXPR_RANGE to EXPR_RANGE_VALUE, this
conversion happens at a later stage.

constant_range_expr_print() translates this structure to constant values
to reuse the existing datatype_print() which relies in singleton values.

The automerge routine has been updated to use EXPR_RANGE_VALUE.

This requires a follow up patch to rework the conversion from range
expression to singleton element to provide a noticeable memory
consumption reduction.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agointervals: do not merge intervals with different timeout
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:40:52 +0000 (17:40 +0100)] 
intervals: do not merge intervals with different timeout

If timeout/expiration of contiguous intervals is different, then do not
merge them.

Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agointervals: add helper function to set previous element
Pablo Neira Ayuso [Fri, 3 Jan 2025 16:40:48 +0000 (17:40 +0100)] 
intervals: add helper function to set previous element

Add helper function to set previous element during the automerge
iteration. No functional changes are intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agorule: make cmd_free(NULL) valid
Florian Westphal [Wed, 8 Jan 2025 11:30:15 +0000 (12:30 +0100)] 
rule: make cmd_free(NULL) valid

bison uses cmd_free($$) as destructor, but base_cmd can
set it to NULL, e.g.

  |       ELEMENT         set_spec        set_block_expr
  {
    if (nft_cmd_collapse_elems(CMD_ADD, state->cmds, &$2, $3)) {
       handle_free(&$2);
       expr_free($3);
       $$ = NULL;   // cmd set to NULL
       break;
    }
    $$ = cmd_alloc(CMD_ADD, CMD_OBJ_ELEMENTS, &$2, &@$, $3);

expr_free(NULL) is legal, cmd_free() causes crash.  So just allow
this to avoid cluttering parser_bison.y with "if ($$)".

Also add the afl-generated bogon input to the test files.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agoparser_bison: fix UaF when reporting table parse error
Florian Westphal [Tue, 7 Jan 2025 22:55:06 +0000 (23:55 +0100)] 
parser_bison: fix UaF when reporting table parse error

It passed already-freed memory to erec function.  Found with afl++ and asan.

Fixes: 4955ae1a81b7 ("Add support for table's persist flag")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agotests: shell: add cgroupv2 socket match test case
Florian Westphal [Thu, 2 Jan 2025 17:08:30 +0000 (18:08 +0100)] 
tests: shell: add cgroupv2 socket match test case

This is a test case for nft_socket cgroupv2 matching, including
support for matching inside a cgroupv2 mount space added in kernel
commit 7f3287db6543 ("netfilter: nft_socket: make cgroupsv2 matching work with namespaces").

Test is thus run twice, once in the initial namespace and once with
a changed cgroupv2 root.

In case we can't create a cgroup or the 2nd half (unshared re-run)
fails, indicate SKIP.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agomain: prepend error tag to printed errors when parsing options
Pablo Neira Ayuso [Tue, 17 Dec 2024 17:35:38 +0000 (18:35 +0100)] 
main: prepend error tag to printed errors when parsing options

Prepend "Error: " tag to printed errors in the option parsing step.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agolibnftables: include canonical path to avoid duplicates
Pablo Neira Ayuso [Thu, 12 Dec 2024 21:41:52 +0000 (22:41 +0100)] 
libnftables: include canonical path to avoid duplicates

A recent commit adds base directory of -f/--filename to include paths by
default to address a silly use of -I/--include to make this work:

  # nft -I /path/to -f /path/to/main.nft

instead users can simply invoke:

  # nft -f /path/to/main.nft

because /path/to/ is added at the end of the list of include paths.

This example above assumes main.nft includes more files that are
contained in /path/to/.

However, globbing can cause duplicates after this recent update, eg.

  # cat test/main
  table inet test {
        chain test {
                include "include/*";
        }
  }
  # nft -I /tmp/test/ -f test/main

because /tmp/test and test/ twice refer to the same directory and both
are added to the list of include path.

Use realpath() to canonicalize include paths. Then, search and skip
duplicated include paths.

Fixes: 302e9f8b3a13 ("libnftables: add base directory of -f/--filename to include path")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: shrink line_offset in struct location to 4 bytes
Pablo Neira Ayuso [Thu, 12 Dec 2024 23:06:14 +0000 (00:06 +0100)] 
src: shrink line_offset in struct location to 4 bytes

line_offset of 2^32 bytes should be enough.

This requires the removal of the last_line field (in a previous patch) to
shrink struct expr to 112 bytes.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: remove last_line from struct location
Pablo Neira Ayuso [Thu, 12 Dec 2024 23:04:40 +0000 (00:04 +0100)] 
src: remove last_line from struct location

This 4 bytes field is never used, remove it.

This does not shrink struct location in x86_64 due to alignment.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: remove unused token_offset from struct location
Pablo Neira Ayuso [Mon, 9 Dec 2024 00:23:08 +0000 (01:23 +0100)] 
src: remove unused token_offset from struct location

This saves 8 bytes in x86_64 in struct location which is embedded in
every expression.

This shrinks struct expr to 120 bytes according to pahole.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoexpression: remove elem_flags from EXPR_SET_ELEM to shrink struct expr size
Pablo Neira Ayuso [Sun, 8 Dec 2024 20:46:13 +0000 (21:46 +0100)] 
expression: remove elem_flags from EXPR_SET_ELEM to shrink struct expr size

Move NFTNL_SET_ELEM_F_INTERVAL_OPEN flag to the existing flags field in
struct expr.

This saves 4 bytes in struct expr, shrinking it to 128 bytes according to
pahole. This reworks:

6089630f54ce ("segtree: Introduce flag for half-open range elements")

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agointervals: set internal element location with the deletion trigger
Pablo Neira Ayuso [Wed, 4 Dec 2024 22:36:05 +0000 (23:36 +0100)] 
intervals: set internal element location with the deletion trigger

set location of internal elements (already in the kernel) to the one
that partial or fully deletes it.

Otherwise, error reporting refers to internal location.

Before this patch:

 # nft delete element x y { 1.1.1.3 }
 Error: Could not process rule: Too many open files in system
 delete element x y { 1.1.1.3 }
                      ^^^^^^^

After this patch:

 # nft delete element x y { 1.1.1.3 }
 Error: Could not process rule: Too many open files in system
 delete element x y { 1.1.1.3 }
                      ^^^^^^^

This occurs after splitting an existing interval in two:

 remove: [1010100-10101ff]
 add: [1010100-1010102]
 add: [1010104-10101ff]

which results in two additions after removing the existing interval
that is split.

Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: shell: add a test case for netdev ruleset flush + parallel link down
Florian Westphal [Sat, 7 Dec 2024 11:17:02 +0000 (12:17 +0100)] 
tests: shell: add a test case for netdev ruleset flush + parallel link down

Test for bug added with kernel commit
c03d278fdf35 ("netfilter: nf_tables: wait for rcu grace period on net_device removal")

Signed-off-by: Florian Westphal <fw@strlen.de>
7 months agosrc: allow binop expressions with variable right-hand operands
Jeremy Sowden [Mon, 18 Nov 2024 23:18:28 +0000 (00:18 +0100)] 
src: allow binop expressions with variable right-hand operands

Hitherto, the kernel has required constant values for the `xor` and
`mask` attributes of boolean bitwise expressions.  This has meant that
the right-hand operand of a boolean binop must be constant.  Now the
kernel has support for AND, OR and XOR operations with right-hand
operands passed via registers, we can relax this restriction.  Allow
non-constant right-hand operands if the left-hand operand is not
constant, e.g.:

  ct mark & 0xffff0000 | meta mark & 0xffff

The kernel now supports performing AND, OR and XOR operations directly,
on one register and an immediate value or on two registers, so we need
to be able to generate and parse bitwise boolean expressions of this
form.

If a boolean operation has a constant RHS, we continue to send a
mask-and-xor expression to the kernel.

Add tests for {ct,meta} mark with variable RHS operands.

JSON support is also included.

This requires Linux kernel >= 6.13-rc.

[ Originally posted as patch 1/8 and 6/8 which has been collapsed and
  simplified to focus on initial {ct,meta} mark support. Tests have
  been extracted from 8/8 including a tests/py fix to payload output
  due to incorrect output in original patchset. JSON support has been
  extracted from patch 7/8 --pablo]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 months agooptimize: compare expression length
Pablo Neira Ayuso [Mon, 18 Nov 2024 11:44:06 +0000 (12:44 +0100)] 
optimize: compare expression length

do not merge raw payload expressions with different length.

Other expression rely on key comparison which is assumed to have the
same length already.

Fixes: 60dcc01d6351 ("optimize: add __expr_cmp()")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 months agomnl: fix basehook comparison
Donald Yandt [Fri, 22 Nov 2024 22:04:49 +0000 (17:04 -0500)] 
mnl: fix basehook comparison

When comparing two hooks, if both device names are null,
the comparison should return true, as they are considered equal.

Fixes: b8872b83eb365 ("src: mnl: prepare for listing all device netdev device hooks")
Signed-off-by: Donald Yandt <donald.yandt@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
7 months agomnl: restore --debug=netlink output with chains
Pablo Neira Ayuso [Thu, 7 Nov 2024 08:00:55 +0000 (09:00 +0100)] 
mnl: restore --debug=netlink output with chains

table and chain name are not displayed with --debug=netlink:

 # nft --debug=netlink -f /tmp/x
 inet (null) (null) use 0
 ...

Similar to 79acbfdbe536 ("mnl: restore --debug=netlink output with sets").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 months agosrc: allow to map key to nfqueue number
Florian Westphal [Fri, 25 Oct 2024 07:47:25 +0000 (09:47 +0200)] 
src: allow to map key to nfqueue number

Allow to specify a numeric queue id as part of a map.
The parser side is easy, but the reverse direction (listing) is not.

'queue' is a statement, it doesn't have an expression.

Add a generic 'queue_type' datatype as a shim to the real basetype with
constant expressions, this is used only for udata build/parse, it stores
the "key" (the parser token, here "queue") as udata in kernel and can
then restore the original key.

Add a dumpfile to validate parser & output.

JSON support is missing because JSON allow typeof only since quite
recently.

Joint work with Pablo.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1455
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 months agodatatype: remove unused flags field
Pablo Neira Ayuso [Wed, 6 Nov 2024 22:31:06 +0000 (23:31 +0100)] 
datatype: remove unused flags field

Leftover unused struct datatype field, remove it.

Fixes: e35aabd511c4 ("datatype: replace DTYPE_F_ALLOC by bitfield")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 months agotests: monitor: Become $PWD agnostic
Phil Sutter [Thu, 7 Nov 2024 13:39:51 +0000 (14:39 +0100)] 
tests: monitor: Become $PWD agnostic

The call to 'cd' is problematic since later the script tries to 'exec
unshare -n $0'. This is not the only problem though: Individual test
cases specified on command line are expected to be relative to the
script's directory, too. Just get rid of these nonsensical restrictions.

Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agotests: monitor: Run in own netns
Phil Sutter [Wed, 2 Oct 2024 16:17:07 +0000 (18:17 +0200)] 
tests: monitor: Run in own netns

Have the script call itself prefixed by unshare. This won't prevent
clashing test case contents, but at least leave the host netns alone.

Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agomonitor: Recognize flowtable add/del events
Phil Sutter [Wed, 15 May 2024 14:01:20 +0000 (16:01 +0200)] 
monitor: Recognize flowtable add/del events

These were entirely ignored before, add the necessary code analogous to
e.g. objects.

Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agotests: py: Fix for storing payload into missing file
Phil Sutter [Wed, 2 Oct 2024 17:55:49 +0000 (19:55 +0200)] 
tests: py: Fix for storing payload into missing file

When running a test for which no corresponding *.payload file exists,
the *.payload.got file name was incorrectly constructed due to
'payload_path' variable not being set.

Fixes: 2cfab7a3e10fc ("tests/py: Write dissenting payload into the right file")
Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agojson: Support typeof in set and map types
Phil Sutter [Fri, 27 Sep 2024 22:55:34 +0000 (00:55 +0200)] 
json: Support typeof in set and map types

Implement this as a special "type" property value which is an object
with sole property "typeof". The latter's value is the JSON
representation of the expression in set->key, so for concatenated
typeofs it is a concat expression.

All this is a bit clumsy right now but it works and it should be
possible to tear it down a bit for more user-friendliness in a
compatible way by either replacing the concat expression by the array it
contains or even the whole "typeof" object - the parser would just
assume any object (or objects in an array) in the "type" property value
are expressions to extract a type from.

Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agojson: collapse set element commands from parser
Pablo Neira Ayuso [Thu, 31 Oct 2024 20:38:02 +0000 (21:38 +0100)] 
json: collapse set element commands from parser

Update json parser to collapse {add,create} element commands to reduce
memory consumption in the case of large sets defined by one element per
command:

{"nftables": [{"add": {"element": {"family": "ip", "table": "x", "name":
"y", "elem": [{"set": ["1.1.0.0"]}]}}},...]}

Add CTX_F_COLLAPSED flag to report that command has been collapsed.

This patch reduces memory consumption by ~32% this case.

Fixes: 20f1c60ac8c8 ("src: collapse set element commands from parser")
Reported-by: Eric Garver <eric@garver.life>
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agotests: shell: move device to different namespace
Pablo Neira Ayuso [Mon, 28 Oct 2024 22:17:14 +0000 (23:17 +0100)] 
tests: shell: move device to different namespace

This actually triggers a UNREGISTER event, it is similar to existing
tests, but add this test to improve coverage for this scenario.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agodoc: extend description of fib expression
Florian Westphal [Thu, 10 Oct 2024 13:37:42 +0000 (15:37 +0200)] 
doc: extend description of fib expression

Describe the input keys and the result types.
Mention which input keys are mandatory and which keys are mutually
exclusive.

Describe which hooks can be used with the various lookup modifiers
and extend the examples with more information on fib expression
capabilities.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1663
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agotests: monitor: fix up test case breakage
Florian Westphal [Tue, 29 Oct 2024 20:12:19 +0000 (21:12 +0100)] 
tests: monitor: fix up test case breakage

Monitor test fails:

 echo: running tests from file set-simple.t
 echo output differs!
  -add element ip t portrange { 1024-65535 }
   add element ip t portrange { 100-200 }
  +add element ip t portrange { 1024-65535 }
  +# new generation 510 by process 129009 (nft)

I also noticed -j mode did not work correctly, add missing json annotations
in set-concat-interval.t while at it.

Signed-off-by: Florian Westphal <fw@strlen.de>
8 months agotests: shell: don't rely on writable test directory
Florian Westphal [Tue, 22 Oct 2024 13:26:54 +0000 (15:26 +0200)] 
tests: shell: don't rely on writable test directory

Running shell tests from a virtme-ng instance with ro mapped test dir
hangs due to runaway 'awk' reading from stdin instead of the intended
$tmpfile (variable is empty), so add quotes where needed.

0002relative_0 wants to check relative includes. It tries to create a
temporary file in the current directory, which fails as thats readonly
inside the virtme vm instance.

[ -w ! $foo ... did not catch this due to missing "".
Add quotes and return the skip retval so the test gets flagged as skipped.

0013input_descriptors_included_files_0 and 0020include_chain_0 are
switched to normal tmpfiles, there is nothing in the test that needs
relative includes.

Also, get rid of some error tests for subsequent mktemp calls for
scripts that already called 'set -e'.

Signed-off-by: Florian Westphal <fw@strlen.de>
8 months agosrc: fix extended netlink error reporting with large set elements
Pablo Neira Ayuso [Wed, 23 Oct 2024 22:24:55 +0000 (00:24 +0200)] 
src: fix extended netlink error reporting with large set elements

Large sets can expand into several netlink messages, use sequence number
and attribute offset to correlate the set element and the location.

When set element command expands into several netlink messages,
increment sequence number for each netlink message. Update struct cmd to
store the range of netlink messages that result from this command.

struct nlerr_loc remains in the same size in x86_64.

 # nft -f set-65535.nft
 set-65535.nft:65029:22-32: Error: Could not process rule: File exists
 create element x y { 1.1.254.253 }
                      ^^^^^^^^^^^

Fixes: f8aec603aa7e ("src: initial extended netlink error reporting")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agorule: netlink attribute offset is uint32_t for struct nlerr_loc
Pablo Neira Ayuso [Wed, 23 Oct 2024 22:08:24 +0000 (00:08 +0200)] 
rule: netlink attribute offset is uint32_t for struct nlerr_loc

The maximum netlink message length (nlh->nlmsg_len) is uint32_t, struct
nlerr_loc stores the offset to the netlink attribute which must be
uint32_t, not uint16_t.

While at it, remove check for zero netlink attribute offset in
nft_cmd_error() which should not ever happen, likely this check was
there to prevent the uint16_t offset overflow.

Fixes: f8aec603aa7e ("src: initial extended netlink error reporting")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agomnl: update cmd_add_loc() to take struct nlmsghdr
Pablo Neira Ayuso [Wed, 23 Oct 2024 21:07:31 +0000 (23:07 +0200)] 
mnl: update cmd_add_loc() to take struct nlmsghdr

To prepare for a fix for very large sets.

No functional change is intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agomnl: rename to mnl_seqnum_alloc() to mnl_seqnum_inc()
Pablo Neira Ayuso [Wed, 23 Oct 2024 20:15:24 +0000 (22:15 +0200)] 
mnl: rename to mnl_seqnum_alloc() to mnl_seqnum_inc()

rename mnl_seqnum_alloc() to mnl_seqnum_inc().

No functional change is intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agosrc: collapse set element commands from parser
Pablo Neira Ayuso [Wed, 23 Oct 2024 09:43:58 +0000 (11:43 +0200)] 
src: collapse set element commands from parser

498a5f0c219d ("rule: collapse set element commands") does not help to
reduce memory consumption in the case of large sets defined by one
element per line:

 add element ip x y { 1.1.1.1 }
 add element ip x y { 1.1.1.2 }
 ...

This patch reduces memory consumption by ~75%, set elements are
collapsed into an existing cmd object wherever possible to reduce the
number of cmd objects.

This patch also adds a special case for variables for sets similar to:

  be055af5c58d ("cmd: skip variable set elements when collapsing commands")

This patch requires this small kernel fix:

 commit b53c116642502b0c85ecef78bff4f826a7dd4145
 Author: Pablo Neira Ayuso <pablo@netfilter.org>
 Date:   Fri May 20 00:02:06 2022 +0200

    netfilter: nf_tables: set element extended ACK reporting support

which is already included in recent -stable kernels:

 # cat ruleset.nft
 add table ip x
 add chain ip x y
 add set ip x y { type ipv4_addr; }
 create element ip x y { 1.1.1.1 }
 create element ip x y { 1.1.1.1 }

 # nft -f ruleset.nft
 ruleset.nft:5:25-31: Error: Could not process rule: File exists
 create element ip x y { 1.1.1.1 }
                         ^^^^^^^

since there is no need to relate commands via sequence number anymore,
this allows also removes the uncollapse step.

Fixes: 498a5f0c219d ("rule: collapse set element commands")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
8 months agolibnftables-json: fix raw payload expression documentation
Eric Long [Thu, 17 Oct 2024 15:33:17 +0000 (23:33 +0800)] 
libnftables-json: fix raw payload expression documentation

Raw payload expression accesses payload data in bits, not bytes.

Fixes: 872f373dc50f7 ("doc: Add JSON schema documentation")
Signed-off-by: Eric Long <i@hack3r.moe>
Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agotests: shell: Join arithmetic statements in maps/vmap_timeout
Phil Sutter [Fri, 11 Oct 2024 09:22:44 +0000 (11:22 +0200)] 
tests: shell: Join arithmetic statements in maps/vmap_timeout

In light of the recent typo fix, go an extra step and merge the modulo
and offset adjustment in a single term.

Signed-off-by: Phil Sutter <phil@nwl.cc>
8 months agotests: shell: fix spurious dump failure in vmap timeout test
Florian Westphal [Fri, 11 Oct 2024 00:32:08 +0000 (02:32 +0200)] 
tests: shell: fix spurious dump failure in vmap timeout test

Blamed commit can update the timeout to 6s, but last line waits
for 5 seconds and expects that to be enough to have all elements vanish.

Fix the typo to limit update timeout also to 5 seconds and not 6.
This fixes spurious dump failures like this one:

-               elements = { 1.2.3.4 . 22 : jump ssh_input }
+               elements = { 1.2.3.4 . 22 : jump ssh_input,
+                            10.0.95.144 . 38023 timeout 6s expires 545ms : jump other_input }

Fixes: db80037c0279 ("tests: shell: extend vmap test with updates")
Signed-off-by: Florian Westphal <fw@strlen.de>
9 months agobuild: Bump version to 1.1.1 v1.1.1
Pablo Neira Ayuso [Wed, 2 Oct 2024 20:45:45 +0000 (22:45 +0200)] 
build: Bump version to 1.1.1

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agocache: initialize filter when fetching implicit chains
Pablo Neira Ayuso [Tue, 17 Sep 2024 17:18:09 +0000 (19:18 +0200)] 
cache: initialize filter when fetching implicit chains

ASAN reports:

  src/cache.c:734:25: runtime error: load of value 189, which is not a valid value for type '_Bool'

because filter->reset.rule remains uninitialized.

Initialize filter and replace existing construct to initialize table and
chain which leaves remaining fields uninitialized.

Fixes: dbff26bfba83 ("cache: consolidate reset command")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agodoc: tproxy is non-terminal in nftables
Pablo Neira Ayuso [Sun, 15 Sep 2024 22:34:27 +0000 (00:34 +0200)] 
doc: tproxy is non-terminal in nftables

iptables TPROXY issues NF_ACCEPT while nftables tproxy allows for
post-processing. Update examples. For more info, see:

https://lore.kernel.org/netfilter-devel/ZuSh_Io3Yt8LkyUh@orbyte.nwl.cc/T/

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agosrc: support for timeout never in elements
Pablo Neira Ayuso [Mon, 2 Sep 2024 23:32:05 +0000 (01:32 +0200)] 
src: support for timeout never in elements

Allow to specify elements that never expire in sets with global
timeout.

    set x {
        typeof ip saddr
        timeout 1m
        elements = { 1.1.1.1 timeout never,
                     2.2.2.2,
                     3.3.3.3 timeout 2m }
    }

in this example above:

 - 1.1.1.1 is a permanent element
 - 2.2.2.2 expires after 1 minute (uses default set timeout)
 - 3.3.3.3 expires after 2 minutes (uses specified timeout override)

Use internal NFT_NEVER_TIMEOUT marker as UINT64_MAX to differenciate
between use default set timeout and timeout never if "timeout N" is used
in set declaration. Maximum supported timeout in milliseconds which is
conveyed within a netlink attribute is 0x10c6f7a0b5ec.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agotests: shell: more randomization for timeout parameter
Florian Westphal [Sat, 14 Sep 2024 21:56:14 +0000 (23:56 +0200)] 
tests: shell: more randomization for timeout parameter

Either pass no timeout argument, pass timeout+expires or omit
timeout (uses default timeout, if any).

This should not expose further kernel code to run at this time, but unlike
the existing (deterministic) element-update test case this script does
have live traffic and different set types, including rhashtable which has
async gc.

Signed-off-by: Florian Westphal <fw@strlen.de>
9 months agotests: py: fix up udp csum fixup output
Florian Westphal [Wed, 11 Sep 2024 12:23:01 +0000 (14:23 +0200)] 
tests: py: fix up udp csum fixup output

Preceeding commit switched udp to use the inkernel csum parser, so tests
warn:

WARNING: line 7: 'add rule ip test-ip4 input iif "lo" udp checksum set 0':
'[ payload write reg 1 => 2b @ transport header + 6 csum_type 1 csum_off 6 csum_flags 0x0 ]' mismatches
'[ payload write reg 1 => 2b @ transport header + 6 csum_type 0 csum_off 0 csum_flags 0x1 ]'

Fixes: f89abfb4068d ("proto: use NFT_PAYLOAD_L4CSUM_PSEUDOHDR flag to mangle UDP checksum")
Signed-off-by: Florian Westphal <fw@strlen.de>
9 months agoproto: use NFT_PAYLOAD_L4CSUM_PSEUDOHDR flag to mangle UDP checksum
Pablo Neira Ayuso [Mon, 9 Sep 2024 10:48:33 +0000 (12:48 +0200)] 
proto: use NFT_PAYLOAD_L4CSUM_PSEUDOHDR flag to mangle UDP checksum

There are two mechanisms to update the UDP checksum field:

 1) _CSUM_TYPE and _CSUM_OFFSET which specify the type of checksum
    (e.g. inet) and offset where it is located.
 2) use NFT_PAYLOAD_L4CSUM_PSEUDOHDR flag to use layer 4 kernel
    protocol parser.

The problem with 1) is that it is inconditional, that is, csum_type and
csum_offset cannot deal with zero UDP checksum.

Use NFT_PAYLOAD_L4CSUM_PSEUDOHDR flag instead since it relies on the
layer 4 kernel parser which skips updating zero UDP checksum.

Extend test coverage for the UDP mangling with and without zero
checksum.

Fixes: e6c9174e13b2 ("proto: add checksum key information to struct proto_desc")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agotests: shell: stabilize packetpath/payload
Pablo Neira Ayuso [Mon, 9 Sep 2024 10:47:35 +0000 (12:47 +0200)] 
tests: shell: stabilize packetpath/payload

- Add sleep calls after setting up container topology.
- Extend TCP connect timeout to 4 seconds. Test has no listener, this is
  just sending SYN packets that are rejected but it works to test the
  payload mangling ruleset.
- fix incorrect logic to check for 0 matching packets through grep.

Fixes: 84da729e067a ("tests: shell: add test to cover payload transport match and mangle")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
9 months agotests: shell: add test case for timeout updates
Florian Westphal [Mon, 9 Sep 2024 15:26:01 +0000 (17:26 +0200)] 
tests: shell: add test case for timeout updates

Needs a feature check file, so add one:
Add element with 1m timeout, then update expiry to 1ms.
If element still exists after 1ms, update request was ignored.

Test case checks timeouts can both be incremented and decremented,
checks error recovery (update request but transaction fails) and
that expiry is restored in addion to timeout.

Signed-off-by: Florian Westphal <fw@strlen.de>
9 months agotests: shell: extend vmap test with updates
Florian Westphal [Mon, 9 Sep 2024 15:13:46 +0000 (17:13 +0200)] 
tests: shell: extend vmap test with updates

It won't validate that the update is actually effective,
but it will trigger relevant update logic in kernel.

This means the updated test works even if the kernel doesn't
support updates.

A dedicated test will be added to check timeout updates work.

Signed-off-by: Florian Westphal <fw@strlen.de>
9 months agotests: shell: add test for kernel stack recursion bug
Florian Westphal [Tue, 10 Sep 2024 09:47:44 +0000 (11:47 +0200)] 
tests: shell: add test for kernel stack recursion bug

Validate that such ruleset updates get rejected.

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agolibnftables: Zero ctx->vars after freeing it
Phil Sutter [Tue, 3 Sep 2024 15:43:19 +0000 (17:43 +0200)] 
libnftables: Zero ctx->vars after freeing it

Leaving the invalid pointer value in place will cause a double-free when
users call nft_ctx_clear_vars() first, then nft_ctx_free(). Moreover,
nft_ctx_add_var() passes the pointer to mrealloc() and thus assumes it
to be either NULL or valid.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1772
Fixes: 9edaa6a51eab4 ("src: add --define key=value")
Signed-off-by: Phil Sutter <phil@nwl.cc>
10 months agotests: shell: extend coverage for meta l4proto netdev/egress matching
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:05 +0000 (12:42 +0200)] 
tests: shell: extend coverage for meta l4proto netdev/egress matching

Extend coverage to match on small UDP packets from netdev/egress.

While at it, cover bridge/input and bridge/output hooks too.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: position does not require full cache
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:17 +0000 (12:42 +0200)] 
cache: position does not require full cache

position refers to the rule handle, it has similar cache requirements as
replace rule command, relax cache requirements.

Commit e5382c0d08e3 ("src: Support intra-transaction rule references")
uses position.id for index support which requires a full cache, but
only in such case.

Fixes: 01e5c6f0ed03 ("src: add cache level flags")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: relax requirement for replace rule command
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:14 +0000 (12:42 +0200)] 
cache: relax requirement for replace rule command

No need for full cache, this command relies on the rule handle which is
not validated from userspace. Cache requirements are similar to those
of add/create/delete rule commands.

This speeds up incremental updates with large rulesets.

Extend tests/coverage for rule replacement.

Fixes: 01e5c6f0ed03 ("src: add cache level flags")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: remove full cache requirement when echo flag is set on
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:12 +0000 (12:42 +0200)] 
cache: remove full cache requirement when echo flag is set on

The echo flag does not use the cache infrastructure yet, it relies on
the monitor cache which follows the netlink_echo_callback() path.

Fixes: 01e5c6f0ed03 ("src: add cache level flags")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: clean up evaluate_cache_del()
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:10 +0000 (12:42 +0200)] 
cache: clean up evaluate_cache_del()

Move NFT_CACHE_TABLE flag to default case to disentangle this.

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: assert filter when calling nft_cache_evaluate()
Pablo Neira Ayuso [Thu, 29 Aug 2024 10:42:08 +0000 (12:42 +0200)] 
cache: assert filter when calling nft_cache_evaluate()

nft_cache_evaluate() always takes a non-null filter, remove superfluous
checks when calculating cache requirements via flags.

Note that filter is still option from netlink dump path, since this can
be called from error path to provide hints.

Fixes: 08725a9dc14c ("cache: filter out rules by chain")
Fixes: b3ed8fd8c9f3 ("cache: missing family in cache filtering")
Fixes: 635ee1cad8aa ("cache: filter out sets and maps that are not requested")
Fixes: 3f1d3912c3a6 ("cache: filter out tables that are not requested")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agotests: shell: cover reset command with counter and quota
Pablo Neira Ayuso [Sun, 25 Aug 2024 22:41:45 +0000 (00:41 +0200)] 
tests: shell: cover reset command with counter and quota

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agotests: shell: cover anonymous set with reset command
Pablo Neira Ayuso [Sun, 25 Aug 2024 22:41:43 +0000 (00:41 +0200)] 
tests: shell: cover anonymous set with reset command

Extend existing test to reset counters for rules with anonymous set.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1763
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: consolidate reset command
Pablo Neira Ayuso [Sun, 25 Aug 2024 22:41:42 +0000 (00:41 +0200)] 
cache: consolidate reset command

Reset command does not utilize the cache infrastructure.

This implicitly fixes a crash with anonymous sets because elements are
not fetched. I initially tried to fix it by toggling the missing cache
flags, but then ASAN reports memleaks.

To address these issues relies on Phil's list filtering infrastructure
which updates is expanded to accomodate filtering requirements of the
reset commands, such as 'reset table ip' where only the family is sent
to the kernel.

After this update, tests/shell reports a few inconsistencies between
reset and list commands:

- reset rules chain t c2

  display sets, but it should only list the given chain.

- reset rules table t
  reset rules ip

  do not list elements in the set. In both cases, these are fully
  listing a given table and family, elements should be included.

The consolidation also ensures list and reset will not differ.

A few more notes:

- CMD_OBJ_TABLE is used for:

rules family table

  from the parser, due to the lack of a better enum, same applies to
  CMD_OBJ_CHAIN.

- CMD_OBJ_ELEMENTS still does not use the cache, but same occurs in
  the CMD_GET command case which needs to be consolidated.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1763
Fixes: 83e0f4402fb7 ("Implement 'reset {set,map,element}' commands")
Fixes: 1694df2de79f ("Implement 'reset rule' and 'reset rules' commands")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: only dump rules for the given table
Pablo Neira Ayuso [Sun, 25 Aug 2024 22:41:40 +0000 (00:41 +0200)] 
cache: only dump rules for the given table

Only family is set on in the dump request, set on table and chain
otherwise, rules for the given family are fetched for each existing
table.

Fixes: afbd102211dc ("src: do not use the nft_cache_filter object from mnl.c")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: add filtering support for objects
Pablo Neira Ayuso [Sun, 25 Aug 2024 22:41:37 +0000 (00:41 +0200)] 
cache: add filtering support for objects

Currently, full ruleset flag is set on to fetch objects.

Follow a similar approach to these patches from Phil:

 de961b930660 ("cache: Filter set list on server side") and
 cb4b07d0b628 ("cache: Support filtering for a specific flowtable")

in preparation to update the reset command to use the cache
infrastructure.

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: accumulate flags in batch
Pablo Neira Ayuso [Mon, 26 Aug 2024 08:19:39 +0000 (10:19 +0200)] 
cache: accumulate flags in batch

Recent updates are relaxing cache requirements:

  babc6ee8773c ("cache: populate chains on demand from error path")

Flags describe cache requirements for a given batch, accumulate flags
that are inferred from commands in this batch.

Fixes: 7df42800cf89 ("src: single cache_update() call to build cache before evaluation")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: reset filter for each command
Pablo Neira Ayuso [Mon, 26 Aug 2024 08:18:34 +0000 (10:18 +0200)] 
cache: reset filter for each command

Inconditionally reset filter for each command in the batch, this is safer.

Fixes: 3f1d3912c3a6 ("cache: filter out tables that are not requested")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agoparser_json: fix crash in json_parse_set_stmt_list
Sebastian Walz (sivizius) [Mon, 19 Aug 2024 22:09:26 +0000 (00:09 +0200)] 
parser_json: fix crash in json_parse_set_stmt_list

Due to missing `NULL`-check, there will be a segfault for invalid statements.

Fixes: 07958ec53830 ("json: add set statement list support")
Signed-off-by: Sebastian Walz (sivizius) <sebastian.walz@secunet.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agoparser_json: fix handle memleak from error path
Pablo Neira Ayuso [Mon, 19 Aug 2024 19:34:49 +0000 (21:34 +0200)] 
parser_json: fix handle memleak from error path

Based on patch from Sebastian Walz.

Fixes: 586ad210368b ("libnftables: Implement JSON parser")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agoparser_json: fix several expression memleaks from error path
Sebastian Walz (sivizius) [Mon, 19 Aug 2024 18:11:44 +0000 (20:11 +0200)] 
parser_json: fix several expression memleaks from error path

Fixes: 586ad210368b ("libnftables: Implement JSON parser")
Signed-off-by: Sebastian Walz (sivizius) <sebastian.walz@secunet.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agoparser_json: release buffer returned by json_dumps
Sebastian Walz (sivizius) [Mon, 19 Aug 2024 17:58:14 +0000 (19:58 +0200)] 
parser_json: release buffer returned by json_dumps

The signature of `json_dumps` is:

`char *json_dumps(const json_t *json, size_t flags)`:

It will return a pointer to an owned string, the caller must free it.
However, `json_error` just borrows the string to format it as `%s`, but
after printing the formatted error message, the pointer to the string is
lost and thus never freed.

Fixes: 586ad210368b ("libnftables: Implement JSON parser")
Signed-off-by: Sebastian Walz (sivizius) <sebastian.walz@secunet.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agodatatype: replace DTYPE_F_ALLOC by bitfield
Pablo Neira Ayuso [Mon, 19 Aug 2024 19:09:04 +0000 (21:09 +0200)] 
datatype: replace DTYPE_F_ALLOC by bitfield

Only user of the datatype flags field is DTYPE_F_ALLOC, replace it by
bitfield, squash byteorder to 8 bits which is sufficient.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agosrc: remove DTYPE_F_PREFIX
Pablo Neira Ayuso [Mon, 19 Aug 2024 19:03:02 +0000 (21:03 +0200)] 
src: remove DTYPE_F_PREFIX

only ipv4 and ipv6 datatype support this, add datatype_prefix_notation()
helper function to report that datatype prefers prefix notation, if
possible.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agosrc: mnl: always dump all netdev hooks if no interface name was given
Florian Westphal [Tue, 20 Aug 2024 22:12:27 +0000 (00:12 +0200)] 
src: mnl: always dump all netdev hooks if no interface name was given

Instead of not returning any results for
  nft list hooks netdev

Iterate all interfaces and then query all of them.

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agosrc: mnl: prepare for listing all device netdev device hooks
Florian Westphal [Tue, 20 Aug 2024 22:12:26 +0000 (00:12 +0200)] 
src: mnl: prepare for listing all device netdev device hooks

Change output foramt slightly so device name is included for netdev
family.

% nft list hooks netdev device eth0
family netdev {
        hook ingress device eth0 {
                 0000000000 chain inet ingress in_public [nf_tables]
                 0000000000 chain netdev ingress in_public [nf_tables]
        }
        hook egress device eth0 {
                 0000000000 chain netdev ingress out_public [nf_tables]
        }
}

Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agodoc: update outdated route and pkttype info
谢致邦 (XIE Zhibang) [Tue, 20 Aug 2024 09:15:03 +0000 (09:15 +0000)] 
doc: update outdated route and pkttype info

inet family supports route type.
unicast pkttype changed to host pkttype.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
10 months agoparser_bison: allow 0 burst in limit rate byte mode
Pablo Neira Ayuso [Thu, 15 Aug 2024 11:56:21 +0000 (13:56 +0200)] 
parser_bison: allow 0 burst in limit rate byte mode

Unbreak restoring elements in set with rate limit that fail with:

> /dev/stdin:3618:61-61: Error: limit burst must be > 0
>                  elements = { 1.2.3.4 limit rate over 1000 kbytes/second timeout 1s,

no need for burst != 0 for limit rate byte mode.

Add tests/shell too.

Fixes: 702eff5b5b74 ("src: allow burst 0 for byte ratelimit and use it as default")
Fixes: 285baccfea46 ("src: disallow burst 0 in ratelimits")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: do not fetch set inconditionally on delete
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:47:54 +0000 (12:47 +0200)] 
cache: do not fetch set inconditionally on delete

This is only required to remove elements, relax cache requirements for
anything else.

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: populate flowtables on demand from error path
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:34:17 +0000 (12:34 +0200)] 
cache: populate flowtables on demand from error path

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

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
10 months agocache: populate objects on demand from error path
Pablo Neira Ayuso [Thu, 15 Aug 2024 10:34:13 +0000 (12:34 +0200)] 
cache: populate objects on demand from error path

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

Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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>
10 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>
10 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>
10 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>