summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* libxtables: Attenuate effects of functions' internal static buffersHEADmasterPhil Sutter2024-04-101-11/+9
| | | | | | | | | | | | | | | | | | | | While functions returning pointers to internal static buffers have obvious limitations, users are likely unaware how they call each other internally and thus won't notice unsafe use. One such case is calling both xtables_ipaddr_to_numeric() and xtables_ipmask_to_numeric() as parameters for a single printf() call. Defuse this trap by avoiding the internal calls to xtables_ip{,6}addr_to_numeric() which is easily doable since callers keep their own static buffers already. While being at it, make use of inet_ntop() everywhere and also use INET_ADDRSTRLEN/INET6_ADDRSTRLEN defines for correct (and annotated) static buffer sizes. Reported-by: Vitaly Chikunov <vt@altlinux.org> Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Vitaly Chikunov <vt@altlinux.org>
* xshared: Fix parsing of empty string arg in '-c' optionPhil Sutter2024-04-102-1/+6
| | | | | | | | | | | | | | Calling iptables with '-c ""' resulted in a call to strchr() with an invalid pointer as 'optarg + 1' points to past the buffer. The most simple fix is to drop the offset: The global optstring part specifies a single colon after 'c', so getopt() enforces a valid pointer in optarg. If it contains a comma at first position, packet counter value parsing will fail so all cases are covered. Reported-by: gorbanev.es@gmail.com Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1741 Fixes: 60a6073690a45 ("Make --set-counters (-c) accept comma separated counters") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xlate: libip6t_mh: Fix and simplify plain '-m mh' matchPhil Sutter2024-04-092-4/+2
| | | | | | | | | | Since core xlate code now ignores '-p mh' if an mh extension is also present in the rule, mh extension has to emit the l4proto match itself. Therefore emit the exthdr match irrespective of '-p' argument value just like other IPv6 extension header matches do. Fixes: 83f60fb37d594 ("extensions: mh: Save/xlate inverted full ranges") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xlate: Improve redundant l4proto match avoidancePhil Sutter2024-04-095-28/+42
| | | | | | | | | | | | | | | xtables-translate tries to avoid 'ip protocol'/'meta l4proto' matches if following expressions add this as dependency anyway. E.g.: | # iptables-translate -A FOO -p tcp -m tcp --dport 22 -j ACCEPT | nft 'add rule ip filter FOO tcp dport 22 counter accept' This worked by searching protocol name in loaded matches, but that approach is flawed as the protocol name and corresponding extension may differ ("mobility-header" vs. "mh"). Improve this by searching for all names (cached or resolved) for a given protocol number. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: Do not combine inverted payload matchesSriram Rajagopalan2024-03-191-4/+2
| | | | | | | | Fixed the issue with combining the payload in case of invert filter for tcp src and dst ports. Signed-off-by: Sriram Rajagopalan <sriramr@arista.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: xt_TPROXY: add txlate supportFlorian Westphal2024-03-092-0/+79
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: xt_socket: add txlate support for socket matchFlorian Westphal2024-03-062-0/+51
| | | | | | | | | | | | | | | | | | | v2: document the match semantics of -m socket. Ignore --nowildcard if used with other options when translating and add "wildcard 0" if the option is missing. "-m socket" will ignore sockets bound to 0.0.0.0/:: by default, unless --nowildcard is given. So, xlate must always append "wildcard 0", can elide "wildcard" if other options are present along with --nowildcard. To emulate "-m socket --nowildcard", check for "wildcard <= 1" to get a "socket exists" type matching. Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Phil Sutter <phil@nwl.cc>
* xtables-translate: Leverage stored protocol namesPhil Sutter2024-02-295-30/+53
| | | | | | | | | | | Align output of ip(6)tables-translate for --protocol arguments with that of ip(6)tables -L/-S by calling proto_to_name() from xshared.c. The latter will consult xtables_chain_protos list first to make sure (the right) names are used for "common" protocol values and otherwise falls back to getprotobynumber() which it replaces here. Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1738 Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: Fix for broken recover_rule_compat()Phil Sutter2024-02-272-4/+35
| | | | | | | | | | | | | | | When IPv4 rule generator was changed to emit payload instead of meta expressions for l4proto matches, the code reinserting NFTNL_RULE_COMPAT_* attributes into rules being reused for counter zeroing was broken by accident. Make rule compat recovery aware of the alternative match, basically reinstating the effect of commit 7a373f6683afb ("nft: Fix -Z for rules with NFTA_RULE_COMPAT") but add a test case this time to make sure things stay intact. Fixes: 69278f9602b43 ("nft: use payload matching for layer 4 protocol") Signed-off-by: Phil Sutter <phil@nwl.cc>
* iptables-save: Avoid /etc/protocols lookupsPhil Sutter2024-02-072-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | Instrument proto_to_name() to abort if given protocol number is not among the well-known ones in xtables_chain_protos. Along with xtables_parse_protocol() preferring said array for lookups as well, this ensures reliable dump'n'restore regardless of /etc/protocols contents. Another benefit is rule dump performance. A simple test-case dumping 100k rules matching on dccp protocol shows an 8s delta (2s vs. 10s for legacy, 0.5s vs. 8s for nft) with this patch applied. For reference: | for variant in nft legacy; do | ( | echo "*filter" | for ((i = 0; i < 100000; i++)); do | echo "-A FORWARD -p dccp -j ACCEPT" | done | echo "COMMIT" | ) | iptables-${variant}-restore | time iptables-${variant}-save | wc -l | iptables-${variant} -F | done Signed-off-by: Phil Sutter <phil@nwl.cc>
* libxtables: Add dccp and ipcomp to xtables_chain_protosPhil Sutter2024-02-071-0/+2
| | | | | | | | | | | There are "protocol extensions" for both just like with TCP or UDP. Caching their values allows for implicit extension lookup after '-p' flag, for instance: | iptables -A FORWARD -p dccp --dport 1 | iptables -A FORWARD -p ipcomp --ipcompspi 18 Signed-off-by: Phil Sutter <phil@nwl.cc>
* Revert "xshared: Print protocol numbers if --numeric was given"Phil Sutter2024-02-074-11/+11
| | | | | | | | | | | | | | This reverts commit da8ecc62dd765b15df84c3aa6b83dcb7a81d4ffa. The patch's original intention is not entirely clear anymore. If it was to reduce delays involved by calling getprotobynumber() though, commit b6196c7504d4d ("xshared: Prefer xtables_chain_protos lookup over getprotoent") avoids those if --numeric flag was given already. Also, this numeric protocol output did not cover iptables-save which is a more relevant candidate for such optimizations anyway. Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1729 Signed-off-by: Phil Sutter <phil@nwl.cc>
* libxtables: xtoptions: Respect min/max values when completing rangesPhil Sutter2024-02-022-5/+8
| | | | | | | | If an extension defines a minimum/maximum valid value for an option's range argument, treat this as the lower/upper boundary to use when completing (half) open ranges. Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: tcp/udp: Save/xlate inverted full rangesPhil Sutter2024-02-026-43/+64
| | | | | | | | Also translate a bare '-m tcp/udp' to 'meta l4proto' match. Fixes: 04f569ded54a7 ("extensions: libxt_udp: add translation to nft") Fixes: fb2593ebbf656 ("extensions: libxt_tcp: add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: Do not omit full ranges if invertedPhil Sutter2024-02-023-6/+6
| | | | | | | Otherwise this turns a never matching rule into an always matching one. Fixes: c034cf31dd1a9 ("nft: prefer native expressions instead of udp match") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: ipcomp: Save inverted full rangesPhil Sutter2024-02-022-4/+5
| | | | | Fixes: 0bb8765cc28cf ("iptables: Add IPv4/6 IPcomp match support") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: esp: Save/xlate inverted full rangesPhil Sutter2024-02-023-13/+23
| | | | | | | | | Also add a translation for plain '-m esp' match which depends on the address family: While ip6tables-translate may emit an exthdr exists match, iptables-translate must stick to meta l4proto. Fixes: 6cfa723a83d45 ("extensions: libxt_esp: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: rt: Save/xlate inverted full rangesPhil Sutter2024-02-023-11/+23
| | | | | | | Also translate plain '-m rt' match into an exthdr exists one. Fixes: 9dbb616c2f0c3 ("extensions: libip6t_rt.c: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: mh: Save/xlate inverted full rangesPhil Sutter2024-02-023-7/+19
| | | | | | | | | Also translate '-m mh' into an exthdr exists match unless '-p mh' is also present. The latter is converted into 'meta l4proto mh' which might need fixing itself at a later point. Fixes: 6d4b93485055a ("extensions: libip6t_mh: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: frag: Save/xlate inverted full rangesPhil Sutter2024-02-023-12/+21
| | | | | | | Also translate plain '-m frag' match into an exthdr exists one. Fixes: bd5bbc7a0fbd8 ("extensions: libip6t_frag: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: ah: Save/xlate inverted full rangesPhil Sutter2024-02-026-23/+33
| | | | | | | | | | | While at it, fix xlate output for plain '-m ah' matches: With ip6tables-translate, one should emit an extdhr exists match since ip6t_ah.c in kernel also uses ipv6_find_hdr(). With iptables-translate, a simple 'meta l4proto ah' was missing. Fixes: bb498c8ba7bb3 ("extensions: libip6t_ah: Fix translation of plain '-m ah'") Fixes: b9a46ee406165 ("extensions: libipt_ah: Add translation to nft") Signed-off-by: Phil Sutter <phil@nwl.cc>
* libxtables: Reject negative port rangesPhil Sutter2024-02-024-9/+14
| | | | | | | Analogous to XTTYPE_UINT*RC value parsing, assert consecutive port values are not lower than previous ones. Signed-off-by: Phil Sutter <phil@nwl.cc>
* libxtables: xtoptions: Assert ranges are monotonic increasingPhil Sutter2024-02-0211-28/+22
| | | | | | | | | | | Extensions commonly require the upper range value to be larger or equal to the lower one. Performing this check in the parser is easier and covers all extensions at once. One notable exception is NFQUEUE which requires strict monotonicity. Hence leave its checks in place. Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: *.t/*.txlate: Test range corner-casesPhil Sutter2024-02-0226-0/+253
| | | | | | | | | | | For every extension option accepting a range, test open and half-open as well as single element and invalid (negative) ranges. The added tests merely reflect the status quo, not the expected outcome. Following patches will fix results and the already existing test cases highlight the fixes' effects. Signed-off-by: Phil Sutter <phil@nwl.cc>
* ebtables: Fix for memleak with change counters commandPhil Sutter2024-02-011-0/+1
| | | | | | | | | | Just like with check command, change counters command creates a temporary rule from rulespec on command line for a search by spec in rule cache. It is not used anymore afterwards, so nft_cmd_free() should free it. Fixes: f340b7b6816be ("ebtables: Implement --change-counters command") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xshared: Introduce xtables_clear_args()Phil Sutter2024-02-016-23/+14
| | | | | | | | | | | Perform struct xtables_args object deinit in a common place, even though it merely consists of freeing any IP addresses and masks. This fixes for a memleak in arptables-translate as the check for h->family didn't catch the value NFPROTO_ARP. Fixes: 5b7324e0675e3 ("nft-arp: add arptables-translate") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xshared: Fix for memleak in option merging with ebtablesPhil Sutter2024-02-013-19/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The crucial difference in ebtables is that all extensions are loaded up front instead of while parsing -m/-j flags. Since this loading of all extensions before every call to do_parse() is pointless overhead (cf. ebtables-restore), other tools' mechanism of freeing all merged options in xtables_free_opts() after handling each command and resetting xt_params->opts at the start of the parser loop is problematic. Fixed commit entailed a hack to defeat the xt_params->opts happening at start of do_parse() by assigning to xt_params->orig_opts after loading all extensions. This approach caused a memleak though since xtables_free_opts() called from xtables_merge_options() will free the opts pointer only if it differs from orig_opts. Resolve this via a different approach which eliminates the xt_params->opts reset at the start of do_parse(): Make xt_params->opts be NULL until the first extension is loaded. Option merging in command_match() and command_jump() tolerates a NULL pointer there after minimal adjustment. Deinit in xtables_free_opts() is already fine as it (re)turns xt_params->opts to a NULL pointer. With do_parse() expecting that and falling back to xt_params->orig_opts, no explicit initialization is required anymore and thus ebtables' init is not mangled by accident. A critical part is that do_parse() checks xt_params->opts pointer upon each call to getopt_long() as it may get assigned while parsing. Fixes: 58d364c7120b5 ("ebtables: Use do_parse() from xshared") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables-eb: Eliminate 'opts' definePhil Sutter2024-02-011-3/+7
| | | | | | | | | | | It is more harm than good as it hides assignments to xt_params->opts field and does funny things if statements actually use xt_params->opts instead of the define. Replace it by local variables where sensible (cf. command_match() and command_jump() in xshared.c). Signed-off-by: Phil Sutter <phil@nwl.cc>
* libxtables: Fix memleak of matches' udataPhil Sutter2024-02-011-0/+4
| | | | | | | | | | If the extension specifies a non-zero udata_size, field 'udata' points to an allocated buffer which needs to be freed upon extension deinit. Interestingly, this bug was identified by ASAN and missed by valgrind. Fixes: 2dba676b68ef8 ("extensions: support for per-extension instance "global" variable space") Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: ruleparse: Add missing braces around ternaryPhil Sutter2024-02-011-1/+1
| | | | | | | | | | | | | | | | The expression evaluated the sum before the ternay, consequently not adding target->size if tgsize was zero. Identified by ASAN for a simple rule using standard target: | # ebtables -A INPUT -s de:ad:be:ef:0:00 -j RETURN | # ebtables -D INPUT -s de:ad:be:ef:0:00 -j RETURN | ================================================================= | ==18925==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000000120 at pc 0x7f627a4c75c5 bp 0x7ffe882b5180 sp 0x7ffe882b4928 | READ of size 8 at 0x603000000120 thread T0 | [...] Fixes: 2a6eee89083c8 ("nft-ruleparse: Introduce nft_create_target()") Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: iptables-test: Increase non-fast mode strictnessPhil Sutter2024-02-013-3/+7
| | | | | | | | | | | | | | | | | | | | | | | | The simple search for the rule in save output accepted arbitrary leading and trailing rule parts. This was partly desired as it allowed to omit the leading '-A' flag or ignore the mandatory '-j CONTINUE' in ebtables rules, though it could hide bugs. Introduction of fast mode mitigated this due to the way how it searches for multiple rules at the same time, but there are cases which fast mode does not support yet (e.g. test cases containing variant-specific rule output). Given save output format will never contain the rule in first or last line, so enclosing the searched rule in newline characters is sufficient to make the search apply to full lines only. The only drawback is having to add '-A' and '-j CONTINUE' parts if needed. The hidden bugs this revealed were: - Long --nflog-prefix strings are not cut to 64 chars with iptables-nft - The TCPMSS rule supposed to fail with legacy only must specify an expected save output Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: libebt_stp: fix range checkingFlorian Westphal2024-01-251-1/+1
| | | | | | | | | | | | | | This has to either consider ->nvals > 1 or check the values post-no-range-fixup: ./iptables-test.py extensions/libebt_stp.t extensions/libebt_stp.t: ERROR: line 12 (cannot load: ebtables -A INPUT --stp-root-cost 1) (it tests 0 < 1 and fails, but test should be 1 < 1). Fixes: dc6efcfeac38 ("extensions: libebt_stp: Use guided option parser") Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
* iptables: Add missing error codesJacek Tomasiak2024-01-241-3/+9
| | | | | | | | | | Without these, commands like `iptables -n -L CHAIN` sometimes print "Incompatible with this kernel" instead of "No chain/target/match by that name". Signed-off-by: Jacek Tomasiak <jacek.tomasiak@gmail.com> Signed-off-by: Jacek Tomasiak <jtomasiak@arista.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
* ebtables: Default to extrapositioned negationsPhil Sutter2024-01-1027-236/+241
| | | | | | | | | | | | | | | ebtables-nft has always supported both intra- and extrapositioned negations but defaulted to intrapositioned when printing/saving rules. With commit 58d364c7120b5 ("ebtables: Use do_parse() from xshared") though, it started to warn about intrapositioned negations. So change the default to avoid mandatory warnings when e.g. loading previously dumped rulesets. Also adjust test cases, help texts and ebtables-nft.8 accordingly. Cc: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
* extensions: libxt_HMARK: Review HMARK_parse()Phil Sutter2024-01-101-51/+9
| | | | | | | | * With XTOPT_NBO support in UINT types, the manual byteorder conversion calls are no longer needed * Setting bits in cb->xflags is done by xtables_option_parse() already * Since O_HMARK_* values match XT_HMARK_* ones, all but the O_HMARK_TYPE case fold together into a single default one
* extensions: libebt_mark_m: Use guided option parserPhil Sutter2024-01-101-36/+22
| | | | | | | Can't use XTTYPE_MARKMASK32 here because in 'val/mask', 'val' is optional. Would have to extend xtopt_parse_markmask() to accept this, maybe guarded by a new XTOPT_ flag to avoid unexpected changes in behaviour?
* extensions: libebt_pkttype: Use guided option parserPhil Sutter2024-01-101-22/+23
| | | | | Not much to gain here. Maybe implement number parsing with fallback to get rid of that part from extension parsers?
* extensions: libxt_limit: Use guided option parser for NFPROTO_BRIDGE, tooPhil Sutter2024-01-101-48/+2
|
* extensions: libebt_arp: Use guided option parserPhil Sutter2024-01-102-130/+78
|
* extensions: libebt_vlan: Use guided option parserPhil Sutter2024-01-101-68/+34
|
* extensions: libebt_802_3: Use guided option parserPhil Sutter2024-01-102-55/+21
|
* extensions: libebt_redirect: Use guided option parserPhil Sutter2024-01-101-23/+17
|
* extensions: libebt_snat: Use guided option parserPhil Sutter2024-01-102-42/+34
|
* extensions: libebt_nflog: Use guided option parserPhil Sutter2024-01-101-64/+18
|
* extensions: libebt_mark: Use guided option parserPhil Sutter2024-01-101-86/+54
|
* extensions: libebt_log: Use guided option parserPhil Sutter2024-01-101-91/+30
|
* extensions: libebt_ip: Use guided option parserPhil Sutter2024-01-102-123/+84
|
* extensions: libebt_ip6: Use guided option parserPhil Sutter2024-01-102-124/+83
|
* extensions: libebt_dnat: Use guided option parserPhil Sutter2024-01-101-37/+27
|
* extensions: libebt_arpreply: Use guided option parserPhil Sutter2024-01-102-34/+22
|