summaryrefslogtreecommitdiffstats
path: root/tests/shell/testcases
Commit message (Collapse)AuthorAgeFilesLines
...
* evaluate: fix stack overflow with huge priority stringFlorian Westphal2023-12-151-0/+5
| | | | | | | | Alternative would be to refactor this and move this into the parsers (bison, json) instead of this hidden re-parsing. Fixes: 627c451b2351 ("src: allow variables in the chain priority specification") Signed-off-by: Florian Westphal <fw@strlen.de>
* netlink: fix stack buffer overflow with sub-reg sized prefixesFlorian Westphal2023-12-151-0/+5
| | | | | | | | | | | | | | | | | | | | | | | The calculation of the dynamic on-stack array is incorrect, the scratch space can be too low which gives stack corruption: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdb454f064.. #1 0x7fabe92aaac4 in __mpz_export_data src/gmputil.c:108 #2 0x7fabe92d71b1 in netlink_export_pad src/netlink.c:251 #3 0x7fabe92d91d8 in netlink_gen_prefix src/netlink.c:476 div_round_up() cannot be used here, it fails to account for register padding. A 16 bit prefix will need 2 registers (start, end -- 8 bytes in total). Remove the dynamic sizing and add an assertion in case upperlayer ever passes invalid expr sizes down to us. After this fix, the combination is rejected by the kernel because of the maps' wrong data size, before the fix userspace may crash before. Signed-off-by: Florian Westphal <fw@strlen.de>
* src: reject large raw payload and concat expressionsFlorian Westphal2023-12-152-0/+10
| | | | | | | | | | | | | | | | | | The kernel will reject this too, but unfortunately nft may try to cram the data into the underlying libnftnl expr. This causes heap corruption or BUG: nld buffer overflow: want to copy 132, max 64 After: Error: Concatenation of size 544 exceeds maximum size of 512 udp length . @th,0,512 . @th,512,512 { 47-63 . 0xe373135363130 . 0x33131303735353203 } ^^^^^^^^^ resp. same warning for an over-sized raw expression. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: exthdr: statement arg must be not be a rangeFlorian Westphal2023-12-141-0/+1
| | | | | | | | Else we get: BUG: unknown expression type range nft: src/netlink_linearize.c:909: netlink_gen_expr: Assertion `0' failed. Signed-off-by: Florian Westphal <fw@strlen.de>
* Revert "evaluate: error out when existing set has incompatible key"Florian Westphal2023-12-144-12/+30
| | | | | | | | | | This breaks existing behaviour, add a test case so this is caught in the future. The reverted test case will be brought back once a better fix is available. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: fix gmp assertion with too-large reject codeFlorian Westphal2023-12-141-0/+1
| | | | | | | | Before: nft: gmputil.c:77: mpz_get_uint8: Assertion `cnt <= 1' failed. After: Error: reject code must be integer in range 0-255 Signed-off-by: Florian Westphal <fw@strlen.de>
* meta: fix tc classid parsing out-of-bounds accessFlorian Westphal2023-12-131-0/+6
| | | | | | | | | | | | | | | | | AddressSanitizer: heap-buffer-overflow on address 0x6020000003af ... #0 0x7f9a83cbb402 in tchandle_type_parse src/meta.c:89 #1 0x7f9a83c6753f in symbol_parse src/datatype.c:138 strlen() - 1 can underflow if length was 0. Simplify the function, there is no need to duplicate the string while scanning it. Expect the first strtol to stop at ':', scan for the minor number next. The second scan is required to stop at '\0'. Fixes: 6f2eb8548e0d ("src: meta priority support using tc classid") Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: error out when existing set has incompatible keyFlorian Westphal2023-12-131-0/+12
| | | | | | | | | | | | | | Before: BUG: invalid range expression type symbol nft: expression.c:1494: range_expr_value_high: Assertion `0' failed. After: range_expr_value_high_assert:5:20-27: Error: Could not resolve protocol name elements = { 100-11.0.0.0, } ^^^^^^^^ range_expr_value_high_assert:7:6-7: Error: set definition has conflicting key (ipv4_addr vs inet_proto) Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: stmt_nat: set reference must point to a mapFlorian Westphal2023-12-131-0/+10
| | | | | | | | | | nat_concat_map() requires a datamap, else we crash: set->data is dereferenced. Also update expr_evaluate_map() so that EXPR_SET_REF is checked there too. Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: fix memory leaks on hookspec error processingFlorian Westphal2023-12-131-0/+21
| | | | | | | prio_spec may contain an embedded expression, release it. We also need to release the device expr and the hook string. Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: close chain scope before chain releaseFlorian Westphal2023-12-131-0/+5
| | | | | | | | | | | | | | | | cmd_alloc() will free the chain, so we must close the scope opened in chain_block_alloc beforehand. The included test file will cause a use-after-free because nft attempts to search for an identifier in a scope that has been freed: AddressSanitizer: heap-use-after-free on address 0x618000000368 at pc 0x7f1cbc0e6959 bp 0x7ffd3ccb7850 sp 0x7ffd3ccb7840 #0 0x7f1cbc0e6958 in symbol_lookup src/rule.c:629 #1 0x7f1cbc0e66a1 in symbol_get src/rule.c:588 #2 0x7f1cbc120d67 in nft_parse src/parser_bison.y:4325 Fixes: a66b5ad9540d ("src: allow for updating devices on existing netdev chain") Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: fix ct scope underflow if ct helper section is duplicatedFlorian Westphal2023-12-121-0/+14
| | | | | | | | | | | | | | | | | | | | | table inet filter { ct helper sip-5060u { type "sip" protocol udp l3proto ip }5060t { type "sip" protocol tcp l3pownerip } Will close the 'ct' scope twice, it has to be closed AFTER the separator has been parsed. While not strictly needed, also error out if the protocol is already given, this provides a better error description. Also make sure we release the string in all error branches. Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: make sure obj_free releases timeout policiesFlorian Westphal2023-12-121-0/+5
| | | | | | | | | obj_free() won't release them because ->type is still 0 at this point. Init this to CT_TIMEOUT. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: fix bogus assertion failure with boolean datatypeFlorian Westphal2023-12-122-0/+26
| | | | | | | | | | | | The assertion is too strict, as found by afl++: typeof iifname . ip saddr . meta ipsec elements = { "eth0" . 10.1.1.2 . 1 } meta ipsec is boolean (1 bit), but datasize of 1 is set at 8 bit. Fixes: 22b750aa6dc9 ("src: allow use of base integer types as set keys in concatenations") Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_bison: fix memleak in meta set error handlingFlorian Westphal2023-12-111-0/+5
| | | | | | | 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-112-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-112-0/+12
| | | | | | | | | 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>
* tests: rename file to lowercaseFlorian Westphal2023-12-061-0/+0
| | | | | | | Thanks to autocomplete i didn't notice this earlier, make this lowercase. Signed-off-by: Florian Westphal <fw@strlen.de>
* parser: tcpopt: fix tcp option parsing with NUM + length fieldFlorian Westphal2023-12-062-0/+55
| | | | | | | | | | | | | | | | | | | | | | 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>
* tests: shell: add test case for sets without keyFlorian Westphal2023-12-061-0/+12
| | | | | | | | | | | 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 bug doesn't trigger anymore due to 1949a63215b4 ("evaluate: reject set definition with no key") ] Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: fix double free on dtype releaseFlorian Westphal2023-12-051-0/+6
| | | | | | | 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-051-0/+1
| | | | | | | | 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-051-0/+11
| | | | | | | | | | | 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: disable meta set with rangesFlorian Westphal2023-12-051-0/+5
| | | | | | | | | | | | | | | | ... this will cause an assertion in netlink linearization, catch this at eval stage instead. before: BUG: unknown expression type range nft: netlink_linearize.c:908: netlink_gen_expr: Assertion `0' failed. after: /unknown_expr_type_range_assert:3:31-40: Error: Meta expression cannot be a range meta mark set 0x001-3434 ^^^^^^^^^^ Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: error out if basetypes are differentFlorian Westphal2023-12-051-0/+5
| | | | | | | | | | 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-051-0/+1
| | | | | | i->dtype->basetype can be NULL. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: handle invalid mapping expressions gracefullyFlorian Westphal2023-12-051-0/+1
| | | | | | | | | | | | 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>
* evaluate: turn assert into real error checkFlorian Westphal2023-12-051-0/+5
| | | | | | | | | large '& VAL' results in: src/evaluate.c:531: expr_evaluate_bits: Assertion `masklen <= NFT_REG_SIZE * BITS_PER_BYTE' failed. Turn this into expr_error(). Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: prevent assert when evaluating very large shift valuesFlorian Westphal2023-12-031-0/+5
| | | | | | | Error out instead of 'nft: gmputil.c:67: mpz_get_uint32: Assertion `cnt <= 1' failed.'. Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand") Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: add missing .nodump fileFlorian Westphal2023-12-011-0/+0
| | | | | | | | We don't want a dump file here, the test has elements with timeouts, listing will differ depending on timing ("expires $random seconds"). Fixes: 4890211e188a ("tests: shell: add test case for catchall gc bug") Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: reject sets with no keyFlorian Westphal2023-12-012-0/+10
| | | | | | | | nft --check -f tests/shell/testcases/bogons/nft-f/set_without_key Segmentation fault (core dumped) Fixes: 56c90a2dd2eb ("evaluate: expand sets and maps before evaluation") Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: add test case for catchall gc bugFlorian Westphal2023-11-281-0/+13
| | | | | | | | Check for bug fixed with kernel commit 93995bf4af2c ("netfilter: nf_tables: remove catchall element in GC sync path"). Reported-by: lonial con <kongln9170@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* json: deal appropriately with multidevice in chainPablo Neira Ayuso2023-11-233-3/+3
| | | | | | | | | | | | | | | | 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-222-0/+28
| | | | | | | | 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>
* tests: shell: Fix sets/reset_command_0 for current kernelsPhil Sutter2023-11-221-8/+2
| | | | | | | | | Since kernel commit 4c90bba60c26 ("netfilter: nf_tables: do not refresh timeout when resetting element"), element reset won't touch expiry anymore. Invert the one check to make sure it remains unaltered, drop the other testing behaviour for per-element timeouts. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests/shell: sanitize "handle" in JSON outputThomas Haller2023-11-22277-277/+277
| | | | | | | | | The "handle" in JSON output is not stable. Sanitize/normalize to zero. Adjust the sanitize code, and regenerate the .json-nft files. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip if kernel does not allow to restore set element expirationPablo Neira Ayuso2023-11-221-0/+2
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip secmark tests if kernel does not support itPablo Neira Ayuso2023-11-221-0/+1
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: split nat inet testsPablo Neira Ayuso2023-11-224-27/+32
| | | | | | Detach nat inet from existing tests not to reduce test coverage. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip nat inet if kernel does not support itPablo Neira Ayuso2023-11-221-0/+2
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip synproxy test if kernel does not support itPablo Neira Ayuso2023-11-221-0/+2
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: detach synproxy testPablo Neira Ayuso2023-11-224-33/+52
| | | | | | Old kernels do not support synproxy, split existing tests with stateful objects. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip stateful object updates if unsupportedPablo Neira Ayuso2023-11-221-0/+2
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: connlimit tests requires set expression supportPablo Neira Ayuso2023-11-221-0/+2
| | | | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: bogus error when adding devices to flowtablePablo Neira Ayuso2023-11-221-0/+8
| | | | | | | | | | | | | | | | | | | | | | Bail out if flowtable declaration is missing and no devices are specified. Otherwise, this reports a bogus error when adding new devices to an existing flowtable. # nft -v nftables v1.0.9 (Old Doc Yak #3) # ip link add dummy1 type dummy # ip link set dummy1 up # nft 'create flowtable inet filter f1 { hook ingress priority 0; counter }' # nft 'add flowtable inet filter f1 { devices = { dummy1 } ; }' Error: missing hook and priority in flowtable declaration add flowtable inet filter f1 { devices = { dummy1 } ; } ^^^^^^^^^^^^^^^^^^^^^^^^ Fixes: 5ad475fce5a1 ("evaluate: bail out if new flowtable does not specify hook and priority") Reported-by: Martin Gignac <martin.gignac@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: flush connlimit setsPablo Neira Ayuso2023-11-212-2/+3
| | | | | | | | | | | Restored elements via set declaration are removed almost inmediately by GC, this is causing spurious failures in test runs. Flush sets to ensure dump is always consistent. Still, cover that restoring a set with connlimit elements do not. Fixes: 95d348d55a9e ("tests: shell: extend connlimit test") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: skip meta time test meta expression lacks supportFlorian Westphal2023-11-211-0/+2
| | | | Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: skip maps delete test if dynset lacks delete opFlorian Westphal2023-11-211-0/+2
| | | | Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: skip ct expectation test if feature is missingFlorian Westphal2023-11-211-0/+2
| | | | Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: quote reference to array to iterate over empty stringPablo Neira Ayuso2023-11-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | This patch restores coverage for non-interval set backend. Use "${FLAGS[@]}" in loop, otherwise empty string is skipped in the iteration. This snippet: FLAGS=("") available_flags FLAGS "single" for flags in "${FLAGS[@]}" ; do echo $flags done ... now shows the empty string: # bash test.sh interval Fixes: ed927baa4fd8 ("tests: shell: skip pipapo set backend in transactions/30s-stress") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>