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>
Phil Sutter [Tue, 11 Oct 2022 15:07:33 +0000 (17:07 +0200)]
extensions: Collate ICMP types/codes in libxt_icmp.h
Put the most extensive version of icmp_codes, icmpv6_codes and
igmp_codes into the header. Have to rename the function
xt_print_icmp_types's parameter to avoid a compiler warning.
Phil Sutter [Tue, 12 May 2020 16:46:39 +0000 (18:46 +0200)]
xshared: Share make_delete_mask() between ip{,6}tables
Function bodies were mostly identical, the only difference being the use
of struct ipt_entry or ip6t_entry for size calculation. Pass this value
via parameter to make them fully identical.
Phil Sutter [Fri, 21 Oct 2022 10:15:21 +0000 (12:15 +0200)]
tests: xlate-test: Replay results for reverse direction testing
Call nft with translation output as input, then check xtables-save
output to make sure iptables-nft can handle anything it suggests nft to
turn its ruleset into.
This extends the test case syntax to cover for expected asymmetries.
When the existing syntax was something like this:
To keep things terse, <replay rule part> may omit the obligatory '-A
<chain>' argument. If missing, <xlate command> is sanitized for how it
would appear in xtables-save output: '-I' is converted into '-A' and an
optional table spec is removed.
Since replay mode has to manipulate the ruleset in-kernel, abort if
called by unprivileged user. Also try to run in own net namespace to
reduce collateral damage.
Phil Sutter [Fri, 21 Oct 2022 08:28:09 +0000 (10:28 +0200)]
tests: xlate-test: Cleanup file reading loop
Put the actual translation test into a function to call from the loop
and clean it up a bit. Preparation work for running a second test on the
same data.
Phil Sutter [Tue, 12 Apr 2022 19:19:39 +0000 (21:19 +0200)]
extensions: Merge SNAT, DNAT, REDIRECT and MASQUERADE
REDIRECT was already merged into DNAT. Given the callback generator and
generalized inner parsing routines, merging the other "flavors" is
relatively simple. Rename the extension into "libxt_NAT.so" while doing
so and turn the old DSOs into symlinks.
Phil Sutter [Sat, 9 Jul 2022 09:01:35 +0000 (11:01 +0200)]
extensions: DNAT: Generate print, save and xlate callbacks
Each extension's callbacks follow the same scheme so introduce a
generator which accepts the specifics as parameter - including the
method to transform from per-extension data into struct nf_nat_range2.
Also move the different parser frontends and fcheck callbacks in one
spot for clarity.
Phil Sutter [Wed, 5 Oct 2022 18:30:44 +0000 (20:30 +0200)]
tests: libebt_vlan.t: Drop trailing whitespace from rules
Fast iptables-test.py mode is picky and it has to: Plain redirect target
prints a trailing whitespace, generally stripping the rules in test
cases won't work therefore.
Phil Sutter [Wed, 5 Oct 2022 17:51:08 +0000 (19:51 +0200)]
tests: iptables-test: Cover for obligatory -j CONTINUE in ebtables
Unlike iptables, ebtables includes the default rule target in output.
Instead of adding it to every rule in ebtables tests, add special casing
to the testscript checking if the expected rule output contains a target
already and adding the default one if not.
Phil Sutter [Fri, 30 Sep 2022 16:15:59 +0000 (18:15 +0200)]
tests: iptables-test: Implement fast test mode
Implement a faster mode of operation for suitable test files:
1) Collect all rules to add and all expected output in lists
2) Any supposedly failing rules are checked immediately like in slow
mode.
3) Create and load iptables-restore input from the list in (1)
5) Construct the expected iptables-save output from (1) and check it in
a single search
5) If any of the steps above fail, fall back to slow mode for
verification and detailed error analysis. Fast mode failures are not
fatal, merely warn about them.
To keep things simple (and feasible), avoid complicated test files
involving external commands, multiple tables or variant-specific
results.
Aside from speeding up testsuite run-time, rule searching has become
more strict since EOL char is practically part of the search string.
This revealed many false positives where the expected string was
actually a substring of the printed rule.
The range is not communicated as "min and max queue number", but "first
queue number and count" instead. With 16bits for each value, it is not
possible to balance between all 65536 possible queues. Although probably
never used in practice, point this detail out in man page and make the
parser complain instead of the cryptic "xt_NFQUEUE: number of total
queues is 0" emitted by the kernel module.
Phil Sutter [Fri, 7 Oct 2022 16:29:07 +0000 (18:29 +0200)]
libiptc: Fix for segfault when renaming a chain
This is an odd bug: If the number of chains is right and one renames the
last one in the list, libiptc dereferences a NULL pointer. Add fix and
test case for it.
Fixes: 64ff47cde38e4 ("libiptc: fix chain rename bug in libiptc") Reported-by: Julien Castets <castets.j@gmail.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 30 Sep 2022 16:06:10 +0000 (18:06 +0200)]
tests: IDLETIMER.t: Fix syntax, support for restore input
Expected output was wrong in the last OK test, probably defeating rule
search check. Also use a different label, otherwise the kernel will
reject the second idletimer with same label but different type if both
rules are added at once.
Phil Sutter [Fri, 30 Sep 2022 15:51:55 +0000 (17:51 +0200)]
extensions: among: Fix for use with ebtables-restore
When restoring multiple rules which use among match, new size may be
smaller than the old one which caused invalid writes by the memcpy()
call. Expect this and realloc the match only if it needs to grow. Also
use realloc instead of freeing and allocating from scratch.
Fixes: 26753888720d8 ("nft: bridge: Rudimental among extension support") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 29 Sep 2022 17:11:55 +0000 (19:11 +0200)]
extensions: among: Remove pointless fall through
This seems to be a leftover from an earlier version of the switch().
This fall through is never effective as the next case's code will never
apply. So just break instead.
Fixes: 26753888720d8 ("nft: bridge: Rudimental among extension support") Signed-off-by: Phil Sutter <phil@nwl.cc>