Phil Sutter [Fri, 28 Apr 2023 12:41:08 +0000 (14:41 +0200)]
xshared: Fix parsing of option arguments in same word
When merging commandline parsers, a decision between 'argv[optind - 1]'
and 'optarg' had to be made in some spots. While the implementation of
check_inverse() required the former, use of the latter allows for the
common syntax of '--opt=arg' or even '-oarg' as 'optarg' will point at
the suffix while 'argv[optind - 1]' will just point at the following
option.
Fix the mess by making check_inverse() update optarg pointer if needed
so calling code may refer to and always correct 'optarg'.
Fixes: 0af80a91b0a98 ("nft: Merge xtables-arp-standalone.c into xtables-standalone.c") Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1677 Signed-off-by: Phil Sutter <phil@nwl.cc>
The function is deprecated. Eliminate the warning by use of
pcap_open_dead(), pcap_compile() and pcap_close() just how
pcap_compile_nopcap() is implemented internally in libpcap.
Phil Sutter [Wed, 5 Apr 2023 11:18:24 +0000 (13:18 +0200)]
tests: shell: Test for false-positive rule check
Rule comparison in legacy ip6tables was broken by commit eb2546a846776
("xshared: Share make_delete_mask() between ip{,6}tables"): A part of
the rules' data was masked out for comparison by accident.
Use new 'meta broute set 1' to emulate -t broute. If '-t broute' is given,
automatically translate -j DROP to 'meta broute set 1 accept' internally.
Reverse translation zaps the broute and pretends verdict was DROP.
Note that BROUTING is internally handled via PREROUTING, i.e. 'redirect'
and 'nat' targets are not available, they will need to be emulated via
nft expressions.
Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Alyssa Ross [Sun, 2 Apr 2023 23:29:40 +0000 (23:29 +0000)]
build: use pkg-config for libpcap
If building statically, with libpcap built with libnl support, linking
will fail, as the compiler won't be able to find the libnl symbols
since static libraries don't contain dependency information. To fix
this, use pkg-config to find the flags for linking libpcap, since the
pkg-config files contain the neccesary dependency information.
autoconf will add code to the configure script for initializing
pkg-config the first time it seems PKG_CHECK_MODULES, so make the
libnfnetlink check the first one in the script, so the initialization
code is run unconditionally.
Signed-off-by: Alyssa Ross <hi@alyssa.is> Signed-off-by: Phil Sutter <phil@nwl.cc>
Markus Boehme [Mon, 3 Apr 2023 21:13:47 +0000 (23:13 +0200)]
ip6tables: Fix checking existence of rule
Pass the proper entry size when creating a match mask for checking the
existence of a rule. Failing to do so causes wrong results.
Reported-by: Jonathan Caicedo <jonathan@jcaicedo.com> Fixes: eb2546a846776 ("xshared: Share make_delete_mask() between ip{,6}tables") Signed-off-by: Markus Boehme <markubo@amazon.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
In most distros 'python' means python2, which is not available anywhere.
This is a problem when, for example, building the Debian package. This
script is called as part of the build but 'python' is not available.
Mention python3 explictly. The script runs just fine in python3.
Phil Sutter [Tue, 28 Feb 2023 17:09:25 +0000 (18:09 +0100)]
nft-restore: Fix for deletion of new, referenced rule
Combining multiple corner-cases here:
* Insert a rule before another new one which is not the first. Triggers
NFTNL_RULE_ID assignment of the latter.
* Delete the referenced new rule in the same batch again. Causes
overwriting of the previously assigned RULE_ID.
Consequently, iptables-nft-restore fails during *insert*, because the
reference is dangling.
Reported-by: Eric Garver <eric@garver.life> Fixes: 760b35b46e4cc ("nft: Fix for add and delete of same rule in single batch") Signed-off-by: Phil Sutter <phil@nwl.cc> Tested-by: Eric Garver <eric@garver.life>
Phil Sutter [Wed, 22 Feb 2023 15:36:16 +0000 (16:36 +0100)]
include: Add missing linux/netfilter/xt_LOG.h
When merging IP-version-specific LOG extensions, a dependency to that
header was introduced without caching it. Fix this and drop the now
unused ip{,6}t_LOG.h files.
Reported-by: Thomas Devoogdt <thomas@devoogdt.com> Fixes: 87e4f1bf0b87b ("extensions: libip*t_LOG: Merge extensions") Signed-off-by: Phil Sutter <phil@nwl.cc>
Xin Long [Tue, 21 Feb 2023 17:19:42 +0000 (12:19 -0500)]
xt_sctp: add the missing chunk types in sctp_help
Add the missing chunk types in sctp_help(), so that the help cmd can
display these chunk types as below:
# iptables -p sctp --help
chunktypes - ... I_DATA RE_CONFIG PAD ... I_FORWARD_TSN ALL NONE
Fixes: 6b04d9c34e25 ("xt_sctp: support a couple of new chunk types") Signed-off-by: Xin Long <lucien.xin@gmail.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 3 Feb 2023 17:25:21 +0000 (18:25 +0100)]
xtables-translate: Support insert with index
Translation is pretty simple due to nft's 'insert rule ... index'
support. Testing the translation is sadly not: index 1 vanishes (as it
should), higher indexes are rejected in replay mode since no rules
previously exist.
Phil Sutter [Fri, 3 Feb 2023 16:37:40 +0000 (17:37 +0100)]
extensions: libebt_ip: Translation has to match on ether type
On one hand, nft refuses th expression in bridge family if layer3
protocol has not been assured by a previous match. On the other, ebt_ip
kernel module will only match on IPv4 packets, so there might be a
functional change in the translation versus the original.
Instead of just always emitting an 'ether type' match, decide whether
it's actually needed - explicit "ip <something>" payload matches (or
icmp ones) cause implicit creation of a match on IPv4 by nft.
Fixes: 03ecffe6c2cc0 ("ebtables-compat: add initial translations") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 3 Feb 2023 17:58:36 +0000 (18:58 +0100)]
extensions: libebt_ip: Do not use 'ip dscp' for translation
Converting from TOS field match to DSCP one is irreversible, so replay
testing is not possible. Use a raw payload expression to produce
something that translates 1:1 back into an 'ip' match.
Fixes: 03ecffe6c2cc0 ("ebtables-compat: add initial translations") Signed-off-by: Phil Sutter <phil@nwl.cc>
While EBT_ACCEPT is the default verdict for ebtables targets, omitting
it from translation implicitly converts it into 'continue'. Omit the
non-default EBT_CONTINUE instead.
Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 16 Feb 2023 21:24:16 +0000 (22:24 +0100)]
nft-shared: Simplify using nft_create_match()
Perform the nft_family_ops::parse_match call from inside
nft_create_match(). It frees callers from having to access the match
itself.
Then return a pointer to match data instead of the match itself.
Phil Sutter [Thu, 16 Feb 2023 20:55:54 +0000 (21:55 +0100)]
nft-shared: Lookup matches in iptables_command_state
Some matches may turn into multiple nft statements (naturally or via
translation). Such statements must parse into a single extension again
in order to rebuild the rule as it was.
Introduce nft_find_match_in_cs() to iterate through the lists and drop
tcp/udp port match caching in struct nft_xt_ctx which is not needed
anymore.
Note: Match reuse is not enabled unconditionally for all matches,
because iptables supports having multiple instances of the same
extension.
Phil Sutter [Wed, 25 Jan 2023 01:01:56 +0000 (02:01 +0100)]
ebtables: Refuse unselected targets' options
Unlike legacy, ebtables-nft would allow e.g.:
| -t nat -A PREROUTING --to-dst fe:ed:00:00:ba:be
While the result is correct, it may mislead users into believing
multiple targets are possible per rule. Better follow legacy's behaviour
and reject target options unless they have been "enabled" by a previous
'-j' option.
To achieve this, one needs to distinguish targets from watchers also
attached to 'xtables_targets' and otherwise behaving like regular
matches. Introduce XTABLES_EXT_WATCHER to mark the two.
The above works already, but error messages are misleading when using
the now unsupported syntax since target options have been merged
already. Solve this by not pre-loading the targets at all, code will
just fall back to loading ad '-j' parsing time as iptables does.
Note how this also fixes for 'counter' statement being in wrong position
of ebtables-translate output.
Fixes: fe97f60e5d2a9 ("ebtables-compat: add watchers support") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 25 Jan 2023 00:51:43 +0000 (01:51 +0100)]
Proper fix for "unknown argument" error message
While commit 1b8210f848631 kind of fixed the corner-case of invalid
short-options packed with others, it broke error reporting for
long-options. Revert it and deploy a proper solution:
When passing an invalid short-option, e.g. 'iptables -vaL', getopt_long
sets the variable 'optopt' to the invalid character's value. Use it for
reporting instead of optind if set.
To distinguish between invalid options and missing option arguments,
ebtables-translate optstring needs adjustment.
Fixes: 1b8210f848631 ("ebtables: Fix error message for invalid parameters") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 17 Jan 2023 15:38:43 +0000 (16:38 +0100)]
etc: Drop xtables.conf
The file is not used since the commit this one fixes. Also it wasn't
installed until recently, when commit 3822a992bc277 ("Makefile: Fix for
'make distcheck'") added it in the wrong spot in an attempt to reduce
differences between tarballs generated by 'make tarball' and 'make
dist'.
While being at it, drop stale xtables_config_main() prototype from
xtables-multi.h.
Fixes: 06fd5e46d46f7 ("xtables: Drop support for /etc/xtables.conf") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 22 Dec 2022 16:14:34 +0000 (17:14 +0100)]
ebtables-translate: Install symlink
Make this officially a tool, we have enough test coverage in place. Also
update xtables-translate.8 to mention it at least and generate
ebtables-translate.8 which points to it.
Phil Sutter [Thu, 22 Dec 2022 14:58:27 +0000 (15:58 +0100)]
nft: Reject tcp/udp extension without proper protocol match
Internally, 'th' expression is used, which works but matches both
protocols. Since users won't expect '-m tcp --dport 1' to match UDP
packets, catch missing/wrong '-p' argument.
Fixes: c034cf31dd1a9 ("nft: prefer native expressions instead of udp match") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 15 Dec 2022 15:06:11 +0000 (16:06 +0100)]
arptables: Check the mandatory ar_pln match
This match is added by nft_arp_add() to every rule with same value, so
when parsing just check it is as expected and otherwise ignore it. This
allows to treat matches on all other offsets/lengths as error.
Fixes: 84909d171585d ("xtables: bootstrap ARP compatibility layer for nftables") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Sat, 15 Oct 2022 10:25:28 +0000 (12:25 +0200)]
Makefile.am: Integrate testsuites
Support calling 'make check' in topdir to run all three testsuites.
While updating .gitignore, also add 'configure~' my autotools create and
the tags file.
Phil Sutter [Sat, 15 Oct 2022 09:43:01 +0000 (11:43 +0200)]
tests: Adjust testsuite return codes to automake guidelines
As per the manual[1]:
"When no test protocol is in use, an exit status of 0 from a test script
will denote a success, an exit status of 77 a skipped test, an exit
status of 99 a hard error, and any other exit status will denote a
failure."
Phil Sutter [Tue, 6 Dec 2022 19:35:42 +0000 (20:35 +0100)]
Makefile: Fix for 'make distcheck'
Since extensions/ directory does not use automake, some targets have to
be added manually. Apart from that, several Makefiles either missed to
specify relevant files or did not specify them correctly for 'make dist'
to add them to the tarball.
Phil Sutter [Mon, 5 Dec 2022 17:56:09 +0000 (18:56 +0100)]
iptables/Makefile: Split nft-variant man page list
Some of them are not generated and must therefore be distributed. Hence
add them to a 'dist_man_MANS' variable. This leaves only generated
entries in the non-dist one, so use that to reduce the CLEANFILES list.
Phil Sutter [Mon, 5 Dec 2022 16:29:30 +0000 (17:29 +0100)]
iptables/Makefile: Reorg variable assignments
Introduce helper variables holding SOURCES, LDADD and CFLAGS used by
both legacy and nft builds. Specify also internal header files, builds
should depend on them. Doing that, reorder lists for clarity.
Phil Sutter [Sat, 3 Dec 2022 21:45:59 +0000 (22:45 +0100)]
Drop libiptc/linux_stddef.h
This header was never included anywhere.
Fixes: aae69bed01982 ("complete libiptc rewrite. Time to load 10k rules goes down from 2.20 minutes to 1.255 seconds (!). Might still contain bugs, use with caution.") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 1 Dec 2022 14:16:43 +0000 (15:16 +0100)]
xtables-translate: Fix for interfaces with asterisk mid-string
For nft, asterisk is special at end of the interface name only. Escaping
it mid-string makes the escape char part of the interface name, so avoid
this.
In the test case, also drop the ticks around interface names in
*-translate command - since there's no shell involved which would eat
them, they become part of the interface name.
Fixes: e179e87a1179e ("xtables-translate: Fix for interface name corner-cases") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Sat, 22 Oct 2022 13:26:56 +0000 (15:26 +0200)]
nft: Recognize INVAL/D interface name
It is just a hack to translate '! -i +' into a never matching nft rule,
but recognize it anyway for completeness' sake and to make xlate replay
test pass.
Phil Sutter [Thu, 1 Dec 2022 14:08:01 +0000 (15:08 +0100)]
nft: Fix match generator for '! -i +'
It's actually nonsense since it will never match, but iptables accepts
it and the resulting nftables rule must behave identically. Reuse the
solution implemented into xtables-translate (by commit e179e87a1179e)
and turn the above match into 'iifname INVAL/D'.
The commit this fixes merely ignored the fact that "any interface" match
might be inverted.
Phil Sutter [Thu, 1 Dec 2022 00:38:26 +0000 (01:38 +0100)]
tests: xlate: Use --check to verify replay
After applying the translated rule using nft, pass the untranslated rule
to --check instead of dumping the ruleset and performing a string
search. This fixes for mandatory match reordering (e.g. addresses before
interfaces) and minor differences like /32 netmasks or even just
whitespace changes.
Fixes: 223e34b057b95 ("tests: xlate-test: Replay results for reverse direction testing") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 1 Dec 2022 12:06:25 +0000 (13:06 +0100)]
ebtables: Implement --check command
Sadly, '-C' is in use already for --change-counters (even though
ebtables-nft does not implement this), so add a long-option only. It is
needed for xlate testsuite in replay mode, which will use '--check'
instead of '-C'.
Phil Sutter [Wed, 30 Nov 2022 19:03:30 +0000 (20:03 +0100)]
libiptc: Eliminate garbage access
When adding a rule, valgrind prints:
Syscall param socketcall.setsockopt(optval) points to uninitialised byte(s)
at 0x4A8165A: setsockopt (in /lib64/libc.so.6)
by 0x4857A48: iptc_commit (libiptc.c:2676)
by 0x10E4BB: iptables_main (iptables-standalone.c:61)
by 0x49A3349: (below main) (in /lib64/libc.so.6)
Address 0x4b63788 is 40 bytes inside a block of size 1,448 alloc'd
at 0x484659F: calloc (vg_replace_malloc.c:1328)
by 0x4857654: iptc_commit (libiptc.c:2564)
by 0x10E4BB: iptables_main (iptables-standalone.c:61)
by 0x49A3349: (below main) (in /lib64/libc.so.6)
This is because repl->counters is not initialized upon allocation. Since
the field is an array, make use of calloc() which implicitly does the
initialization.
Fixes: e37c0dc100c51 ("Revert the recent addition of memset()'s to TC_COMMIT. One of them is bogus and the other one needs more investigation to why valgrind is complaining.") Signed-off-by: Phil Sutter <phil@nwl.cc>
When adding a rule with a target which defines a udata_size, valgrind
prints:
8 bytes in 1 blocks are definitely lost in loss record 1 of 1
at 0x484659F: calloc (vg_replace_malloc.c:1328)
by 0x486B128: xtables_calloc (xtables.c:434)
by 0x1128B4: xs_init_target (xshared.c:238)
by 0x113CD3: command_jump (xshared.c:877)
by 0x114969: do_parse (xshared.c:1644)
by 0x10EEB9: do_command4 (iptables.c:691)
by 0x10E45B: iptables_main (iptables-standalone.c:59)
by 0x49A2349: (below main) (in /lib64/libc.so.6)
It is not sufficient to free cs.target->t, so call
xtables_clear_iptables_command_state() which takes care of all the
details.
Fixes: 2dba676b68ef8 ("extensions: support for per-extension instance "global" variable space") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 25 Nov 2022 20:35:28 +0000 (21:35 +0100)]
iptables: Plug memleaks in print_firewall()
When adding a rule in verbose mode, valgrind prints:
192 bytes in 1 blocks are definitely lost in loss record 1 of 2
at 0x48417E5: malloc (vg_replace_malloc.c:381)
by 0x486B158: xtables_malloc (xtables.c:446)
by 0x486C1F6: xtables_find_match (xtables.c:826)
by 0x10E684: print_match (iptables.c:115)
by 0x10E684: print_firewall (iptables.c:169)
by 0x10FC0C: print_firewall_line (iptables.c:196)
by 0x10FC0C: append_entry (iptables.c:221)
by 0x10FC0C: do_command4 (iptables.c:776)
by 0x10E45B: iptables_main (iptables-standalone.c:59)
by 0x49A2349: (below main) (in /lib64/libc.so.6)
200 bytes in 1 blocks are definitely lost in loss record 2 of 2
at 0x48417E5: malloc (vg_replace_malloc.c:381)
by 0x486B158: xtables_malloc (xtables.c:446)
by 0x486BBD6: xtables_find_target (xtables.c:956)
by 0x10E579: print_firewall (iptables.c:145)
by 0x10FC0C: print_firewall_line (iptables.c:196)
by 0x10FC0C: append_entry (iptables.c:221)
by 0x10FC0C: do_command4 (iptables.c:776)
by 0x10E45B: iptables_main (iptables-standalone.c:59)
by 0x49A2349: (below main) (in /lib64/libc.so.6)
If the match/target was cloned, it needs to be freed. Basically a bug since
day 1.
Phil Sutter [Fri, 25 Nov 2022 20:21:22 +0000 (21:21 +0100)]
nft: Plug memleak in nft_rule_zero_counters()
When zeroing a specific rule, valgrind reports:
40 bytes in 1 blocks are definitely lost in loss record 1 of 1
at 0x484659F: calloc (vg_replace_malloc.c:1328)
by 0x48DE128: xtables_calloc (xtables.c:434)
by 0x11C7C6: nft_parse_immediate (nft-shared.c:1071)
by 0x11C7C6: nft_rule_to_iptables_command_state (nft-shared.c:1236)
by 0x119AF5: nft_rule_zero_counters (nft.c:2877)
by 0x11A3CA: nft_prepare (nft.c:3445)
by 0x11A7A8: nft_commit (nft.c:3479)
by 0x114258: xtables_main.isra.0 (xtables-standalone.c:94)
by 0x1142D9: xtables_ip6_main (xtables-standalone.c:118)
by 0x49F2349: (below main) (in /lib64/libc.so.6)
Have to free the matches/target in populated iptables_command_state object
again. While being at it, call the proper family_ops callbacks since this is
family-agnostic code.
Fixes: a69cc575295ee ("xtables: allow to reset the counters of an existing rule") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 25 Nov 2022 18:24:38 +0000 (19:24 +0100)]
iptables-restore: Free handle with --test also
When running 'iptables-restore -t', valgrind reports:
1,496 (160 direct, 1,336 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
at 0x48417E5: malloc (vg_replace_malloc.c:381)
by 0x4857A46: alloc_handle (libiptc.c:1279)
by 0x4857A46: iptc_init (libiptc.c:1342)
by 0x1167CE: create_handle (iptables-restore.c:72)
by 0x1167CE: ip46tables_restore_main (iptables-restore.c:229)
by 0x116DAE: iptables_restore_main (iptables-restore.c:388)
by 0x49A2349: (below main) (in /lib64/libc.so.6)
Free the handle pointer before parsing the next table.
Fixes: 1c9015b2cb483 ("libiptc: remove indirections") Signed-off-by: Phil Sutter <phil@nwl.cc>
Florian Westphal [Wed, 30 Nov 2022 09:31:52 +0000 (10:31 +0100)]
xlate: get rid of escape_quotes
Its not necessary to escape " characters, we can let xtables-translate
print the entire translation/command enclosed in '' chracters, i.e. nft
'add rule ...', this also takes care of [, { and other special characters
that some shells might parse otherwise (when copy-pasting translated output).
The escape_quotes struct member is retained to avoid an ABI breakage.
This breaks all xlate test cases, fixup in followup patches.
v3: no need to escape ', replace strcmp(x, "") with x[0] (Phil Sutter)
Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 25 Nov 2022 02:13:14 +0000 (03:13 +0100)]
libxtables: xt_xlate_add() to take care of spacing
Try to eliminate most of the whitespace issues by separating strings
from separate xt_xlate_add() calls by whitespace if needed.
Cover the common case of consecutive range, list or MAC/IP address
printing by inserting whitespace only if the string to be appended
starts with an alphanumeric character or a brace. The latter helps to
make spacing in anonymous sets consistent.
Provide *_nospc() variants which disable the auto-spacing for the
mandatory exception to the rule.
Make things round by dropping any trailing whitespace before returning
the buffer via xt_xlate_get().
Phil Sutter [Thu, 17 Nov 2022 14:30:11 +0000 (15:30 +0100)]
extensions: CONNMARK: Fix xlate callback
Bail out if nfmask != ctmask with XT_CONNMARK_SAVE and
XT_CONNMARK_RESTORE. Looks like this needs a similar implementation to
the one for XT_CONNMARK_SET.
Fix shift mark translation: xt_connmark_shift_ops does not contain
useful strings for nftables. Also add needed braces around the term
being shifted.
Fixes: db7b4e0de960c ("extensions: libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 11 Mar 2022 17:28:49 +0000 (18:28 +0100)]
extensions: libebt_mark: Fix xlate test case
The false suffix effectively disabled this test file, but it also has
problems: Apart from brmark_xlate() printing 'meta mark' instead of just
'mark', target is printed in the wrong position (like with any other
target-possessing extension.
Fixes: e67c08880961f ("ebtables-translate: add initial test cases") Signed-off-by: Phil Sutter <phil@nwl.cc>
Florian Westphal [Wed, 23 Nov 2022 13:44:22 +0000 (14:44 +0100)]
iptables-nft: exit nonzero when iptables-save cannot decode all expressions
We always return 0, even if we printed some error message half-way.
Increment an error counter whenever an error message was printed so that
the chain-loop can exit with an error if this counter is nonzero.
Another effect is that iptables-save will no longer print the COMMIT line anmore.
Reported-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Phil Sutter <phil@nwl.cc>
projects / thirdparty / iptables.git / log
