Phil Sutter [Fri, 7 Oct 2022 20:23:59 +0000 (22:23 +0200)]
ebtables: Support for guided option parser
Adjust ebt_load_match() and ebt_command_default() to expect
x6_options/x6_parse fiels to be set instead of the traditional ones.
Much of this is c'n'p from command_default() in xshared.c, but due to
ebtables' custom match data structure (combining matches and watchers),
sharing the code is probably not feasible.
Phil Sutter [Wed, 13 Dec 2023 20:14:58 +0000 (21:14 +0100)]
libxtables: xtoptions: Treat NFPROTO_BRIDGE as IPv4
When parsing for XTTYPE_HOST(MASK), the return value of afinfo_family()
is used to indicate the expected address family.
Make guided option parser expect IPv4 by default for ebtables as this is
the more common case. The exception is libebt_ip6, which will
temporarily adjust afinfo->family while parsing.
Accept an Ethernet MAC address with optional mask in the format
xtables_parse_mac_and_mask() expects it. Does not support XTOPT_PUT (for
now) due to the lack of defined data structure.
Phil Sutter [Sun, 17 Dec 2023 12:02:36 +0000 (13:02 +0100)]
libxtables: xtoptions: Prevent XTOPT_PUT with XTTYPE_HOSTMASK
Do as the comment in xtopt_parse_hostmask() claims and omit
XTTYPE_HOSTMASK from xtopt_psize array so xtables_option_metavalidate()
will catch the incompatibility.
Phil Sutter [Mon, 18 Dec 2023 23:56:07 +0000 (00:56 +0100)]
iptables-legacy: Fix for mandatory lock waiting
Parameter 'wait' passed to xtables_lock() signals three modes of
operation, depending on its value:
0: --wait not specified, do not wait if lock is busy
-1: --wait specified without value, wait indefinitely until lock becomes
free
>0: Wait for 'wait' seconds for lock to become free, abort otherwise
Since fixed commit, the first two cases were treated the same apart from
calling alarm(0), but that is a nop if no alarm is pending. Fix the code
by requesting a non-blocking flock() in the second case. While at it,
restrict the alarm setup to the third case only.
Cc: Jethro Beekman <jethro@fortanix.com> Cc: howardjohn@google.com Cc: Antonio Ojea <antonio.ojea.garcia@gmail.com> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1728 Fixes: 07e2107ef0cbc ("xshared: Implement xtables lock timeout using signals") Signed-off-by: Phil Sutter <phil@nwl.cc>
Jeremy Sowden [Thu, 14 Dec 2023 16:44:03 +0000 (16:44 +0000)]
build: use standard automake verbosity variables
The previous implementation ignored the default verbosity defined by
configure, and controlled by --{enable,disable}-silent-rules, and
treated V="" as V=0. Instead, follow the guide-lines given in the
automake manual.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 9 Nov 2023 15:59:51 +0000 (16:59 +0100)]
ebtables: Use do_parse() from xshared
Drop the custom commandline parsers from ebtables and
ebtables-translate, extend and use the shared one instead.
ebtables gains a few new features from doing this:
- Rule counters may be specified in the '-c N,M' syntax
- Support for --replace command
- Support for --list-rules command
- Zero individual rules
There is one known regression in this patch, namely maximum chain name
length shrinks to 28 characters (from 32). Since this limit changed for
iptables in the past as well (e.g. with commit 5429b41c2bb4a), assume
nobody really relies upon it anyway.
Phil Sutter [Fri, 24 Nov 2023 19:22:44 +0000 (20:22 +0100)]
ebtables: Make 'h' case just a call to print_help()
Move the special ebtables help parameter handling into its print_help()
function to prepare for it turning into a callback. Add new field 'argc'
to struct iptables_command_state to make this possible. It is actually
kind of consistent as it holds 'argv' already.
Phil Sutter [Wed, 22 Nov 2023 02:05:19 +0000 (03:05 +0100)]
ebtables: Change option values to avoid clashes
In order to parse input using do_parse(), distinct ebtables option's
values have to be distinct from others. Since arptables uses values 2-8
already, resort to values >10.
Phil Sutter [Fri, 24 Nov 2023 17:13:00 +0000 (18:13 +0100)]
xshared: Support for ebtables' --change-counters command
This is tricky because the short-option clashes with the --check
command. OTOH, ebtables supports --check as well (though without
short-option), so making do_parse() detect ebtables based on struct
xtables_args::family is probably still the least messy option.
Phil Sutter [Wed, 15 Nov 2023 12:09:27 +0000 (13:09 +0100)]
xshared: Support rule range deletion in do_parse()
This is a distinct ebtables feature. Introduce struct
xt_cmd_parse::rule_ranges boolean indicating support for it and bail
otherwise if a range was specified by the user.
Phil Sutter [Wed, 15 Nov 2023 15:58:15 +0000 (16:58 +0100)]
xshared: Turn command_default() into a callback
Ebtables' variant is pretty different since all extensions are loaded up
front and some targets serve as "watcher" extensions, so let variants
specify the function to call for extension parameters.
Phil Sutter [Tue, 28 Nov 2023 19:21:49 +0000 (20:21 +0100)]
libxtables: xtoptions: Fix for non-CIDR-compatible hostmasks
In order to parse the mask, xtopt_parse_hostmask() calls
xtopt_parse_plenmask() thereby limiting netmask support to prefix
lengths (alternatively specified in IP address notation).
In order to lift this impractical restriction, make
xtopt_parse_plenmask() aware of the fact that xtopt_parse_plen() may
fall back to xtopt_parse_mask() which correctly initializes val.hmask
itself and indicates non-CIDR-compatible masks by setting val.hlen to
-1.
So in order to support these odd masks, it is sufficient for
xtopt_parse_plenmask() to skip its mask building from val.hlen value and
take whatever val.hmask contains.
Fixes: 66266abd17adc ("libxtables: XTTYPE_HOSTMASK support") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 28 Nov 2023 19:11:57 +0000 (20:11 +0100)]
libxtables: xtoptions: Fix for garbage access in xtables_options_xfrm()
Allocation of the temporary array did not account for a terminating NULL
entry, causing array boundary overstepping in the called
xtables_merge_options(), causing spurious errors in extension parameter
parsing.
Fixes: ed8c3ea4015f0 ("libxtables: Combine the two extension option mergers") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Fri, 17 Nov 2023 16:27:56 +0000 (17:27 +0100)]
nft: Leave interface masks alone when parsing from kernel
The mask is entirely unused by nft-variants in general and legacy ones
when printing. It is relevant only when inserting a legacy rule into
kernel as it doesn't detect the '+'-suffix.
Phil Sutter [Sat, 18 Nov 2023 03:28:25 +0000 (04:28 +0100)]
xshared: Do not populate interface masks per default
These are needed by legacy variants only, so introduce a simplified
xtables_parse_interface() replacement which does not deal with them and
a small function which sets the mask based on given interface name for
use by legacy tools.
Replace the awkward inverse_for_options array with basically a few
switch() statements clearly identifying the relation between option and
inverse values and relieve callers from having to find the option flag
bit's position.
Phil Sutter [Tue, 14 Nov 2023 17:51:59 +0000 (18:51 +0100)]
xshared: Introduce xt_cmd_parse_ops::option_name
The old opt2char() function was flawed: Since not every field in
optflags contains a printable character, typical use of its return value
in print statements could lead to garbage on screen.
Replace this by a mechanism to retrieve an option's long name which
supports family-specific overrides. and get rid of optflags field
altogether and define NUMBER_OF_OPT similar to NUMBER_OF_CMD.
Phil Sutter [Fri, 3 Sep 2021 17:23:43 +0000 (19:23 +0200)]
extensions: MARK: arptables: Use guided option parser
It expects mark values in hex which is possible by setting the base
field.
The only adjustment needed to use the revision 2 parser is to fill the
mask for --set-mark: With XTTYPE_MARKMASK32, an omitted mask sets all
mask bits, XTTYPE_UINT32 leaves it uninitialized, though.
Phil Sutter [Thu, 2 Sep 2021 14:53:26 +0000 (16:53 +0200)]
extensions: libarpt_mangle: Use guided option parser
Sadly not the best conversion, struct arpt_mangle is not ideal for use
as storage backend: With MAC addresses, xtopt_parse_ethermac() refuses
to write into *_devaddr fields as they are larger than expected. With
XTTYPE_HOSTMASK OTOH, XTOPT_PUT is not supported in the first place.
As a side-effect, network names (from /etc/networks) are no longer
accepted. But earlier migrations to guided option parser had this
side-effect as well, so probably not a frequently used feature.
Phil Sutter [Wed, 22 Nov 2023 19:41:34 +0000 (20:41 +0100)]
libxtables: Introduce xtables_strtoul_base()
Semantically identical to xtables_strtoul() but accepts the base as
parameter so callers may force it irrespective of number prefix. The old
xtables_strtoul() becomes a shallow wrapper.
Phil Sutter [Thu, 2 Sep 2021 14:51:15 +0000 (16:51 +0200)]
libxtables: Fix guided option parser for use with arptables
With an unexpected value in afinfo->family, guided option parser was
rather useless when called from arptables extensions. Introduce
afinfo_family() wrapper to sanitize at least NFPROTO_ARP value.
Phil Sutter [Fri, 3 Sep 2021 21:43:41 +0000 (23:43 +0200)]
libxtables: Combine the two extension option mergers
For extending the command parser's struct option array, there is
xtables_merge_options() and xtables_options_xfrm(). Since their bodies
were almost identical, make the latter a wrapper of the former by
transforming the passed struct xt_option_entry array into a temporary
struct option one before handing over.
Phil Sutter [Tue, 21 Nov 2023 22:14:47 +0000 (23:14 +0100)]
ebtables: Implement --change-counters command
Treat it like --replace against the same rule with changed counters.
The operation is obviously not atomic, so rule counters may change in
kernel while the rule is fetched, modified and replaced.
Phil Sutter [Wed, 22 Nov 2023 02:58:18 +0000 (03:58 +0100)]
xshared: do_parse: Ignore '-j CONTINUE'
While iptables does not support his NOP, arptables man page claims it
does (although legacy arptables rejects it) and ebtables prefers to
print it instead of omitting the '-j' option.
Accept and ignore the target when parsing to at least fix for
arptables-nft and prepare for ebtables-nft using do_parse() as well.
Phil Sutter [Sun, 19 Nov 2023 10:23:09 +0000 (11:23 +0100)]
tests: xlate: Print failing command line
If the command segfaults, 'error' variable is empty and the resulting
error message is even misleading as the called program may not have been
iptables-translate.
Phil Sutter [Thu, 16 Nov 2023 16:03:10 +0000 (17:03 +0100)]
xshared: Drop pointless CMD_REPLACE check
All current users set default source and destination addresses in their
post_parse callbacks, so legacy variants are safe and nft variants don't
have this restriction anyway.
Phil Sutter [Sun, 19 Nov 2023 12:18:26 +0000 (13:18 +0100)]
xshared: struct xt_cmd_parse::xlate is unused
Drop the boolean, it was meant to disable some existence checks in
do_parse() prior to the caching rework. Now that do_parse() runs before
any caching is done, the checks in question don't exist anymore so drop
this relict.
Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Wed, 15 Nov 2023 16:41:50 +0000 (17:41 +0100)]
nft-bridge: nft_bridge_add() uses wrong flags
When checking whether -s or -d was given, invflags were used by
accident. This change has no functional effect since the values remain
the same, but this way it's clear where the previously assigned flags
are used.
Jan Engelhardt [Mon, 13 Nov 2023 10:17:35 +0000 (11:17 +0100)]
man: more backslash-encoding of characters
"-" is the dash, "\-" is minus as we know, but groff lists some more
characters: "^" is "modifier circumflex" and "~" is "modifier tilde",
which, too, need to be escaped for our use.
Phil Sutter [Wed, 8 Nov 2023 03:08:44 +0000 (04:08 +0100)]
arptables: Fix --proto-type mask formatting
Arptables accepts numeric --proto-type values and masks in any numeral
system identified by (absence of) prefix. Yet it prints the mask value
in hex without '0x'-prefix, breaking save and restore the same way
numeric --h-type output did.
In theory, this could be fixed either by adding the missing prefix or
printing the mask in decimal (like most other builtin matches do), but
since the value is printed in hex with prefix already, align mask output
with that.
Also a day 1 bug and consistent with legacy, so no Fixes: tag here as
well.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 8 Nov 2023 02:58:42 +0000 (03:58 +0100)]
arptables: Fix formatting of numeric --h-type output
Arptables expects numeric arguments to --h-type option in hexadecimal
form, even if no '0x'-prefix is present. In contrast, it prints such
values in decimal. This is not just inconsistent, but makes it
impossible to save and later restore a ruleset without fixing up the
values in between.
Assuming that the parser side can't be changed for compatibility
reasons, fix the output side instead.
This is a day 1 bug and present in legacy arptables as well, so treat
this as a "feature" of arptables-nft and omit a Fixes: tag.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Add test cases for libarpt_mangle and extend the generic
tests to cover basic arptables matches.
Note that there are several historic artefacts that could be revised.
For example, arptables-legacy and arptables-nft both ignore "-p"
instead of returning an error about an unsupported option.
The ptype could be hard-wired to 0x800 and set unconditionally.
OTOH, this should always match for ethernet arp packets anyway.
Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 7 Nov 2023 18:12:14 +0000 (19:12 +0100)]
ebtables: Fix corner-case noflush restore bug
Report came from firwalld, but this is actually rather hard to trigger.
Since a regular chain line prevents it, typical dump/restore use-cases
are unaffected.
Fixes: 73611d5582e72 ("ebtables-nft: add broute table emulation") Cc: Eric Garver <eric@garver.life> Signed-off-by: Phil Sutter <phil@nwl.cc>
ARPT_ and IPT_INV flags are not interchangeable, e.g.:
define IPT_INV_SRCDEVADDR 0x0080
define ARPT_INV_SRCDEVADDR 0x0010
as these flags can be tested by libarp_foo.so such checks can yield
incorrect results.
Because arptables-nft uses existing code, e.g. xt_mark, it makes
sense to unify this completely by converting the last users of
ARPT_INV_ constants.
Note that arptables-legacy does not do run-time module loading via
dlopen(). Functionaliy implemented by "extensions" in the
arptables-legacy git tree are built-in, so this doesn't break
arptables-legacy binaries.
Fixes: 44457c080590 ("xtables-arp: Don't use ARPT_INV_*") Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Jan Engelhardt [Tue, 24 Oct 2023 13:04:03 +0000 (15:04 +0200)]
man: encode hyphens the way groff/man requires it
Edit a few spots where indeed a hyphens (U+2010) rather than U+002D is desired.
("set-name" is not something you input, it is a placeholder in the context of
documentation. "out-of-flow" is part of the regular flowed text, so should not
use anything but hyphens.)
Signed-off-by: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Jan Engelhardt [Tue, 24 Oct 2023 12:58:06 +0000 (14:58 +0200)]
man: encode minushyphen the way groff/man requires it
Sparked by a recent LWN article[1], sweeps over the iptables manpages
for incorrectly encoded dashes was made by Phil Sutter and myself.
An ASCII minushyphen in the source manpage translates to a hyphen in
output, so one has to use the sequence "\-" to get a minushyphen in
the output, as groff_char(7) explains.
[1] https://lwn.net/Articles/947941/ (paywalled until about 2023-11-06)
Signed-off-by: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Phil Sutter <phil@nwl.cc>
Since kernel commit c4eee56e14fe ("net: skb_find_text: Ignore patterns
extending past 'to'"), pattern scanning no longer happens past --to
offset even if skb_seq_read() returned a larger block. Point this out in
the description and also drop the '-1' offset which is not true as
kernel's selftest in tools/testing/selftests/netfilter/xt_string.sh
shows.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1707 Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 12 Oct 2023 15:54:53 +0000 (17:54 +0200)]
extensions: string: Clarify description of --to
String match indeed returns a match as long as the given pattern starts
in the range of --from and --to, update the text accordingly.
Also add a note regarding fragment boundaries.
Phil Sutter [Thu, 12 Oct 2023 15:27:42 +0000 (17:27 +0200)]
libiptc: Fix for another segfault due to chain index NULL pointer
Chain rename code missed to adjust the num_chains value which is used to
calculate the number of chain index buckets to allocate during an index
rebuild. So with the right number of chains present, the last chain in a
middle bucket being renamed (and ending up in another bucket) triggers
an index rebuild based on false data. The resulting NULL pointer index
bucket then causes a segfault upon reinsertion.
Phil Sutter [Wed, 6 Sep 2023 17:02:52 +0000 (19:02 +0200)]
include: linux: Update kernel.h
Its contents were moved into const.h and sysinfo.h, apply these changes
to the cached copies. Fixes for the following warning when compiling
xtables-monitor.c with new kernel headers in /usr/include:
| In file included from ../include/linux/netfilter/x_tables.h:3,
| from ../include/xtables.h:19,
| from xtables-monitor.c:36:
| ../include/linux/kernel.h:7: warning: "__ALIGN_KERNEL" redefined
| 7 | #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)
| |
| In file included from /usr/include/linux/netlink.h:5,
| from /home/n0-1/git/libmnl/install/include/libmnl/libmnl.h:9,
| from xtables-monitor.c:30:
| /usr/include/linux/const.h:31: note: this is the location of the previous definition
| 31 | #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
| |
Phil Sutter [Wed, 6 Sep 2023 14:32:47 +0000 (16:32 +0200)]
nft: Fix for useless meta expressions in rule
A relict of legacy iptables' mandatory matching on interfaces and IP
addresses is support for the '-i +' notation, basically a "match any
input interface". Trying to make things better than its predecessor,
iptables-nft boldly optimizes that nop away - not entirely though, the
meta expression loading the interface name was left in place. While not
a problem (apart from pointless overhead) in current HEAD, v1.8.7 would
trip over this as a following cmp expression (for another match) was
incorrectly linked to that stale meta expression, loading strange values
into the respective interface name field.
While being at it, merge and generalize the functions into a common one
for use with ebtables' NFT_META_BRI_(I|O)IFNAME matches, too.
Phil Sutter [Fri, 1 Sep 2023 11:16:56 +0000 (13:16 +0200)]
tests: shell: Fix for ineffective 0007-mid-restore-flush_0
The test did not catch non-zero exit status of the spawned coprocess. To
make it happen, Drop the line killing it (it will exit anyway) and pass
its PID to 'wait'.
While being at it, put the sleep into the correct spot (otherwise the
check for chain 'foo' existence fails as it runs too early) and make
said chain existence check effective.
Fixes: 4e3c11a6f5a94 ("nft: Fix for ruleset flush while restoring") Signed-off-by: Phil Sutter <phil@nwl.cc>
Quentin Armitage [Sat, 23 Nov 2013 08:41:58 +0000 (08:41 +0000)]
extensions: Fix checking of conntrack --ctproto 0
There are three issues in the code:
1) the check (sinfo->invflags & XT_INV_PROTO) is using the wrong mask
2) in conntrack_mt_parse it is testing (info->invert_flags &
XT_INV_PROTO) before the invert bit has been set.
3) the sense of the error message is the wrong way round
1) To get the error, ! -ctstatus XXX has to be specified, since
XT_INV_PROTO == XT_CONNTRACK_STATUS e.g.
| iptables -I CHAIN -m conntrack ! --ctstatus ASSURED --ctproto 0 ...
3) Unlike --proto 0 (where 0 means all protocols), in the conntrack
match --ctproto 0 appears to mean protocol 0, which can never be.
Therefore --ctproto 0 could never match and ! --ctproto 0 will always
match. Both of these should be rejected, since the user clearly
cannot be intending what was specified.
The attached patch resolves the issue, and also produces an error
message if --ctproto 0 is specified (as well as ! --ctproto 0 ), since
--ctproto 0 will never match, and ! --ctproto 0 will always match.
[Phil: - Added Fixes: tag - it's a day 1 bug
- Copied patch description from Bugzilla
- Reorganized changes to reduce diff
- Added test cases]
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=874 Fixes: 5054e85be3068 ("general conntrack match module userspace support files") Signed-off-by: Quentin Armitage <quentin@armitage.org.uk> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 15 Aug 2023 11:47:28 +0000 (13:47 +0200)]
Revert --compat option related commits
This reverts the following commits:
b14c971db6db0 ("tests: Test compat mode") 11c464ed015b5 ("Add --compat option to *tables-nft and *-nft-restore commands") ca709b5784c98 ("nft: Introduce and use bool nft_handle::compat") 402b9b3c07c81 ("nft: Pass nft_handle to add_{target,action}()")
This implementation of a compatibility mode implements rules using
xtables extensions if possible and thus relies upon existence of those
in kernel space. Assuming no viable replacement for the internal
mechanics of this mode will be found in foreseeable future, it will
effectively block attempts at deprecating and removing of these xtables
extensions in favor of nftables expressions and thus hinder upstream's
future plans for iptables.
Phil Sutter [Fri, 5 May 2023 18:04:41 +0000 (20:04 +0200)]
Add --compat option to *tables-nft and *-nft-restore commands
The flag sets nft_handle::compat boolean, indicating a compatible rule
implementation is wanted. Users expecting their created rules to be
fetched from kernel by an older version of *tables-nft may use this to
avoid potential compatibility issues.
Changes since v1:
- Expect short option '-C' in {ip,ip6,eb}tables-nft-restore command line
parser
- Support -C/--compat in arptables-nft-restore, too
- Update man pages with the new flag
Phil Sutter [Fri, 5 May 2023 15:39:08 +0000 (17:39 +0200)]
nft: Introduce and use bool nft_handle::compat
If set, create rules using compat expressions where possible and disable
the bitwise expression avoidance introduced in 323259001d617 ("nft:
Optimize class-based IP prefix matches").
The change can't be right: A simple rule append call will reset all
built-in chains' counters. The old code works fine even given the
mentioned "empty restore" use-case, at least if counters don't change on
the fly in-kernel.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=912 Fixes: 7c4d668c9c2ee ("libiptc: fix wrong maptype of base chain counters on restore") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 10 Aug 2023 09:30:59 +0000 (11:30 +0200)]
nft: Create builtin chains with counters enabled
The kernel enables policy counters for nftables chains only if
NFTA_CHAIN_COUNTERS attribute is present. For this to be generated, one
has to set NFTNL_CHAIN_PACKETS and NFTNL_CHAIN_BYTES attributes in the
allocated nftnl_chain object.
The above happened for base chains only with iptables-nft-restore if
called with --counters flag. Since this is very unintuitive to users,
fix the situation by adding counters to base chains in any case.
Fixes: 384958620abab ("use nf_tables and nf_tables compatibility interface") Signed-off-by: Phil Sutter <phil@nwl.cc>