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>
Phil Sutter [Thu, 2 Aug 2018 15:05:12 +0000 (17:05 +0200)]
xtables: Fix compilation with NLDEBUG defined
In libnftnl-1.0.5, symbol name prefix changed from 'nft_' to 'nftnl_'.
This patch fixes for two places forgotten by the relevant commit.
Fixes: 742baabd185c3 ("iptables-compat: use new symbols in libnftnl") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:11 +0000 (17:05 +0200)]
xtables: Free chains in NFT_COMPAT_CHAIN_ADD jobs
Chains in NFT_COMPAT_CHAIN_ADD usually have to be freed because they are
not added to the cache.
There is one exception though, namely when zeroing counters:
nft_chain_zero_counters() adds a chain object it took from chain cache.
To distinguish this situation from the others, introduce
NFT_COMPAT_CHAIN_ZERO batch object type, which is treated just like
NFT_COMPAT_CHAIN_ADD but batch_obj_del() does not free it's chain.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:09 +0000 (17:05 +0200)]
xtables: Fix for nft_rule_flush() returning garbage
Due to variable 'ret' not being initialized in all situations, return
code of the function depends on garbage in stack. Fix this by
initializing 'ret' to zero upon declaration.
While being at it, make nftnl_chain_list_get() failure as well as
nftnl_chain_list_iter_create() failure an error condition since both
functions should succeed even if the current ruleset does not contain
any chains at all.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Aug 2018 15:05:08 +0000 (17:05 +0200)]
xtables: Allocate rule cache just once
For each parsed table, xtables-restore calls nft_table_flush() which
each time allocates a new rule cache, possibly overwriting the pointer
to the previously allocated one. Fix this by checking the pointer value
and only allocate if it's NULL.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Eric Garver [Wed, 1 Aug 2018 19:36:26 +0000 (15:36 -0400)]
nft: don't print rule counters unless verbose
Currently rule counters are always printed, but that's not the desired
behavior. We should only print them with the verbose flag. This broke
when the arguments of nft_rule_print_save() were changed to accept the
format instead of a counters flag.
Fixes: cdc78b1d6bd7 ("nft: convert rule into a command state structure") Signed-off-by: Eric Garver <e@erig.me> Signed-off-by: Florian Westphal <fw@strlen.de>
Joel Goguen [Wed, 11 Jul 2018 23:32:20 +0000 (16:32 -0700)]
iptables-restore: free the table lock when skipping a table
Currently, when running `iptables-restore --table=X`, where `X` is not the first
table in the rules dump, the restore will fail when parsing the second table:
- a lock is acquird when parsing the first table name
- the table name does not match the parameter to `--table` so processing
continues until the next table
- when processing the next table a lock is acquired, which fails because a lock
is already held
Another app is currently holding the xtables lock. Perhaps you want to use the -w option?
This will release the lock as soon as it's decided the current table won't be
used.
Signed-off-by: Joel Goguen <contact+netfilter@jgoguen.ca> Signed-off-by: Florian Westphal <fw@strlen.de>
when using custom nft tables + iptables-nft, iptables-nft -L
may fail with
iptables v1.8.0 (nf_tables): table `filter' is incompatible, use 'nft' tool.
even if filter table is compatible.
Problem is that the chain cache tracks ALL chains.
The "old" compat-check only walked chains in the table to checked
(filter in this case), now we will see all other
chains including base chains of another table.
It seems better to extend the chain cache long-term to track chains
per table instead, but for now skip the foreign ones.
Phil Sutter [Thu, 19 Jul 2018 16:32:09 +0000 (18:32 +0200)]
xtables: implement ebtables-{save,restore}
The code for ebtables-restore was derived from legacy code,
ebtables-save is actually a new implementation using the existing
infrastructure and trying to adhere to legacy perl script output
formatting as much as possible.
This introduces a new format flag (FMT_EBT_SAVE) to allow
nft_bridge_save_rule() to distinguish between ruleset listing (i.e.,
ebtables -L) and saving via ebtables-save - the two differ in how
counters are being formatted. Odd, but that's how it is.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 19 Jul 2018 16:32:02 +0000 (18:32 +0200)]
xtables: eliminate nft_ipv{4,6}_rule_find()
Both functions just pass their parameters 1:1 to nft_ipv46_rule_find, so
replace them by the latter after minor adjustment to match expected
callback signature.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Differences between both structs are marginal (apart from
arptables_command_state being much smaller), so merge them into one.
Struct iptables_command_state is already shared between iptables,
ip6tables and ebtables.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 11 Jul 2018 10:09:05 +0000 (12:09 +0200)]
xtables: Support nft suffix for arptables and ebtables
Since the names without suffix clash with legacy tools, support the
suffixed versions as well to help distributions packaging for parallel
installation of both nft and legacy variants.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
1) README is added to run test suite.
2) Rename two test-case scripts to follow proper numerical order.
3) "echo -en "\033[1A\033[K" # clean the [EXECUTING] foobar line" command
should only used when verbose("-v") option is not there else instead of
clearing "[EXECUTING]" prompt it is clearing last prompt of the test file.
Florian Westphal [Fri, 29 Jun 2018 14:14:31 +0000 (16:14 +0200)]
xtables: display legacy/nf_tables flavor in error messages, too
Also, in nf_tables backend case, only show more than one error
if we're iptables-restore, else we get very long concatenated errorline.
old:
iptables v1.6.2: can't initialize iptables table `security': Table does not exist (do you need to insmod?)
iptables v1.6.2: iptables: CHAIN_ADD failed (Device or resource busy): chain PREROUTINGCHAIN_ADD failed (Device or resource busy): chain INPUTCHAIN_ADD failed (Device or resource busy): chain POSTROUTINGCHAIN_ADD failed (Device or resource busy): chain OUTPUT
iptables-restore v1.6.2: iptables-restore:
line 1: CHAIN_ADD failed (Device or resource busy): chain PREROUTING
line 1: CHAIN_ADD failed (Device or resource busy): chain INPUT
line 1: CHAIN_ADD failed (Device or resource busy): chain POSTROUTING
line 1: CHAIN_ADD failed (Device or resource busy): chain OUTPUT
line 6: RULE_INSERT failed (No such file or directory): rule in chain PREROUTING
now:
iptables v1.6.2 (legacy): can't initialize iptables table `security': Table does not exist (do you need to insmod?)
iptables v1.6.2 (nf_tables): CHAIN_ADD failed (Device or resource busy): chain PREROUTING
iptables-restore v1.6.2 (nf_tables):
line 1: CHAIN_ADD failed (Device or resource busy): chain PREROUTING
line 1: CHAIN_ADD failed (Device or resource busy): chain INPUT
line 1: CHAIN_ADD failed (Device or resource busy): chain POSTROUTING
line 1: CHAIN_ADD failed (Device or resource busy): chain OUTPUT
line 6: RULE_INSERT failed (No such file or directory): rule in chain PREROUTING
Florian Westphal [Fri, 29 Jun 2018 17:14:18 +0000 (19:14 +0200)]
tests: add script that mimics firewalld startup
Mimic firewalld startup, i.e. "iptables-restore -n" use.
First script is normal startup,
second script restores ruleset, then re-runs first one (i.e., with
existing rules rather than non-existent tables).
Jan Engelhardt [Wed, 27 Jun 2018 23:02:48 +0000 (01:02 +0200)]
doc: fix some spellos and the dash escape
Commands, options, filenames, and possibly references to other
manpages, should always use the minus. (Important for copy-n-paste
and e.g. following manpage links.) Everything else can do with the
dash.
Florian Westphal [Tue, 26 Jun 2018 21:16:22 +0000 (23:16 +0200)]
tests: add initial save/restore test cases
Add script to restore ipt-save files and compare it with save output.
This should be extended to cover as many rulesets as possible, so this
is only a start.
The test script is changed to pass XT_MULTI instead of
iptables/ip6tables.
This allows ip(6)tables/ebt/arp only test scripts and avoids running all scripts
multiple times for ip/ip6tables.