Phil Sutter [Mon, 24 Sep 2018 17:25:24 +0000 (19:25 +0200)]
Combine parse_target() and command_jump() implementations
Merge these two functions from xtables, iptables, ip6tables and
arptables. Both functions were basically identical in the first three,
only the last one required a bit more attention.
To eliminate access to 'invflags' in variant-specific location, move the
call to set_option() into callers. This is actually consistent with
parsing of other options in them.
As with command_match(), use xt_params instead of the different
*_globals objects to refer to 'opts' and 'orig_opts'.
It was necessary to rename parse_target() as it otherwise clashes with a
static function of same name in libxt_SET.
In arptables, the maximum allowed target name is a bit larger, so
introduce xtables_globals.target_maxnamelen defining the value. It is
used in the shared xt_parse_target() implementation.
Implementation of command_jump() in arptables diverted from the others
for no obvious reason. The call to parse_target() was done outside of it
and a pointer to cs->arp was passed but not used inside.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 24 Sep 2018 17:25:23 +0000 (19:25 +0200)]
Combine command_match() implementations
This merges the basically identical implementations of command_match()
from xtables, iptables and ip6tables into one. The only required
adjustment was to make use of xt_params instead of the different
*_globals objects.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 24 Sep 2018 17:25:22 +0000 (19:25 +0200)]
libiptc: NULL-terminate errorname
In struct chain_head, field 'name' is of size TABLE_MAXNAMELEN, hence
copying its content into 'error_name' field of struct xt_error_target
which is two bytes shorter may overflow. Make sure this doesn't happen
by using strncpy() and set the last byte to zero.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:17:07 +0000 (15:17 +0200)]
arptables: Fix incorrect strcmp() in nft_arp_rule_find()
Since nft_arp_rule_to_cs() may not set cs->jumpto, later call to
strcmp() may be passed a NULL pointer. Therefore check if the pointer is
valid before doing so.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:17:06 +0000 (15:17 +0200)]
xtables: Don't read garbage in nft_ipv4_parse_payload()
The problem here is that get_frag() does not set 'inv' in any case, so
when later checking its value, garbage may be read. Sanitize this case
by setting 'inv' to false before calling get_frag().
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:17:05 +0000 (15:17 +0200)]
libxtables: Use posix_spawn() instead of vfork()
According to covscan, vfork() may lead to a deadlock in the parent
process. It suggests to use posix_spawn() instead. Since the latter
combines vfork() and exec() calls, use it for xtables_insmod().
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:17:03 +0000 (15:17 +0200)]
extensions: libebt_ip{, 6}: Drop pointless error checking
Since info->protocol is of type __u8, its value will never become -1.
Apart from that, xtables_parse_protocol() calls xt_params->exit_err() in
case of error, so this code is dead anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:17:00 +0000 (15:17 +0200)]
iptables: Use print_ifaces() from xtables
Move the function to xshared.c for common use between legacy and xtables
sources. While being at it, silence a covscan warning triggered by that
function as it couldn't verify input buffers won't exceed IFNAMSIZ.
Therefore use snprintf() when writing to the local buffer.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:59 +0000 (15:16 +0200)]
Share print_ipv{4,6}_addr() from xtables
These functions contain code which occurs in legacy's print_firewall()
functions, so use them there.
Rename them to at least make clear they print more than a single
address.
Also introduce ipv{4,6}_addr_to_string() which take care of converting
an address/netmask pair into string representation in a way which
doesn't upset covscan (since that didn't detect that 'buf' may not be
exceeded by the strings written into it.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:55 +0000 (15:16 +0200)]
libxtables: Don't read garbage in xtables_strtoui()
If xtables_strtoul() fails, it returns false and data pointed to by
parameter 'value' is undefined. Hence avoid copying that data in
xtables_strtoui() if the call failed.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:54 +0000 (15:16 +0200)]
libxtables: Avoid calling memcpy() with NULL source
Both affected functions check if 'oldopts' is NULL once but later seem
to ignore that possibility. To catch up on that, increment the pointer
only if it isn't NULL, also don't copy its content into the merged
options buffer in that case.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:53 +0000 (15:16 +0200)]
libiptc: Simplify alloc_handle() function signature
This change originated from covscan complaining about the strcpy() call
with an unknown size source buffer. But in fact, the size is known (and
equal to the destination size), so pass a pointer to STRUCT_GETINFO to
alloc_handle() instead of it's fields separately. Hopefully this will
silence covscan.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:52 +0000 (15:16 +0200)]
libxt_time: Drop initialization of variable 'year'
The variable is not read before being assigned the return value of
strtoul(), thefore the initialization is useless. And since after this
change parameter 'end' becomes unused, drop it as well.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:51 +0000 (15:16 +0200)]
libxt_ipvs: Avoid potential buffer overrun
Just like with libxt_conntrack, get rid of the temporary buffer. The
comment even states that it was copied from there, so just make them
identical again.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:50 +0000 (15:16 +0200)]
libxt_conntrack: Avoid potential buffer overrun
In print_addr(), a resolved hostname is written into a buffer without
size check. Since BUFSIZ is typically 8192 bytes, this shouldn't be an
issue, though covscan complained about it. Fix the code by using
conntrack_dump_addr() as an example.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:49 +0000 (15:16 +0200)]
libxt_conntrack: Version 0 does not support XT_CONNTRACK_DIRECTION
Since sinfo->flags is only 8 bytes large, checking for
XT_CONNTRACK_DIRECTION bit (which has value 1 << 12) will always return
false, so drop this dead code.
Phil Sutter [Wed, 19 Sep 2018 13:16:46 +0000 (15:16 +0200)]
ip{, 6}tables-restore: Fix for uninitialized array 'curtable'
When reading sufficiently malformed input, parser might hit end of
loop without having written the current table name into curtable and
therefore calling strcmp() with uninitialized buffer. Avoid this by
setting curtable to zero upon declaration.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:45 +0000 (15:16 +0200)]
Mark fall through cases in switch() statements
Typical covscan complaint, non-empty fall throughs should be marked as
such. There was but a single case which should break instead, namely in
libebt_log.c: It is not critical, since the next case merely asserts
'invert' being zero (which can't be as it was checked before). But while
being at it, introduce log_chk_inv() to consolidate the semantically
equal cases for the various log types.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:44 +0000 (15:16 +0200)]
libxtables: Integrate getethertype.c from xtables core
This moves getethertype.c into libxtables so that both extensions and
xtables-nft-multi may use the implementations therein. New users are
libebt_arp and libebt_vlan which drop their own duplicated
implementations of getethertypebyname() for the shared one.
This change originated from a covscan report of extensions'
implementations not checking fopen() return value which should be
implicitly fixed by this as well.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 19 Sep 2018 13:16:43 +0000 (15:16 +0200)]
xtables: Fix for wrong assert() in __nft_table_flush()
The code obviously tries to assert that nft_table_builtin_find()
returned a valid pointer before dereferencing it, but the wrong argument
was given. Assume this is just a typo and insert the missing underscore.
Phil Sutter [Wed, 19 Sep 2018 13:16:42 +0000 (15:16 +0200)]
nfnl_osf: Drop pointless check in xt_osf_strchr()
Although it remains unclear what the original intention behind the
affected code was, but 'tmp + 1' always evaluates true since 'tmp' is a
pointer value.
Phil Sutter [Mon, 17 Sep 2018 11:38:33 +0000 (13:38 +0200)]
libxt_string: Fix array out of bounds check
Commit 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds
access") tried to fix parse_hex_string() for overlong strings but the
change still allowed for 'sindex' to become XT_STRING_MAX_PATTERN_SIZE
which leads to access of first byte after info->pattern. This is not
really a problem because it merely overwrites info->patlen before
calling xtables_error() later, but covscan still detects it so it's
still worth fixing.
The crucial bit here is that 'sindex' has to be incremented at end of
the last iteration since its value is used for info->patlen. Hence just
move the overflow check to the beginning of the loop.
Fixes: 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds access") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 10 Sep 2018 21:32:34 +0000 (23:32 +0200)]
xtables-save: Ignore uninteresting tables
When running iptables-nft-save with other tables present, the dump
succeeded but the tool complained about those other tables. In an
environment where iptables-nft and nftables are uses in parallel, this
is an expected situation, so only complain about incompatible builtin
tables.
While being at it, move the table existence check from __do_output()
into do_output() since the former may be called from
nft_for_each_table() in which case the table is guaranteed to exist.
Also use nft_table_builtin_find() in nft_is_table_compatible() instead
of open-coding the search by name in h->tables.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 10 Sep 2018 21:35:17 +0000 (23:35 +0200)]
extensions: REJECT: Merge reject tables
Initial motivation for this was a covscan report for potential array out
of bounds access in REJECT_xlate (a false-positive, because all possible
values of reject->with occur in reject_table_xlate).
Use reject types as array indices of reject_table so that reject->with
serves as array index. Also merge reject_table_xlate into it.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 10 Sep 2018 21:35:16 +0000 (23:35 +0200)]
libxt_string: Avoid potential array out of bounds access
The pattern index variable 'sindex' is bounds checked before
incrementing it, which means in the next loop iteration it might already
match the bounds check condition but is used anyway.
Fix this by incrementing the index before performing the bounds check.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 10 Sep 2018 21:35:14 +0000 (23:35 +0200)]
libiptc: Avoid side-effect in memset() calls
These calls to memset() are passed a length argument which exceeds
t->target.u.user.name's length by one byte and hence overwrite
t->target.u.user.revision as well (relying upon no padding to happen
between both).
Avoid this obscure behaviour by passing the correct field size and
explicitly overwriting 'revision' field.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 5 Sep 2018 17:14:40 +0000 (19:14 +0200)]
xtables: Accept --wait in iptables-nft-restore
Passing --wait option to iptables-nft-restore led to program abort
because the flag parameter was not skipped. Mimick iptables-restore
behaviour when encountering --wait or --wait-interval options (but still
ignore the parameter).
Fixes: b9d7b49d84bc2 ("xtables-compat: restore: sync options with iptables-restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 7 Sep 2018 15:06:21 +0000 (17:06 +0200)]
xtables: Don't check all rules for being compatible
Commit f8e29a13fed8d ("xtables: avoid bogus 'is incompatible' warning")
fixed for compatibility checking to extend over all chains, not just the
relevant ones. This patch does the same for rules: Make sure only rules
belonging to the relevant table are being considered.
Note that comparing the rule's table name is sufficient here since the
table family is already considered when populating the rule cache.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
The logic to replicate 'iptables-restore --noflush' behaviour of
flushing custom chains if listed in the dump was broken for chains being
referenced. A minimal dump reproducing the issue is:
With --noflush, this can be restored just once in iptables-nft-restore.
Consecutive attempts return an error since xtables tries to delete the
referenced chain and recreate it instead of performing a real flush.
Fix this by really flushing the custom chain in 'chain_user_flush'
callback and running 'chain_user_add' callback only if the chain doesn't
exist already.
Fixes: df3d92bec6007 ("xtables-compat-restore: flush user-defined chains with -n") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 31 Aug 2018 20:30:58 +0000 (22:30 +0200)]
xtables: Drop use of IP6T_F_PROTO
Setting this bit in cs->fw6.ipv6.flags was done only for rules parsed
from command line, not for those read from kernel. As a result,
appropriate rules could not be deleted. A simple test case is:
| # ip6tables-nft -A INPUT -p tcp -j ACCEPT
| # ip6tables-nft -D INPUT -p tcp -j ACCEPT
| iptables: Bad rule (does a matching rule exist in that chain?).
Since the flag is not used anywhere in xtables-nft, dropping its use fixes
the bug as well as setting it in both cases.
Fixes: 5ee03e6df4172 ("xtables: Use meta l4proto for -p match") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 28 Aug 2018 19:44:16 +0000 (21:44 +0200)]
xtables: Fix for deleting rules with comment
Comment match allocation in command_match() and
nft_rule_to_iptables_command_state() were misaligned in that the latter
set match_size to just what is required instead of what the match needs
at maximum like the further. This led to failure when comparing them
later and therefore a rule with a comment could not be deleted.
For comments of a specific length, the udata buffer is padded by
libnftnl so nftnl_rule_get_data() returns a length value which is larger
than the string (including NULL-byte). The trailing data is supposed to
be ignored, but compare_matches() can't not know about that detail and
therefore returns a false-negative if trailing data contains junk. To
overcome this, use strncpy() when populating match data in
nft_rule_to_iptables_command_state(). While being at it, make sure
comment match allocation in that function is identical to what
command_match() does with regards to data allocation size. Also use
xtables_calloc() which does the required error checking.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 23 Aug 2018 15:43:28 +0000 (17:43 +0200)]
ebtables-translate: Fix for libebt_limit.txlate
The xlate function sharing here does not quite work since in
ebtables-translate, extensions are supposed to append whitespace. Fix
this by introducing a simple wrapper.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:27 +0000 (17:43 +0200)]
xtables: Add missing deinitialization
These fix reports for definitely lost blocks in valgrind. Not really
memleaks, but due to nft_handle going out of scope they're counted as
lost. Still worth fixing though since it reduces noise when auditing
code for real issues.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:26 +0000 (17:43 +0200)]
ebtables: Review match/target lookup once more
This is a partial revert of my previous commit with similar subject - it
missed to apply the needed changes to ebtables-translate as well and on
top of that still left some leaks and use-after-frees in place. The new
strategy is to make ebtables extension loading compatible with that of
xtables, because otherwise the heavy code sharing between
ebtables-translate and iptables-translate will cause trouble.
Basically, ebt_add_match() and ebt_add_watcher() copy what xtables'
command_match() does, but after the actual extension argument parsing
has already happened. Therefore they duplicate the loaded match along
with its data and reset the original one to default state for being
reused (e.g., by ebtables-restore). Since mflags/tflags are cleared
while doing so, clearing them for all loaded extensions in
do_commandeb() is not necessary anymore.
In ebt_command_default() (where extension parameter parsing happens),
the list of added extensions to the current rule are consolidated first
so no duplicate extension loading happens.
With the above in place, ebt_cs_clean() can be reverted to its old
state.
Apart from sharing command_jump() function with ebtables-translate, make
use of nft_init_eb() there, as well.
Fixes: aa7fb04fcf72c ("ebtables: Review match/target lookup") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:25 +0000 (17:43 +0200)]
extensions: libebt_mark: Drop mark_supplied check
Use of this static variable causes trouble as it affects all instances
of this target. So calling xs_init_target() for one instance invalidates
all the others.
Moving the variable into target private data seems not possible since
that would change the target's size and therefore it wouldn't match
anymore with what kernel expects.
So just get rid of it entirely. If a user "forgets" to set a mark value,
the default value of zero applies.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:24 +0000 (17:43 +0200)]
xtables: Add a few missing exit calls
Mostly to reduce noise from valgrind output, add missing calls to
destroy iterators in nft.c and add cleanup for the populated nft_handle
in xtables_eb_save_main().
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:23 +0000 (17:43 +0200)]
ebtables-translate: Fix segfault while parsing extension options
Previous review of match/target lookup did not consider
xtables-eb-translate.c which contains the same code. Fix parsing of
target/match arguments there as well by introducing
ebt_command_default() which consolidates the previously duplicated code.
One notable quirk in comparison to the similar xtables code: Since
ebtables allows for negations in ugly places (e.g. '--arp-opcode ! 1'),
ebt_check_inverse2() has to be called first.
Fixes: aa7fb04fcf72c ("ebtables: Review match/target lookup") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:22 +0000 (17:43 +0200)]
ebtables: trivial: Leverage C99-style initializers a bit more
This nit was discovered when comparing do_commandeb() with
do_commandeb_xlate(): Since 'cs' is initialized upon declaration
already, initialization of field '.eb.bitmask' may be moved there as
well.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 23 Aug 2018 15:43:20 +0000 (17:43 +0200)]
extensions: AUDIT: Provide translation
With audit logging being supported by nftables as a simple (fake) log
level, translating AUDIT target is easy. Especially since xt_AUDIT in
kernel doesn't quite care about --type parameter.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 20 Aug 2018 13:30:03 +0000 (15:30 +0200)]
xtables: Use meta l4proto for -p match
Use of payload expression to match against IPv6 nexthdr field does not
work if extension headers are present. A simple example for that is
matching for fragmented icmpv6 traffic. Instead, generate a 'meta
l4proto' expression which works even if extension headers are present.
For consistency, apply the same change to iptables-nft as well.
No adjustment to reverse path required as the needed bits were added by
commit 6ea7579e6fe24 ("nft: decode meta l4proto") already.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Heena Sirwani [Tue, 21 Aug 2018 11:55:56 +0000 (17:25 +0530)]
xtables: Fix for segfault when registering hashlimit extension
This patch fixes the crash when registering the hashlimit extension
with xtables during init_extensions(when built with static libs) .
The option validation function xtables_option_metavalidate has a
loop termination condition of the entry name being NULL. The loop
does not terminate when validating hashlimit_mt_opts_v2 which causes
a crash on derefencing an invalid entry.
Phil Sutter [Fri, 17 Aug 2018 13:35:47 +0000 (15:35 +0200)]
xtables: Fix for segfault in iptables-nft
Trying to set a chain's policy in an invalid table resulted in a
segfault. Reproducer was:
| # iptables -t broute -P BROUTING ACCEPT
Fix this by aborting in nft_chain_new() if nft_table_builtin_find()
returned NULL for the given table name.
For an illustrative error message, set errno to ENXIO in the above case
and add an appropriate Mesage to nft_strerror().
While being at it, improve the error message if an invalid policy was
given. Before:
| # iptables-nft -t filter -P INPUT ACCEPTdf
| iptables: Incompatible with this kernel.
After:
| # iptables-nft -t filter -P INPUT ACCEPTdf
| iptables: Bad policy name. Run `dmesg' for more information.
Third unrelated change in this patch: Drop error checking of
nft_chain_set() in do_commandx(): The function never returns negative,
so that check never yielded true.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 16 Aug 2018 16:14:36 +0000 (18:14 +0200)]
ebtables: Fix entries count in chain listing
The previous fix for reference counts in iptables-nft output wasn't
complete: While iptables lists the number of references for each custom
chain (i.e., the number of jumps to it), ebtables lists number of
entries (i.e., the number of rules contained) for each chain. Both used
the same value for it, although they are different metrics.
Fix this by passing both numbers separately to the 'print_header'
callback so that each tool may print the desired value.
Fixes: a0698de9866d2 ("xtables: Do not count rules as chain references") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 16 Aug 2018 16:07:07 +0000 (18:07 +0200)]
xtables: Make 'iptables -S nonexisting' return non-zero
To be consistent with legacy iptables, calling -S with a non-existing
chain should lead to an error message. This is how some scripts find out
whether a user-defined chain exists or not.
Make sure doing the same for an existing chain does succeed, even if an
invalid rule number was given.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 15 Aug 2018 10:34:24 +0000 (12:34 +0200)]
ebtables: Fix for listing of non-existent chains
When trying to list a non-existent chain, ebtables-nft would just print
the table header and then exit with a code of zero. In order to be more
consistent with legacy ebtables, change the code to:
* Print table header only if chosen chain is found and
* propagate the error condition if chain was not found to print an error
message.
Note that this does not establish full parity with legacy ebtables due
to the error code being 1 instead of 255 and the error message differing
from the legacy one.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 10 Aug 2018 15:07:36 +0000 (17:07 +0200)]
ebtables: Merge libebt_limit.c into libxt_limit.c
Both extensions were very similar already, but now that they both are
translated into native nftables code, their actual difference (i.e.
match size) doesn't matter anymore.
This change comes with one caveat: Since ebtables limit match is not in
its own file anymore, match preloading automatically also loads the
NFPROTO_UNSPEC limit match. This is not a problem per se since match
lookup will prefer the family-specific one, but when parsing unknown
options, a match without 'parse' callback is encountered. Therefore
do_commandeb() has to check existence of that callback prior to
dereferencing it.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 10 Aug 2018 15:07:35 +0000 (17:07 +0200)]
xtables: Use native nftables limit expression
The original issue was that for a rule with limit match added by
ebtables-nft, the kernel might attempt to use xt_limit instead of
ebt_limit (and fail due to that). This happens if xt_limit.ko is loaded
but ebt_limit.ko is not, because the kernel prefers the
family-independent variants.
There are multiple ways to avoid above issue, but using neither xt_limit
nor ebt_limit with nft-variants should be the most effective one.
Therefore translate a created limit match in userspace into native
nftables code before sending it to kernel and do the reverse translation
when listing rules. Apart from the translation routines, this requires
slight adjustment of nft_is_expr_compatible() since neither xt_limit nor
ebt_limit support byte-based limits or inverted limit match.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 10 Aug 2018 09:42:16 +0000 (11:42 +0200)]
ebtables: Remove flags misinterpretations
This is actually quite a mess: xtables-eb.c defines names for bits in
'flags' variable of do_commandeb(), though these tend to clash with bit
names defined in xshared.h due to the same 'OPT_' prefix. Therefore
checking for bits OPT_NUMERIC and OPT_VERBOSE is syntactically correct,
but semantically wrong as they actually refer to bits OPT_COMMAND and
OPT_PROTOCOL.
Since ebtables doesn't support numeric nor verbose modes, just replace
the checks with zero values.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 7 Aug 2018 10:29:35 +0000 (12:29 +0200)]
xtables: Fix for wrong counter format in -S output
Legacy iptables uses '-c PCNT BCNT' format in listed rules, nft-variant
used '[PCNT BCNT]' prefix like with iptables-save.
In order to pass the counter format preference along, FMT_C_COUNTS is
introduced and related 'format' checks adjusted.
Since legacy iptables prints the counters between matches and target,
this change affects save_matches_and_target() function. In order to get
access to the rule counters, it's declaration is adjusted to receive
iptables_command_state pointer instead of match, target and jumpto
pointers from the same object.
While being at it, integrate jump to user-defined chain into it as well
since the related code in both callers was almost identical. Though
since different rule flags are used between iptables and ip6tables, pass
a 'goto_flag' boolean instead of the actual 'flags' bitfield.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 9 Aug 2018 16:06:56 +0000 (18:06 +0200)]
xtables: Don't pass full invflags to add_compat()
The function expects a boolean, not a bitfield. This bug caused
inversion in another match to carry over to protocol match by accident.
The supplied testcase contains rules which then fail because they
contain matches requiring that protocol.
Fixes: 4ef77b6d1b52e ("xtables: fix missing protocol and invflags") Fixes: 4143a08819a07 ("ebtables-compat: add nft rule compat information to bridge rules") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Thu, 9 Aug 2018 16:06:28 +0000 (18:06 +0200)]
tests: Fix skipping for recent nft-only tests
In an attempt to sanitize shell scripting, exit test in recent testcases
was altered, which led to them being skipped even in nft test runs. Drop
the quotes so that globbing happens again.
While here, improve the check a bit to glob only on leading path part,
not also the file name. Also print "skip ..." just like
nft-only/0001compat_0 testcase does.
Phil Sutter [Tue, 7 Aug 2018 11:15:34 +0000 (13:15 +0200)]
xtables: Spelling fixes in xtables-monitor
Fix a few minor spelling issues in xtables-monitor help output and man
page. While being at it, change 'ipv4' and 'ipv6' to 'IPv4' and 'IPv6',
respectively.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 6 Aug 2018 15:23:23 +0000 (17:23 +0200)]
xtables: Fix potential segfault in nft_rule_append()
If batch_rule_add() failed (ENOMEM), nft_rule_append() frees the
rule and then tries to add it to the rule cache. Better return 0
(failure) instead of continuing.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 6 Aug 2018 15:21:56 +0000 (17:21 +0200)]
ebtables: Review match/target lookup
Since ebtables does not indicate extension use on commandline via '-m'
flag as in iptables, loading of matches has to happen prior to
commandline parsing. While parsing, the right extension is searched for
unknown parameters by passing it to its 'parse' callback and checking if
it succeeds. As an unavoidable side-effect, custom data in
xtables_targets objects is being altered if the extension parser
succeeds.
If called multiple times, do_commandeb() leaks memory and fixing this
requires to properly treat the above quirk:
* Load extensions just once at program startup, thereby reusing the
existing ones for several calls of do_commandeb().
* In ebt_cs_clean(), don't free memory which is being reused. Instead
reinit custom extension data if it was used in current do_commandeb()
call (i.e., it is contained in cs->match_list).
On the other hand, target lookup in command_jump() can be simplified a
lot: The only target it may have loaded is 'standard', so just load that
at as well at program startup and reduce command_jump() to a simple
linked list search. Since 'standard' target does not prove a 'parse'
callback, a check is necessary when parsing target options.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 6 Aug 2018 15:21:55 +0000 (17:21 +0200)]
ebtables-restore: Use xtables_restore_parse()
This drops the dedicated input parser (which was broken in many ways
anyway) and replaces it by the common one now that all required knobs
are in place.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 6 Aug 2018 15:21:54 +0000 (17:21 +0200)]
xtables-restore: Make COMMIT support configurable
Legacy ebtables-restore does not support COMMIT directive, so allow for
callers of xtables_restore_parse() to toggle whether it is required or
not.
In iptables, omitting COMMIT may be used for syntax checking, so we must
not add an implicit commit at EOF. Although ebtables/arptables legacy
does not support COMMIT lines at all, this patch allows them in nft
variants. If omitted, an implicit commit happens for them at EOF.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Legacy ebtables-save does not use a policy string of '-' to denote
user-defined chains but instead lists them with a policy of ACCEPT.
In order to use ebtables_restore_parse() for ebtables-save
implementation, make use of builtin table definitions to decide whether
a given chain is a builtin one or not.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Sat, 4 Aug 2018 11:10:19 +0000 (13:10 +0200)]
xtables: Match verbose ip{,6}tables output with legacy
Legacy ip{,6}tables prints feedback for various commands if in verbose
mode, make sure nft variants do the same.
There is one difference, namely when checking a rule (-C command):
Legacy ip{,6}tables print the rule in any case, nft variants don't in
case the rule wasn't found. Changing this though would require to
populate the nftnl_rule object just for printing, which is probably not
feasible.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 3 Aug 2018 15:26:46 +0000 (17:26 +0200)]
xtables: Reserve space for 'opt' column in ip6tables output
Although ip6tables does not support matching on fragments, the 'opt'
column is included in ruleset listing nevertheless. So
nft_ipv6_print_rule() has to fill that space up with blanks.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 3 Aug 2018 13:55:55 +0000 (15:55 +0200)]
xtables: Fix for no output on first iptables-nft invocation
Fix the same issue commit a4e78370af849 ("iptables-compat: fix empty
chains after first invocation of iptables-compat -L") fixed back in
2014. Seems like some changes since then broke it again.
This time, existing cache not containing the added table/chains gets
into the way, so clear it if nft_commit() was called.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Fri, 3 Aug 2018 13:33:02 +0000 (15:33 +0200)]
xtables: Do not count rules as chain references
Unlike iptables, nftables counts rules in a chain as references to that
chain. Align output of 'iptables-nft -L' with that of legacy iptables by
counting the number of rules in a chain and subtracting that value from
reference count before printing the chain header.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:23 +0000 (17:05 +0200)]
arptables: Fix jumps into user-defined chains
Trying to jump into a user-defined chain was not possible:
| arptables-nft -N foo
| arptables-nft -A INPUT -j foo
| (null) v1.8.0 (nf_tables): RULE_APPEND failed (No such file or directory): rule in chain INPUT
Since nft_arp_add() already does the right thing if cs->target is NULL
and cs->jumpto contains a non-empty string, simply drop the block of
code trying to deal with the situation.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:18 +0000 (17:05 +0200)]
xtables: Fix symlinks/names for ebtables-{save, restore}
While xtables-nft-multi only recognized ebtables-save and -restore,
Makefile did install only ebtables-nft-save and -restore symlinks. Clean
this up by making both name variants known and installing respective
symlinks, just like for ebtables and ebtables-nft.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:32 +0000 (17:05 +0200)]
ebtables: Support --init-table command
This effectively flushes all built-in chains and removes user-defined
ones. Since compat layer takes care of built-in table/chain creation, it
is sufficient to just drop the relevant table.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:25 +0000 (17:05 +0200)]
arptables: Fix for trailing spaces in output
This changes mangle target to print whitespace before each option, not
afterwards. This fixes any cases of trailing or double whitespace in
arptables output.
While being at it, introduce ipaddr_to() helper in libarpt_mangle.c to
simplify arpmangle_print() a bit.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:24 +0000 (17:05 +0200)]
arptables: Fix memleaks in do_commandarp()
The function did not free memory allocated in parse_hostnetworkmask()
and command_jump(). To fix the latter, code was aligned a bit more with
xtables.c (especially opts handling).
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:21 +0000 (17:05 +0200)]
ebtables: Print non-standard target parameters
If a rule has a non-standard target (i.e., cs->target != NULL), it may
contain parameters. This patch enables printing them.
The code assumed that a non-standard target is only present if
cs->jumpto is not set, but that is wrong: If
nft_rule_to_iptables_command_state() encounters a target expression, it
calls nft_parse_target() which in turn calls the family-specific
parse_target callback. All of them assign cs->target, whose name is
later assigned to cs->jumpto by the first function.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:17 +0000 (17:05 +0200)]
xshared: Consolidate argv construction routines
Implementations were equal in {ip,ip6,x}tables-restore.c. The one in
iptables-xml.c differed slightly. For now, collect all features
together. Maybe it would make sense to migrate iptables-xml.c to using
add_param_to_argv() at some point and therefore extend the latter to
store whether a given parameter was quoted or not.
While being at it, a few improvements were done:
* free_argv() now also resets 'newargc' variable, so users don't have to
do that anymore.
* Indenting level in add_param_to_argv() was reduced a bit.
* That long error message is put into a single line to aid in grepping
for it.
* Explicit call to exit() after xtables_error() is removed since the
latter does not return anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:16 +0000 (17:05 +0200)]
xshared: Consolidate parse_counters()
Move this helper function into xshared. While being at it, drop the need
for temporary variables and take over null pointer tolerance from the
implementation in iptables-xml.c.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:15 +0000 (17:05 +0200)]
Consolidate DEBUGP macros
This debug printing macro was defined in various places, always
identical. Move it into xshared.h and drop it from sources including
that header. There are a few exceptions:
* iptables-xml.c did not include xshared.h, which this patch changes.
* Sources in extensions and libiptc mostly left alone since they don't
include xshared.h (and maybe shouldn't). Only libxt_set.h does, so
it's converted, too.
This also converts DEBUG define use in libip6t_hbh.c to avoid a compiler
warning.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:13 +0000 (17:05 +0200)]
xtables: Use correct built-in chain count
In nft_chain_builtin_init(), The wrong macro was used for iterating over
the built-in chains of a given table. That array's length is defined
using NF_INET_NUMHOOKS, not NF_IP_NUMHOOKS. Though this change is rather
cosmetic since both macros resolve into the same value.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
projects / thirdparty / iptables.git / log
