]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
7 weeks agoscanner: better error reporting for CRLF line terminators
Pablo Neira Ayuso [Mon, 6 Jan 2025 23:00:50 +0000 (00:00 +0100)] 
scanner: better error reporting for CRLF line terminators

commit 8c35615297983227cac1437edbe0cdedf4c2227b upstream.

Provide a hint to users that file is coming with CRLF line terminators,
maybe from a non-Linux OS.

Extend scanner.l to provide hint on CRLF in files:

 # file test.nft
 test.nft: ASCII text, with CRLF, LF line terminators
 # nft -f test.nft
 test.nft:1:13-14: Error: syntax error, unexpected CRLF line terminators
 table ip x {
             ^^

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
7 weeks 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

commit 581e051ae26b503484b7634b8799a9b9b531e95d upstream.

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>
7 weeks 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

commit 93077e35accccd8cc056b67f70bfb3182c819fd4 upstream.

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 agoevaluate: reset statement length context before evaluating statement
Pablo Neira Ayuso [Wed, 6 Dec 2023 17:48:29 +0000 (18:48 +0100)] 
evaluate: reset statement length context before evaluating statement

commit 8d3de823b622136e1d05a6fed11ff2dc0e804f8a upstream.

This patch consolidates ctx->stmt_len reset in stmt_evaluate() to avoid
this problem. Note that stmt_evaluate_meta() and stmt_evaluate_ct()
already reset it after the statement evaluation.

Moreover, statement dependency can be generated while evaluating a meta
and ct statement. Payload statement dependency already manually stashes
this before calling stmt_evaluate(). Add a new stmt_dependency_evaluate()
function to stash statement length context when evaluating a new statement
dependency and use it for all of the existing statement dependencies.

Florian also says:

'meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }' will
crash. Reason is that the l2 dependency generated here is errounously
expanded to a 32bit-one, so the evaluation path won't recognize this
as a L2 dependency.  Therefore, pctx->stacked_ll_count is 0 and
__expr_evaluate_payload() crashes with a null deref when
dereferencing pctx->stacked_ll[0].

nft-test.py gains a fugly hack to tolerate '!map typeof vlan id : meta mark'.
For more generic support we should find something more acceptable, e.g.

!map typeof( everything here is a key or data ) timeout ...

tests/py update and assert(pctx->stacked_ll_count) by Florian Westphal.

Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agooptimize: compare expression length
Pablo Neira Ayuso [Mon, 18 Nov 2024 11:44:06 +0000 (12:44 +0100)] 
optimize: compare expression length

commit bc0311378285d41850e3508df905d75959ba4239 upstream.

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>
6 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

commit 058246016188c8418cae1b3db70b16b935b1fe7c upstream.

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>
6 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

commit 570320ab9a0752c7749a6c9cc85b34a5e7ab91b5 upstream.

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>
6 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

commit bb6312484af93a83a9ec8716f3887a43566a775a upstream.

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>
6 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

commit e3d2a5e852ceea587bfff5878e6e5c569f15116a upstream.

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>
6 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

commit f89abfb4068d31f7279cae298abf25e0c077d2d3 upstream.

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>
6 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

commit d361be1f8734461e27117f6c569acf2189fcf81e upstream.

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>
6 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

commit d414f756af9d638fe0c0002b2df31c8c17a15002 upstream.

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>
6 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

commit 4984da8cc427974ea63796fa60a791b714a71440 upstream.

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>
6 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

commit 53a503ad4a1abfa0374b3d12e884b69dc6df4b4f upstream.

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>
6 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()

commit 19702ae3d5da18fef64248f95df471c6664dd08e upstream.

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>
6 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()

commit 4dd20f3bbd606eed4869ebe449debee8b2ac7900 upstream.

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>
6 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

commit ebd06f85a3257c294572005d0fa6b8ab0f213486 upstream.

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>
6 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

commit 68c8fb5f7c988a38a694c77c65e789e0cb8dfd8a upstream.

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>
6 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

commit 29cb49d0ca92b840938823dec697d8c5488d7253 upstream.

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>
6 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

commit 26d9cbefb10e6bc3765df7e9e7a4fc3b951a80f3 upstream.

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>
6 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

commit 47e18c0eba51a538e1110322d1a9248b0501d7c8 upstream.

Based on patch from Sebastian Walz.

Fixes: 586ad210368b ("libnftables: Implement JSON parser")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 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

commit bae7b4d283826efbeb28c21aecd7b355e86da170 upstream.

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>
6 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

commit 46700fbdbbbaab0d7db716fce3a438334c58ac9e upstream.

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>
6 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

commit 5089d0f46676aa13ab679f6e4820a08957f2e7e6 upstream.

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>
6 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

commit cea05ae5bdc50949d4c734796d6db5717187055a upstream.

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>
6 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

commit ba13acf4be081129d5c943db9f607a13954be5f6 upstream.

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>
6 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

commit 52d99078521f0ae245ad0145348bebdba9f665ab upstream.

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>
6 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

commit aab2fe87a665c0cba2676096b49b5c8ea21910f8 upstream.

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>
6 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

commit babc6ee8773cfeed78167f78827b35e3141e04c6 upstream.

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>
6 months agocmd: provide better hint if chain is already declared with different type/hook/priority
Pablo Neira Ayuso [Mon, 10 Jun 2024 17:36:21 +0000 (19:36 +0200)] 
cmd: provide better hint if chain is already declared with different type/hook/priority

commit 1f321f86c45fce88a5bcd6f8eafa0157248c8b38 upstream.

Display the following error in such case:

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

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

Fixes: 573788e05363 ("src: improve error reporting for unsupported chain type")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agometa: stash context statement length when generating payload/meta dependency
Pablo Neira Ayuso [Tue, 18 Jul 2023 21:10:01 +0000 (23:10 +0200)] 
meta: stash context statement length when generating payload/meta dependency

commit 5f1676ac9f1aeb36d7695c3c354dade013a1e4f3 upstream.

... meta mark set ip dscp

generates an implicit dependency from the inet family to match on meta
nfproto ip.

The length of this implicit expression is incorrectly adjusted to the
statement length, ie. relational to compare meta nfproto takes 4 bytes
instead of 1 byte. The evaluation of 'ip dscp' under the meta mark
statement triggers this implicit dependency which should not consider
the context statement length since it is added before the statement
itself.

This problem shows when listing the ruleset, since netlink_parse_cmp()
where left->len < right->len, hence handling the implicit dependency as
a concatenation, but it is actually a bug in the evaluation step that
leads to incorrect bytecode.

Fixes: 3c64ea7995cb ("evaluate: honor statement length in integer evaluation")
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Tested-by: Brian Davidson <davidson.brian@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agonetlink_linearize: use div_round_up in byteorder length
Pablo Neira Ayuso [Thu, 6 Jul 2023 08:26:39 +0000 (10:26 +0200)] 
netlink_linearize: use div_round_up in byteorder length

commit 25e7b99cc450490c38becb03d8bddd0199cfd3f9 upstream.

Use div_round_up() to calculate the byteorder length, otherwise fields
that take % BITS_PER_BYTE != 0 are not considered by the byteorder
expression.

Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: honor statement length in integer evaluation
Pablo Neira Ayuso [Thu, 23 Mar 2023 11:52:39 +0000 (12:52 +0100)] 
evaluate: honor statement length in integer evaluation

commit 3c64ea7995cbbc4f1d9d7707f907667325eb62b9 upstream.

Otherwise, bogus error is reported:

 # nft --debug=netlink add rule ip x y 'ct mark set ip dscp & 0x0f << 1 | 0xff000000'
 Error: Value 4278190080 exceeds valid range 0-63
 add rule ip x y ct mark set ip dscp & 0x0f << 1 | 0xff000000
                                                   ^^^^^^^^^^

Use the statement length as the maximum value in the mark statement
expression.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: improve error reporting for unsupported chain type
Pablo Neira Ayuso [Fri, 10 Mar 2023 18:20:50 +0000 (19:20 +0100)] 
src: improve error reporting for unsupported chain type

commit 573788e053631a5c069f887caed7c62d521b022d upstream.

8c75d3a16960 ("Reject invalid chain priority values in user space")
provides error reporting from the evaluation phase. Instead, this patch
infers the error after the kernel reports EOPNOTSUPP.

test.nft:3:28-40: Error: Chains of type "nat" must have a priority value above -200
                type nat hook prerouting priority -300;
                                         ^^^^^^^^^^^^^

This patch also adds another common issue for users compiling their own
kernels if they forget to enable CONFIG_NFT_NAT in their .config file.

Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 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

commit 161beaeacd2e5218d66febc3db825bf6a27119c5 upstream.

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>
6 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

commit 6bcaef6a1ea6dc60250ed6124f3b49a8cd29434c upstream.

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>
6 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

commit 8ed6fa6d66b2df50d118423c1cb0e98cdd45cdbd upstream.

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>
6 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

commit bc1f910f502701f1a1d28c7bd723e4be3bac1d8c upstream.

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>
6 months agolibnftables: skip useable checks for /dev/stdin
Pablo Neira Ayuso [Tue, 9 Jul 2024 14:59:53 +0000 (16:59 +0200)] 
libnftables: skip useable checks for /dev/stdin

commit 477fd8218777b75bdfa3a5643f692adae4f002fe upstream.

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

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

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

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

commit e48f32701ff65d522c2f29f34bf4f3ce8e562057 upstream.

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

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

Fixes: 935f82e7dd49 ("Support 'nft -f -' to read from stdin")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: clone counter before insertion into set element
Pablo Neira Ayuso [Fri, 5 Jul 2024 12:03:33 +0000 (14:03 +0200)] 
optimize: clone counter before insertion into set element

commit ac77f3805c71f14c51730a9c5cb726ee67f14159 upstream.

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

Fixes: 686ab8b6996e ("optimize: do not remove counter in verdict maps")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser_bison: recursive table declaration in deprecated meter statement
Pablo Neira Ayuso [Tue, 2 Jul 2024 22:08:01 +0000 (00:08 +0200)] 
parser_bison: recursive table declaration in deprecated meter statement

commit a70a217079ef83482fc093d8549f8cdeaeaa3cae upstream.

This is allowing for recursive table NAME declarations such as:

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

remove it.

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

commit 551a4ad68b922fa6c942f5e79ac59f723a12e233 upstream.

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

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

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

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

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

commit dc6950a80110d6e6f63bd6f5c308d202db698f46 upstream.

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

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

commit b523008535f3de78ed5834a302ba07cda4b4c8fd upstream.

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

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

Fixes: 419d19688688 ("src: add set element catch-all support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodynset: avoid errouneous assert with ipv6 concat data
Son Dinh [Tue, 9 Apr 2024 06:23:31 +0000 (16:23 +1000)] 
dynset: avoid errouneous assert with ipv6 concat data

commit aa791a12cb2b23fb49c233e40b6e12e0e3ce6b9b upstream.

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

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

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

Also add test cases.

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

commit b40bebbcee3602e2d849e48f3a50676bd8987204 upstream.

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

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agocmd: skip variable set elements when collapsing commands
Pablo Neira Ayuso [Tue, 11 Jun 2024 15:40:23 +0000 (17:40 +0200)] 
cmd: skip variable set elements when collapsing commands

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

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

Skip such commands to address this issue.

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

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1754
Fixes: 498a5f0c219d ("rule: collapse set element commands")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agomonitor: too large shift exponent displaying payload expression
Pablo Neira Ayuso [Mon, 10 Jun 2024 17:08:20 +0000 (19:08 +0200)] 
monitor: too large shift exponent displaying payload expression

commit 016f37f1268fa1003c46c66655697d3f58d86598 upstream.

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

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

skip if proto_unknown_template is set on in this payload expression.

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

commit fda913d712925e8a8d6460b361d49774dc5d34b8 upstream.

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

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1730
Fixes: fd513de78bc0 ("scanner: IPv4-Mapped IPv6 addresses support")
Fixes: 3f82ef3d0dbf ("scanner: Support rfc4291 IPv4-compatible addresses")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agocache: check for NFT_CACHE_REFRESH in current requested cache too
Pablo Neira Ayuso [Mon, 27 May 2024 16:12:05 +0000 (18:12 +0200)] 
cache: check for NFT_CACHE_REFRESH in current requested cache too

commit bbb0e944b59d355085e8f50b4b7b5057ae0d33a4 upstream.

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

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

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

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

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

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

commit f6b579344eee17e5587b6a7fcc444fe997cd8cb6 upstream.

Found by RASU JSC.

Fixes: 2b29ea5f3c3e ("src: ct: add eval part to inject dependencies for ct saddr/daddr")
Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: bogus protocol conflicts in vlan with implicit dependencies
Pablo Neira Ayuso [Wed, 15 May 2024 17:23:49 +0000 (19:23 +0200)] 
evaluate: bogus protocol conflicts in vlan with implicit dependencies

commit d1a7e74d1e065d244439fdb0f1c1cba83f921609 upstream.

The following command:

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

fails with:

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

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

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

but nft should really handle this itself.

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

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

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

 ether type ip vlan id 1

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

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

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

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

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

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

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

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

Fixes: 39f15c243912 ("nft: support listing expressions that use non-byte header fields")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: add support for variables in map expressions
Jeremy Sowden [Mon, 29 Apr 2024 19:27:53 +0000 (20:27 +0100)] 
evaluate: add support for variables in map expressions

commit c6127ff0c4480ccefc5c29548409898fb315a2ca upstream.

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

  define dst_map = { ::1234 : 5678 }

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

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

  define dst_map = { ::1234 : 5678 }

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

nft rejects it:

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

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

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

thus:

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

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

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

commit 52a7af9bec15a4fb4bfea86e40b70f96098f7dfd upstream.

Currently, they are reported as assertion failures:

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

Instead, report them more informatively as errors:

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

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodoc: nft.8: Highlight "hook" in flowtable description
Phil Sutter [Wed, 24 Apr 2024 21:48:56 +0000 (23:48 +0200)] 
doc: nft.8: Highlight "hook" in flowtable description

commit 9da7b00aa886012c0e59e73aa19e05a8d1568540 upstream.

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

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

commit ef659df2a45c38ad18f703febeae8c4febf9dd04 upstream.

Just a missing asterisk somewhere.

Fixes: 1dd08fcfa07a4 ("src: add ct expectations support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agojson: Fix for memleak in __binop_expr_json
Phil Sutter [Wed, 24 Apr 2024 21:35:00 +0000 (23:35 +0200)] 
json: Fix for memleak in __binop_expr_json

commit a0a15e4dd0576bc4efd9b01fdd4ee1c565effac9 upstream.

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

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

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 0ac39384fd9e4 ("json: Accept more than two operands in binary expressions")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agomergesort: Avoid accidental set element reordering
Phil Sutter [Fri, 22 Mar 2024 12:31:10 +0000 (13:31 +0100)] 
mergesort: Avoid accidental set element reordering

commit bf52af188b306acf5a30134d6a670f41f16a9459 upstream.

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

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

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

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

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

commit 0ac39384fd9e48ff6bcc5605df2cbeb33af64b9e upstream.

The most common use case is ORing flags like

| syn | ack | rst

but nft seems to be fine with less intuitive stuff like

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

so support all of them.

Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agosrc: disentangle ICMP code types
Pablo Neira Ayuso [Mon, 1 Apr 2024 22:28:24 +0000 (00:28 +0200)] 
src: disentangle ICMP code types

commit 5fecd2a6ef614eca7b0829e684449ee25982c233 upstream.

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

Thus, the output of:

  nft describe icmp_code

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

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

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

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

After this patch:

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

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

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

And update tests/shell accordingly.

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

commit 3f776e8b37d8022d4492ed8be136e99f5a88ab9e upstream.

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

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

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

Add test for cross-day scenario in EADT timezone.

Fixes: 44d144cd593e ("netlink_delinearize: reverse cross-day meta hour range")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodatatype: use DTYPE_F_PREFIX only for IP address datatype
Pablo Neira Ayuso [Thu, 21 Mar 2024 11:40:53 +0000 (12:40 +0100)] 
datatype: use DTYPE_F_PREFIX only for IP address datatype

commit a2a7fbdfdd7f8dc5baa4cc8a23753b96bbc8a433 upstream.

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

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

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

  meta mark & 0xffffff00 == 0xffffff00

is used instead.

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

commit c0a5b8c6a6433ec1d4e41646dc42ccb8444c96be upstream.

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

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

commit 44d144cd593e3af9f3b3618ea510ea02bba4bc4c upstream.

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

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

which results in (Sidney, Australia AEDT time):

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

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

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

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

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

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

commit b11b6c68e61ea294eb4c313705ccfe3e7b0eda87 upstream.

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

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

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

commit ea011231c06cbe828cf6056bc9c3d116e1f528d5 upstream.

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

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

However, nft continues evaluation.

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

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

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

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

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

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

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser: json: Support for synproxy objects
Phil Sutter [Fri, 8 Mar 2024 23:29:36 +0000 (00:29 +0100)] 
parser: json: Support for synproxy objects

commit 938a135f2200766661e42a20e02d87555f5bacfa upstream.

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

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

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

commit c56d77cc82988391ab8f2514214c0088cbc7d89e upstream.

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

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

commit 38f04196ebef64a6672c55c27a6afe9af811c8f7 upstream.

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

Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agonftables: do mot merge payloads on negation
Sriram Rajagopalan [Wed, 13 Mar 2024 08:32:42 +0000 (01:32 -0700)] 
nftables: do mot merge payloads on negation

commit f35a0d78fe870737fa39d859bd2e3ac25bf1b12e upstream.

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

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

Signed-off-by: Sriram Rajagopalan <sriramr@arista.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: allow to define maps that contain ct helpers
Florian Westphal [Fri, 1 Mar 2024 12:59:37 +0000 (13:59 +0100)] 
parser: allow to define maps that contain ct helpers

commit acbac5e3ada4bdf16cf24056a8a7a7739429a8ab upstream.

Its currently not possible to use ct helpers in objref maps.

Simply adding "CT HELPER" to "map_block_obj_type" does not work
due to a conflict with the "ct helper" ct_expr block.

  map m {
    type ipv4_addr : ct helper ..

... declares a map storing ip addresses and conntrack helper names
(string type).  This does not make sense, there is no way
to use the associated value (the names) in any sensible way:

ct helper "ftp" - this matches if the packet has a conntrack entry that
was accepted via the "ftp" conntrack helper. In nft vm terms, this is
translated to:

 [ ct load helper => reg 1 ]
 [ cmp eq reg 1 0x00707466 0x00000000 0x00000000 0x00000000 ]

Or one can query a set, e.g. 'ct helper { "ftp", "sip" }'.
"ftp" and "sip" are the kernel-defined names of these connection
tracking helpers.

ct helper set "ftp" is something else, however:

This is used to assign a *userspace defined helper objrect reference*.
Nftables will translate this to:

 [ objref type 3 name ftp ]

.. where "ftp" is a arbitrary user-chosen name.

  ct helper "ftp" {
    type "ftp" protocol tcp
    l3proto ip
  }

IOW, "ct helper" is ambiguous.  Without the "set" keyword (first case),
it places the kernel-defined name of the active connection tracking helper
in the chosen register (or it will cancel rule evaluation if no helper was
active).

With the set keyword (second case), the expected argument is a user-defined
object reference which will then tell the connection tracking engine to
monitor all further packets of the new flow with the given helper template.

This change makes it so that

  map m {
    type ipv4_addr : ct helper ..

declares a map storing ct helper object references suitable for
'ct helper set'.

The better alternative would be to resolve the ambiguity
by adding an additional postfix keyword, for example

 ct helper name (case one)
 ct helper object (case two).

But this needs a kernel change that adds
NFT_CT_HELPER_NAME and NFT_CT_HELPER_OBJECT to enum nft_ct_keys.

While a new kernel could handle old nftables binaries that still use
NFT_CT_HELPER key, new nftables would need to probe support first.

Furthermore,

 ct helper name set "foo"

... would make no sense, as the kernel-defined helper names are
readonly.

 ct helper object "foo"

... would make no sense, unless we extend the kernel to store
the nftables userspace-defined name in a well-known location
in the kernel.  Userdata area cannot work for this, because the
nft conntrack expression in the kernel would need to know how to
retrieve this info again.

Also, I cannot think of a sensible use case for this.
So the only remaining, useful commands are:

ct helper name "ftp"
ct helper object set "foo"

... which is identical to what we already support, just with
extra keyword.

So a much simpler solution that does not need any kernel changes
is make "ct helper" have different meaning depending on wheter it
is placed on the key side, i.e.:

    "typeof ct helper", "typeof ct helper : $value"

versus when its on placed on the data (value) side of maps:

    "typeof $key : ct helper".

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: allow to define maps that contain timeouts and expectations
Pablo Neira Ayuso [Fri, 1 Mar 2024 12:59:36 +0000 (13:59 +0100)] 
parser: allow to define maps that contain timeouts and expectations

commit bb2054cfd99097973d98e231066297e9e8632a61 upstream.

Its currently not possible to use ct timeouts/expectations/helpers
in objref maps, bison parser lacks the relevant keywords.

This change adds support for timeouts and expectations.
Ct helpers are more problematic, this will come in a different change.

Support is only added for the "typeof" keyword, otherwise we'd
need to add pseudo-datatypes as well, but making "ct expectation"
available as "type" as well might be confusing.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agorule: fix ASAN errors in chain priority to textual names
Pablo Neira Ayuso [Thu, 29 Feb 2024 15:50:37 +0000 (16:50 +0100)] 
rule: fix ASAN errors in chain priority to textual names

commit ff6135270616ccf4712990246cae850e64253516 upstream.

ASAN reports several errors when listing this ruleset:

 table ip x {
        chain y {
                type filter hook input priority -2147483648; policy accept;
        }
 }

src/rule.c:1002:8: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
src/rule.c:1001:11: runtime error: signed integer overflow: -2147483648 - 50 cannot be represented in type 'int'

Use int64_t for the offset to avoid an underflow when calculating
closest existing priority definition.

Use llabs() because abs() is undefined with INT32_MIN.

Fixes: c8a0e8c90e2d ("src: Set/print standard chain prios with textual names")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoproto: use hexadecimal to display ip frag-off field
Pablo Neira Ayuso [Tue, 29 Aug 2023 15:46:03 +0000 (17:46 +0200)] 
proto: use hexadecimal to display ip frag-off field

commit 67edd1f2330a31e93789639d31315b64d01a79a3 upstream.

The ip frag-off field in the protocol definition is 16-bits long
and it contains the DF (0x2000) and MF (0x4000) bits too.

iptables-translate also suggests:

ip frag-off & 0x1ffff != 0

to match on fragments. Use hexadecimal for listing this header field.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser: deduplicate map with data interval
Florian Westphal [Wed, 2 Aug 2023 15:48:24 +0000 (17:48 +0200)] 
parser: deduplicate map with data interval

commit 28d202216535ac54216f825e511a92d9acea5d3c upstream.

Its copypasted, the copy is same as original
except that it specifies a map key that maps to an interval.

Add an exra rule that returns 0 or EXPR_F_INTERVAL, then
use that in a single rule.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agonetlink: allow typeof keywords with objref maps during listing
Florian Westphal [Thu, 29 Feb 2024 10:41:24 +0000 (11:41 +0100)] 
netlink: allow typeof keywords with objref maps during listing

commit 070ec7ce350f8139f9b97dbcb78ad1d7b7bf1196 upstream.

Without this,

  typeof meta l4proto . ip saddr . tcp sport : limit

... is shown as

  type inet_proto . ipv4_addr . inet_service : limit

The "data" element is a value (the object type number).
It doesn't support userinfo data.

There is no reason to add it, the value is the object type
number that the object-reference map stores.

So, if we have an objref map, DO NOT discard the key part,
as we do for normal maps.

For normal maps, we support either typeof notation, i.e.:

  typeof meta l4proto . ip saddr . tcp sport : ip saddr

or the data type version:
  type inet_proto . ipv4_addr . inet_service : ipv4_addr

... but not a mix, a hyptothetical

  typeof meta l4proto . ip saddr . tcp sport : ipv4_addr

... does not work.

If nft finds no udata attached to the data element, for normal
map case, it has to fall back to the "type" form.

But for objref maps this is expected, udata for key but not for data.
Hence, for objref case, keep the typeof part if its valid.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: allow typeof in objref maps
Florian Westphal [Thu, 29 Feb 2024 10:41:23 +0000 (11:41 +0100)] 
parser: allow typeof in objref maps

commit 25f1e6ccc0590e1081879022809afb83fc45f673 upstream.

Its currently not possible to declare a map that
stores object references with the "typeof" keyword, e.g.

map m {
type ipv4_addr : limit

will work, but

map m {
typeof ip saddr : limit

will give a syntax error ("unexpected limit").
Followup pach will add support for listing side too.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: compact type/typeof set rules
Florian Westphal [Tue, 27 Feb 2024 14:53:19 +0000 (15:53 +0100)] 
parser: compact type/typeof set rules

commit fb98cb91c575880617e5da2cb6f094525516cb78 upstream.

Set/maps keys can be declared either by 'type' or 'typeof' keyword.

Compact this to use a common block for both cases.

The datatype_set call is redundant, remove it:
at this point $3 == $1->key, so this is a no-op.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: compact interval typeof rules
Florian Westphal [Tue, 27 Feb 2024 14:50:05 +0000 (15:50 +0100)] 
parser: compact interval typeof rules

commit 81fc7aee0d523d61077518534036ff10beddb3e9 upstream.

There are two nearly identical blocks for typeof maps:
one with INTERVAL keyword present and one without.

Compact this into a single block.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser_json: allow 0 offsets again
Florian Westphal [Mon, 26 Feb 2024 09:34:59 +0000 (10:34 +0100)] 
parser_json: allow 0 offsets again

commit e4f59f2dc68fb2e6d62a6c37e04366a8ebd199cf upstream.

Its valid in case of tcp option removal:

[ {
   "reset": {
     "tcp option": {
       "base": 123,
       "len": 0,
       "offset": 0
   }

This makes nft-test.py -j pass again.

Fixes: e08627257ecf ("parser: reject raw payload expressions with 0 length")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoexpression: missing line in describe command with invalid expression
Pablo Neira Ayuso [Tue, 13 Feb 2024 16:09:20 +0000 (17:09 +0100)] 
expression: missing line in describe command with invalid expression

commit 2b24dd29c5fa1c7e4cf44f0753752d25106273a0 upstream.

Before:

 duh@testbed:~# nft describe blah
 symbol expression, datatype invalid (invalid)duh@testbed:#

After:

 duh@testbed:~# nft describe blah
 symbol expression, datatype invalid (invalid)
 duh@testbed:#

Fixes: 48aca2de80a7 ("iptopt: fix crash with invalid field/type combo")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: permit use of host-endian constant values in set lookup keys
Pablo Neira Ayuso [Mon, 12 Feb 2024 15:46:29 +0000 (16:46 +0100)] 
evaluate: permit use of host-endian constant values in set lookup keys

commit c0080feb0d034913409944d23873cce4bf9edf9e upstream.

AFL found following crash:

table ip filter {
map ipsec_in {
typeof ipsec in reqid . iif : verdict
flags interval
}

chain INPUT {
type filter hook input priority filter; policy drop;
ipsec in reqid . 100 @ipsec_in
}
}

Which yields:
nft: evaluate.c:1213: expr_evaluate_unary: Assertion `!expr_is_constant(arg)' failed.

All existing test cases with constant values use big endian values, but
"iif" expects host endian values.

As raw values were not supported before, concat byteorder conversion
doesn't handle constants.

Fix this:

1. Add constant handling so that the number is converted in-place,
   without unary expression.

2. Add the inverse handling on delinearization for non-interval set
   types.
   When dissecting the concat data soup, watch for integer constants where
   the datatype indicates host endian integer.

Last, extend an existing test case with the afl input to cover
in/output.

A new test case is added to test linearization, delinearization and
matching.

Based on original patch from Florian Westphal, patch subject and
description wrote by him.

Fixes: b422b07ab2f9 ("src: permit use of constant values in set lookup keys")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: permit use of constant values in set lookup keys
Florian Westphal [Wed, 24 May 2023 11:25:22 +0000 (13:25 +0200)] 
src: permit use of constant values in set lookup keys

commit b422b07ab2f96436001f33dfdfd937238033c799 upstream.

Something like:

Given: set s { type ipv4_addr . ipv4_addr . inet_service .. } something
like
   add rule ip saddr . 1.2.3.4 . 80 @s goto c1

fails with: "Error: Can't parse symbolic invalid expressions".

This fails because the relational expression first evaluates
the left hand side, so when concat evaluation sees '1.2.3.4'
no key context is available.

Check if the RHS is a set reference, and, if so, evaluate
the right hand side.

This sets a pointer to the set key in the evaluation context
structure which then makes the concat evaluation step parse
1.2.3.4 and 80 as ipv4 address and 16bit port number.

On delinearization, extend relop postprocessing to
copy the datatype from the rhs (set reference, has
proper datatype according to set->key) to the lhs (concat
expression).

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agonetlink_delinearize: move concat and value postprocessing to helpers
Florian Westphal [Fri, 19 Jan 2024 12:47:08 +0000 (13:47 +0100)] 
netlink_delinearize: move concat and value postprocessing to helpers

commit 7ef933d16fd1b14afb276ab26f4114a2c9d6a3ea upstream.

No functional changes intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agocache: Always set NFT_CACHE_TERSE for list cmd with --terse
Phil Sutter [Thu, 8 Feb 2024 01:10:48 +0000 (02:10 +0100)] 
cache: Always set NFT_CACHE_TERSE for list cmd with --terse

commit cd4e947032a57a585b1a457ce03f546afc7ba033 upstream.

This fixes at least 'nft -t list table ...' and 'nft -t list set ...'.

Note how --terse handling for 'list sets/maps' remains in place since
setting NFT_CACHE_TERSE does not fully undo NFT_CACHE_SETELEM: setting
both enables fetching of anonymous sets which is pointless for that
command.

Reported-by: anton.khazan@gmail.com
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1735
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agonetlink_linearize: add assertion to catch for buggy byteorder
Pablo Neira Ayuso [Thu, 8 Feb 2024 12:20:43 +0000 (13:20 +0100)] 
netlink_linearize: add assertion to catch for buggy byteorder

commit 8a34cca265a290a0b23cec27d5258bc432cef3d3 upstream.

Add assertion to catch buggy bytecode where unary expression is present
with 1-byte selectors, where no byteorder conversion is required.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: skip byteorder conversion for selector smaller than 2 bytes
Pablo Neira Ayuso [Wed, 7 Feb 2024 22:53:32 +0000 (23:53 +0100)] 
evaluate: skip byteorder conversion for selector smaller than 2 bytes

commit 9fe58952c45a1643dc7df1a0b1f5d88e8ae1a978 upstream.

Add unary expression to trigger byteorder conversion for host byteorder
selectors only if selectors length is larger or equal than 2 bytes.

 # cat test.nft
 table ip x {
        set test {
                type ipv4_addr . ether_addr . inet_proto
                flags interval
        }

        chain y {
                ip saddr . ether saddr . meta l4proto @test counter
        }
 }

 # nft -f test.nft
 ip x y
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 4b @ network header + 12 => reg 1 ]
  [ payload load 6b @ link header + 6 => reg 9 ]
  [ meta load l4proto => reg 11 ]
  [ byteorder reg 11 = hton(reg 11, 2, 1) ] <--- should not be here
  [ lookup reg 1 set test ]
  [ counter pkts 0 bytes 0 ]

Fixes: 1017d323cafa ("src: support for selectors with different byteorder with interval concatenations")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agocache: Optimize caching for 'list tables' command
Phil Sutter [Tue, 6 Feb 2024 18:26:57 +0000 (19:26 +0100)] 
cache: Optimize caching for 'list tables' command

commit 674eb7fa2895813b25f6fbfcc9417fc0788fade1 upstream.

No point in fetching anything other than existing tables from kernel:
'list tables' merely prints existing table names, no contents.

Also populate filter's family field to reduce overhead when listing
tables in one family with many tables in another one. It works without
further adjustments because nftnl_nlmsg_build_hdr() will use the value
for nfgen_family.

Reported-by: anton.khazan@gmail.com
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1735
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoMakefile.am: don't silence -Wimplicit-function-declaration
Sam James [Wed, 7 Feb 2024 02:12:35 +0000 (02:12 +0000)] 
Makefile.am: don't silence -Wimplicit-function-declaration

This becomes an error in GCC 14 and Clang 16. It's a common
misconception that these warnings are invalid or simply noise for
Bison/parser files, but even if that were true, we'd need to handle it
somehow anyway. Silencing them does nothing, so stop doing that.

Further, I don't actually get any warnings to fix with bison-3.8.2. This
mirrors changes we've done in other netfilter.org projects.

Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoevaluate: fix check for unknown in cmd_op_to_name
谢致邦 (XIE Zhibang) [Wed, 7 Feb 2024 15:10:20 +0000 (15:10 +0000)] 
evaluate: fix check for unknown in cmd_op_to_name

commit 9f76bb63c0c706ea5c0d55931ee690ca5dccaf16 upstream.

Example:
nft --debug=all destroy table ip missingtable

Before:
Evaluate unknown

After:
Evaluate destroy

Fixes: e1dfd5cc4c46 ("src: add support to command "destroy"")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoevaluate: don't assert on net/transport header conflict
Florian Westphal [Tue, 16 Jan 2024 14:21:00 +0000 (15:21 +0100)] 
evaluate: don't assert on net/transport header conflict

commit 3a734d60813193974a4a0e8ed0af3349f8857ec9 upstream.

before:
nft: evaluate.c:467: conflict_resolution_gen_dependency: Assertion `expr->payload.base == PROTO_BASE_LL_HDR' failed.
Aborted (core dumped)

conflict_resolution_gen_dependency() can only handle linklayer
conflicts, hence the assert.

Rename it accordingly.  Also rename resolve_protocol_conflict, it doesn't
do anything for != PROTO_BASE_LL_HDR and extend the assertion to that
function too.

Callers now enforce PROTO_BASE_LL_HDR prerequisite.

after:
Error: conflicting transport layer protocols specified: comp vs. udp
 ip6 nexthdr comp udp dport 4789
                  ^^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodatatype: display 0s time datatype
Pablo Neira Ayuso [Thu, 25 Jan 2024 16:31:45 +0000 (17:31 +0100)] 
datatype: display 0s time datatype

commit 072c83e9a18399249fbc4343f7d0b5b04c29e6fb upstream.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agojson: Support sets' auto-merge option
Phil Sutter [Wed, 31 Jan 2024 16:30:24 +0000 (17:30 +0100)] 
json: Support sets' auto-merge option

commit 0021d264d975ac7db1ed32699487f95b41e4c2bf upstream.

If enabled, list the option as additional attribute with boolean value.

Fixes: e70354f53e9f6 ("libnftables: Implement JSON output support")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1734
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agorule: fix sym refcount assertion
Florian Westphal [Mon, 15 Jan 2024 13:27:15 +0000 (14:27 +0100)] 
rule: fix sym refcount assertion

commit b73298405cda74b3a87a1818bb92f53298d34170 upstream.

Scope release must happen last.
afl provided a reproducer where policy is a define, because
scope is released too early we get:
nft: src/rule.c:559: scope_release: Assertion `sym->refcnt == 1' failed.

... because chain->policy is EXPR_SYMBOL.

Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: error out when store needs more than one 128bit register of align fixup
Florian Westphal [Mon, 15 Jan 2024 13:11:17 +0000 (14:11 +0100)] 
evaluate: error out when store needs more than one 128bit register of align fixup

commit 8a66de2a15943b2fbf960967cdbcbd0a148cb114 upstream.

Else this gives:
nft: evaluate.c:2983: stmt_evaluate_payload: Assertion `sizeof(data) * BITS_PER_BYTE >= masklen' failed.

For loads, this is already prevented via expr_evaluate_bits() which has:

  if (masklen > NFT_REG_SIZE * BITS_PER_BYTE)
      return expr_error(ctx->msgs, expr, "mask length %u exceeds allowed maximum of %u\n",
                        masklen, NFT_REG_SIZE * BITS_PER_BYTE);

But for the store path this isn't called.
The reproducer asks to store a 128 bit integer at bit offset 1, i.e.
17 bytes would need to be munged, but we can only handle up to 16 bytes
(one pseudo-register).

Fixes: 78936d50f306 ("evaluate: add support to set IPv6 non-byte header fields")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: reject raw payload expressions with 0 length
Florian Westphal [Fri, 12 Jan 2024 12:27:23 +0000 (13:27 +0100)] 
parser: reject raw payload expressions with 0 length

commit e08627257ecfa7dfb68a34a1c8866e7a7e012b15 upstream.

Reject this at parser stage.  Fix up the json input side too, else
reproducer gives:
nft: src/netlink.c:243: netlink_gen_raw_data: Assertion `len > 0' failed.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agopayload: only assert if l2 header base has no length
Florian Westphal [Thu, 11 Jan 2024 12:11:22 +0000 (13:11 +0100)] 
payload: only assert if l2 header base has no length

commit 9cc41467c75ab6beb35e0d7c34d04acd1a44861b upstream.

nftables will assert in some cases because the sanity check is done even
for network and transport header bases.

However, stacked headers are only supported for the link layer.
Move the assertion around and add a test case for this.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: release mpz type in expr_evaluate_list() error path
Pablo Neira Ayuso [Thu, 11 Jan 2024 21:14:34 +0000 (22:14 +0100)] 
evaluate: release mpz type in expr_evaluate_list() error path

commit 172b660843501463a0894b0d2ca1dd48c898dc4d upstream.

Detected when running:

 # nft -f tests/shell/testcases/bogons/nft-f/no_integer_basetype_crash
 ==383222==ERROR: LeakSanitizer: detected memory leaks

 Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fe7b54a9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7fe7b538b9a9 in __gmp_default_allocate (/lib/x86_64-linux-gnu/libgmp.so.10+0xc9a9)

Fixes: 3671c4897003 ("evaluate: guard against NULL basetype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: release key expression in error path of implicit map with unknown datatype
Pablo Neira Ayuso [Thu, 11 Jan 2024 21:04:27 +0000 (22:04 +0100)] 
evaluate: release key expression in error path of implicit map with unknown datatype

commit 4622a8c6372ccf3103e38d1f15227eadb0485f54 upstream.

Detected when running:

 # nft -f tests/shell/testcases/bogons/nft-f/mapping_with_invalid_datatype_crash
 ==382584==ERROR: LeakSanitizer: detected memory leaks

 Direct leak of 144 byte(s) in 1 object(s) allocated from:
    #0 0x7fde06ca9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7fde062924af in xmalloc src/utils.c:31
    #2 0x7fde0629266c in xzalloc src/utils.c:70
    #3 0x7fde06167299 in expr_alloc src/expression.c:46
    #4 0x7fde0616b014 in constant_expr_alloc src/expression.c:420
    #5 0x7fde06128e43 in expr_evaluate_map src/evaluate.c:2027
    #6 0x7fde06137b06 in expr_evaluate src/evaluate.c:2891
    #7 0x7fde06132417 in expr_evaluate_relational src/evaluate.c:2497
    #8 0x7fde06137b36 in expr_evaluate src/evaluate.c:2895
    #9 0x7fde06137d5f in stmt_evaluate_expr src/evaluate.c:2914
    #10 0x7fde061524c8 in stmt_evaluate src/evaluate.c:4646
    #11 0x7fde0615c9ee in rule_evaluate src/evaluate.c:5202
    #12 0x7fde061600c7 in cmd_evaluate_add src/evaluate.c:5422

Fixes: 70054e6e1c87 ("evaluate: catch implicit map expressions without known datatype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>