summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* tests: shell: use /bin/bash in sets/elem_opts_compat_01.0.6.yPablo Neira Ayuso2023-12-121-1/+1
| | | | | | | | | | 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>
* parser_bison: fix memleak in meta set error handlingFlorian Westphal2023-12-122-0/+6
| | | | | | | | | commit 21608263cc1ae489326e743957bfe34b05414a44 upstream. We must release the expression here, found via afl++ and -fsanitize-address build. Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: fix objref statement corruptionFlorian Westphal2023-12-123-37/+71
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* evaluate: validate chain max lengthFlorian Westphal2023-12-123-1/+45
| | | | | | | | | | | 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>
* parser: tcpopt: fix tcp option parsing with NUM + length fieldFlorian Westphal2023-12-124-10/+93
| | | | | | | | | | | | | | | | | | | | | | | | 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>
* evaluate: reject set definition with no keyPablo Neira Ayuso2023-12-121-2/+6
| | | | | | | | | | | | | | 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>
* monitor: add support for concatenated set rangesPablo Neira Ayuso2023-12-122-2/+21
| | | | | | | | | 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>
* evaluate: fix double free on dtype releaseFlorian Westphal2023-12-122-1/+7
| | | | | | | | | 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>
* evaluate: catch implicit map expressions without known datatypeFlorian Westphal2023-12-122-0/+5
| | | | | | | | | | 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>
* evaluate: reject attempt to update a setFlorian Westphal2023-12-122-0/+15
| | | | | | | | | | | | | 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>
* evaluate: error out if basetypes are differentFlorian Westphal2023-12-052-2/+10
| | | | | | | | | | | | 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>
* evaluate: guard against NULL basetypeFlorian Westphal2023-12-052-1/+2
| | | | | | | | commit 3671c48970031e617ee713b79caf8ef0a1b096c2 upstream. i->dtype->basetype can be NULL. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: handle invalid mapping expressions gracefullyPablo Neira Ayuso2023-12-052-2/+3
| | | | | | | | | | | | | | 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>
* json: deal appropriately with multidevice in chainPablo Neira Ayuso2023-11-272-58/+58
| | | | | | | | | | | | | | | | | | 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>
* evaluate: clone unary expression datatype to deal with dynamic datatypePablo Neira Ayuso2023-11-273-1/+29
| | | | | | | | | | 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>
* evaluate: fix rule replacement with anon setsFlorian Westphal2023-11-211-0/+1
| | | | | | | | | | | | 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>
* json: fix use after free in table_flags_json()Thomas Haller2023-11-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* src: expand create commandsPablo Neira Ayuso2023-11-215-1/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* netlink: fix buffer size for user data in netlink_delinearize_chain()Thomas Haller2023-11-211-1/+1
| | | | | | | | | | | | 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>
* meta: fix hour decoding when timezone offset is negativeFlorian Westphal2023-11-213-4/+61
| | | | | | | | | | | | | | | | | | | | | | | 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>
* tproxy: Drop artificial port printing restrictionPhil Sutter2023-11-214-1/+50
| | | | | | | | | | | | | | | 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>
* parser_bison: fix length check for ifname in ifname_expr_alloc()Thomas Haller2023-11-212-1/+21
| | | | | | | | | | | | | | | | 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>
* parser_bison: Fix for broken compatibility with older dumpsPhil Sutter2023-11-032-0/+35
| | | | | | | | | | | | | | 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>
* evaluate: validate maximum log statement prefix lengthPablo Neira Ayuso2023-11-031-1/+6
| | | | | | | | | | 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>
* tests: never merge across non-expression statements redux 2Florian Westphal2023-11-032-0/+95
| | | | | | | | | 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>
* rule: never merge across non-expression statementsFlorian Westphal2023-11-036-4/+40
| | | | | | | | | | | | | | | | | | | | 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>
* json: add missing map statement stubPablo Neira Ayuso2023-11-031-0/+1
| | | | | | | | | 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>
* netlink_linearize: skip set element expression in map statement keyPablo Neira Ayuso2023-11-0315-3/+204
| | | | | | | | | | | | | | | | | | | 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>
* json: expose dynamic flagPablo Neira Ayuso2023-11-032-0/+3
| | | | | | | | | | | | 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>
* tests: py: add map supportPablo Neira Ayuso2023-11-031-4/+66
| | | | | | | | | | | | 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>
* tests: never merge across non-expression statements reduxFlorian Westphal2023-11-033-0/+54
| | | | | | | | | 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>
* parser_json: Default meter size to zeroPhil Sutter2023-11-031-1/+1
| | | | | | | | | | 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>
* netlink: handle invalid etype in set_make_key()Thomas Haller2023-11-031-0/+2
| | | | | | | | | | | 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>
* datatype: initialize TYPE_CT_EVENTBIT slot in datatype arrayPablo Neira Ayuso2023-11-033-1/+3
| | | | | | | | | | | | | | 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>
* datatype: initialize TYPE_CT_LABEL slot in datatype arrayPablo Neira Ayuso2023-11-033-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* parser_json: Catch nonsense ops in match statementPhil Sutter2023-11-031-4/+9
| | | | | | | | | | 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>
* parser_json: Wrong check in json_parse_ct_timeout_policy()Phil Sutter2023-11-031-1/+1
| | | | | | | | | | | 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>
* parser_json: Fix synproxy object mss/wscale parsingPhil Sutter2023-11-031-3/+4
| | | | | | | | | | 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>
* parser_json: Fix limit object burst value parsingPhil Sutter2023-11-031-1/+1
| | | | | | | | | 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>
* parser_json: Fix flowtable prio value parsingPhil Sutter2023-11-031-1/+1
| | | | | | | | | | 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>
* parser_json: Proper ct expectation attribute parsingPhil Sutter2023-11-031-6/+7
| | | | | | | | | | | 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>
* parser_json: Fix typo in json_parse_cmd_add_object()Phil Sutter2023-11-031-1/+1
| | | | | | | | | | 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>
* parser_json: Catch wrong "reset" payloadPhil Sutter2023-11-031-1/+8
| | | | | | | | | | | | | | 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>
* limit: display default burst when listing rulesetPablo Neira Ayuso2023-11-038-20/+29
| | | | | | | | | | | | | | 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>
* evaluate: expand sets and maps before evaluationPablo Neira Ayuso2023-11-033-40/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* scanner: restrict include directive to regular filesFlorian Westphal2023-11-032-3/+67
| | | | | | | | | | | | | | | | 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>
* libnftables: refuse to open onput files other than named pipes or regular filesFlorian Westphal2023-11-031-0/+34
| | | | | | | | | | | | | 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>
* datatype: fix leak and cleanup reference counting for struct datatypeThomas Haller2023-11-038-50/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* evaluate: fix get element for concatenated setFlorian Westphal2023-11-031-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* evaluate: revisit anonymous set with single element optimizationPablo Neira Ayuso2023-11-031-20/+40
| | | | | | | | | | | | | | | | | | | | | | | 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>