summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* build: Bump version to 1.0.6.1v1.0.6.11.0.6.yPablo Neira Ayuso2025-09-021-1/+1
| | | | | | This is -stable release including 412 backported commits. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: mnl: always dump all netdev hooks if no interface name was givenFlorian Westphal2025-09-014-7/+159
| | | | | | | | | | | commit db70959a5ccf2952b218f51c3d529e186a5a43bb upstream. Instead of not returning any results for nft list hooks netdev Iterate all interfaces and then query all of them. Signed-off-by: Florian Westphal <fw@strlen.de>
* src: mnl: prepare for listing all device netdev device hooksFlorian Westphal2025-09-011-3/+26
| | | | | | | | | | | | | | | | | | | | commit b8872b83eb365fcc921f2c59ac3ea055ca22c7e7 upstream. Change output foramt slightly so device name is included for netdev family. % nft list hooks netdev device eth0 family netdev { hook ingress device eth0 { 0000000000 chain inet ingress in_public [nf_tables] 0000000000 chain netdev ingress in_public [nf_tables] } hook egress device eth0 { 0000000000 chain netdev ingress out_public [nf_tables] } } Signed-off-by: Florian Westphal <fw@strlen.de>
* src: drop obsolete hook argument form hook dump functionsFlorian Westphal2025-09-013-19/+15
| | | | | | | | | | commit 4d66136082ce32f979bf992e68385ed033af057e upstream. since commit b98fee20bfe2 ("mnl: revisit hook listing"), handle.chain is never set in this path, so 'hook' is always set to -1, so the hook arg can be dropped. Signed-off-by: Florian Westphal <fw@strlen.de>
* src: mnl: make family specification more strict when listingFlorian Westphal2025-09-011-29/+24
| | | | | | | | | | | | | | | | | | | | | | | commit 194dc496177a8197c617743ba3cce7f4dc8fffa7 upstream. make "nft list hooks <family>" more strict. nft list hooks: query/list all NFPROTO_XXX values, i.e. arp, bridge, ipv4, ipv6. If a device is also given, then do include the netdev family for the given device as well. "nft list hooks arp" will only dump the hooks registered for NFPROTO_ARP (or nothing at all if none are active). "bridge", "ip", "ip6" will list the pre/in/forward/output/postrouting hooks for these families, if any. "inet" serves as an alias for "ip" and "ip6". Link: https://lore.kernel.org/netfilter-devel/20240729153211.GA26048@breakpoint.cc/ Signed-off-by: Florian Westphal <fw@strlen.de>
* src: mnl: clean up hook listing codeFlorian Westphal2025-09-011-63/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit af712117df54d8620ed046d19a37ee62516d5f38 upstream. mnl_nft_dump_nf_hooks() can call itself for the UNSPEC case, this avoids the second switch/case to handle printing for inet/unspec. As for the error handling, 'nft list hooks' should not print an error, even if nothing is printed, UNLESS there was also a lowlevel (syscall) error from the kernel. We don't want to indicate failure just because e.g. kernel doesn't support NFPROTO_ARP. This also fixes a display bug, 'nft list hooks device foo' would show hooks registered for that device as 'bridge' family instead of the expected 'netdev' family. This was because UNSPEC handling did not query 'netdev' family and did pass the device name to the lowlevel function. Add it, and pass NULL device name for those families that don't support device attachment. The lowelevel function currently always queries NFPROTO_NETDEV to handle the 'inet' ingress case. This is dubious, as 'inet ingress' is a pseudo-alias to netdev family (inet itself is a pseudo-family that ends up registering for both ipv4 and ipv6 hooks). This is resolved in next patch. Signed-off-by: Florian Westphal <fw@strlen.de>
* src: remove decnet supportFlorian Westphal2025-09-013-100/+0
| | | | | | | | commit 80258b03640a729113195fad027d61fd17692db9 upstream. Removed two years ago with v6.1, ditch this from hook list code as well. Signed-off-by: Florian Westphal <fw@strlen.de>
* src: remove utf-8 character in printf linesFlorian Westphal2025-09-012-12/+12
| | | | | | | | commit c92ec3b21979fe446fafa139c06ee99cb17ffd54 upstream. replace "‘" (UTF-8, 0xe280 0x98) with "'" (ASCII 0x27). Signed-off-by: Florian Westphal <fw@strlen.de>
* mergesort: avoid cloning value in expr_msort_cmp()Thomas Haller2025-09-011-16/+15
| | | | | | | | | | commit f749f344331a03ce1f5035ef6d652786f48643ce upstream. If we have a plain EXPR_VALUE value, there is no need to copy it via mpz_set(). Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: fix "const static" declarationThomas Haller2025-09-011-2/+2
| | | | | | | | | | | | | | | | commit 06810e15bcfea356ccecde9317dd860b0f29df06 upstream. Gcc warns against this with "-Wextra": src/rule.c:869:1: error: static is not at beginning of declaration [-Werror=old-style-declaration] 869 | const static struct prio_tag std_prios[] = { | ^~~~~ src/rule.c:878:1: error: static is not at beginning of declaration [-Werror=old-style-declaration] 878 | const static struct prio_tag bridge_std_prios[] = { | ^~~~~ Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* cache: chain listing implicitly sets on terse optionPablo Neira Ayuso2025-09-011-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 70d99ce8bf8bd3dab84ea0a6249812b04ec95b8c upstream. If user specifies a chain to be listed (which is internally handled via filtering options), then toggle NFT_CACHE_TERSE to skip fetching set content from kernel for non-anonymous sets. With a large IPv6 set with bogons, before this patch: # time nft list chain inet raw x table inet raw { chain x { ip6 saddr @bogons6 ip6 saddr { aaaa::, bbbb:: } } } real 0m2,913s user 0m1,345s sys 0m1,568s After this patch: # time nft list chain inet raw prerouting table inet raw { chain x { ip6 saddr @bogons6 ip6 saddr { aaaa::, bbbb:: } } } real 0m0,056s user 0m0,018s sys 0m0,039s This speeds up chain listing in the presence of a large set. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* INSTALL: provide examples to install python bindingsPablo Neira Ayuso2025-09-011-3/+9
| | | | | | | | | | | commit 97c28c926096950f1646c99b85a31de309429a0c upstream. Provide a bit more details on how to install python bindings with legacy setup.py and pip with .toml file. Original text from Jeremy Sowden <jeremy@azazel.net>. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* json: use strtok_r() instead of strtok()Thomas Haller2025-09-011-2/+3
| | | | | | | | | | | commit 4646d656466b1f05bd765bbfb4d6d7bf1529bdbd upstream. strtok_r() is probably(?) everywhere available where we care. Use it. It is thread-safe, and libnftables shouldn't make assumptions about what other threads of the process are doing. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* py: fix exception during cleanup of half-initialized NftablesThomas Haller2025-09-011-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 33706f99ce56e178b058b180661aacbea2e79ce9 upstream. When we create a Nftables instance against an older library version, we might not find a symbol and fail with an exception when initializing the context object. Then, __del__() is still called, but resulting in a second exception because self.__ctx is not set. Avoid that second exception. $ python -c 'import nftables; nftables.Nftables()' Traceback (most recent call last): File "<string>", line 1, in <module> File "/data/src/nftables/py/nftables.py", line 90, in __init__ self.nft_ctx_input_get_flags = lib.nft_ctx_input_get_flags ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.11/ctypes/__init__.py", line 389, in __getattr__ func = self.__getitem__(name) ^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.11/ctypes/__init__.py", line 394, in __getitem__ func = self._FuncPtr((name_or_ordinal, self)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: /lib64/libnftables.so.1: undefined symbol: nft_ctx_input_get_flags Exception ignored in: <function Nftables.__del__ at 0x7f6315a2c540> Traceback (most recent call last): File "/data/src/nftables/py/nftables.py", line 166, in __del__ self.nft_ctx_free(self.__ctx) ^^^^^^^^^^^^^^^^^ AttributeError: 'Nftables' object has no attribute 'nft_ctx_free' Signed-off-by: Thomas Haller <thaller@redhat.com> Reviewed-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* py: add pyproject.toml to support PEP-517-compatible build-systemsJeremy Sowden2025-09-013-2/+6
| | | | | | | | | | | | | | | commit 8e603e0f7eec7c0000344a004228a30fbf0ece5c upstream. This makes it possible to build and install the module without directly invoking setup.py which has been deprecated. Retain the setup.py script for backwards-compatibility. Update INSTALL to mention the new config-file. Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* py: use setup.cfg to configure setuptoolsJeremy Sowden2025-09-013-22/+27
| | | | | | | | | | | | | | | | | | | | commit 8ae4dc1f40aa04e499d941faca45fe7e914f0b4d upstream. Setuptools has had support for declarative configuration for several years. To quote their documentation: Setuptools allows using configuration files (usually setup.cfg) to define a package’s metadata and other options that are normally supplied to the setup() function (declarative config). This approach not only allows automation scenarios but also reduces boilerplate code in some cases. Additionally, this allows us to introduce support for PEP-517-compatible build-systems. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* py: move package source into src directoryJeremy Sowden2025-09-015-2/+2
| | | | | | | | | | | commit ce443afc214553b9fa6f02a640a3cd2f71a23ec9 upstream. Separate the actual package source from the build files. In addition to being a bit tidier, this will prevent setup.py being erroneously installed when we introduce PEP-517 support in a later commit. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: allow src/dstnat prios in input and outputFlorian Westphal2025-09-011-2/+4
| | | | | | | | | | | | | | | | commit 8beafab74c391130fbb9111bfccab8613644e3b9 upstream. Dan Winship says: The "dnat" command is usable from either "prerouting" or "output", but the "dstnat" priority is only usable from "prerouting". (Likewise, "snat" is usable from either "postrouting" or "input", but "srcnat" is only usable from "postrouting".) No need to restrict those priorities to pre/postrouting. Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1694 Signed-off-by: Florian Westphal <fw@strlen.de>
* py: remove setup.py integration with autotoolsPablo Neira Ayuso2025-09-014-57/+9
| | | | | | | | | | | | | | | commit b3def33efecb2f7be39fc9aefc9546907202056c upstream. With Python distutils and setuptools going deprecated, remove integration with autotools. This integration is causing issues in modern environments. Note that setup.py is still left in place under the py/ folder. Update INSTALL file to refer to Python support and setup.py. Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* update INSTALL filePablo Neira Ayuso2025-09-011-12/+29
| | | | | | | | commit 8e339bae3c9918b38bd72ddacf7765a12c1dcda9 upstream. Update it to current library dependencies and existing options. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: set SO_SNDBUF before SO_SNDBUFFORCEPablo Neira Ayuso2025-09-015-5/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 375505a4a8068bf7cb623e18c3aedb831c17fd0e upstream. Set SO_SNDBUF before SO_SNDBUFFORCE: Unpriviledged user namespace does not have CAP_NET_ADMIN on the host (user_init_ns) namespace. SO_SNDBUF always succeeds in Linux, always try SO_SNDBUFFORCE after it. Moreover, suggest the user to bump socket limits if EMSGSIZE after having see EPERM previously, when calling SO_SNDBUFFORCE. Provide a hint to the user too: # nft -f test.nft netlink: Error: Could not process rule: Message too long Please, rise /proc/sys/net/core/wmem_max on the host namespace. Hint: 4194304 bytes Dave Pfike says: Prior to this patch, nft inside a systemd-nspawn container was failing to install my ruleset (which includes a large-ish map), with the error netlink: Error: Could not process rule: Message too long strace reveals: setsockopt(3, SOL_SOCKET, SO_SNDBUFFORCE, [524288], 4) = -1 EPERM (Operation not permitted) This is despite the nspawn process supposedly having CAP_NET_ADMIN. A web search reveals at least one other user having the same issue: https://old.reddit.com/r/Proxmox/comments/scnoav/lxc_container_debian_11_nftables_geoblocking/ Reported-by: Dave Pifke <dave@pifke.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* py: replace distutils with setuptoolsJose M. Guisado Gomez2025-09-011-1/+1
| | | | | | | | | | | | | | | | | | | | commit 1acc2fd48c755a8931fa87b8d0560b750316059f upstream. Removes a deprecation warning when using distutils and python >=3.10. Python distutils module is formally marked as deprecated since python 3.10 and will be removed from the standard library from Python 3.12. (https://peps.python.org/pep-0632/) From https://setuptools.pypa.io/en/latest/setuptools.html """ Packages built and distributed using setuptools look to the user like ordinary Python packages based on the distutils. """ Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net> Signed-off-by: Florian Westphal <fw@strlen.de>
* src: Add GPLv2+ header to .c files of recent creationPablo Neira Ayuso2025-09-0118-29/+77
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 77fd4fa2827087dc00615137da78730500823259 upstream. This patch comes after a proposal of mine at NFWS 2022 that resulted in agreement to license recent .c files under GPLv2+ by the attendees at this meeting: - Stefano Brivio - Fernando F. Mancera - Phil Sutter - Jozsef Kadlecsik - Florian Westphal - Laura Garcia - Arturo Borrero - Pablo Neira It has already happened that one of the external library dependencies was moved to GPLv3+ (libreadline), resulting in a change to libedit by default in b4dded0ca78d ("configure: default to libedit for cli"). I have added the GPLv2+ header to the following files: Authors ------- src/cmd.c Pablo src/fib.c Florian src/hash.c Pablo src/iface.c Pablo src/json.c Phil + fixes from occasional contributors src/libnftables.c Eric Leblond and Phil src/mergesort.c Elise Lenion src/misspell.c Pablo src/mnl.c Pablo + fixes from occasional contributors src/monitor.c Arturo src/numgen.c Pablo src/osf.c Fernando src/owner.c Pablo src/parser_json.c Phil + fixes from occasional contributors src/print.c Phil src/xfrm.c Florian src/xt.c Pablo Eric Leblond and Elise Lennion did not attend NFWS 2022, but they acknowledged this license update already in the past when I proposed this to them in private emails. Update COPYING file too to refer that we are now moving towards GPLv2 or any later. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: netlink: netlink_delinearize_table() may return NULLPhil Sutter2025-08-172-1/+6
| | | | | | | | | commit a69d552a005ba467d37e225032e35d01d9491241 upstream. Catch the error condition in callers to avoid crashes. Fixes: c156232a530b3 ("src: add comment support when adding tables") Signed-off-by: Phil Sutter <phil@nwl.cc>
* segtree: incorrect type when aggregating concatenated set rangesPablo Neira Ayuso2025-08-131-1/+1
| | | | | | | | | | | | | | | commit 87f23fe0357da8f951faebbe2fa0b306048c2394 upstream. Uncovered by the compound_expr_remove() replacement by type safe function coming after this patch. Add expression to the concatenation which is reachable via expr_value(). This bug is subtle, I could not spot any reproducible buggy behaviour when using the wrong type when running the existing tests. Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* doc: nft.8: Minor NAT STATEMENTS section reviewPhil Sutter2025-08-131-6/+11
| | | | | | | | | | | | | | | commit 9e1cbf667da2b9c30b41ff887de212b2c38b2eb7 upstream. Synopsis insinuates an IP address argument is mandatory in snat/dnat statements although specifying ports alone is perfectly fine. Adjust it accordingly and add a paragraph briefly describing the behaviour. While at it, update the redirect statement description with more relevant examples, the current one is wrong: To *only* alter the destination port, dnat statement must be used, not redirect. Fixes: 6908a677ba04c ("nft.8: Enhance NAT documentation") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_bison: fix memory leak when parsing flowtable hook declarationFlorian Westphal2025-08-132-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 7265bf1252f66d5ca5b5dc4aa06df43f60f551a6 upstream. When the hook location is invalid we error out but we do leak both the priority expression and the flowtable name. Example: valgrind --leak-check=full nft -f flowtable-parser-err-memleak [..] Error: unknown chain hook hook enoent priority filter + 10 ^^^^^^ [..] 2 bytes in 1 blocks are definitely lost in loss record 1 of 3 at: malloc (vg_replace_malloc.c:446) by: strdup (in libc.so.6) by: xstrdup (in libnftables.so.1.1.0) by: nft_lex (in libnftables.so.1.1.0) by: nft_parse (in libnftables.so.1.1.0) by: __nft_run_cmd_from_filename (in libnftables.so.1.1.0) by: nft_run_cmd_from_filename (in libnftables.so.1.1.0) First two reports are due to the priority expression: this needs to call expr_free(). Third report is due to the flowtable name, the destructor was missing so add one. After fix: All heap blocks were freed -- no leaks are possible Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* parser_json: fix assert due to empty interface nameFlorian Westphal2025-08-132-11/+42
| | | | | | | | | | | | | | | commit 26f6ac378a49b3151a8c7e4bb0a94211b54708cc upstream. Before: nft: src/mnl.c:744: nft_dev_add: Assertion `ifname_len > 0' failed. After: internal:0:0-0: Error: empty interface name Bison checks this upfront, do same in json. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* parser_json: reject non-concat expressionFlorian Westphal2025-08-132-1/+48
| | | | | | | | | | | | | | | | commit f4d3e5e2f6595b6628b2aa948ff45ffaec40fb65 upstream. Before "src: detach set, list and concatenation expression layout": internal:0:0-0: Error: Concatenation with 0 elements is illegal After this change, expr->size access triggers assert() failure, add explicit test for etype to avoid this and error out: internal:0:0-0: Error: Expected concat element, got symbol. Fixes: e0d92243be1c ("src: detach set, list and concatenation expression layout") Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: maps: check element data mapping matches set data definitionFlorian Westphal2025-08-132-0/+54
| | | | | | | | | | | | | | | | | | | commit bc1eeb8fe709b2c0322a6b0e447517256cc9c18b upstream. This change is similar to 7f4d7fef31bd ("evaluate: check element key vs. set definition") but this time for data mappings. The included bogon asserts with: BUG: invalid data expression type catch-all set element nft: src/netlink.c:596: __netlink_gen_data: Assertion `0' failed. after: internal:0:0-0: Error: Element mapping mismatches map definition, expected packet mark, not 'invalid' Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* json: BASECHAIN flag no longer implies presence of priority expressionFlorian Westphal2025-08-132-12/+26
| | | | | | | | | | commit 715010c61ba25627b57d95d096138013e7c0e194 upstream. This is a followup to 44ea19364637 ("src: BASECHAIN flag no longer implies presence of priority expression"): feeding the same bogon file into nft -j we get a very similar crash. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: fix crash with invalid elements in setFlorian Westphal2025-08-132-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | commit 8cb7cfc2d8c7f2d8dec804ab028883c1d260e717 upstream. ctx->ectx.key can be cleared, causing a crash: src/nft --check -f tests/shell/testcases/bogons/nft-f/set_with_bad_elem AddressSanitizer:DEADLYSIGNAL #0 0x7ffb57098c0d in elem_key_compatible src/evaluate.c:1934 #1 0x7ffb5709926d in expr_evaluate_set_elem src/evaluate.c:1979 #2 0x7ffb570a540f in expr_evaluate src/evaluate.c:3159 #3 0x7ffb57095f33 in list_member_evaluate src/evaluate.c:1652 #4 0x7ffb57099f92 in expr_evaluate_set src/evaluate.c:2066 #5 0x7ffb570a53f7 in expr_evaluate src/evaluate.c:3157 .. AddressSanitizer: SEGV src/evaluate.c:1934 in elem_key_compatible After: set_with_bad_elem:4:39-46: Error: Element mismatches set definition, expected IPv4 address, not 'integer' elements = { 1.2.3.4, tcp << 8 } ^^^^^^^^ Use ctx->set->key instead. Fixes: 7f4d7fef31bd ("evaluate: check element key vs. set definition") Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: bogons: fix missing file name when loggingFlorian Westphal2025-08-131-1/+1
| | | | | | | | | | commit 85b9124868886fc1015ca3f37da5c138123819a4 upstream. When the json is parsed without returning an error the test fails. Its supposed to log the name of the failed input which it does for -f but not for -j -f. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: check element key vs. set definitionFlorian Westphal2025-08-132-7/+30
| | | | | | | | | | | | | | | | | commit 7f4d7fef31bd282b8e37d6d401208535a1e74d17 upstream. Included bogon asserts with: src/datatype.c:253: symbolic_constant_print: Assertion `expr->len / BITS_PER_BYTE <= sizeof(val)' failed. Resolve this by validating that the set element key matches the set key definition. After this, loading the bogon file gives: Error: Element mismatches set definition, expected concatenation of (IPv4 address, integer), not 'ICMP type' elements = {redirect } ^^^^^^^^ Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: monitor: enclose device names in quotesPablo Neira Ayuso2025-08-131-1/+1
| | | | | | | | | | | | | | commit 26746952952bba8c19aebbd03a55decbc0d0c5fc upstream. Update test to enclose flowtable device names in quotes, otherwise, it reports a spurious issue: @@ -1,2 +1,3 @@ add table ip t -add flowtable ip t ft { hook ingress priority 0; devices = { lo }; } +add flowtable ip t ft { hook ingress priority 0; devices = { "lo" }; } Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: validate set expression type before accessing flagsPablo Neira Ayuso2025-08-131-1/+2
| | | | | | | | | | commit 2022e8bb5cf0e0fa81ab0a5087bd1ab6e20280ee upstream. Validate set->init is of EXPR_SET expression type before accessing set_flags. Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: print chain and flowtable devices in quotesPablo Neira Ayuso2025-08-1312-21/+21
| | | | | | | | | | | | | | | | | | | commit eb30f236d91a8d61ece789e28e6540b3a3fa2a6a upstream. Print devices in quotes, for consistency with: - the existing chain listing with single device: type filter hook ingress device "lo" priority filter; policy accept - the ifname datatype used in sets. In general, tokens that are user-defined, not coming in the datatype symbol list, are enclosed in quotes. Fixes: 3fdc7541fba0 ("src: add multidevice support for netdev chain") Acked-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: prevent merge of sets with incompatible keysFlorian Westphal2025-08-133-1/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | commit c9d6f089f0eb2cb615cbca3e4c99b07c5639960f upstream. Its not enough to check for interval flag, this would assert in interval code due to concat being passed to the interval code: BUG: unhandled key type 13 After fix: same_set_name_but_different_keys_assert:8:6-7: Error: set already exists with different datatype (concatenation of (IPv4 address, network interface index) vs network interface index) set s4 { ^^ This also improves error verbosity when mixing datamap and objref maps: invalid_transcation_merge_map_and_objref_map:9:13-13: Error: map already exists with different datatype (IPv4 address vs string) .. instead of 'Cannot merge map with incompatible existing map of same name'. The 'Cannot merge map with incompatible existing map of same name' check is kept in place to catch when ruleset contains a set and map with same name and same key definition. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: check that set type is identical before mergingFlorian Westphal2025-08-133-2/+52
| | | | | | | | | | | | | | | | | | | | | | | | | commit 5335452966c4e5da2f3a5cf617cf431d711b215e upstream. Reject maps and sets of the same name: BUG: invalid range expression type catch-all set element nft: src/expression.c:1704: range_expr_value_low: Assertion `0' failed. After: Error: Cannot merge set with existing datamap of same name set z { ^ v2: Pablo points out that we shouldn't merge datamaps (plain value) and objref maps either, catch this too and add another test: nft --check -f invalid_transcation_merge_map_and_objref_map invalid_transcation_merge_map_and_objref_map:9:13-13: Error: Cannot merge map with incompatible existing map of same name We should also make sure that both data (for map case) and set keys are identical, this is added in a followup patch. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: avoid double-free on error handling of bogus objref mapsFlorian Westphal2025-08-132-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit ab1139f807f0d0519a25704e75c442ccb71f7a60 upstream. commit 98c51aaac42b ("evaluate: bail out if anonymous concat set defines a non concat expression") clears set->init to avoid a double-free. Extend this to also handle object maps. The included bogon triggers a double-free of set->init expression: Error: unqualified type invalid specified in map definition. Try "typeof expression" instead of "type datatype". ct helper set ct saddr map { 1c3:: : "p", dead::beef : "myftp" } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This might not crash, depending on libc/malloc, but ASAN reports this: ==17728==ERROR: AddressSanitizer: heap-use-after-free on address 0x50b0000005e8 at .. READ of size 4 at 0x50b0000005e8 thread T0 #0 0x7f1be3cb7526 in expr_free src/expression.c:87 #1 0x7f1be3cbdf29 in map_expr_destroy src/expression.c:1488 #2 0x7f1be3cb74d5 in expr_destroy src/expression.c:80 #3 0x7f1be3cb75c6 in expr_free src/expression.c:96 #4 0x7f1be3d5925e in objref_stmt_destroy src/statement.c:331 #5 0x7f1be3d5831f in stmt_free src/statement.c:56 #6 0x7f1be3d583c2 in stmt_list_free src/statement.c:66 #7 0x7f1be3d42805 in rule_free src/rule.c:495 #8 0x7f1be3d48329 in cmd_free src/rule.c:1417 #9 0x7f1be3cd2c7c in __nft_run_cmd_from_filename src/libnftables.c:759 #10 0x7f1be3cd340c in nft_run_cmd_from_filename src/libnftables.c:847 #11 0x55dcde0440be in main src/main.c:535 Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: make sure chain jump name comes with a null byteFlorian Westphal2025-08-132-5/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit ca0c49d1bdb944534851c3dcb4c8ce16f1675074 upstream. There is a stack oob read access in netlink_gen_chain(): mpz_export_data(chain, expr->chain->value, BYTEORDER_HOST_ENDIAN, len); snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain); There is no guarantee that chain[] is null terminated, so snprintf can read past chain[] array. ASAN report is: AddressSanitizer: stack-buffer-overflow on address 0x7ffff5f00520 at .. READ of size 257 at 0x7ffff5f00520 thread T0 #0 0x00000032ffb6 in printf_common(void*, char const*, __va_list_tag*) (src/nft+0x32ffb6) #1 0x00000033055d in vsnprintf (src/nft+0x33055d) #2 0x000000332071 in snprintf (src/nft+0x332071) #3 0x0000004eef03 in netlink_gen_chain src/netlink.c:454:2 #4 0x0000004eef03 in netlink_gen_verdict src/netlink.c:467:4 Reject chain jumps that exceed 255 characters, which matches the netlink policy on the kernel side. The included reproducer fails without asan too because the kernel will reject the too-long chain name. But that happens after the asan detected bogus read. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* json: reject too long interface namesFlorian Westphal2025-08-132-2/+105
| | | | | | | | | | | | | | | | | | | | | | | | commit bed99830c4c63eae205c28a7ff914737bedb199d upstream. Blamed commit added a length check on ifnames to the bison parser. Unfortunately that wasn't enough, json parser has the same issue. Bogon results in: BUG: Interface length 44 exceeds limit nft: src/mnl.c:742: nft_dev_add: Assertion `0' failed. After patch, included bogon results in: Error: Invalid device at index 0. name d2345678999999999999999999999999999999012345 too long I intentionally did not extend evaluate.c to catch this, past sentiment was that frontends should not send garbage. I'll send a followup patch to also catch this from eval stage in case there are further reports for frontends passing in such long names. Fixes: fa52bc225806 ("parser: reject zero-length interface names") Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
* parser_bison: allow delete command with map via handlePablo Neira Ayuso2025-08-133-1/+6
| | | | | | | | | commit 640312b1529c548790117635c91886a6c83e83f2 upstream. For consistency with sets, allow delete via handle for maps too. Fixes: f4a34d25f6d5 ("src: list set handle and delete set via set handle") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* cache: assert name is non-nul when looking upPablo Neira Ayuso2025-08-131-2/+9
| | | | | | | | | | commit f15bc7d368b7c1d897fd830f91e7db6929175b27 upstream. {table,chain,set,obj,flowtable}_cache_find() should not be called when handles are used Fixes: 5ec5c706d993 ("cache: add hashtable cache for table") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: skip fuzzy lookup if object name is not availablePablo Neira Ayuso2025-08-131-0/+12
| | | | | | | | | | | | | | commit de8396358f869d6d7640eae6d6287c2f7fb0d3dc upstream. Skip fuzzy lookup for suggestions when handles are used. Note that 4cf97abfee61 ("rule: Avoid segfault with anonymous chains") already skips it for chain. Fixes: 285bb67a11ad ("src: introduce simple hints on incorrect set") Fixes: 9f7817a4e022 ("src: introduce simple hints on incorrect chain") Fixes: d7476ddd5f7d ("src: introduce simple hints on incorrect table") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: BASECHAIN flag no longer implies presence of priority expressionFlorian Westphal2025-08-132-6/+16
| | | | | | | | | | | | | commit 44ea1936463728475768861073ca4ba34a5c2f75 upstream. The included bogon will crash nft because print side assumes that BASECHAIN flag presence also means that priority expression is available. Make the print side conditional. Fixes: a66b5ad9540d ("src: allow for updating devices on existing netdev chain") Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: restrict allowed subtypes of concatenationsFlorian Westphal2025-08-133-1/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 3cec07327ea2b91ac8395e0c0ee2a635a5e9fcd5 upstream. We need to restrict this, included bogon asserts with: BUG: unknown expression type prefix nft: src/netlink_linearize.c:940: netlink_gen_expr: Assertion `0' failed. Prefix expressions are only allowed if the concatenation is used within a set element, not when specifying the lookup key. For the former, anything that represents a value is allowed. For the latter, only what will generate data (fill a register) is permitted. At this time we do not have an annotation that tells if the expression is on the left hand side (lookup key) or right hand side (set element). Add a new list recursion counter for this. If its 0 then we're building the lookup key, if its the latter the concatenation is the RHS part of a relational expression and prefix, ranges and so on are allowed. IOW, we don't really need a recursion counter, another type of annotation that would tell if the expression is placed on the left or right hand side of another expression would work too. v2: explicitly list all 'illegal' expression types instead of using a default label for them. This will raise a compiler warning to remind us to adjust the case labels in case a new expression type gets added in the future. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: rename recursion counter to recursion.binopFlorian Westphal2025-08-132-7/+11
| | | | | | | | | | | | | | | | | | | | | | | | commit 10b44319a53a131ed943e2b6eeb62d197178bf4d upstream. The existing recursion counter is used by the binop expression to detect if we've completely followed all the binops. We can only chain up to NFT_MAX_EXPR_RECURSION binops, but the evaluation step can perform constant-folding, so we must recurse until we found the rightmost (last) binop in the chain. Then we can check the post-eval chain to see if it is something that can be serialized later (i.e., if we are within the NFT_MAX_EXPR_RECURSION after constant folding) or not. Thus we can't reuse the existing ctx->recursion counter for other expressions; entering the initial expr_evaluate_binop with ctx->recursion > 0 would break things. Therefore rename this to an embedded structure. This allows us to add a new recursion counter in a followup patch. Signed-off-by: Florian Westphal <fw@strlen.de>
* test: shell: Don't use system nft binaryYi Chen2025-08-133-4/+4
| | | | | | | | | | | commit c73eadca05c781ebad631331a6864fa8c54a5024 upstream. Use the defined $NFT variable instead of calling the system nft binary directly. Add a nat_ftp.nodump file to avoid the following check-tree.sh error: ERR: "tests/shell/testcases/packetpath/nat_ftp" has no "tests/shell/testcases/packetpath/dumps/nat_ftp.{nft,nodump}" file. Signed-off-by: Yi Chen <yiche@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: don't BUG on unexpected base datatypeFlorian Westphal2025-08-132-1/+13
| | | | | | | | | commit 845b8d7208077310e77560a64b698973fb047ef2 upstream. Included bogon will cause a crash but this is the evaluation stage where we can just emit an error instead. Signed-off-by: Florian Westphal <fw@strlen.de>