Phil Sutter [Tue, 27 Nov 2018 19:07:11 +0000 (20:07 +0100)]
xtables: Don't use native nftables comments
The problem with converting libxt_comment into nftables comment is that
rules change when parsing from kernel due to comment match being moved
to the end of the match list. And since match ordering matters, the rule
may not be found anymore when checking or deleting. Apart from that,
iptables-nft didn't support multiple comments per rule anymore. This is
a compatibility issue without technical reason.
Leave conversion from nftables comment to libxt_comment in place so we
don't break running systems during an update.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Baruch Siach [Sat, 17 Nov 2018 20:20:08 +0000 (22:20 +0200)]
xtables-monitor: fix build with musl libc
Commit 7c8791edac3 ("xtables-monitor: fix build with older glibc")
changed the code to use GNU style tcphdr fields. Unfortunately, musl
libc requires _GNU_SOURCE definition to expose these fields.
Fix the following build failure:
xtables-monitor.c: In function ‘trace_print_packet’:
xtables-monitor.c:406:43: error: ‘const struct tcphdr’ has no member named ‘source’
printf("SPORT=%d DPORT=%d ", ntohs(tcph->source), ntohs(tcph->dest));
^~
xtables-monitor.c:406:64: error: ‘const struct tcphdr’ has no member named ‘dest’
printf("SPORT=%d DPORT=%d ", ntohs(tcph->source), ntohs(tcph->dest));
^~
...
Baruch Siach [Fri, 16 Nov 2018 07:30:33 +0000 (09:30 +0200)]
include: fix build with kernel headers before 4.2
Commit 672accf1530 (include: update kernel netfilter header files)
updated linux/netfilter.h and brought with it the update from kernel
commit a263653ed798 (netfilter: don't pull include/linux/netfilter.h
from netns headers). This triggers conflict of headers that is fixed in
kernel commit 279c6c7fa64f (api: fix compatibility of linux/in.h with
netinet/in.h) included in kernel version 4.2. For earlier kernel headers
we need a workaround that prevents the headers conflict.
Fixes the following build failure:
In file included from .../sysroot/usr/include/netinet/ip.h:25:0,
from ../include/libiptc/ipt_kernel_headers.h:8,
from ../include/libiptc/libiptc.h:6,
from libip4tc.c:29:
.../sysroot/usr/include/linux/in.h:26:3: error: redeclaration of enumerator ‘IPPROTO_IP’
IPPROTO_IP = 0, /* Dummy protocol for TCP */
^
.../sysroot/usr/include/netinet/in.h:33:5: note: previous definition of ‘IPPROTO_IP’ was here
IPPROTO_IP = 0, /* Dummy protocol for TCP. */
^~~~~~~~~~
Phil Sutter [Thu, 15 Nov 2018 13:53:02 +0000 (14:53 +0100)]
xtables: Introduce per table chain caches
Being able to omit the previously obligatory table name check when
iterating over the chain cache might help restore performance with large
rulesets in xtables-save and -restore.
There is one subtle quirk in the code: flush_chain_cache() did free the
global chain cache if not called with a table name but didn't if a table
name was given even if it emptied the chain cache. In other places,
chain_cache being non-NULL prevented a cache update from happening, so
this patch establishes the same behaviour (for each individual chain
cache) since otherwise unexpected cache updates lead to weird problems.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Baruch Siach [Fri, 16 Nov 2018 05:23:32 +0000 (07:23 +0200)]
xtables-monitor: fix build with older glibc
glibc older than 2.19 only expose BSD style fields of struct tcphdr when
_BSD_SOURCE is define. Current glibc however, warn that _BSD_SOURCE is
deprecated. Migrate to the GNU style of tcphdr fields to make the code
compatible with any glibc version.
Fix the following build failure:
xtables-monitor.c: In function 'trace_print_packet':
xtables-monitor.c:406:43: error: 'const struct tcphdr' has no member named 'th_sport'
printf("SPORT=%d DPORT=%d ", ntohs(tcph->th_sport), ntohs(tcph->th_dport));
^
xtables-monitor.c:406:66: error: 'const struct tcphdr' has no member named 'th_dport'
printf("SPORT=%d DPORT=%d ", ntohs(tcph->th_sport), ntohs(tcph->th_dport));
^
...
Adam Gołębiowski [Wed, 14 Nov 2018 06:35:28 +0000 (07:35 +0100)]
extensions: format-security fixes in libip[6]t_icmp
commit 61d6c3834de3 ("xtables: add 'printf' attribute to xlate_add")
introduced support for gcc feature to check format string against passed
argument. This commit adds missing bits to extenstions's libipt_icmp.c
and libip6t_icmp6.c that were causing build to fail.
Fixes: 61d6c3834de3 ("xtables: add 'printf' attribute to xlate_add") Signed-off-by: Adam Gołębiowski <adamg@pld-linux.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Build with musl libc fails because of conflicting struct ethhdr
definitions:
In file included from .../sysroot/usr/include/net/ethernet.h:10:0,
from ../iptables/nft-bridge.h:8,
from libebt_vlan.c:18:
.../sysroot/usr/include/netinet/if_ether.h:107:8: error: redefinition of ‘struct ethhdr’
struct ethhdr {
^~~~~~
In file included from libebt_vlan.c:16:0:
.../sysroot/usr/include/linux/if_ether.h:160:8: note: originally defined here
struct ethhdr {
^~~~~~
Include the userspace header first for the definition suppression logic
to do the right thing.
Signed-off-by: Baruch Siach <baruch@tkos.co.il> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 19 Sep 2018 14:25:58 +0000 (16:25 +0200)]
ip6tables-save: Merge into iptables-save.c
Both implementations were very similar already. Differences were mostly
in which libiptc functions were called. Therefore introduce struct
iptables_save_cb to point to the right functions for each variant.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Florian Westphal [Mon, 12 Nov 2018 13:40:41 +0000 (14:40 +0100)]
xtables: add 'printf' attribute to xlate_add
This allows gcc to check format string vs. passed arguments.
Fix the fallout from this as well, typical warning produced is:
libebt_mark_m.c:112:28: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
xt_xlate_add(xl, "and 0x%x %s0 ", info->mask, ...
~^ ~~~~~~~~~~
so add the required casts or fixup format strings as needed.
libxt_conntrack also passed an unneeded argument (port), so remove that.
In arptables-classic, the kernel will add dev->addr_len to the
arp header base address to obtain the correct location, but we cannot
do this in nf_tables, at least not at this time (we need a fixed offset
value).
but if user did not provide "--h-length 6" argument, then this won't
work even for ethernet, as the payload expression will be told to load
the first 4 bytes of arp header source mac address (sender hw address).
Fix this by pre-initialising arhlen to 6.
We also need to set up arhrd. Otherwise, src/dst mac can't be used:
arptables -A INPUT -i lo --destination-mac 11:22:33:44:55:66
arptables v1.8.1 (nf_tables): RULE_APPEND failed (Invalid argument): rule in chain INPUT
This means that matching won't work for AX25, NETROM etc, however,
arptables "classic" can't parse non-ethernet addresses, and makes
ETH_ALEN assumptions in several spots, so this should be fine from
compatibility point of view.
1. check both address and mask, not just first byte of mac
2. use add_addr() for this so mask is also handled via bitwise expr.
3. use the correct offsets.
4. add dissector so we can reverse translate the payload expressions
generated for this.
arptables classic doesn't have arptables-save, it only has a perl
script that attempts to emulate iptables-save. It supports no options,
and thus has no way to dump counters. Add -c option, like iptables to
enable this.
Florian Westphal [Mon, 12 Nov 2018 11:49:11 +0000 (12:49 +0100)]
ebtables: use extrapositioned negation consistently
in the iptables universe, we enforce extrapositioned negation:
! -i foo
"-i ! foo" is not even supported anymore.
At least make sure that ebtables prints the former syntax everywhere as
well so we don't have a mix of both ways.
Parsing of --option ! 42 will still work for backwards compat reasons.
iptables-tests: do not append xtables-multi to external commands
Lines starting by @ can be used to invoke an external command of any
kind. Do not add xtables-multi here since we may want to execute a
non-iptables command.
-j CONTINUE can be added, but it can't be removed:
extensions/libebt_standard.t: ERROR: line 5 (cannot find: ebtables -I INPUT -d de:ad:be:ef:00:00 -j CONTINUE)
This problem stems from silly ambiguity in ebtables-nft vs. iptables.
In iptables, you can do
iptables -A INPUT
(no -j)
in ebtables, you can do either
ebtables -A INPUT
or
ebtables -A INPUT -j CONTINUE
both are *supposed* to be the same (and they do the same even
in ebtables-nft on netlink side).
However, the temprary binary representation within ebtables-nft is not
the same: when parsing -j CONTINUE, we add a standard target, then omit
it later in _add_target().
When translating netlink representation to ebt binary one,
we do not add a standard target and instead just print '-j CONTINUE'
when listing rules.
So when doing
-I INPUT -j CONTINUE
-D INPUT -j CONTINUE
the -D operation fails because it has a standard target in the binary
representation, whereas the rule we obtained from translating
nftables netlink back to ebtables' binary represenation doesn't.
Phil Sutter [Wed, 31 Oct 2018 19:13:34 +0000 (20:13 +0100)]
xtables: Fix for matching rules with wildcard interfaces
Due to xtables_parse_interface() and parse_ifname() being misaligned
regarding interface mask setting, rules containing a wildcard interface
added with iptables-nft could neither be checked nor deleted.
As suggested, introduce extensions/iptables.t to hold checks for
built-in selectors. This file is picked up by iptables-test.py as-is.
The only limitation is that iptables is being used for it, so no
ip6tables-specific things can be tested with it (for now).
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 11 Oct 2018 11:30:38 +0000 (13:30 +0200)]
xtables: Remove target_maxnamelen field
This is a partial revert of commit 9f075031a1973 ("Combine
parse_target() and command_jump() implementations"): Upstream prefers to
reduce max chain name length of arptables by two characters instead of
the introduced struct xtables_globals field which requires to bump
library API version.
Fixes: 9f075031a1973 ("Combine parse_target() and command_jump() implementations") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Chenbo Feng [Tue, 2 Oct 2018 01:23:07 +0000 (18:23 -0700)]
extensions: libxt_quota: Allow setting the remaining quota
The current xt_quota module cannot track the current remaining quota
of a specific rule. Everytime an unrelated rule is updated in the same
iptables table, the quota will be reset. This is not a very useful
function for iptables that get changed at run time. This patch fixes the
above problem by adding a new field in the struct that records the
current remaining quota.
Fixed a print out bug in verbose print out wrt. inversion.
Signed-off-by: Chenbo Feng <fengc@google.com> Suggested-by: Maciej Żenczykowski <maze@google.com> Reviewed-by: Maciej Żenczykowski <maze@google.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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.