]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
4 months agotests: shell: use /bin/bash in sets/elem_opts_compat_0 1.0.6.y
Pablo Neira Ayuso [Tue, 24 Oct 2023 10:41:57 +0000 (12:41 +0200)] 
tests: shell: use /bin/bash in sets/elem_opts_compat_0

commit d3e941668be1d3922832fd70788fb248fd11f6c7 upstream.

[ All 1.0.6.y tests/shell report success after this ]

So running this test with /bin/sh != /bin/bash works.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 months agoparser_bison: fix memleak in meta set error handling
Florian Westphal [Fri, 8 Dec 2023 12:37:27 +0000 (13:37 +0100)] 
parser_bison: fix memleak in meta set error handling

commit 21608263cc1ae489326e743957bfe34b05414a44 upstream.

We must release the expression here, found via afl++ and
-fsanitize-address build.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoparser_bison: fix objref statement corruption
Florian Westphal [Fri, 8 Dec 2023 18:41:39 +0000 (19:41 +0100)] 
parser_bison: fix objref statement corruption

commit 78dffb470fcf7b1c0b1b3d6f43fcc056c337a808 upstream.

Consider this:

counter_stmt            :       counter_stmt_alloc
                        |       counter_stmt_alloc      counter_args

counter_stmt_alloc      :       COUNTER { $$ = counter_stmt_alloc(&@$); }
                        |       COUNTER         NAME    stmt_expr
                        {
                                $$ = objref_stmt_alloc(&@$);
                                $$->objref.type = NFT_OBJECT_COUNTER;
                                $$->objref.expr = $3;
                        }
                        ;

counter_args            :       counter_arg { $<stmt>$        = $<stmt>0; }
                        |       counter_args    counter_arg
                        ;

counter_arg             :       PACKETS NUM { $<stmt>0->counter.packets = $2; }

[..]

This has 'counter_stmt_alloc' EITHER return counter or objref statement.
Both are the same structure but with different (union'd) trailer content.

counter_stmt permits the 'packet' and 'byte' argument.

But the 'counter_arg' directive only works with a statement
coming from counter_stmt_alloc().

afl++ came up with following input:

table inet x {
        chain y {
                counter name ip saddr bytes 1.1.1. 1024
        }
}

This clobbers $<stmt>->objref.expr pointer, we then crash when
calling expr_evaluate() on it.

Split the objref related statements into their own directive.

After this, the input will fail with:
"syntax error, unexpected bytes, expecting newline or semicolon".

Also split most of the other objref statements into their own blocks.
synproxy seems to have same problem, limit and quota appeared to be ok.

v1 added objref_stmt to stateful_stmt list, this is wrong, we will
assert when generating the 'counter' statement.
Place it in the normal statement list so netlink_gen_stmt_stateful_assert
throws the expected parser error.

Fixes: dccab4f646b4 ("parser_bison: consolidate stmt_expr rule")
Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoevaluate: validate chain max length
Florian Westphal [Fri, 8 Dec 2023 23:37:09 +0000 (00:37 +0100)] 
evaluate: validate chain max length

commit 08925ba0daf19753df933fed69f4572a7c9d3d47 upstream.

The includes test files cause:
BUG: chain is too large (257, 256 max)nft: netlink.c:418: netlink_gen_chain: Assertion `0' failed.

Error out in evaluation step instead.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoparser: tcpopt: fix tcp option parsing with NUM + length field
Florian Westphal [Tue, 5 Dec 2023 11:56:08 +0000 (12:56 +0100)] 
parser: tcpopt: fix tcp option parsing with NUM + length field

commit 59a33d08ab3a75b2ae370b6816942793f49fa8db upstream.

tcp option 254 length ge 4

... will segfault.
The crash bug is that tcpopt_expr_alloc() can return NULL if we cannot
find a suitable template for the requested kind + field combination,
so add the needed error handling in the bison parser.

However, we can handle this.  NOP and EOL have templates, all other
options (known or unknown) must also have a length field.

So also add a fallback template to handle both kind and length, even
if only a numeric option is given that nft doesn't recognize.

Don't bother with output, above will be printed via raw syntax, i.e.
tcp option @254,8,8 >= 4.

Fixes: 24d8da308342 ("tcpopt: allow to check for presence of any tcp option")
Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoevaluate: reject set definition with no key
Pablo Neira Ayuso [Wed, 6 Dec 2023 12:40:22 +0000 (13:40 +0100)] 
evaluate: reject set definition with no key

commit 1949a63215b423b914d3a7a9de7511cb48af3c09 upstream.

 tests/shell/testcases/bogons/nft-f/set_definition_with_no_key_assert
 BUG: unhandled key type 2
 nft: src/intervals.c:59: setelem_expr_to_range: Assertion `0' failed.

This patch adds a new unit tests/shell courtesy of Florian Westphal.

Fixes: 3975430b12d9 ("src: expand table command before evaluation")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 months agomonitor: add support for concatenated set ranges
Pablo Neira Ayuso [Tue, 5 Dec 2023 16:20:05 +0000 (17:20 +0100)] 
monitor: add support for concatenated set ranges

commit 0d9392eef5f2c79ac7c19f59754a0aee574b5617 upstream.

monitor is missing concatenated set ranges support.

Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 months agoevaluate: fix double free on dtype release
Florian Westphal [Tue, 5 Dec 2023 12:08:17 +0000 (13:08 +0100)] 
evaluate: fix double free on dtype release

commit ee73e9dcd46dc5a1fe3be7caa8b9323819e394b8 upstream.

We release ->dtype twice, will either segfault or assert
on dtype->refcount != 0 check in datatype_free().

Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoevaluate: catch implicit map expressions without known datatype
Florian Westphal [Mon, 4 Dec 2023 21:51:21 +0000 (22:51 +0100)] 
evaluate: catch implicit map expressions without known datatype

commit 70054e6e1c879475779d82d19f450ac521700fe4 upstream.

mapping_With_invalid_datatype_crash:1:8-65: Error: Implicit map expression without known datatype
bla to tcp dport map { 80 : 1.1.1.1 . 8001, 81 : 2.2.2.2 . 9001 } bla
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
4 months agoevaluate: reject attempt to update a set
Florian Westphal [Mon, 4 Dec 2023 21:00:06 +0000 (22:00 +0100)] 
evaluate: reject attempt to update a set

commit 5f43ea807bb0f5b30f332c2c96f13e33c9243d22 upstream.

This will crash as set->data is NULL, so check that SET_REF is pointing
to a map:

Error: candidates_ipv4 is not a map
tcp dport 10003 ip saddr . tcp dport @candidates_ipv4 add @candidates_ipv4 { ip saddr . 10 :0004 timeout 1s }
                                     ~~~~~~~~~~~~~~~~

Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agoevaluate: error out if basetypes are different
Florian Westphal [Mon, 4 Dec 2023 17:30:51 +0000 (18:30 +0100)] 
evaluate: error out if basetypes are different

commit 45a4d4434742b425d019623812f2cce293033cdf upstream.

prefer
binop_with_different_basetype_assert:3:29-35: Error: Binary operation (<<) with different base types (string vs integer) is not supported
oifname set ip9dscp << 26 | 0x10
            ^^^^^^^~~~~~~
to assertion failure.

Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agoevaluate: guard against NULL basetype
Florian Westphal [Mon, 4 Dec 2023 17:18:07 +0000 (18:18 +0100)] 
evaluate: guard against NULL basetype

commit 3671c48970031e617ee713b79caf8ef0a1b096c2 upstream.

i->dtype->basetype can be NULL.

Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agoevaluate: handle invalid mapping expressions gracefully
Pablo Neira Ayuso [Tue, 5 Dec 2023 14:26:10 +0000 (15:26 +0100)] 
evaluate: handle invalid mapping expressions gracefully

commit 778e4e113673c2a4daa798634c554c40f2808276 upstream.

Before:
BUG: invalid mapping expression binop
nft: src/evaluate.c:2027: expr_evaluate_map: Assertion `0' failed.

After:
tests/shell/testcases/bogons/nft-f/invalid_mapping_expr_binop_assert:1:22-25: Error: invalid mapping expression binop
xy mame ip saddr map h& p p
        ~~~~~~~~     ^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agojson: deal appropriately with multidevice in chain
Pablo Neira Ayuso [Thu, 23 Nov 2023 09:36:50 +0000 (10:36 +0100)] 
json: deal appropriately with multidevice in chain

commit 4dfb5b2010917da3f9f11c83931069931a7d6fb3 upstream.

Chain device support is broken in JSON: listing does not include devices
and parser only deals with one single device.

Use existing json_parse_flowtable_devs() function, rename it to
json_parse_devs() to parse the device array.

Use the dev_array that contains the device names (as string) for the
listing.

Update incorrect .json-nft files in tests/shell.

Fixes: 3fdc7541fba0 ("src: add multidevice support for netdev chain")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agoevaluate: clone unary expression datatype to deal with dynamic datatype
Pablo Neira Ayuso [Wed, 22 Nov 2023 19:35:07 +0000 (20:35 +0100)] 
evaluate: clone unary expression datatype to deal with dynamic datatype

upstream faa6908fad6053ae9549c45b88d0402cc69cf1ed commit.

When allocating a unary expression, clone the datatype to deal with
dynamic datatypes.

Fixes: 6b01bb9ff798 ("datatype: concat expression only releases dynamically allocated datatype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agoevaluate: fix rule replacement with anon sets
Florian Westphal [Sun, 19 Nov 2023 12:05:55 +0000 (13:05 +0100)] 
evaluate: fix rule replacement with anon sets

commit 256904b1ded6314974dddc75726149f7b19d33f4 upstream.

nft replace rule t c handle 3 'jhash ip protocol . ip saddr mod 170 vmap { 0-94 : goto wan1, 95-169 : goto wan2, 170-269 }"'
BUG: unhandled op 2
nft: src/evaluate.c:1748: interval_set_eval: Assertion `0' failed.

Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Reported-by: Tino Reichardt <milky-netfilter@mcmilk.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agojson: fix use after free in table_flags_json()
Thomas Haller [Tue, 14 Nov 2023 15:29:25 +0000 (16:29 +0100)] 
json: fix use after free in table_flags_json()

commit b04512cf30de1ba6657facba5ebe2321e17c2727 upstream.

Add `$NFT -j list ruleset` to the end of "tests/shell/testcases/transactions/table_onoff".
Then valgrind will find this issue:

  $ make -j && ./tests/shell/run-tests.sh tests/shell/testcases/transactions/table_onoff -V

Gives:

  ==286== Invalid read of size 4
  ==286==    at 0x49B0261: do_dump (dump.c:211)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B04F7: do_dump (dump.c:273)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B0E84: json_dump_callback (dump.c:465)
  ==286==    by 0x48AF22A: do_command_list_json (json.c:2016)
  ==286==    by 0x48732F1: do_command_list (rule.c:2335)
  ==286==    by 0x48737F5: do_command (rule.c:2605)
  ==286==    by 0x48A867D: nft_netlink (libnftables.c:42)
  ==286==    by 0x48A92B1: nft_run_cmd_from_buffer (libnftables.c:597)
  ==286==    by 0x402CBA: main (main.c:533)

Fixes: e70354f53e9f ("libnftables: Implement JSON output support")
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agosrc: expand create commands
Pablo Neira Ayuso [Mon, 13 Nov 2023 13:39:23 +0000 (14:39 +0100)] 
src: expand create commands

commit 04a1ddc2012964c0a00350973328f5954887cedb upstream.

create commands also need to be expanded, otherwise elements are never
evaluated:

 # cat ruleset.nft
 define ip-block-4 = { 1.1.1.1 }
 create set netdev filter ip-block-4-test {
        type ipv4_addr
flags interval
auto-merge
elements = $ip-block-4
 }
 # nft -f ruleset.nft
 BUG: unhandled expression type 0
 nft: src/intervals.c:211: interval_expr_key: Assertion `0' failed.
 Aborted

Same applies to chains in the form of:

 create chain x y {
counter
 }

which is also accepted by the parser.

Update tests/shell to improve coverage for these use cases.

Fixes: 56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agonetlink: fix buffer size for user data in netlink_delinearize_chain()
Thomas Haller [Wed, 8 Nov 2023 18:22:20 +0000 (19:22 +0100)] 
netlink: fix buffer size for user data in netlink_delinearize_chain()

commit 505a6794422238f9f1d590fe8c1ee3ea7fd46579 upstream.

The correct define is NFTNL_UDATA_CHAIN_MAX and not NFTNL_UDATA_OBJ_MAX.
In current libnftnl, they both are defined as 1, so (with current libnftnl)
there is no difference.

Fixes: 702ac2b72c0e ("src: add comment support for chains")
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agometa: fix hour decoding when timezone offset is negative
Florian Westphal [Thu, 2 Nov 2023 14:34:13 +0000 (15:34 +0100)] 
meta: fix hour decoding when timezone offset is negative

commit d392ddf243dcbf8a34726c777d2c669b1e8bfa85 upstream.

Brian Davidson says:

 meta hour rules don't display properly after being created when the
 hour is on or after 00:00 UTC. The netlink debug looks correct for
 seconds past midnight UTC, but displaying the rules looks like an
 overflow or a byte order problem. I am in UTC-0400, so today, 20:00
 and later exhibits the problem, while 19:00 and earlier hours are
 fine.

meta.c only ever worked when the delta to UTC is positive.
We need to add in case the second counter turns negative after
offset adjustment.

Also add a test case for this.

Fixes: f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
Reported-by: Brian Davidson <davidson.brian@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
5 months agotproxy: Drop artificial port printing restriction
Phil Sutter [Thu, 2 Nov 2023 13:48:10 +0000 (14:48 +0100)] 
tproxy: Drop artificial port printing restriction

commit e4c9f9f7e0d1f83be18f6c4a418da503e9021b24 upstream.

It does not make much sense to omit printing the port expression if it's
not a value expression: On one hand, input allows for more advanced
uses. On the other, if it is in-kernel, best nft can do is to try and
print it no matter what. Just ignoring ruleset elements can't be
correct.

Fixes: 2be1d52644cf7 ("src: Add tproxy support")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1721
Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 months agoparser_bison: fix length check for ifname in ifname_expr_alloc()
Thomas Haller [Mon, 23 Oct 2023 17:00:47 +0000 (19:00 +0200)] 
parser_bison: fix length check for ifname in ifname_expr_alloc()

commit 122dce6b35205a3df419a5cae9acfd6e83e8725a upstream.

IFNAMSIZ is 16, and the allowed byte length of the name is one less than
that. Fix the length check and adjust a test for covering the longest
allowed interface name.

This is obviously a change in behavior, because previously interface
names with length 16 were accepted and were silently truncated along the
way. Now they are rejected as invalid.

Fixes: fa52bc225806 ("parser: reject zero-length interface names")
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser_bison: Fix for broken compatibility with older dumps
Phil Sutter [Thu, 19 Oct 2023 16:40:04 +0000 (18:40 +0200)] 
parser_bison: Fix for broken compatibility with older dumps

commit 22fab8681a50014174cdd02ace90f74b9e9eefe9 upstream.

Commit e6d1d0d611958 ("src: add set element multi-statement
support") changed the order of expressions and other state attached to set
elements are expected in input. This broke parsing of ruleset dumps
created by nft commands prior to that commit.

Restore compatibility by also accepting the old ordering.

Fixes: e6d1d0d611958 ("src: add set element multi-statement support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoevaluate: validate maximum log statement prefix length
Pablo Neira Ayuso [Tue, 17 Oct 2023 13:50:21 +0000 (15:50 +0200)] 
evaluate: validate maximum log statement prefix length

commit 6ceec21204e0260af2d50e9e987d0fe3c79c28d4 upstream.

Otherwise too long string overruns the log prefix buffer.

Fixes: e76bb3794018 ("src: allow for variables in the log prefix string")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1714
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: never merge across non-expression statements redux 2
Florian Westphal [Fri, 6 Oct 2023 08:07:01 +0000 (10:07 +0200)] 
tests: never merge across non-expression statements redux 2

commit 8b9ae77598b4d074cfa6dc263e6064d9bd5610d4 upstream.

Turns out I also love to forget about nft-test.py -j.

Fixes: 99ab1b8feb16 ("rule: never merge across non-expression statements")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agorule: never merge across non-expression statements
Florian Westphal [Thu, 28 Sep 2023 21:27:55 +0000 (23:27 +0200)] 
rule: never merge across non-expression statements

commit 99ab1b8feb16741a83fb8b887bacae8fa07d29a2 upstream.

The existing logic can merge across non-expression statements,
if there is only one payload expression.

Example:
ether saddr 00:11:22:33:44:55 counter ether type 8021q

is turned into
counter ether saddr 00:11:22:33:44:55 ether type 8021q

which isn't the same thing.

Fix this up and add test cases for adjacent vlan and ip header
fields.  'Counter' serves as a non-merge fence.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agojson: add missing map statement stub
Pablo Neira Ayuso [Thu, 28 Sep 2023 18:34:09 +0000 (20:34 +0200)] 
json: add missing map statement stub

commit c0f820fd34549239e73758dda0b88f3d339e1a5c upstream.

Add map statement stub to restore compilation without json support.

Fixes: 27a2da23d508 ("netlink_linearize: skip set element expression in map statement key")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agonetlink_linearize: skip set element expression in map statement key
Pablo Neira Ayuso [Tue, 26 Sep 2023 08:02:23 +0000 (10:02 +0200)] 
netlink_linearize: skip set element expression in map statement key

commit 27a2da23d5085cfa3765fb5172e93d9eb060e7df upstream.

This fix is similar to 22d201010919 ("netlink_linearize: skip set element
expression in set statement key") to fix map statement.

netlink_gen_map_stmt() relies on the map key, that is expressed as a set
element. Use the set element key instead to skip the set element wrap,
otherwise get_register() abort execution:

  nft: netlink_linearize.c:650: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.

This includes JSON support to make this feature complete and it updates
tests/shell to cover for this support.

Reported-by: Luci Stanescu <luci@cnix.ro>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agojson: expose dynamic flag
Pablo Neira Ayuso [Tue, 26 Sep 2023 14:55:50 +0000 (16:55 +0200)] 
json: expose dynamic flag

commit 57f5aca0006ebf984ffc1f66d48cf3b74a3d1f59 upstream.

The dynamic flag is not exported via JSON, this triggers spurious
ENOTSUPP errors when restoring rulesets in JSON with dynamic flags
set on.

Fixes: 6e45b102650a2 ("nft: set: print dynamic flag when set")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: py: add map support
Pablo Neira Ayuso [Tue, 26 Sep 2023 14:53:07 +0000 (16:53 +0200)] 
tests: py: add map support

commit d97cf78385fd116bf97ff64f8a1c2b8f309e58f8 upstream.

Add basic map support to this infrastructure, eg.

  !map1 ipv4_addr : mark;ok

Adding elements to map is still not supported.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: never merge across non-expression statements redux
Florian Westphal [Fri, 29 Sep 2023 10:50:56 +0000 (12:50 +0200)] 
tests: never merge across non-expression statements redux

commit 4e8aa050312822400124260bf6b630c3c05cb04d upstream.

Forgot to 'git add' inet/bridge/netdev payload records.

Fixes: 99ab1b8feb16 ("rule: never merge across non-expression statements")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser_json: Default meter size to zero
Phil Sutter [Wed, 20 Sep 2023 15:16:33 +0000 (17:16 +0200)] 
parser_json: Default meter size to zero

commit 343006f5e0a6cdf907877218a354505dcc9d9864 upstream.

JSON parser was missed when performing the same change in standard
syntax parser.

Fixes: c2cad53ffc22a ("meters: do not set a defaut meter size from userspace")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agonetlink: handle invalid etype in set_make_key()
Thomas Haller [Wed, 20 Sep 2023 14:26:07 +0000 (16:26 +0200)] 
netlink: handle invalid etype in set_make_key()

commit c4186c5376ee73efff005dbd23dd73a8e06e6ad8 upstream.

It's not clear to me, what ensures that the etype is always valid.
Handle a NULL.

Fixes: 6e48df5329ea ('src: add "typeof" build/parse/print support')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodatatype: initialize TYPE_CT_EVENTBIT slot in datatype array
Pablo Neira Ayuso [Tue, 19 Sep 2023 16:15:17 +0000 (18:15 +0200)] 
datatype: initialize TYPE_CT_EVENTBIT slot in datatype array

commit 2d9e23a9e9c3c729784a3add41639cbd3f72d504 upstream.

Matching on ct event makes no sense since this is mostly used as
statement to globally filter out ctnetlink events, but do not crash
if it is used from concatenations.

Add the missing slot in the datatype array so this does not crash.

Fixes: 2595b9ad6840 ("ct: add conntrack event mask support")
Reported-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agodatatype: initialize TYPE_CT_LABEL slot in datatype array
Pablo Neira Ayuso [Tue, 19 Sep 2023 16:09:31 +0000 (18:09 +0200)] 
datatype: initialize TYPE_CT_LABEL slot in datatype array

commit 1b235f9962a059a599d9a9ecce477ed71e328e89 upstream.

Otherwise, ct label with concatenations such as:

 table ip x {
        chain y {
                ct label . ct mark  { 0x1 . 0x1 }
        }
 }

crashes:

../include/datatype.h:196:11: runtime error: member access within null pointer of type 'const struct datatype'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==640948==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc970d3199b bp 0x7fffd1f20560 sp 0x7fffd1f20540 T0)
==640948==The signal is caused by a READ memory access.
==640948==Hint: address points to the zero page.
sudo     #0 0x7fc970d3199b in datatype_equal ../include/datatype.h:196

Fixes: 2fcce8b0677b ("ct: connlabel matching support")
Reported-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser_json: Catch nonsense ops in match statement
Phil Sutter [Wed, 13 Sep 2023 20:07:46 +0000 (22:07 +0200)] 
parser_json: Catch nonsense ops in match statement

commit 7df0b2f1a1c64e2bdc652fd2418b4f7218c93f1f upstream.

Since expr_op_symbols array includes binary operators and more, simply
checking the given string matches any of the elements is not sufficient.

Fixes: 586ad210368b7 ("libnftables: Implement JSON parser")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Wrong check in json_parse_ct_timeout_policy()
Phil Sutter [Wed, 13 Sep 2023 18:53:41 +0000 (20:53 +0200)] 
parser_json: Wrong check in json_parse_ct_timeout_policy()

commit 1e5ad2eeb38af0af2e06d4cba0ec4d84009855fa upstream.

The conditional around json_unpack() was meant to accept a missing
policy attribute. But the accidentally inverted check made the function
either ignore a given policy or access uninitialized memory.

Fixes: c82a26ebf7e9f ("json: Add ct timeout support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Fix synproxy object mss/wscale parsing
Phil Sutter [Wed, 20 Sep 2023 17:40:11 +0000 (19:40 +0200)] 
parser_json: Fix synproxy object mss/wscale parsing

commit d73e269f7bffc04b1163ffd16e0bc1689d4127d2 upstream.

The fields are 16 and 8 bits in size, introduce temporary variables to
parse into.

Fixes: f44ab88b1088e ("src: add synproxy stateful object support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Fix limit object burst value parsing
Phil Sutter [Wed, 20 Sep 2023 17:37:21 +0000 (19:37 +0200)] 
parser_json: Fix limit object burst value parsing

commit 29aeff471496b6b1ae9a08dada21a5a9278467cf upstream.

The field is of type uint32_t, use lower case 'i' format specifier.

Fixes: c36288dbe2ba3 ("JSON: Fix parsing and printing of limit objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Fix flowtable prio value parsing
Phil Sutter [Wed, 20 Sep 2023 17:33:40 +0000 (19:33 +0200)] 
parser_json: Fix flowtable prio value parsing

commit 407e947dfc53827bd27689f2de9ed7f14f1b092e upstream.

Using format specifier 'I' requires a 64bit variable to write into. The
temporary variable 'prio' is of type int, though.

Fixes: 586ad210368b7 ("libnftables: Implement JSON parser")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Proper ct expectation attribute parsing
Phil Sutter [Wed, 20 Sep 2023 17:11:45 +0000 (19:11 +0200)] 
parser_json: Proper ct expectation attribute parsing

commit 34c1337296807b3a3147c95268f5e4ca70811779 upstream.

Parts of the code were unsafe (parsing 'I' format into uint32_t), the
rest just plain wrong (parsing 'o' format into char *tmp). Introduce a
temporary int variable to parse into.

Fixes: 1dd08fcfa07a4 ("src: add ct expectations support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Fix typo in json_parse_cmd_add_object()
Phil Sutter [Wed, 13 Sep 2023 18:44:19 +0000 (20:44 +0200)] 
parser_json: Fix typo in json_parse_cmd_add_object()

commit 300edcfbb357ef1785b5a16424952fcd06146cb3 upstream.

A case of bad c'n'p in the fixed commit broke ct timeout objects
parsing.

Fixes: c7a5401943df8 ("parser_json: Fix for ineffective family value checks")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_json: Catch wrong "reset" payload
Phil Sutter [Wed, 13 Sep 2023 18:32:37 +0000 (20:32 +0200)] 
parser_json: Catch wrong "reset" payload

commit 22febeea80043f5fe4eb1aa7723da0a0a6953802 upstream.

The statement happily accepted any valid expression as payload and
assumed it to be a tcpopt expression (actually, a special case of
exthdr). Add a check to make sure this is the case.

Standard syntax does not provide this flexibility, so no need to have
the check there as well.

Fixes: 5d837d270d5a8 ("src: add tcp option reset support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agolimit: display default burst when listing ruleset
Pablo Neira Ayuso [Tue, 19 Sep 2023 13:25:43 +0000 (15:25 +0200)] 
limit: display default burst when listing ruleset

commit 7360ab610164c7457b1024419ee046a4d05a6e2f upstream.

Default burst for limit is 5 for historical reasons but it is not
displayed when listing the ruleset.

Update listing to display the default burst to disambiguate.

man nft(8) has been recently updated to document this, no action in this
front is therefore required.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: expand sets and maps before evaluation
Pablo Neira Ayuso [Sat, 16 Sep 2023 13:42:48 +0000 (15:42 +0200)] 
evaluate: expand sets and maps before evaluation

commit 56c90a2dd2eb9cb63a6d74d0f5ce8075bef3895b upstream.

3975430b12d9 ("src: expand table command before evaluation") moved
ruleset expansion before evaluation, except for sets and maps. For
sets and maps there is still a post_expand() phase.

This patch moves sets and map expansion to allocate an independent
CMD_OBJ_SETELEMS command to add elements to named set and maps which is
evaluated, this consolidates the ruleset expansion to happen always
before the evaluation step for all objects, except for anonymous sets
and maps.

This approach avoids an interference with the set interval code which
detects overlaps and merges of adjacents ranges. This set interval
routine uses set->init to maintain a cache of existing elements. Then,
the post_expand() phase incorrectly expands set->init cache and it
triggers a bogus ENOENT errors due to incorrect bytecode (placing
element addition before set creation) in combination with user declared
sets using the flat syntax notation.

Since the evaluation step (coming after the expansion) creates
implicit/anonymous sets and maps, those are not expanded anymore. These
anonymous sets still need to be evaluated from set_evaluate() path and
the netlink bytecode generation path, ie. do_add_set(), needs to deal
with anonymous sets.

Note that, for named sets, do_add_set() does not use set->init. Such
content is part of the existing cache, and the CMD_OBJ_SETELEMS command
is responsible for adding elements to named sets.

Fixes: 3975430b12d9 ("src: expand table command before evaluation")
Reported-by: Jann Haber <jannh@selfnet.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoscanner: restrict include directive to regular files
Florian Westphal [Thu, 14 Sep 2023 09:42:16 +0000 (11:42 +0200)] 
scanner: restrict include directive to regular files

commit 999ca7dade519ad5757f07a9c488b326a5e7d785 upstream.

Similar to previous change, also check all

include "foo"

and reject those if they refer to named fifos, block devices etc.

Directories are still skipped, I don't think we can change this
anymore.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1664
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agolibnftables: refuse to open onput files other than named pipes or regular files
Florian Westphal [Thu, 14 Sep 2023 09:42:15 +0000 (11:42 +0200)] 
libnftables: refuse to open onput files other than named pipes or regular files

commit 149b1c95d129f8ec8a3df16aeca0e9063e8d45bf upstream backport.

Don't start e.g. parsing a block device.
nftables is typically run as privileged user, exit early if we
get unexpected input.

Only exception: Allow character device if input is /dev/stdin.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1664
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agodatatype: fix leak and cleanup reference counting for struct datatype
Thomas Haller [Tue, 12 Sep 2023 07:30:54 +0000 (09:30 +0200)] 
datatype: fix leak and cleanup reference counting for struct datatype

commit 8519ab031d8022999603a69ee9f18e8cfb06645d upstream.

Test `./tests/shell/run-tests.sh -V tests/shell/testcases/maps/nat_addr_port`
fails:

==118== 195 (112 direct, 83 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==118==    at 0x484682C: calloc (vg_replace_malloc.c:1554)
==118==    by 0x48A39DD: xmalloc (utils.c:37)
==118==    by 0x48A39DD: xzalloc (utils.c:76)
==118==    by 0x487BDFD: datatype_alloc (datatype.c:1205)
==118==    by 0x487BDFD: concat_type_alloc (datatype.c:1288)
==118==    by 0x488229D: stmt_evaluate_nat_map (evaluate.c:3786)
==118==    by 0x488229D: stmt_evaluate_nat (evaluate.c:3892)
==118==    by 0x488229D: stmt_evaluate (evaluate.c:4450)
==118==    by 0x488328E: rule_evaluate (evaluate.c:4956)
==118==    by 0x48ADC71: nft_evaluate (libnftables.c:552)
==118==    by 0x48AEC29: nft_run_cmd_from_buffer (libnftables.c:595)
==118==    by 0x402983: main (main.c:534)

I think the reference handling for datatype is wrong. It was introduced
by commit 01a13882bb59 ('src: add reference counter for dynamic
datatypes').

We don't notice it most of the time, because instances are statically
allocated, where datatype_get()/datatype_free() is a NOP.

Fix and rework.

- Commit 01a13882bb59 comments "The reference counter of any newly
  allocated datatype is set to zero". That seems not workable.
  Previously, functions like datatype_clone() would have returned the
  refcnt set to zero. Some callers would then then set the refcnt to one, but
  some wouldn't (set_datatype_alloc()). Calling datatype_free() with a
  refcnt of zero will overflow to UINT_MAX and leak:

       if (--dtype->refcnt > 0)
          return;

  While there could be schemes with such asymmetric counting that juggle the
  appropriate number of datatype_get() and datatype_free() calls, this is
  confusing and error prone. The common pattern is that every
  alloc/clone/get/ref is paired with exactly one unref/free.

  Let datatype_clone() return references with refcnt set 1 and in
  general be always clear about where we transfer ownership (take a
  reference) and where we need to release it.

- set_datatype_alloc() needs to consistently return ownership to the
  reference. Previously, some code paths would and others wouldn't.

- Replace

    datatype_set(key, set_datatype_alloc(dtype, key->byteorder))

  with a __datatype_set() with takes ownership.

Fixes: 01a13882bb59 ('src: add reference counter for dynamic datatypes')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: fix get element for concatenated set
Florian Westphal [Tue, 5 Sep 2023 14:32:16 +0000 (16:32 +0200)] 
evaluate: fix get element for concatenated set

commit 988e83a1ce61a829473082221a790e72f8d43519 upstream.

given:
table ip filter {
set test {
type ipv4_addr . ether_addr . mark
flags interval
elements = { 198.51.100.0/25 . 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00 . 0x0000006f, }
}
}

We get lookup failure:

nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f }
Error: Could not process rule: No such file or directory

Its possible to work around this via dummy range somewhere in the key, e.g.
nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f-0x6f }

but that shouldn't be needed, so make sure the INTERVAL flag is enabled
for the queried element if the set is of interval type.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: revisit anonymous set with single element optimization
Pablo Neira Ayuso [Sat, 2 Sep 2023 08:37:39 +0000 (10:37 +0200)] 
evaluate: revisit anonymous set with single element optimization

commit fa17b17ea74a21a44596f3212466ff3d2d3ede8e upstream.

This patch reworks it to perform this optimization from the evaluation
step of the relational expression. Hence, when optimizing for protocol
flags, use OP_EQ instead of OP_IMPLICIT, that is:

tcp flags { syn }

becomes (to represent an exact match):

tcp flags == syn

given OP_IMPLICIT and OP_EQ are not equivalent for flags.

01167c393a12 ("evaluate: do not remove anonymous set with protocol flags
and single element") disabled this optimization, which is enabled again
after this patch.

Fixes: 01167c393a12 ("evaluate: do not remove anonymous set with protocol flags and single element")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoinclude: drop "format" attribute from nft_gmp_print()
Thomas Haller [Tue, 29 Aug 2023 12:53:37 +0000 (14:53 +0200)] 
include: drop "format" attribute from nft_gmp_print()

commit 0ba90cceaf8693a392196112c809302271259758 upstream.

nft_gmp_print() passes the format string and arguments to
gmp_vfprintf(). Note that the format string is then interpreted
by gmp, which also understand special specifiers like "%Zx".

Note that with clang we get various compiler warnings:

  datatype.c:299:26: error: invalid conversion specifier 'Z' [-Werror,-Wformat-invalid-specifier]
          nft_gmp_print(octx, "0x%Zx [invalid type]", expr->value);
                                 ~^

gcc doesn't warn, because to gcc 'Z' is a deprecated alias for 'z' and
because the 3rd argument of the attribute((format())) is zero (so gcc
doesn't validate the arguments). But Z specifier in gmp expects a
"mpz_t" value and not a size_t. It's really not the same thing.

The correct solution is not to mark the function to accept a printf format
string.

Fixes: 2535ba7006f2 ('src: get rid of printf')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: fix check for truncation in stmt_evaluate_log_prefix()
Thomas Haller [Tue, 29 Aug 2023 12:53:33 +0000 (14:53 +0200)] 
evaluate: fix check for truncation in stmt_evaluate_log_prefix()

commit 7e6aa6db1fe5b14b5d224da11b077c50cc954efa upstream.

Otherwise, nft crashes with prefix longer than 127 bytes:

 # nft add rule x y log prefix \"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\"

==159385==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffed5bf4a10 at pc 0x7f3134839269 bp 0x7ffed5bf48b0 sp 0x7ffed5bf4060
WRITE of size 129 at 0x7ffed5bf4a10 thread T0
    #0 0x7f3134839268 in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:778
    #1 0x7f3133e3074e in __mpz_export_data /tmp/nftables/src/gmputil.c:110
    #2 0x7f3133d21d3c in expr_to_string /tmp/nftables/src/expression.c:192
    #3 0x7f3133ded103 in netlink_gen_log_stmt /tmp/nftables/src/netlink_linearize.c:1148
    #4 0x7f3133df33a1 in netlink_gen_stmt /tmp/nftables/src/netlink_linearize.c:1682
    [...]

Fixes: e76bb3794018 ('src: allow for variables in the log prefix string')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: monitor: Fix for wrong ordering in expected JSON output
Phil Sutter [Tue, 29 Aug 2023 13:44:25 +0000 (15:44 +0200)] 
tests: monitor: Fix for wrong ordering in expected JSON output

commit 830d280740bb414dadef0eba44f41f9fd1e7c6bf upstream.

Adjust JSON for delete before add for replace after respective kernel
fix, too.

Fixes: ba786ac758fba ("tests: monitor: update insert and replace commands")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agotests: monitor: Fix for wrong syntax in set-interval.t
Phil Sutter [Tue, 29 Aug 2023 13:41:13 +0000 (15:41 +0200)] 
tests: monitor: Fix for wrong syntax in set-interval.t

commit 3dc073e0c0836430c8692816ad8855f932debc67 upstream.

Expected JSON output must be prefixed 'J'.

Fixes: 7ab453a033c9a ("monitor: do not call interval_map_decompose() for concat intervals")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agotests: monitor: Fix time format in ct timeout test
Phil Sutter [Tue, 29 Aug 2023 13:22:56 +0000 (15:22 +0200)] 
tests: monitor: Fix time format in ct timeout test

commit eac68b84227a3fc46572f3bae1113eaa91d13f24 upstream.

The old "plain" numbers are still accepted (and assumed to be in
seconds), but output will use units which is unexpected due to 'O -'.
Adjust input instead of adding this subtly different output line.

Fixes: 5c25c5a35cbd2 ("parser: allow ct timeouts to use time_spec values")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agotests: monitor: Fix monitor JSON output for insert command
Phil Sutter [Tue, 29 Aug 2023 13:17:13 +0000 (15:17 +0200)] 
tests: monitor: Fix monitor JSON output for insert command

commit f66ef82e714aa502a2f59ffc31a6ffb16590db09 upstream.

Looks like commit ba786ac758fba ("tests: monitor: update insert and
replace commands") missed to also fix expected JSON output.

Fixes: 48d20b8cf162e ("monitor: honor NLM_F_APPEND flag for rules")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoevaluate: do not remove anonymous set with protocol flags and single element
Pablo Neira Ayuso [Mon, 28 Aug 2023 20:47:05 +0000 (22:47 +0200)] 
evaluate: do not remove anonymous set with protocol flags and single element

commit 01167c393a12c74c6f9a3015b75702964ff5bcda upstream.

Set lookups with flags search for an exact match, however:

tcp flags { syn }

gets transformed into:

tcp flags syn

which is matching on the syn flag only (non-exact match).

This optimization is safe for ct state though, because only one bit is
ever set on in the ct state bitmask.

Since protocol flags allow for combining flags, skip this optimization
to retain exact match semantics.

Another possible solution is to turn OP_IMPLICIT into OP_EQ for exact
flag match to re-introduce this optimization and deal with this corner
case.

Fixes: fee6bda06403 ("evaluate: remove anon sets with exactly one element")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: Drop dead code from expr_evaluate_mapping()
Phil Sutter [Fri, 25 Aug 2023 11:49:31 +0000 (13:49 +0200)] 
evaluate: Drop dead code from expr_evaluate_mapping()

commit 41f8628f6fb8340dde21e1295191a2d131c1a8df upstream.

Since commit 343a51702656a ("src: store expr, not dtype to track data in
sets"), set->data is allocated for object maps in set_evaluate(), all
other map types have set->data initialized by the parser already,
set_evaluate() also checks that.

Drop the confusing check, later in the function set->data is
dereferenced unconditionally.

Fixes: 343a51702656a ("src: store expr, not dtype to track data in sets")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agotests: shell: Stabilize sets/0043concatenated_ranges_0 test
Phil Sutter [Wed, 23 Aug 2023 16:18:49 +0000 (18:18 +0200)] 
tests: shell: Stabilize sets/0043concatenated_ranges_0 test

commit c791765cb0d62ba261f0b495e07315437b3ae914 upstream.

On a slow system, one of the 'delete element' commands would
occasionally fail. Assuming it can only happen if the 2s timeout passes
"too quickly", work around it by adding elements with a 2m timeout
instead and when wanting to test the element expiry just drop and add
the element again with a short timeout.

Fixes: 6231d3fa4af1e ("tests: shell: Fix for unstable sets/0043concatenated_ranges_0")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser: permit gc-interval in map declarations
Florian Westphal [Mon, 21 Aug 2023 14:12:52 +0000 (16:12 +0200)] 
parser: permit gc-interval in map declarations

commit 61eab46ee62a72629fa86c1929e73a2bfa95bc02 upstream.

Maps support timeouts, so allow to set the gc interval as well.

Fixes: 949cc39eb93f ("parser: support of maps with timeout")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: allow ct timeouts to use time_spec values
Florian Westphal [Wed, 2 Aug 2023 15:47:14 +0000 (17:47 +0200)] 
parser: allow ct timeouts to use time_spec values

commit 5c25c5a35cbd27911d233efd01efcb9be35c85af upstream.

For some reason the parser only allows raw numbers (seconds)
for ct timeouts, e.g.

ct timeout ttcp {
protocol tcp;
policy = { syn_sent : 3, ...

Also permit time_spec, e.g. "established : 5d".
Print the nicer time formats on output, but retain
raw numbers support on input for compatibility.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agotests: fix inet nat prio tests
Florian Westphal [Wed, 2 Aug 2023 14:19:45 +0000 (16:19 +0200)] 
tests: fix inet nat prio tests

commit 76835278e00af29d708e0085461c1cd79fb4050e upstream.

Its legal to DNAT in output and SNAT in input chain, so don't test
for that being illegal.

Fixes: 8beafab74c39 ("rule: allow src/dstnat prios in input and output")
Fixes: 34ce4e4a7bb6 ("test: shell: Test cases for standard chain prios")
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agolibnftables: Drop cache in -c/--check mode
Pablo Neira Ayuso [Mon, 31 Jul 2023 10:29:55 +0000 (12:29 +0200)] 
libnftables: Drop cache in -c/--check mode

commit 458e91a954abe4b7fb4ba70901c7da28220c446a upstream.

Extend e0aace943412 ("libnftables: Drop cache in error case") to also
drop the cache with -c/--check, this is a dry run mode and kernel does
not get any update.

This fixes a bug with -o/--optimize, which first runs in an implicit
-c/--check mode to validate that the ruleset is correct, then it
provides the proposed optimization. In this case, if the cache is not
emptied, old objects in the cache refer to scanner data that was
already released, which triggers BUG like this:

 BUG: invalid input descriptor type 151665524
 nft: erec.c:161: erec_print: Assertion `0' failed.
 Aborted

This bug was triggered in a ruleset that contains a set for geoip
filtering. This patch also extends tests/shell to cover this case.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agonetlink: delinearize: copy set keytype if needed
Florian Westphal [Tue, 25 Jul 2023 16:51:34 +0000 (18:51 +0200)] 
netlink: delinearize: copy set keytype if needed

commit 52c200b490d806da47d1f30448470e202e97f635 upstream.

Output before:
add @dynmark { 0xa020304 [invalid type] timeout 1s : 0x00000002 } comment "also check timeout-gc"

after:
add @dynmark { 10.2.3.4 timeout 1s : 0x00000002 } comment "also check timeout-gc"

This is a followup to 76c358ccfea0 ("src: maps: update data expression dtype based on set"),
which did fix the map expression, but not the key.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoexpression: define .clone for catchall set element
Pablo Neira Ayuso [Fri, 30 Jun 2023 07:42:11 +0000 (09:42 +0200)] 
expression: define .clone for catchall set element

commit 5bd6b4981ce649b5e0ae5ec30b7738ef33ef7c6e upstream.

Otherwise reuse of catchall set element expression in variable triggers
a null-pointer dereference.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoparser: reject zero-length interface names in flowtables
Florian Westphal [Mon, 19 Jun 2023 20:43:05 +0000 (22:43 +0200)] 
parser: reject zero-length interface names in flowtables

commit d40c7623837424d4eb8048508b924887b092e050 upstream.

Previous patch wasn't enough, also disable this for flowtable device lists.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: reject zero-length interface names
Florian Westphal [Mon, 19 Jun 2023 20:43:04 +0000 (22:43 +0200)] 
parser: reject zero-length interface names

commit fa52bc22580632b4b78c263e338ddfbe235a8218 upstream.

device "" results in an assertion during evaluation.
Before:
nft: expression.c:426: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
After:
zero_length_devicename_assert:3:42-49: Error: you cannot set an empty interface name
type filter hook ingress device""lo" priority -1
                         ^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoparser: don't assert on scope underflows
Florian Westphal [Mon, 19 Jun 2023 20:43:03 +0000 (22:43 +0200)] 
parser: don't assert on scope underflows

commit bb16416ec82599e41043a52887c37157e6f61984 upstream.

close_scope() gets called from the object destructors;
imbalance can cause us to hit assert().

Before:
nft: parser_bison.y:88: close_scope: Assertion `state->scope > 0' failed.
After:
assertion3:4:7-7: Error: too many levels of nesting jump {
assertion3:5:8-8: Error: too many levels of nesting jump
assertion3:5:9-9: Error: syntax error, unexpected newline, expecting '{'
assertion3:7:1-1: Error: syntax error, unexpected end of file

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: do not abort when prefix map has non-map element
Florian Westphal [Mon, 19 Jun 2023 20:43:02 +0000 (22:43 +0200)] 
evaluate: do not abort when prefix map has non-map element

commit 75217cb7bb78e22fc9317116353149def8a306e9 upstream.

Before:
nft: evaluate.c:1849: __mapping_expr_expand: Assertion `i->etype == EXPR_MAPPING' failed.

after:
Error: expected mapping, not set element
   snat ip prefix to ip saddr map { 10.141.11.0/24 : 192.168.2.0/24, 10.141.12.1 }

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agocache: include set elements in "nft set list"
Florian Westphal [Sun, 18 Jun 2023 16:39:45 +0000 (18:39 +0200)] 
cache: include set elements in "nft set list"

commit 29bed4fa594c3f6e343a8b5669d61e20c7129cca upstream.

Make "nft list sets" include set elements in listing by default.
In nftables 1.0.0, "nft list sets" did not include the set elements,
but with "--json" they were included.

1.0.1 and newer never include them.
This causes a problem for people updating from 1.0.0 and relying
on the presence of the set elements.

Change nftables to always include the set elements.
The "--terse" option is honored to get the "no elements" behaviour.

Fixes: a1a6b0a5c3c4 ("cache: finer grain cache population for list commands")
Link: https://marc.info/?l=netfilter&m=168704941828372&w=2
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: set NFT_SET_EVAL flag if dynamic set already exists
Pablo Neira Ayuso [Thu, 18 May 2023 12:40:38 +0000 (14:40 +0200)] 
evaluate: set NFT_SET_EVAL flag if dynamic set already exists

commit 5629f9c211fff71b72f2ba0c452801605b63cbfb upstream.

nft reports EEXIST when reading an existing set whose NFT_SET_EVAL has
been previously inferred from the ruleset.

 # cat test.nft
 table ip test {
        set dlist {
                type ipv4_addr
                size 65535
        }

        chain output {
                type filter hook output priority filter; policy accept;
                udp dport 1234 update @dlist { ip daddr } counter packets 0 bytes 0
        }
 }
 # nft -f test.nft
 # nft -f test.nft
 test.nft:2:6-10: Error: Could not process rule: File exists
         set dlist {
             ^^^^^

Phil Sutter says:

In the first call, the set lacking 'dynamic' flag does not exist
and is therefore added to the cache. Consequently, both the 'add set'
command and the set statement point at the same set object. In the
second call, a set with same name exists already, so the object created
for 'add set' command is not added to cache and consequently not updated
with the missing flag. The kernel thus rejects the NEWSET request as the
existing set differs from the new one.

Set on the NFT_SET_EVAL flag if the existing set sets it on.

Fixes: 8d443adfcc8c1 ("evaluate: attempt to set_eval flag if dynamic updates requested")
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: do not remove counter in verdict maps
Pablo Neira Ayuso [Sun, 7 May 2023 17:54:30 +0000 (19:54 +0200)] 
optimize: do not remove counter in verdict maps

commit 686ab8b6996e154592a5fc16bd1e15e661201b2a upstream.

Add counter to set element instead of dropping it:

 # nft -c -o -f test.nft
 Merging:
 test.nft:6:3-50:              ip saddr 1.1.1.1 ip daddr 2.2.2.2 counter accept
 test.nft:7:3-48:              ip saddr 1.1.1.2 ip daddr 3.3.3.3 counter drop
 into:
       ip daddr . ip saddr vmap { 2.2.2.2 . 1.1.1.1 counter : accept, 3.3.3.3 . 1.1.1.2 counter : drop }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: skip optimization if anonymous set uses stateful statement
Pablo Neira Ayuso [Sun, 7 May 2023 17:34:19 +0000 (19:34 +0200)] 
evaluate: skip optimization if anonymous set uses stateful statement

commit 033a664e89362e8c0c191a823bc37a6f92e8c89e upstream.

fee6bda06403 ("evaluate: remove anon sets with exactly one element")
introduces an optimization to remove use of sets with single element.
Skip this optimization if set element contains stateful statements.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: allow stateful statements with anonymous verdict maps
Pablo Neira Ayuso [Sun, 7 May 2023 17:30:46 +0000 (19:30 +0200)] 
evaluate: allow stateful statements with anonymous verdict maps

commit aceea86de797bcc315d3e759a44b97cbfb724435 upstream.

Evaluation fails to accept stateful statements in verdict maps, relax
the following check for anonymous sets:

test.nft:4:29-35: Error: missing statement in map declaration
                ip saddr vmap { 127.0.0.1 counter : drop, * counter : accept }
                                          ^^^^^^^

The existing code generates correctly the counter in the anonymous
verdict map.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agonetlink: restore typeof interval map data type
Florian Westphal [Mon, 1 May 2023 16:51:19 +0000 (18:51 +0200)] 
netlink: restore typeof interval map data type

commit 0583bac241ea18c9d7f61cb20ca04faa1e043b78 upstream.

When "typeof ... : interval ..." gets used, existing logic
failed to validate the expressions.

"interval" means that kernel reserves twice the size,
so consider this when validating and restoring.

Also fix up the dump file of the existing test
case to be symmetrical.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agotests: shell: Fix for unstable sets/0043concatenated_ranges_0
Phil Sutter [Thu, 20 Apr 2023 15:39:27 +0000 (17:39 +0200)] 
tests: shell: Fix for unstable sets/0043concatenated_ranges_0

commit 6231d3fa4af1e403555a6d4a139b3867218fab74 upstream.

On my (slow?) testing VM, The test tends to fail when doing a full run
(i.e., calling run-test.sh without arguments) and tends to pass when run
individually.

The problem seems to be the 1s element timeout which in some cases may
pass before element deletion occurs. Simply fix this by doubling the
timeout. It has to pass just once, so shouldn't hurt too much.

Fixes: 618393c6b3f25 ("tests: Introduce test for set with concatenated ranges")
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agonetlink_delinearize: do not reset protocol context for nat protocol expression
Pablo Neira Ayuso [Tue, 4 Apr 2023 13:34:05 +0000 (15:34 +0200)] 
netlink_delinearize: do not reset protocol context for nat protocol expression

commit f3b27274bfdb75dc29301bdd537ee6fec6d4e7c1 upstream backport.

This patch reverts 403b46ada490 ("netlink_delinearize: kill dependency
before eval of 'redirect' stmt"). Since ("evaluate: bogus missing
transport protocol"), this workaround is not required anymore.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: bogus missing transport protocol
Pablo Neira Ayuso [Tue, 4 Apr 2023 13:30:20 +0000 (15:30 +0200)] 
evaluate: bogus missing transport protocol

commit 2c227f00f0df9552b786080a8fd27c6a360e5828 upstream backport.

Users have to specify a transport protocol match such as

meta l4proto tcp

before the redirect statement, even if the redirect statement already
implicitly refers to the transport protocol, for instance:

test.nft:3:16-53: Error: transport protocol mapping is only valid after transport protocol match
                redirect to :tcp dport map { 83 : 8083, 84 : 8084 }
                ~~~~~~~~     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Evaluate the redirect expression before the mandatory check for the
transport protocol match, so protocol context already provides a
transport protocol.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: support for redirect and masquerade
Pablo Neira Ayuso [Tue, 4 Apr 2023 13:30:21 +0000 (15:30 +0200)] 
optimize: support for redirect and masquerade

commit 053566f71a28e9afc792d222a6fd7b55f7d8f4a0 upstream.

The redirect and masquerade statements can be handled as verdicts:

- if redirect statement specifies no ports.
- masquerade statement, in any case.

Exceptions to the rule: If redirect statement specifies ports, then nat
map transformation can be used iif both statements specify ports.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1668
Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: assert nat type on nat statement helper
Pablo Neira Ayuso [Tue, 4 Apr 2023 13:30:16 +0000 (15:30 +0200)] 
optimize: assert nat type on nat statement helper

commit 17a39cb0b082fe5117801d0b1a41407eec7b776c upstream.

Add assert() to helper function to expression from NAT statement.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoxt: Fix translation error path
Phil Sutter [Tue, 28 Mar 2023 11:46:10 +0000 (13:46 +0200)] 
xt: Fix translation error path

commit ce3d71348ee77d2d7ffa6a825afbc7471e92bc89 upstream.

If xtables support was compiled in but the required libxtables DSO is
not found, nft prints an error message and leaks memory:

| counter packets 0 bytes 0 XT target MASQUERADE not found

This is not as bad as it seems, the output combines stdout and stderr.
Dropping stderr produces an incomplete ruleset listing, though. While
this seemingly inline output can't easily be avoided, fix a few things:

* Respect octx->error_fp, libnftables might have been configured to
  redirect stderr somewhere else.
* Align error message formatting with others.
* Don't return immediately, but free allocated memory and fall back to
  printing the expression in "untranslated" form.

Fixes: 5c30feeee5cfe ("xt: Delay libxtables access until translation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agointervals: use expression location when translating to intervals
Pablo Neira Ayuso [Mon, 27 Mar 2023 14:36:31 +0000 (16:36 +0200)] 
intervals: use expression location when translating to intervals

commit 5e39a34b196d68b803911aa13066fef2f83dc98c upstream.

Otherwise, internal location reports:

 # nft -f ruleset.nft
 internal:0:0-0: Error: Could not process rule: File exists

after this patch:

 # nft -f ruleset.nft
 ruleset.nft:402:1-16: Error: Could not process rule: File exists
 1.2.3.0/30,
 ^^^^^^^^^^^

Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: insert byte-order conversions for expressions between 9 and 15 bits
Jeremy Sowden [Fri, 17 Mar 2023 09:16:36 +0000 (10:16 +0100)] 
evaluate: insert byte-order conversions for expressions between 9 and 15 bits

commit fe623a50949203fa979a79adc8f8af35b74b534c upstream.

Round up expression lengths when determining whether to insert a
byte-order conversion.  For example, if one is masking a network header
which spans a byte boundary, the mask will span two bytes and so it will
need to be in NBO.

Fixes: bb03cbcd18a1 ("evaluate: no need to swap byte-order for values of fewer than 16 bits.")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoxt: Fix fallback printing for extensions matching keywords
Phil Sutter [Thu, 9 Mar 2023 13:31:31 +0000 (14:31 +0100)] 
xt: Fix fallback printing for extensions matching keywords

commit aef5330fe7827f760b70d5d27010445c3adb3d3c upstream.

Yet another Bison workaround: Instead of the fancy error message, an
incomprehensible syntax error is emitted:

| # iptables-nft -A FORWARD -p tcp -m osf --genre linux
| # nft list ruleset | nft -f -
| # Warning: table ip filter is managed by iptables-nft, do not touch!
| /dev/stdin:4:29-31: Error: syntax error, unexpected osf, expecting string
|  meta l4proto tcp xt match osf counter packets 0 bytes 0
|                            ^^^

Avoid this by quoting the extension name when printing:

| # nft list ruleset | sudo ./src/nft -f -
| # Warning: table ip filter is managed by iptables-nft, do not touch!
| /dev/stdin:4:20-33: Error: unsupported xtables compat expression, use iptables-nft with this ruleset
|  meta l4proto tcp xt match "osf" counter packets 0 bytes 0
|                   ^^^^^^^^^^^^^^

Fixes: 79195a8cc9e9d ("xt: Rewrite unsupported compat expression dumping")
Fixes: e41c53ca5b043 ("xt: Fall back to generic printing from translation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
6 months agoparser_bison: allow to use quota in sets
Pablo Neira Ayuso [Wed, 1 Mar 2023 10:12:20 +0000 (11:12 +0100)] 
parser_bison: allow to use quota in sets

commit 87e5f35bc58450d352c303a8ff51b02193605d66 upstream.

src: support for restoring element quota

This patch allows you to restore quota in dynamic sets.

 table ip x {
        set y {
                type ipv4_addr
                size 65535
                flags dynamic,timeout
                counter quota 500 bytes
                timeout 1h
                elements = { 8.8.8.8 counter packets 9 bytes 756 quota 500 bytes used 500 bytes timeout 1h expires 56m57s47ms }
        }

        chain z {
                type filter hook output priority filter; policy accept;
                update @y { ip daddr } counter packets 6 bytes 507
        }
 }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agotests: shell: cover rule insertion by index
Pablo Neira Ayuso [Thu, 23 Feb 2023 19:36:43 +0000 (20:36 +0100)] 
tests: shell: cover rule insertion by index

commit 66191ce8b9c03cea1525f3f73f543ecf06cd58c4 upstream.

Original patch including this feature did not include a test, add it.

Fixes: 816d8c7659c1 ("Support 'add/insert rule index <IDX>'")
Reported-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agosrc: expand table command before evaluation
Pablo Neira Ayuso [Thu, 23 Feb 2023 18:55:39 +0000 (19:55 +0100)] 
src: expand table command before evaluation

commit 3975430b12d97c92cdf03753342f2269153d5624 upstream.

The nested syntax notation results in one single table command which
includes all other objects. This differs from the flat notation where
there is usually one command per object.

This patch adds a previous step to the evaluation phase to expand the
objects that are contained in the table into independent commands, so
both notations have similar representations.

Remove the code to evaluate the nested representation in the evaluation
phase since commands are independently evaluated after the expansion.

The commands are expanded after the set element collapse step, in case
that there is a long list of singleton element commands to be added to
the set, to shorten the command list iteration.

This approach also avoids interference with the object cache that is
populated in the evaluation, which might refer to objects coming in the
existing command list that is being processed.

There is still a post_expand phase to detach the elements from the set
which could be consolidated by updating the evaluation step to handle
the CMD_OBJ_SETELEMS command type.

This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
contains rules") which broke rule addition/insertion by index because
the expansion code after the evaluation messes up the cache.

Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agonetlink_delinearize: Sanitize concat data element decoding
Phil Sutter [Tue, 21 Feb 2023 17:36:01 +0000 (18:36 +0100)] 
netlink_delinearize: Sanitize concat data element decoding

commit 1344d9e53ba4d67cedd13a2c76a970fc7ce65683 upstream.

The call to netlink_get_register() might return NULL, catch this before
dereferencing the pointer.

Fixes: db59a5c1204c9 ("netlink_delinearize: fix decoding of concat data element")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
6 months agoevaluate: expand value to range when nat mapping contains intervals
Pablo Neira Ayuso [Fri, 17 Feb 2023 14:10:44 +0000 (15:10 +0100)] 
evaluate: expand value to range when nat mapping contains intervals

commit ddb962604cda323f15589f3b424c4618db7494de upstream.

If the data in the mapping contains a range, then upgrade value to range.
Otherwise, the following error is displayed:

/dev/stdin:11:57-75: Error: Could not process rule: Invalid argument
dnat ip to iifname . ip saddr map { enp2s0 . 10.1.1.136 : 1.1.2.69, enp2s0 . 10.1.1.1-10.1.1.135 : 1.1.2.66-1.84.236.78 }
                                    ^^^^^^^^^^^^^^^^^^^

The kernel rejects this command because userspace sends a single value
while the kernel expects the range that represents the min and the max
IP address to be used for NAT. The upgrade is also done when concatenation
with intervals is used in the rhs of the mapping.

For anonymous sets, expansion cannot be done from expr_evaluate_mapping()
because the EXPR_F_INTERVAL flag is inferred from the elements. For
explicit sets, this can be done from expr_evaluate_mapping() because the
user already specifies the interval flag in the rhs of the map definition.

Update tests/shell and tests/py to improve testing coverage in this case.

Fixes: 9599d9d25a6b ("src: NAT support for intervals in maps")
Fixes: 66746e7dedeb ("src: support for nat with interval concatenation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: infer family from mapping
Pablo Neira Ayuso [Thu, 16 Feb 2023 14:41:30 +0000 (15:41 +0100)] 
evaluate: infer family from mapping

commit 2f4935a8d7aeb6b2e019aebb961a579d9950c74a upstream backport.

If the key in the nat mapping is either ip or ip6, then set the nat
family accordingly, no need for explicit family in the nat statement.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: print error on missing family in nat statement
Pablo Neira Ayuso [Thu, 16 Feb 2023 14:49:11 +0000 (15:49 +0100)] 
evaluate: print error on missing family in nat statement

commit 6968c2632e0c7a625ca57cd4501b6b980fdebc55 upstream.

Print error message in case family cannot be inferred, before this
patch, $? shows 1 after nft execution but no error message was printed.

While at it, update error reporting for consistency in similar use
cases.

Fixes: e5c9c8fe0bcc ("evaluate: stmt_evaluate_nat_map() only if stmt->nat.ipportmap == true")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: infer family for nat mapping
Pablo Neira Ayuso [Wed, 15 Feb 2023 18:20:22 +0000 (19:20 +0100)] 
optimize: infer family for nat mapping

commit 114ba9606cb5ad85b2215e59f8af8ad97a16cac8 upstream.

Infer family from key in nat mapping, otherwise nat mapping via merge
breaks since family is not specified.

Merging:
fw-test-bug2.nft:4:9-78:         iifname enp2s0 ip daddr 72.2.3.66 tcp dport 53122 dnat to 10.1.1.10:22
fw-test-bug2.nft:5:9-77:         iifname enp2s0 ip daddr 72.2.3.66 tcp dport 443 dnat to 10.1.1.52:443
fw-test-bug2.nft:6:9-75:         iifname enp2s0 ip daddr 72.2.3.70 tcp dport 80 dnat to 10.1.1.52:80
into:
        dnat ip to iifname . ip daddr . tcp dport map { enp2s0 . 72.2.3.66 . 53122 : 10.1.1.10 . 22, enp2s0 . 72.2.3.66 . 443 : 10.1.1.52 . 443, enp2s0 . 72.2.3.70 . 80 : 10.1.1.52 . 80 }

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1657
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: ignore existing nat mapping
Pablo Neira Ayuso [Tue, 7 Feb 2023 09:53:41 +0000 (10:53 +0100)] 
optimize: ignore existing nat mapping

commit 9be404a153bc9525d52afabed622843717c37851 upstream.

User might be already using a nat mapping in their ruleset, use the
unsupported statement when collecting statements in this case.

 # nft -c -o -f ruleset.nft
 nft: optimize.c:443: rule_build_stmt_matrix_stmts: Assertion `k >= 0' failed.
 Aborted

The -o/--optimize feature only cares about linear rulesets at this
stage, but do not hit assert() in this case.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1656
Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agorule: expand standalone chain that contains rules
Pablo Neira Ayuso [Mon, 6 Feb 2023 14:28:41 +0000 (15:28 +0100)] 
rule: expand standalone chain that contains rules

commit 27c753e4a8d4744f479345e3f5e34cafef751602 upstream.

Otherwise rules that this chain contains are ignored when expressed
using the following syntax:

chain inet filter input2 {
       type filter hook input priority filter; policy accept;
       ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
}

When expanding the chain, remove the rule so the new CMD_OBJ_CHAIN
case does not expand it again.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1655
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: select merge criteria based on candidates rules
Pablo Neira Ayuso [Mon, 6 Feb 2023 13:18:10 +0000 (14:18 +0100)] 
optimize: select merge criteria based on candidates rules

commit 299823d46b6d0c49040d81ee3eb0f37b3b0520ea upstream.

Select the merge criteria based on the statements that are used
in the candidate rules, instead of using the list of statements
in the given chain.

Update tests to include a rule with a verdict, which triggers
the bug described in the bugzilla ticket.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1657
Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agorule: add helper function to expand chain rules into commands
Pablo Neira Ayuso [Mon, 6 Feb 2023 14:28:40 +0000 (15:28 +0100)] 
rule: add helper function to expand chain rules into commands

commit 784597a4ed63b9decb10d74fdb49a1b021e22728 upstream.

This patch adds a helper function to expand chain rules into commands.
This comes in preparation for the follow up patch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: fix incorrect expansion into concatenation with verdict map
Pablo Neira Ayuso [Thu, 2 Feb 2023 20:47:56 +0000 (21:47 +0100)] 
optimize: fix incorrect expansion into concatenation with verdict map

commit b691e2ea1d643adeb89c576a105f08cfff677cfb upstream.

 # nft -c -o -f ruleset.nft
 Merging:
 ruleset.nft:3:3-53:          meta pkttype broadcast udp dport { 67, 547 } accept
 ruleset.nft:4:17-58:         meta pkttype multicast udp dport 1900 drop
 into:
        meta pkttype . udp dport vmap { broadcast . { 67, 547 } : accept, multicast . 1900 : drop }
 ruleset.nft:3:38-39: Error: invalid data type, expected concatenation of (packet type, internet network service)
                meta pkttype broadcast udp dport { 67, 547 } accept
                                                   ^^

Similar to 187c6d01d357 ("optimize: expand implicit set element when
merging into concatenation") but for verdict maps.

Reported-by: Simon G. Trajkovski <neur0armitage@proton.me>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agooptimize: wrap code to build concatenation in helper function
Pablo Neira Ayuso [Thu, 2 Feb 2023 17:15:22 +0000 (18:15 +0100)] 
optimize: wrap code to build concatenation in helper function

commit 9dbbf397b2f3d9fa40454648cb98c13c7c5515b7 upstream.

Move code to build concatenations into helper function, this routine
includes support for expansion of implicit sets containing singleton
values. This is preparation work to reuse existing code in a follow up
patch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 months agoevaluate: set eval ctx for add/update statements with integer constants
Florian Westphal [Mon, 23 Jan 2023 18:03:28 +0000 (19:03 +0100)] 
evaluate: set eval ctx for add/update statements with integer constants

commit 4cc6b20d31498d90e90ff574ce8b70276afcee8f upstream.

Eric reports that nft asserts when using integer basetype constants with
'typeof' sets.  Example:
table netdev t {
set s {
typeof ether saddr . vlan id
flags dynamic,timeout
}

chain c { }
}

loads fine.  But adding a rule with add/update statement fails:
nft 'add rule netdev t c set update ether saddr . 0 @s'
nft: netlink_linearize.c:867: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.

When the 'ether saddr . 0' concat expression is processed, there is
no set definition available anymore to deduce the required size of the
integer constant.

nft eval step then derives the required length using the data types.
'0' has integer basetype, so the deduced length is 0.

The assertion triggers because serialization step finds that it
needs one more register.

2 are needed to store the ethernet address, another register is
needed for the vlan id.

Update eval step to make the expression context store the set key
information when processing the preceeding set reference, then
let stmt_evaluate_set() preserve the  existing context instead of
zeroing it again via stmt_evaluate_arg().

This makes concat expression evaluation compute the total size
needed based on the sets key definition.

Reported-by: Eric Garver <eric@garver.life>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 months agooptimize: Do not return garbage from stack
Phil Sutter [Fri, 13 Jan 2023 16:09:53 +0000 (17:09 +0100)] 
optimize: Do not return garbage from stack

commit d4d47e5bdf943be494aeb5d5a29b8f5212acbddf upstream.

If input does not contain a single 'add' command (unusual, but
possible), 'ret' value was not initialized by nft_optimize() before
returning its value.

Fixes: fb298877ece27 ("src: add ruleset optimization infrastructure")
Signed-off-by: Phil Sutter <phil@nwl.cc>