]> git.ipfire.org Git - thirdparty/iptables.git/log
thirdparty/iptables.git
5 years agoipables: xtables-restore: output filename option in help text
Florian Westphal [Mon, 16 Sep 2019 14:04:02 +0000 (16:04 +0200)] 
ipables: xtables-restore: output filename option in help text

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1341
Signed-off-by: Florian Westphal <fw@strlen.de>
5 years agolibiptc: silence two comiler warnings
Florian Westphal [Mon, 16 Sep 2019 13:44:48 +0000 (15:44 +0200)] 
libiptc: silence two comiler warnings

avoid hyptothetical truncation by leaving space for triling zero byte.
silcences:

In file included from libip4tc.c:113:
libiptc.c: In function ‘iptcc_alloc_chain_head’:
libiptc.c:163:2: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
  163 |  strncpy(c->name, name, TABLE_MAXNAMELEN);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libiptc.c: In function ‘iptc_rename_chain’:
libiptc.c:2388:2: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
 2388 |  strncpy(c->name, newname, sizeof(IPT_CHAINLABEL));
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Florian Westphal <fw@strlen.de>
5 years agolibiptc: axe non-building debug code
Florian Westphal [Mon, 16 Sep 2019 11:57:45 +0000 (13:57 +0200)] 
libiptc: axe non-building debug code

hasn't built with IPTC_DEBUG=1 since at least 2004, so remove it.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1275
Signed-off-by: Florian Westphal <fw@strlen.de>
5 years agoiptables-test: Support testing host binaries
Phil Sutter [Sat, 14 Sep 2019 00:34:36 +0000 (02:34 +0200)] 
iptables-test: Support testing host binaries

Introduce --host parameter to run the testsuite against host's binaries
instead of built ones.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agoebtables: fix over-eager -o checks on custom chains
Florian Westphal [Tue, 10 Sep 2019 21:10:59 +0000 (23:10 +0200)] 
ebtables: fix over-eager -o checks on custom chains

Arturo reports ebtables-nft reports an error when -o is
used in custom chains:

-A MYCHAIN -o someif
makes ebtables-nft exit with an error:
"Use -o only in OUTPUT, FORWARD and POSTROUTING chains."

Problem is that all the "-o" checks expect <= NF_BR_POST_ROUTING
to mean "builtin", so -1 mistakenly leads to the checks being active.

Reported-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1347
Signed-off-by: Florian Westphal <fw@strlen.de>
5 years agonetfilter: hashlimit: prefer PRIu64 to avoid warnings on 32bit platforms
Duncan Roe [Tue, 10 Sep 2019 21:08:20 +0000 (23:08 +0200)] 
netfilter: hashlimit: prefer PRIu64 to avoid warnings on 32bit platforms

I found this patch attached to an older BZ, apply this finally...

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1107
Signed-off-by: Florian Westphal <fw@strlen.de>
5 years agodoc: Note REDIRECT case of no IP address
Joseph C. Sible [Tue, 20 Aug 2019 20:26:25 +0000 (16:26 -0400)] 
doc: Note REDIRECT case of no IP address

If an IP packet comes in on an interface that lacks a corresponding IP
address (which happens on, e.g., the veth's that Project Calico creates),
attempting to use REDIRECT on it will cause it to be dropped. Take note
of this in REDIRECT's documentation.

Signed-off-by: Joseph C. Sible <josephcsible@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoextensions: nfacct: Fix alignment mismatch in xt_nfacct_match_info
Juliana Rodrigueiro [Tue, 20 Aug 2019 11:30:39 +0000 (13:30 +0200)] 
extensions: nfacct: Fix alignment mismatch in xt_nfacct_match_info

When running a 64-bit kernel with a 32-bit iptables binary, the
size of the xt_nfacct_match_info struct diverges.

    kernel: sizeof(struct xt_nfacct_match_info) : 40
    iptables: sizeof(struct xt_nfacct_match_info)) : 36

This patch is the userspace fix of the memory misalignment.

It introduces a v1 ABI with the correct alignment and stays
compatible with unfixed revision 0 kernels.

Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Drop stale include directive
Phil Sutter [Thu, 1 Aug 2019 11:40:30 +0000 (13:40 +0200)] 
nft: Drop stale include directive

This is a leftover, the file does not exist in fresh clones.

Fixes: 06fd5e46d46f7 ("xtables: Drop support for /etc/xtables.conf")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agodoc: Install ip{6,}tables-restore-translate.8 man pages
Phil Sutter [Tue, 23 Jul 2019 13:24:41 +0000 (15:24 +0200)] 
doc: Install ip{6,}tables-restore-translate.8 man pages

Just like in b738ca3677785 ("doc: Install ip{6,}tables-translate.8
manpages"), create man pages for *-restore-translate tools as semantic
links to xtables-translate.8.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agodoc: Install nft-variant man pages only if enabled
Phil Sutter [Tue, 23 Jul 2019 13:24:40 +0000 (15:24 +0200)] 
doc: Install nft-variant man pages only if enabled

Man pages relevant for nftables backend only (xtables-*, *-translate.8)
were installed even if --disable-nftables was given at configure time.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Drop support for /etc/xtables.conf
Phil Sutter [Thu, 25 Jul 2019 15:19:14 +0000 (17:19 +0200)] 
xtables: Drop support for /etc/xtables.conf

As decided upon at NFWS2019, drop support for configurable nftables base
chains to use with iptables-nft.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Set errno in nft_rule_flush()
Phil Sutter [Thu, 25 Jul 2019 15:19:13 +0000 (17:19 +0200)] 
nft: Set errno in nft_rule_flush()

When trying to flush a non-existent chain, errno gets set in
nft_xtables_config_load(). That is an unintended side-effect and when
support for xtables.conf is later removed, iptables-nft will emit the
generic "Incompatible with this kernel." error message instead of "No
chain/target/match by that name." as it should.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agorestore legacy behaviour of iptables-restore when rules start with -4/-6
Adel Belhouane [Fri, 26 Jul 2019 07:24:37 +0000 (09:24 +0200)] 
restore legacy behaviour of iptables-restore when rules start with -4/-6

v2: moved examples to testcase files

Legacy implementation of iptables-restore / ip6tables-restore allowed
to insert a -4 or -6 option at start of a rule line to ignore it if not
matching the command's protocol. This allowed to mix specific ipv4 and
ipv6 rules in a single file, as still described in iptables 1.8.3's man
page in options -4 and -6. The implementation over nftables doesn't behave
correctly in this case: iptables-nft-restore accepts both -4 or -6 lines
and ip6tables-nft-restore throws an error on -4.

There's a distribution bug report mentioning this problem:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925343

Restore the legacy behaviour:
- let do_parse() return and thus not add a command in those restore
  special cases
- let do_commandx() ignore CMD_NONE instead of bailing out

I didn't attempt to fix all minor anomalies, but just to fix the
regression. For example in the line below, iptables should throw an error
instead of accepting -6 and then adding it as ipv4:

% iptables-nft -6 -A INPUT -p tcp -j ACCEPT

Signed-off-by: Adel Belhouane <bugs.a.b@free.fr>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoutils: nfnl_osf: fix snprintf -Wformat-truncation warning
Fernando Fernandez Mancera [Wed, 24 Jul 2019 07:31:14 +0000 (09:31 +0200)] 
utils: nfnl_osf: fix snprintf -Wformat-truncation warning

Fedora 30 uses very recent gcc (version 9.1.1 20190503 (Red Hat 9.1.1-1)),
osf produces following warnings:

-Wformat-truncation warning have been introduced in the version 7.1 of gcc.
Also, remove a unneeded address check of "tmp + 1" in nf_osf_strchr().

nfnl_osf.c: In function ‘nfnl_osf_load_fingerprints’:
nfnl_osf.c:346:33: warning: ‘%s’ directive output may be truncated writing
up to 1023 bytes into a region of size 128 [-Wformat-truncation=]
  346 |   snprintf(obuf, sizeof(obuf), "%s,", pbeg);
      |                                 ^~
nfnl_osf.c:346:3: note: ‘snprintf’ output between 2 and 1025 bytes into a
destination of size 128
  346 |   snprintf(obuf, sizeof(obuf), "%s,", pbeg);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nfnl_osf.c:354:40: warning: ‘%s’ directive output may be truncated writing
up to 1023 bytes into a region of size 32 [-Wformat-truncation=]
  354 |    snprintf(f.genre, sizeof(f.genre), "%s", pbeg);
      |                                        ^~
nfnl_osf.c:354:4: note: ‘snprintf’ output between 1 and 1024 bytes into a
destination of size 32
  354 |    snprintf(f.genre, sizeof(f.genre), "%s", pbeg);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nfnl_osf.c:363:43: warning: ‘%s’ directive output may be truncated writing
up to 1023 bytes into a region of size 32 [-Wformat-truncation=]
  363 |   snprintf(f.version, sizeof(f.version), "%s", pbeg);
      |                                           ^~
nfnl_osf.c:363:3: note: ‘snprintf’ output between 1 and 1024 bytes into a
destination of size 32
  363 |   snprintf(f.version, sizeof(f.version), "%s", pbeg);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nfnl_osf.c:370:47: warning: ‘%s’ directive output may be truncated writing
up to 1023 bytes into a region of size 32 [-Wformat-truncation=]
  370 |       snprintf(f.subtype, sizeof(f.subtype), "%s", pbeg);
      |                                               ^~
nfnl_osf.c:370:7: note: ‘snprintf’ output between 1 and 1024 bytes into a
destination of size 32
  370 |       snprintf(f.subtype, sizeof(f.subtype), "%s", pbeg);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoebtables-save: Merge into xtables_save_main()
Phil Sutter [Mon, 22 Jul 2019 10:16:28 +0000 (12:16 +0200)] 
ebtables-save: Merge into xtables_save_main()

The only thing missing was handling of EBTABLES_SAVE_COUNTER env var,
but that can be done after parsing parameters in bridge-specific code.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoarptables-save: Merge into xtables_save_main()
Phil Sutter [Mon, 22 Jul 2019 10:16:27 +0000 (12:16 +0200)] 
arptables-save: Merge into xtables_save_main()

With all preparations in place, xtables_save_main() can replace it with
not further changes.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Pass format flags to do_output()
Phil Sutter [Mon, 22 Jul 2019 10:16:26 +0000 (12:16 +0200)] 
xtables-save: Pass format flags to do_output()

Let callers define the flags to pass to nft_rule_save() instead of just
setting the counters boolean.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Make COMMIT line optional
Phil Sutter [Mon, 22 Jul 2019 10:16:25 +0000 (12:16 +0200)] 
xtables-save: Make COMMIT line optional

Explicit commits are not used by either arp- nor ebtables-save. In order
to share code between all the different *-save tools without inducing
changes to ruleset dump contents, allow for callers of do_output() to
turn COMMIT lines on or off.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Pass optstring/longopts to xtables_save_main()
Phil Sutter [Mon, 22 Jul 2019 10:16:24 +0000 (12:16 +0200)] 
xtables-save: Pass optstring/longopts to xtables_save_main()

Introduce variables for the different optstrings so short and long
options live side-by-side.

In order to make xtables_save_main() more versatile, pass optstring and
longopts via parameter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Avoid mixed code and declarations
Phil Sutter [Mon, 22 Jul 2019 10:16:23 +0000 (12:16 +0200)] 
xtables-save: Avoid mixed code and declarations

Also move time() calls to where they are used.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Make nft_for_each_table() more versatile
Phil Sutter [Mon, 22 Jul 2019 10:16:22 +0000 (12:16 +0200)] 
nft: Make nft_for_each_table() more versatile

Support passing arbitrary data (via void pointer) to the callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Fix table compatibility check
Phil Sutter [Mon, 22 Jul 2019 10:16:21 +0000 (12:16 +0200)] 
xtables-save: Fix table compatibility check

The builtin table check guarding the 'is incompatible' warning was
wrong: The idea was to print the warning only for incompatible tables
which are builtin, not for others. Yet the code would print the warning
only for non-builtin ones.

Also reorder the checks: nft_table_builtin_find() is fast and therefore
a quick way to bail for uninteresting tables. The compatibility check is
needed for the remaining tables, only.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Unify *-save header/footer comments
Phil Sutter [Mon, 22 Jul 2019 10:16:20 +0000 (12:16 +0200)] 
xtables-save: Unify *-save header/footer comments

Make eb- and arptables-save print both header and footer comments, too.
Also print them for each table separately - the timing information is
worth the extra lines in output.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoebtables-save: Fix counter formatting
Phil Sutter [Mon, 22 Jul 2019 10:16:19 +0000 (12:16 +0200)] 
ebtables-save: Fix counter formatting

The initial problem was 'ebtables-save -c' printing iptables-style
counters but at the same time not disabling ebtables-style counter
output (which was even printed in wrong format for ebtables-save).

The code around counter output was complicated enough to motivate a
larger rework:

* Make FMT_C_COUNTS indicate the appended counter style for ebtables.

* Use FMT_EBT_SAVE to distinguish between '-c' style counters and the
  legacy pcnt/bcnt ones.

Consequently, ebtables-save sets format to:

FMT_NOCOUNTS - for no counters
FMT_EBT_SAVE - for iptables-style counters
FMT_EBT_SAVE | FMT_C_COUNTS - for '-c' style counters

For regular ebtables, list_rules() always sets FMT_C_COUNTS
(iptables-style counters are never used there) and FMT_NOCOUNTS if no
counters are requested.

The big plus is if neither FMT_NOCOUNTS nor FMT_C_COUNTS is set,
iptables-style counters are to be printed - both in iptables and
ebtables. This allows to drop the ebtables-specific 'save_counters'
callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoebtables: Fix error message for invalid parameters
Phil Sutter [Mon, 22 Jul 2019 10:16:18 +0000 (12:16 +0200)] 
ebtables: Fix error message for invalid parameters

With empty ruleset, ebtables-nft would report the wrong argv:

| % sudo ./install/sbin/ebtables-nft -vnL
| ebtables v1.8.3 (nf_tables): Unknown argument: './install/sbin/ebtables-nft'
| Try `ebtables -h' or 'ebtables --help' for more information.

After a (successful) call to 'ebtables-nft -L', this would even
segfault:

| % sudo ./install/sbin/ebtables-nft -vnL
| zsh: segmentation fault  sudo ./install/sbin/ebtables-nft -vnL

Fixes: acde6be32036f ("ebtables-translate: Fix segfault while parsing extension options")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Use argv[0] as program name
Phil Sutter [Thu, 18 Jul 2019 12:44:09 +0000 (14:44 +0200)] 
xtables-save: Use argv[0] as program name

Don't hard-code program names. This also fixes for bogus 'xtables-save'
name which is no longer used.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: exit in case we can't fetch current genid
Florian Westphal [Sun, 14 Jul 2019 08:49:28 +0000 (10:49 +0200)] 
nft: exit in case we can't fetch current genid

When running iptables -nL as non-root user, iptables would loop indefinitely.

With this change, it will fail with
iptables v1.8.3 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)

Reported-by: Amish <anon.amish@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoextensions/libxt_MASQUERADE.man: random and random-fully are now identical
Florian Westphal [Thu, 11 Jul 2019 08:14:06 +0000 (10:14 +0200)] 
extensions/libxt_MASQUERADE.man: random and random-fully are now identical

Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agonft: Move send/receive buffer sizes into nft_handle
Phil Sutter [Wed, 3 Jul 2019 07:36:26 +0000 (09:36 +0200)] 
nft: Move send/receive buffer sizes into nft_handle

Store them next to the mnl_socket pointer. While being at it, add a
comment to mnl_set_rcvbuffer() explaining why the buffer size is
changed.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Pass nft_handle down to mnl_batch_talk()
Phil Sutter [Wed, 3 Jul 2019 07:36:25 +0000 (09:36 +0200)] 
nft: Pass nft_handle down to mnl_batch_talk()

>From there, pass it along to mnl_nft_socket_sendmsg() and further down
to mnl_set_{snd,rcv}buffer(). This prepares the code path for keeping
stored socket buffer sizes in struct nft_handle.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Set socket receive buffer
Phil Sutter [Tue, 2 Jul 2019 18:30:49 +0000 (20:30 +0200)] 
nft: Set socket receive buffer

When trying to delete user-defined chains in a large ruleset,
iptables-nft aborts with "No buffer space available". This can be
reproduced using the following script:

| #! /bin/bash
| iptables-nft-restore <(
|
| echo "*filter"
| for i in $(seq 0 200000);do
|         printf ":chain_%06x - [0:0]\n" $i
| done
| for i in $(seq 0 200000);do
|         printf -- "-A INPUT -j chain_%06x\n" $i
|         printf -- "-A INPUT -j chain_%06x\n" $i
| done
| echo COMMIT
|
| )
| iptables-nft -X

The problem seems to be the sheer amount of netlink error messages sent
back to user space (one EBUSY for each chain). To solve this, set
receive buffer size depending on number of commands sent to kernel.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoiptables-tests: fix python3
Shekhar Sharma [Thu, 20 Jun 2019 10:49:32 +0000 (16:19 +0530)] 
iptables-tests: fix python3

This converts the iptables-test.py file to run on both python2 and
python3.  The error regarding out.find() has been fixed by using method
.encode('utf-8') in its argument.

Signed-off-by: Shekhar Sharma <shekhar250198@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoextensions: libxt_owner: Add supplementary groups option
Lukasz Pawelczyk [Mon, 10 Jun 2019 10:58:56 +0000 (12:58 +0200)] 
extensions: libxt_owner: Add supplementary groups option

The --suppl-groups option causes GIDs specified with --gid-owner to be
also checked in the supplementary groups of a process.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-restore: Fix program names in help texts
Phil Sutter [Sat, 8 Jun 2019 17:34:13 +0000 (19:34 +0200)] 
xtables-restore: Fix program names in help texts

Avoid referring to wrong or even non-existent commands:

* When calling xtables_restore_main(), pass the actual program name
  taken from argv[0].
* Use 'prog_name' in unknown parameter and help output instead of
  'xtables-restore' which probably doesn't exist.
* While being at it, fix false whitespace in help text.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agosrc: replace IPTABLES_VERSION by PACKAGE_VERSION
Jan Engelhardt [Tue, 28 May 2019 09:43:26 +0000 (11:43 +0200)] 
src: replace IPTABLES_VERSION by PACKAGE_VERSION

The IPTABLES_VERSION C macro replicates the PACKAGE_VERSION C macro
(both have the same definition, "@PACKAGE_VERSION@"). Since
IPTABLES_VERSION, being located in internal.h, is not exposed to
downstream users in any way, it can just be replaced by
PACKAGE_VERSION, which saves a configure-time file substitution.
This goes towards eliminating unnecessary rebuilds after rerunning
./configure.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agobuild: remove -Wl,--no-as-needed and libiptc.so
Jan Engelhardt [Tue, 28 May 2019 09:18:32 +0000 (11:18 +0200)] 
build: remove -Wl,--no-as-needed and libiptc.so

Despite the presence of --no-as-needed, the libiptc.so library as
produced inside the openSUSE Build Service has no links to
libip4tc.so or libip6tc.so. I have not looked into why --no-as-needed
is ignored in this instance, but likewise, the situation must have
been like that ever since openSUSE made as-needed a distro-wide
default (gcc 4.8 timeframe or so).

Since I am not aware of any problem reports within SUSE/openSUSE
about this whole situation, it seems safe to assume no one in the
larger scope is still using a bare "-liptc" on the linker command
line and that all parties have moved on to using pkg-config.

Therefore, libiptc.la/so is hereby removed, as are all parts
related to the -Wl,--no-as-needed flag.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoconfigure: bump versions for 1.8.3 release v1.8.3
Pablo Neira Ayuso [Mon, 27 May 2019 15:05:45 +0000 (17:05 +0200)] 
configure: bump versions for 1.8.3 release

Bump version dependency on libnftnl since this needs new
nftnl_chain_rule_*() functions.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoRevert "build: don't include tests in released tarball"
Phil Sutter [Mon, 20 May 2019 11:43:57 +0000 (13:43 +0200)] 
Revert "build: don't include tests in released tarball"

This reverts commit 4b187eeed49dc507d38438affabe90d36847412d.

Having the testsuites available in release tarball is helpful for
SRPM-based CI at least. The other two suites are included already, so
it's actually 2:1 keep or drop.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoDrop release.sh
Phil Sutter [Mon, 20 May 2019 11:44:07 +0000 (13:44 +0200)] 
Drop release.sh

Last change in 2010, version number hardcoded - strong evidence this
script is not used anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: reset netlink sender buffer size of socket restart
Pablo Neira Ayuso [Mon, 20 May 2019 18:46:40 +0000 (20:46 +0200)] 
nft: reset netlink sender buffer size of socket restart

Otherwise, mnl_set_sndbuffer() skips the buffer update after socket
restart. Then, sendmsg() fails with EMSGSIZE later on when sending the
batch to the kernel.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: do not retry on EINTR
Pablo Neira Ayuso [Mon, 20 May 2019 16:39:31 +0000 (18:39 +0200)] 
nft: do not retry on EINTR

Patch ab1cd3b510fa ("nft: ensure cache consistency") already handles
consistency via generation ID.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: don't care about previous state in ERESTART
Pablo Neira Ayuso [Mon, 20 May 2019 14:10:06 +0000 (16:10 +0200)] 
nft: don't care about previous state in ERESTART

We need to re-evalute based on the existing cache generation.

Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: don't skip table addition from ERESTART
Pablo Neira Ayuso [Mon, 20 May 2019 14:03:33 +0000 (16:03 +0200)] 
nft: don't skip table addition from ERESTART

I don't find a scenario that trigger this case.

Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Fix for explicit rule flushes
Phil Sutter [Mon, 13 May 2019 16:32:37 +0000 (18:32 +0200)] 
xtables: Fix for explicit rule flushes

The commit this fixes added a new parameter to __nft_rule_flush() to
mark a rule flush job as implicit or not. Yet the code added to that
function ignores the parameter and instead always sets batch job's
'implicit' flag to 1.

Fixes: 77e6a93d5c9dc ("xtables: add and set "implict" flag on transaction objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: keep original cache in case of ERESTART
Pablo Neira Ayuso [Sun, 19 May 2019 16:58:40 +0000 (18:58 +0200)] 
nft: keep original cache in case of ERESTART

Phil Sutter says:

"The problem is that data in h->obj_list potentially sits in cache, too.
At least rules have to be there so insert with index works correctly. If
the cache is flushed before regenerating the batch, use-after-free
occurs which crashes the program."

This patch keeps around the original cache until we have refreshed the
batch.

Fixes: 862818ac3a0de ("xtables: add and use nft_build_cache")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: ensure cache consistency
Pablo Neira Ayuso [Mon, 20 May 2019 09:16:21 +0000 (11:16 +0200)] 
nft: ensure cache consistency

Check for generation ID before and after fetching the cache to ensure
consistency.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: cache table list
Pablo Neira Ayuso [Mon, 20 May 2019 08:51:26 +0000 (10:51 +0200)] 
nft: cache table list

nft_table_find() uses the table list cache to look up for existing
tables.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: add flush_cache()
Pablo Neira Ayuso [Sun, 19 May 2019 11:25:23 +0000 (13:25 +0200)] 
nft: add flush_cache()

This new function takes a struct nft_cache as parameter.

This patch also introduces __nft_table_builtin_find() which is required
to look up for built-in tables without the nft_handle structure.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: add __nft_table_builtin_find()
Pablo Neira Ayuso [Sun, 19 May 2019 16:35:02 +0000 (18:35 +0200)] 
nft: add __nft_table_builtin_find()

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: statify nft_rebuild_cache()
Pablo Neira Ayuso [Sun, 19 May 2019 11:04:13 +0000 (13:04 +0200)] 
nft: statify nft_rebuild_cache()

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: add struct nft_cache
Pablo Neira Ayuso [Sun, 19 May 2019 10:54:19 +0000 (12:54 +0200)] 
nft: add struct nft_cache

Add new structure that encloses the cache and update the code to use it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoman: refer to iptables-translate and ip6tables
Pablo Neira Ayuso [Tue, 14 May 2019 12:46:41 +0000 (14:46 +0200)] 
man: refer to iptables-translate and ip6tables

Instead of xtables-translate. Remove old reference to xtables-compat.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agotests: Fix ipt-restore/0004-restore-race_0 testcase
Phil Sutter [Tue, 14 May 2019 11:46:00 +0000 (13:46 +0200)] 
tests: Fix ipt-restore/0004-restore-race_0 testcase

Two issues fixed:

* XTABLES_LIBDIR was set wrong (CWD is not topdir but tests/). Drop the
  export altogether, the testscript does this already.

* $LINES is a variable set by bash, so initial dump sanity check failed
  all the time complaining about a spurious initial dump line count. Use
  $LINES1 instead.

Fixes: 4000b4cf2ea38 ("tests: add test script for race-free restore")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Don't leak iter in error path of __nft_chain_zero_counters()
Phil Sutter [Mon, 13 May 2019 17:12:24 +0000 (19:12 +0200)] 
xtables: Don't leak iter in error path of __nft_chain_zero_counters()

If batch_rule_add() fails, this function leaked the rule iterator
object.

Fixes: 4c54c892443c2 ("xtables: Catch errors when zeroing rule rounters")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoextensions: SYNPROXY: should not be needed anymore on current kernels
Florian Westphal [Fri, 3 May 2019 10:35:38 +0000 (12:35 +0200)] 
extensions: SYNPROXY: should not be needed anymore on current kernels

SYN packets do not require taking the listener socket lock anymore
as of 4.4 kernel, i.e. this target should not be needed anymore.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxshared: check for maximum buffer length in add_param_to_argv()
Pablo Neira Ayuso [Mon, 22 Apr 2019 21:17:27 +0000 (23:17 +0200)] 
xshared: check for maximum buffer length in add_param_to_argv()

Bail out if we go over the boundary, based on patch from Sebastian.

Reported-by: Sebastian Neef <contact@0day.work>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agotests: add test script for race-free restore
Florian Westphal [Tue, 23 Apr 2019 13:16:25 +0000 (15:16 +0200)] 
tests: add test script for race-free restore

xtables-nft-restore ignores -w, check that we don't add
duplicate rules when parallel restores happen.

With a slightly older iptables-nft version this ususally fails with:
I: [EXECUTING] iptables/tests/shell/testcases/ipt-restore/0004-restore-race_0
iptables-restore v1.8.2 (nf_tables):
line 5: CHAIN_USER_ADD failed (File exists): chain UC-0
line 6: CHAIN_USER_ADD failed (File exists): chain UC-1
W: [FAILED] ipt-restore/0004-restore-race_0: expected 0 but got 4

or
I: [EXECUTING]   iptables/tests/shell/testcases/ipt-restore/0004-restore-race_0
iptables-restore v1.8.2 (nf_tables):
line 1: TABLE_FLUSH failed (No such file or directory): table filter

or
/tmp/tmp.SItN4URxxF /tmp/tmp.P1y4LIxhTl differ: byte 7159, line 137

As the legacy version should not have such race (due to nature
of full-table-replace), only do one iteration for legacy case.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: handle concurrent ruleset modifications
Florian Westphal [Tue, 23 Apr 2019 13:16:24 +0000 (15:16 +0200)] 
xtables: handle concurrent ruleset modifications

We currently race when several xtables-nft-restore processes attempt to
handle rules in parallel.  For instance, when no rules are present at
all, then

iptables-nft-restore < X & iptables-nft-restore < X

... can cause rules to be restored twice.

Reason is that both processes might detect 'no rules exist', so
neither issues a flush operation.

We can't unconditionally issue the flush, because it would
cause kernel to fail with -ENOENT unless the to-be-flushed table
exists.

This change passes the generation id that was used to build
the transaction to the kernel.

In case another process changed *any* rule somewhere, the transaction
will now fail with -ERESTART.

We then flush the cache, re-fetch the ruleset and refresh
our transaction.

For example, in the above 'parallel restore' case, the iptables-restore
instance that lost the race would detect that the table has been created
already, and would add the needed flush.

In a similar vein, in case --noflush is used, we will add the flush
op for user-defined chains that were created in the mean-time.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: add and set "implict" flag on transaction objects
Florian Westphal [Tue, 23 Apr 2019 13:16:23 +0000 (15:16 +0200)] 
xtables: add and set "implict" flag on transaction objects

Its used to flag the rule flushes that get added in user-defined-chains
that get redefined with --noflush.

IOW, those objects that are added not by explicit instruction but
to keep semantics.

With --noflush, iptables-legacy-restore will behave as if
-F USERCHAIN was given, in case USERCHAIN exists and USERCHAIN gets
redefined, i.e.:

 iptables-save v1.8.2 on Thu Apr 18 17:11:05 2019
*filter
:USERCHAIN - [0:0]
COMMIT

... will remove all existing rules from USERCHAIN.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: add and use nft_build_cache
Florian Westphal [Tue, 23 Apr 2019 13:16:22 +0000 (15:16 +0200)] 
xtables: add and use nft_build_cache

Will be used with the "generation id" infrastructure.
When we're told that the commit failed because someone else made
changes, we can use this to re-initialize the cache and then
revalidate the transaction list (e.g. to detect that we now have
to flush the user-defined chain 'foo' that we wanted to create, but
was added just now by someone else).

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: add skip flag to objects
Florian Westphal [Tue, 23 Apr 2019 13:16:21 +0000 (15:16 +0200)] 
xtables: add skip flag to objects

This will be used to skip transaction objects when committing to
kernel.  This is needed for example when we restore a table that
doesn't exist yet.  In such a case we would already build a flush
operation so we can just enable it when we hit problem with the
generation id and we find that the table/chain was already created
in the mean time.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: unify user chain add/flush for restore case
Florian Westphal [Tue, 23 Apr 2019 13:16:20 +0000 (15:16 +0200)] 
xtables: unify user chain add/flush for restore case

The idea here is to move the 'flush' decision into the core, rather than
have the decision in the frontend.

This will be required later when "generation id" is passed to kernel.
In this case, we might have to add the flush when re-trying the
transaction.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agotests: return-codes script is bash specific
Florian Westphal [Fri, 19 Apr 2019 20:20:10 +0000 (22:20 +0200)] 
tests: return-codes script is bash specific

The script fails on systems where sh is not bash.

Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: libxt_osf.: Typo in manpage
Sam Banks [Thu, 21 Mar 2019 23:22:47 +0000 (12:22 +1300)] 
extensions: libxt_osf.: Typo in manpage

Signed-off-by: Sam Banks <sam.banks.nz@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-legacy: add missing config.h include
Lucas Stach [Fri, 8 Mar 2019 14:37:09 +0000 (15:37 +0100)] 
xtables-legacy: add missing config.h include

This fixes a IPv4 only build, where this file would have references to
functions that aren't built in this case. I'm not sure how it ends up
with ENABLE_IPV6 defined without the config.h include, but since this
was clearly missing and fixed my issue, I didn't bother tracking down
the chain.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoman: iptables-save: Add note about module autoloading
Phil Sutter [Tue, 26 Mar 2019 18:03:43 +0000 (19:03 +0100)] 
man: iptables-save: Add note about module autoloading

Using '-t' parameter in iptables-save might lead to kernel module
loading, just like with iptables itself. Copy the hint from iptables.8
to inform users.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoextensions: Install symlinks as such
Phil Sutter [Fri, 22 Mar 2019 18:31:06 +0000 (19:31 +0100)] 
extensions: Install symlinks as such

Fake shared objects which are actually symlinks to others are installed
using 'install' tool which follows them and therefore installs a copy of
the file they point at. Fix this by introducing special handling for
them in install target.

Reported-by: Wenle Chen <solachenclever@hotmail.com>
Fixes: 269655d54e22f ("build: remove symlink-only extensions from static object list")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables-save: Point at existing man page in help text
Phil Sutter [Wed, 13 Mar 2019 19:46:17 +0000 (20:46 +0100)] 
xtables-save: Point at existing man page in help text

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxtables-legacy.8: Remove stray colon
Phil Sutter [Wed, 13 Mar 2019 19:46:16 +0000 (20:46 +0100)] 
xtables-legacy.8: Remove stray colon

This obviously doesn't belong there.

Fixes: be70918eab26e ("xtables: rename xt-multi binaries to -nft, -legacy")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agodoc: Adjust ebtables man page
Phil Sutter [Wed, 13 Mar 2019 19:46:15 +0000 (20:46 +0100)] 
doc: Adjust ebtables man page

Change content to match nft-variant, most notably:

* There is no broute table, drop all references to it
* Comment out description of among and string matches, we don't support
  them (yet)

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agodoc: Add ebtables man page
Phil Sutter [Wed, 13 Mar 2019 19:46:14 +0000 (20:46 +0100)] 
doc: Add ebtables man page

This is a 1:1 copy from legacy ebtables repository.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agodoc: Adjust arptables man pages
Phil Sutter [Wed, 13 Mar 2019 19:46:13 +0000 (20:46 +0100)] 
doc: Adjust arptables man pages

Change content to suit the shipped nft-based variant. Most relevant
changes:

* FORWARD chain is not supported
* arptables-nft-save supports a few parameters

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agodoc: Add arptables-nft man pages
Phil Sutter [Wed, 13 Mar 2019 19:46:12 +0000 (20:46 +0100)] 
doc: Add arptables-nft man pages

These are 1:1 copies from legacy arptables repository.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: connlabel: Fallback on missing connlabel.conf
Phil Sutter [Mon, 4 Mar 2019 15:53:46 +0000 (16:53 +0100)] 
extensions: connlabel: Fallback on missing connlabel.conf

If connlabel.conf was not found, fall back to manually parsing arguments
as plain numbers.

If nfct_labelmap_new() has failed, nfct_labelmap_get_name() segfaults.
Therefore make sure it is not called in connlabel_get_name() if that's
the case.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: Add testcase for libxt_ipvs
Phil Sutter [Thu, 21 Feb 2019 19:09:32 +0000 (20:09 +0100)] 
extensions: Add testcase for libxt_ipvs

Given that it is fixed now, make it stay.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: Fix ipvs vproto option printing
Phil Sutter [Thu, 21 Feb 2019 19:09:31 +0000 (20:09 +0100)] 
extensions: Fix ipvs vproto option printing

This was broken since day 1: vproto option was printed as 'proto' which
in turn iptables wouldn't accept anymore.

Fixes: c36d05e424069 ("libxt_ipvs: user-space lib for netfilter matcher xt_ipvs")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: Fix ipvs vproto parsing
Phil Sutter [Thu, 21 Feb 2019 19:09:30 +0000 (20:09 +0100)] 
extensions: Fix ipvs vproto parsing

This was broken by integration into guided option parser:

* Make 'vproto' option XTTYPE_PROTOCOL, otherwise its arguments are
  parsed as garbage only.

* Drop O_VPROTO case from ipvs_mt_parse(), due to XTOPT_POINTER() and
  above change there is nothing to do for it in there.

Fixes: 372203af4c70f ("libxt_ipvs: use guided option parser")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: AUDIT: Document ineffective --type option
Phil Sutter [Thu, 21 Feb 2019 14:38:47 +0000 (15:38 +0100)] 
extensions: AUDIT: Document ineffective --type option

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agodoc: Install ip{6,}tables-translate.8 manpages
Phil Sutter [Wed, 20 Feb 2019 13:02:55 +0000 (14:02 +0100)] 
doc: Install ip{6,}tables-translate.8 manpages

These are just semantic links to xtables-translate.8.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agotests/shell: Support testing host binaries
Phil Sutter [Tue, 19 Feb 2019 19:39:50 +0000 (20:39 +0100)] 
tests/shell: Support testing host binaries

Add -H/--host parameter to run the testsuite against host system's
binaries.

While being at it, rewrite parameter parsing:

* Parse all parameters in a loop, this frees any ordering constraints.
* Set extglob option so strict pattern matching for single testcase mode
  can be done via bash globbing.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxlate-test: Support testing host binaries
Phil Sutter [Tue, 19 Feb 2019 19:39:49 +0000 (20:39 +0100)] 
xlate-test: Support testing host binaries

Introduce --host parameter to run the testsuite against host's binaries
instead of built ones.

Apparently, extending PATH variable in main() was redundant with
explicit full path call in run_test() so drop the former.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoarptables-nft: fix decoding of hlen on bigendian platforms
Florian Westphal [Fri, 22 Feb 2019 12:26:05 +0000 (13:26 +0100)] 
arptables-nft: fix decoding of hlen on bigendian platforms

The existing test fail with:
extensions/libarpt_standard.t: ERROR: line 2 (cannot find: arptables -I INPUT -s 192.168.0.1)

... because hlen is 0 instead of expected "6".
The rule is correct, i.e. this is a decode/display bug: arp_hlen is
specified as 'unsigned short' instead of uint8_t.

On LSB systems, this doesn't matter but on MSB the value then is '0x600'
instead of '0x006' which becomes 0 when assignment to the u8 header field.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Phil Sutter <phil@nwl.cc>
6 years agoarptables: Print space before comma and counters
Phil Sutter [Fri, 15 Feb 2019 14:27:43 +0000 (15:27 +0100)] 
arptables: Print space before comma and counters

Legacy arptables separates counters from rest of rule by ' , '. Assuming
that scripts scraping 'arptables -vL' output match on this, make
arptables-nft output conformant.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agotests: Extend return codes check by error messages
Phil Sutter [Wed, 13 Feb 2019 10:11:27 +0000 (11:11 +0100)] 
tests: Extend return codes check by error messages

Check that error messages match between legacy and nft code.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Fix error message for chain renaming
Phil Sutter [Wed, 13 Feb 2019 10:11:26 +0000 (11:11 +0100)] 
xtables: Fix error message for chain renaming

If the new name already exists, legacy iptables prints "File exists.".
This is a bit exotic, but more appropriate than "No chain/target/match
by that name." printed by iptables-nft without this patch.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Fix error messages in commands with rule number
Phil Sutter [Wed, 13 Feb 2019 10:11:25 +0000 (11:11 +0100)] 
xtables: Fix error messages in commands with rule number

Use E2BIG if rule identified by given number is not found. ENOENT is
used if referenced chain is not found. Without this, a command
specifying a non-existing chain in combination with a rule number like
e.g.: 'iptables-nft -I nonexist 23 -j ACCEPT' returns "Index of
insertion too big." instead of "No chain/target/match by that name."
like legacy iptables does.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Move new chain check to where it belongs
Phil Sutter [Wed, 13 Feb 2019 10:11:24 +0000 (11:11 +0100)] 
xtables: Move new chain check to where it belongs

Instead of checking chain existence in xtables.c, do it in
nft_chain_user_add() and reuse predefined error message.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agoxtables: Fix error message when zeroing a non-existent chain
Phil Sutter [Wed, 13 Feb 2019 10:11:23 +0000 (11:11 +0100)] 
xtables: Fix error message when zeroing a non-existent chain

Previously, error message was a bit misleading:

| # iptables-nft -Z noexist
| iptables: Incompatible with this kernel.

Set errno value so that the typical "No chain/target/match by that
name." is printed instead.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
6 years agonft: Eliminate dead code in __nft_rule_list
Phil Sutter [Thu, 7 Feb 2019 08:20:10 +0000 (09:20 +0100)] 
nft: Eliminate dead code in __nft_rule_list

If passed a rulenum > 0, the function uses nftnl_rule_lookup_byindex()
and returns early. Negative rulenum values are not supposed to happen,
so the remaining code which iterates over the full list of rules does
not need to respect rulenum anymore.

Fixes: 039b048965210 ("nft: Make use of nftnl_rule_lookup_byindex()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoebtables-nft: Support user-defined chain policies
Phil Sutter [Thu, 7 Feb 2019 21:08:55 +0000 (22:08 +0100)] 
ebtables-nft: Support user-defined chain policies

Legacy ebtables supports policies for user-defined chains - and what's
worse, they default to ACCEPT unlike anywhere else. So lack of support
for this braindead feature in ebtables-nft is actually a change of
behaviour which very likely affects all ebtables users out there.

The solution implemented here uses an implicit (and transparent) last
rule in all user-defined ebtables-nft chains with policy other than
RETURN. This rule is identified by an nft comment
"XTABLES_EB_INTERNAL_POLICY_RULE" (since commit ccf154d7420c0 ("xtables:
Don't use native nftables comments") nft comments are not used
otherwise).

To minimize interference with existing code, this policy rule is removed
from chains during cache population and the policy is saved in
NFTNL_CHAIN_POLICY attribute. When committing changes to the kernel,
nft_commit() traverses through the list of chains and (re-)creates
policy rules if required.

In ebtables-nft-restore, table flushes are problematic. To avoid weird
kernel error responses, introduce a custom 'table_flush' callback which
removes any pending policy rule add/remove jobs prior to creating the
NFT_COMPAT_TABLE_FLUSH one.

I've hidden all this mess behind checks for h->family, so hopefully
impact on {ip,ip6,arp}tables-nft should be negligible.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agonft: Introduce UDATA_TYPE_EBTABLES_POLICY
Phil Sutter [Thu, 7 Feb 2019 21:08:54 +0000 (22:08 +0100)] 
nft: Introduce UDATA_TYPE_EBTABLES_POLICY

This will be used later to identify ebtables user-defined chain policy
rules.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agonft: Don't assume NFTNL_RULE_USERDATA holds a comment
Phil Sutter [Thu, 7 Feb 2019 21:08:53 +0000 (22:08 +0100)] 
nft: Don't assume NFTNL_RULE_USERDATA holds a comment

If this rule attribute is present but does not contain a comment,
get_comment() returns NULL which is then fed into strncpy() causing a
crash.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxtables-save: Fix table not found error message
Phil Sutter [Thu, 7 Feb 2019 21:13:31 +0000 (22:13 +0100)] 
xtables-save: Fix table not found error message

First of all, this error message should not appear on stdout, otherwise
it may end in dump files. Next, with completely empty ruleset, even
valid table names cause errors. To avoid this, continue operation if the
not found table is a builtin one.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxshared: Explicitly pass target to command_jump()
Phil Sutter [Tue, 5 Feb 2019 16:01:42 +0000 (17:01 +0100)] 
xshared: Explicitly pass target to command_jump()

The use of global 'optarg' variable inside that function is a mess, but
most importantly it limits its applicability to input parsers. Fix this
by having it take the option argument as a parameter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoRevert "ebtables: use extrapositioned negation consistently"
Phil Sutter [Tue, 5 Feb 2019 17:18:02 +0000 (18:18 +0100)] 
Revert "ebtables: use extrapositioned negation consistently"

This reverts commit 5f508b76a0cebaf91965ffa678089222e2d47964.

While attempts at unifying syntax between arp-, eb- and iptables-nft
increase the opportunity for more code-sharing, they are problematic
when it comes to compatibility. Accepting the old syntax on input helps,
but due to the fact that neither arptables nor ebtables support --check
command we must expect for users to test existence of a rule by
comparing input with output. If that happens in a script, deviating from
the old syntax in output has a high chance of breaking it.

Therefore revert Florian's patch changing inversion character position
in output and review the old code for consistency - the only thing
changed on top of the actual revert is ebtables' own copy of
print_iface() to make it adhere to the intrapositioned negation scheme
used throughout ebtables.

Added extension tests by the reverted commit have been kept.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxtables: Fix for false-positive rule matching
Phil Sutter [Mon, 4 Feb 2019 20:52:53 +0000 (21:52 +0100)] 
xtables: Fix for false-positive rule matching

When comparing two rules with non-standard targets, differences in
targets' payloads wasn't respected.

The cause is a rather hideous one: Unlike xtables_find_match(),
xtables_find_target() did not care whether the found target was already
in use or not, so the same target instance was assigned to both rules
and therefore payload comparison happened over the same memory location.

With legacy iptables it is not possible to reuse a target: The only case
where two rules (i.e., iptables_command_state instances) could exist at
the same time is when comparing rules, but that's handled using libiptc.

The above change clashes with ebtables-nft's reuse of target objects:
While input parsing still just assigns the object from xtables_targets
list, rule conversion from nftnl to iptables_command_state allocates new
data. To fix this, make ebtables-nft input parsing use the common
command_jump() routine instead of its own simplified copy. In turn, this
also eliminates the ebtables-nft-specific variants of parse_target(),
though with a slight change of behaviour: Names of user-defined chains
are no longer allowed to contain up to 31 but merely 28 characters.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoxtables: Fix for crash when comparing rules with standard target
Phil Sutter [Fri, 1 Feb 2019 18:17:50 +0000 (19:17 +0100)] 
xtables: Fix for crash when comparing rules with standard target

When parsing an nftnl_rule with a standard verdict,
nft_rule_to_iptables_command_state() initialized cs->target but didn't
care about cs->target->t. When later comparing that rule to another,
compare_targets() crashed due to unconditional access to t's fields.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoextensions: Fix arptables extension tests
Phil Sutter [Fri, 1 Feb 2019 16:06:19 +0000 (17:06 +0100)] 
extensions: Fix arptables extension tests

With changes to arptables-nft output, many of these tests fail because
rules are not printed as expected anymore. Since most of the tests with
explicitly defined output did so just because of added --h-length and
--h-type options, adjust input a little more (typically reordering of
arguments) to make output match input.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
6 years agoarptables-nft: Set h-type/h-length masks by default, too
Phil Sutter [Fri, 1 Feb 2019 16:06:18 +0000 (17:06 +0100)] 
arptables-nft: Set h-type/h-length masks by default, too

These masks are not used in nftables backend, but mangle extension
checks arhln_mask value to make sure --h-length was given (which is
implicitly the case).

Fixes: 5aecb2d8bfdda ("arptables: pre-init hlen and ethertype")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>