Phil Sutter [Tue, 1 Aug 2023 22:28:03 +0000 (00:28 +0200)]
man: iptables.8: Clarify --goto description
Text speaks about behaviour of RETURN target when used in chains
redirected to using --goto instead of --jump, not the difference between
--jump option and "return".
Fixes: 17fc163babc34 ("add 'goto' support (Henrik Nordstrom <hno@marasystems.com>)") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 1 Aug 2023 22:05:45 +0000 (00:05 +0200)]
man: iptables.8: Trivial spelling fixes
- Missing "and" as well as full stop
- Missing comma in enumeration
- Duplicate "previous"
- Confusions are avoided rather than simplified
- Missing space after comma
Phil Sutter [Tue, 1 Aug 2023 19:24:15 +0000 (21:24 +0200)]
man: iptables.8: Extend exit code description
Codes 3 and 4 were missing.
Reported-by: Steven Barre <steven.barre@dxcas.com> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1353 Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 1 Aug 2023 21:28:20 +0000 (23:28 +0200)]
extensions: libipt_icmp: Fix confusion between 255/255 and any
Per definition, ICMP type "any" is type 255 and the full range of codes
(0-255). Save callback though ignored the actual code values, printing
"any" for every type 255 match. This at least confuses users as they
can't find their rule added as '--icmp-type 255/255' anymore.
It is not entirely clear what the fixed commit was trying to establish,
but the save output is certainly not correct (especially since print
callback gets things right).
Phil Sutter [Tue, 1 Aug 2023 14:56:42 +0000 (16:56 +0200)]
iptables-apply: Eliminate shellcheck warnings
Actual warnings were only about use of '-a' in bracket expressions
(replace by '&&' pipeline) and the immediate evaluation of the variable
in trap command.
The remaining changes silence info-level messages: missing quoting
around variables, pointless '$' in arithmetic expressions, backticks
instead of $(...), missing '-r' parameter when calling read and an
awkward negated '-z' check.
Phil Sutter [Thu, 13 Jul 2023 16:32:02 +0000 (18:32 +0200)]
iptables-restore: Drop dead code
Handle initialization is guarded by 'in_table' boolean, so there can't
be a handle already (because the branch which unsets 'in_table' also
frees the handle).
Phil Sutter [Fri, 28 Jul 2023 11:58:46 +0000 (13:58 +0200)]
tests: shell: Fix and extend chain rename test
The old version exited unintentionally before testing ip6tables. Replace
it by a more complete variant testing for all tools, creating and
renaming of,chains with various illegal names instead of just renaming
to a clashing name.
Fixes: ed9cfe1b48526 ("tests: add initial save/restore test cases") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 28 Jul 2023 11:50:11 +0000 (13:50 +0200)]
ebtables: Improve invalid chain name detection
Fix several issues:
- Most importantly, --new-chain command accepted any name. Introduce
ebt_assert_valid_chain_name() for use with both --new-chain and
--rename-chain.
- Restrict maximum name length to what legacy ebtables allows - this is
a bit more than iptables-nft, subject to be unified.
- Like iptables, legacy ebtables rejects names prefixed by '-' or '!'.
- Use xs_has_arg() for consistency, keep the check for extra args for
now.
Fixes: da871de2a6efb ("nft: bootstrap ebtables-compat") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 21 Jul 2023 18:14:09 +0000 (20:14 +0200)]
*tables: Reject invalid chain names when renaming
While given chain name was sanity checked with --new-chain command,
--rename-chain command allowed to choose an invalid name. Keep things
consistent by adding the missing check.
Fixes: e6869a8f59d77 ("reorganized tree after kernel merge") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 21 Jul 2023 17:40:30 +0000 (19:40 +0200)]
*tables-restore: Enforce correct counters syntax if present
If '--counters' option was not given, restore parsers would ignore
anything following the policy word. Make them more strict, rejecting
anything in that spot which does not look like counter values even if
not restoring counters.
Phil Sutter [Fri, 21 Jul 2023 11:14:36 +0000 (13:14 +0200)]
nft: Special casing for among match in compare_matches()
When other extensions may have "garbage" appended to their data which
should not be considered for match comparison, among match is the
opposite in that it extends its data beyond the value in 'size' field.
Add special casing to cover for this, avoiding false-positive rule
comparison.
Fixes: 26753888720d8 ("nft: bridge: Rudimental among extension support") Signed-off-by: Phil Sutter <phil@nwl.cc>
Some versions of awk (gawk-4.2.1-4.el8 in particular) also print the
non-debug ruleset listing's empty lines, causing the diff to fail. Catch
this by exiting upon seeing the first table heading. For the sake of
comparing bytecode, the actual ruleset listing is not interesting,
anyway.
Fixes: 0f7ea0390b336 ("tests/shell: Fix nft-only/0009-needless-bitwise_0") Signed-off-by: Phil Sutter <phil@nwl.cc>
nft-bridge: pass context structure to ops->add() to improve anonymous set support
Add context structure to improve bridge among support which creates an
anonymous set. This context structure specifies the command and it
allows to optionally store a anonymous set.
Use this context to generate native bytecode only if this is an
add/insert/replace command.
This fixes a dangling anonymous set that is created on rule removal.
Fixes: 26753888720d ("nft: bridge: Rudimental among extension support") Reported-and-tested-by: Igor Raits <igor@gooddata.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jacek Tomasiak [Mon, 19 Jun 2023 11:46:36 +0000 (13:46 +0200)]
iptables: Fix handling of non-existent chains
Since 694612adf87 the "compatibility" check considers non-existent
chains as "incompatible". This broke some scripts which used calls
like `iptables -L CHAIN404` to test for chain existence and expect
"No chain/target/match by that name." in the output.
This patch changes the logic of `nft_is_table_compatible()` to
report non-existent chains as "compatible" which restores the old
behavior.
Fixes: 694612adf87 ("nft: Fix selective chain compatibility checks") Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1648 Signed-off-by: Jacek Tomasiak <jtomasiak@arista.com> Signed-off-by: Jacek Tomasiak <jacek.tomasiak@gmail.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
This is an IPv4 header, which does not require the special handling
as in IPv6, use the payload matching instead of meta l4proto which
is slightly faster in this case.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Phil Sutter <phil@nwl.cc>
Jeremy Sowden [Sun, 11 Jun 2023 11:34:29 +0000 (12:34 +0100)]
man: string: document BM false negatives
For non-linear skb's there's a possibility that the kernel's Boyer-Moore
text-search implementation may miss matches. There's a warning about
this in the kernel source. Include that warning in the man-page.
nft: check for source and destination address in first place
When generating bytecode, check for source and destination address in
first place, then, check for the input and output device. In general,
the first expression in the rule is the most evaluated during the
evaluation process. These selectors are likely to show more variability
in rulesets.
# iptables-nft -vv -I INPUT -s 1.2.3.4 -p tcp
tcp opt -- in * out * 1.2.3.4 -> 0.0.0.0/0
table filter ip flags 0 use 0 handle 0
ip filter INPUT use 0 type filter hook input prio 0 policy accept packets 0 bytes 0
ip filter INPUT
[ payload load 4b @ network header + 12 => reg 1 ]
[ cmp eq reg 1 0x04030201 ]
[ meta load l4proto => reg 1 ]
[ cmp eq reg 1 0x00000006 ]
[ counter pkts 0 bytes 0 ]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 29 Mar 2023 15:53:11 +0000 (17:53 +0200)]
nft: Introduce nft-ruleparse.{c,h}
Extract all code dealing with parsing from struct nftnl_rule into struct
iptables_command_state from nft-shared.c into a separate source file.
Basically this is nft_rule_to_iptables_command_state() and the functions
it calls, plus family-independent parsers called from family-specific
callbacks.
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.