]> git.ipfire.org Git - thirdparty/iptables.git/log
thirdparty/iptables.git
5 years agoxtables-restore: Fix --table parameter check
Phil Sutter [Fri, 20 Sep 2019 15:31:58 +0000 (17:31 +0200)] 
xtables-restore: Fix --table parameter check

Xtables-restore tries to reject rule commands in input which contain a
--table parameter (since it is adding this itself based on the previous
table line). The manual check was not perfect though as it caught any
parameter starting with a dash and containing a 't' somewhere, even in
rule comments:

| *filter
| -A FORWARD -m comment --comment "- allow this one" -j ACCEPT
| COMMIT

Instead of error-prone manual checking, go a much simpler route: All
do_command callbacks are passed a boolean indicating they're called from
*tables-restore. React upon this when handling a table parameter and
error out if it's not the first one.

Fixes: f8e5ebc5986bf ("iptables: Fix crash on malformed iptables-restore")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agoxtables-restore: Drop chain_list callback
Phil Sutter [Thu, 17 Oct 2019 22:09:01 +0000 (00:09 +0200)] 
xtables-restore: Drop chain_list callback

Since commit 0baa08fed43fa ("xtables: unify user chain add/flush for
restore case") it is not used anymore, so just drop it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Drop local xtc_ops instance
Phil Sutter [Thu, 17 Oct 2019 22:03:00 +0000 (00:03 +0200)] 
xtables-restore: Drop local xtc_ops instance

It is merely used to hold nft_strerror() pointer but using that function
in turn does not provide any benefit as it falls back to plain
strerror() if nft_fn is not initialized.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoiptables-restore: Constify struct iptables_restore_cb
Phil Sutter [Thu, 17 Oct 2019 21:20:22 +0000 (23:20 +0200)] 
iptables-restore: Constify struct iptables_restore_cb

Just like with xtables-restore, these callbacks don't change at
run-time.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Constify struct nft_xt_restore_cb
Phil Sutter [Thu, 17 Oct 2019 20:49:26 +0000 (22:49 +0200)] 
xtables-restore: Constify struct nft_xt_restore_cb

There is no need for dynamic callback mangling, so make all instances
static const.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Introduce rule counter tokenizer function
Phil Sutter [Tue, 17 Sep 2019 16:43:21 +0000 (18:43 +0200)] 
xtables-restore: Introduce rule counter tokenizer function

The same piece of code appears three times, introduce a function to take
care of tokenizing and error reporting.

Pass buffer pointer via reference so it can be updated to point to after
the counters (if found).

While being at it, drop pointless casting when passing pcnt/bcnt to
add_argv().

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Use xt_params->program_name
Phil Sutter [Tue, 17 Sep 2019 14:45:20 +0000 (16:45 +0200)] 
xtables-restore: Use xt_params->program_name

Instead of setting newargv[0] to argv[0]'s value, just use whatever
xt_params->program_name contains. The latter is arbitrarily defined, but
may still be more correct than real argv[0] which may simply be for
instance xtables-nft-multi. Either way, there is no practical
significance since newargv[0] is used exclusively in debug output.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Treat struct nft_xt_restore_parse as const
Phil Sutter [Thu, 17 Oct 2019 19:46:53 +0000 (21:46 +0200)] 
xtables-restore: Treat struct nft_xt_restore_parse as const

This structure contains restore parser configuration, parser is not
supposed to alter it.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Optimize flushing all chains of a table
Phil Sutter [Wed, 28 Aug 2019 10:33:55 +0000 (12:33 +0200)] 
nft: Optimize flushing all chains of a table

Leverage nftables' support for flushing all chains of a table by
omitting NFTNL_RULE_CHAIN attribute in NFT_MSG_DELRULE payload.

The only caveat is with verbose output, as that still requires to have a
list of (existing) chains to iterate over. Apart from that, implementing
this shortcut is pretty straightforward: Don't retrieve a chain list and
just call __nft_rule_flush() directly which doesn't set above attribute
if chain name pointer is NULL.

A bigger deal is keeping rule cache consistent: Instead of just clearing
rule list for each flushed chain, flush_rule_cache() is updated to
iterate over all cached chains of the given table, clearing their rule
lists if not called for a specific chain.

While being at it, sort local variable declarations in nft_rule_flush()
from longest to shortest and drop the loop-local 'chain_name' variable
(but instead use 'chain' function parameter which is not used at that
point).

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Support nft_is_table_compatible() per chain
Phil Sutter [Wed, 25 Sep 2019 16:48:07 +0000 (18:48 +0200)] 
nft: Support nft_is_table_compatible() per chain

When operating on a single chain only, compatibility checking causes
unwanted overhead by checking all chains of the current table. Avoid
this by accepting the current chain name as parameter and pass it along
to nft_chain_list_get().

While being at it, introduce nft_assert_table_compatible() which
calls xtables_error() in case compatibility check fails. If a chain name
was given, include that in error message.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Reduce cache overhead of nft_chain_builtin_init()
Phil Sutter [Wed, 25 Sep 2019 16:20:24 +0000 (18:20 +0200)] 
nft: Reduce cache overhead of nft_chain_builtin_init()

There is no need for a full chain cache, fetch only the few builtin
chains that might need to be created.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft-cache: Support partial rule cache per chain
Phil Sutter [Wed, 2 Oct 2019 19:13:47 +0000 (21:13 +0200)] 
nft-cache: Support partial rule cache per chain

Accept an additional chain name pointer in __nft_build_cache() and pass
it along to fetch only that specific chain and its rules.

Enhance nft_build_cache() to take an optional nftnl_chain pointer to
fetch rules for.

Enhance nft_chain_list_get() to take an optional chain name. If cache
level doesn't include chains already, it will fetch only the specified
chain from kernel (if existing) and add that to table's chain list which
is returned. This keeps operations for all chains of a table or a
specific one within the same code path in nft.c.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft-cache: Support partial cache per table
Phil Sutter [Wed, 25 Sep 2019 11:49:19 +0000 (13:49 +0200)] 
nft-cache: Support partial cache per table

Accept a builtin_table pointer in __nft_build_cache() and pass it along
when fetching chains and rules to operate on that table only (unless the
pointer is NULL).

Make use of it in nft_chain_list_get() since that accepts a table name
and performs a builtin table lookup internally already.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft-cache: Cover for multiple fetcher invocation
Phil Sutter [Mon, 7 Oct 2019 16:40:40 +0000 (18:40 +0200)] 
nft-cache: Cover for multiple fetcher invocation

Preparing for partial caches, it is necessary to make sure these
functions don't cause harm if called repeatedly.

* Use h->cache->tables pointer as indicator for existing table cache,
  return immediately from fetch_table_cache() if non-NULL.

* Initialize table's chain list only if non-NULL.

* Search for chain in table's chain list before adding it.

* Don't fetch rules for a chain if it has any rules already. With rule
  list being embedded in struct nftnl_chain, this is the best way left
  to check if rules have been fetched already or not. It will fail for
  empty chains, but causes no harm in that case, either.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft-cache: Fetch only chains in nft_chain_list_get()
Phil Sutter [Mon, 7 Oct 2019 11:49:08 +0000 (13:49 +0200)] 
nft-cache: Fetch only chains in nft_chain_list_get()

The function is used to return the given table's chains, so fetching
chain cache is enough.

Add calls to nft_build_cache() in places where a rule cache is required.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft-cache: Introduce cache levels
Phil Sutter [Tue, 1 Oct 2019 14:23:24 +0000 (16:23 +0200)] 
nft-cache: Introduce cache levels

Replace the simple have_cache boolean by a cache level indicator
defining how complete the cache is. Since have_cache indicated full
cache (including rules), make code depending on it check for cache level
NFT_CL_RULES.

Core cache fetching routine __nft_build_cache() accepts a new level via
parameter and raises cache completeness to that level.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoiptables-test: Run tests in lexical order
Phil Sutter [Fri, 27 Sep 2019 10:07:46 +0000 (12:07 +0200)] 
iptables-test: Run tests in lexical order

To quickly see if a given test was run or not, sort the file list. Also
filter non-test files right when preparing the list.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Extract cache routines into nft-cache.c
Phil Sutter [Tue, 1 Oct 2019 13:09:55 +0000 (15:09 +0200)] 
nft: Extract cache routines into nft-cache.c

The amount of code dealing with caching only is considerable and hence
deserves an own source file.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Avoid nested cache fetching
Phil Sutter [Mon, 7 Oct 2019 10:35:21 +0000 (12:35 +0200)] 
nft: Avoid nested cache fetching

Don't call fetch_table_cache() from within fetch_chain_cache() but
instead from __nft_build_cache(). Since that is the only caller of
fetch_chain_cache(), this change should not have any effect in practice.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Pass nft_handle to flush_cache()
Phil Sutter [Tue, 1 Oct 2019 13:14:48 +0000 (15:14 +0200)] 
nft: Pass nft_handle to flush_cache()

This allows to call nft_table_builtin_find() and hence removes the only
real user of __nft_table_builtin_find(). Consequently remove the latter
by integrating it into its sole caller.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoextensions: libxt_SYNPROXY: add xlate method
Jose M. Guisado Gomez [Mon, 30 Sep 2019 17:28:07 +0000 (19:28 +0200)] 
extensions: libxt_SYNPROXY: add xlate method

This adds translation capabilities when encountering SYNPROXY inside
iptables rules.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables-restore: Minimize caching when flushing
Phil Sutter [Wed, 25 Sep 2019 09:29:59 +0000 (11:29 +0200)] 
xtables-restore: Minimize caching when flushing

Unless --noflush was given, xtables-restore merely needs the list of
tables to decide whether to delete it or not. Introduce nft_fake_cache()
function which populates table list, initializes chain lists (so
nft_chain_list_get() returns an empty list instead of NULL) and sets
'have_cache' to turn any later calls to nft_build_cache() into nops.

If --noflush was given, call nft_build_cache() just once instead of for
each table line in input.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agonft: Make nftnl_table_list_get() fetch only tables
Phil Sutter [Tue, 24 Sep 2019 14:57:24 +0000 (16:57 +0200)] 
nft: Make nftnl_table_list_get() fetch only tables

No need for a full cache to serve the list of tables.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agonft: Fix for add and delete of same rule in single batch
Phil Sutter [Tue, 3 Sep 2019 16:10:55 +0000 (18:10 +0200)] 
nft: Fix for add and delete of same rule in single batch

Another corner-case found when extending restore ordering test: If a
delete command in a dump referenced a rule added earlier within the same
dump, kernel would reject the resulting NFT_MSG_DELRULE command.

Catch this by assigning the rule to delete a RULE_ID value if it doesn't
have a handle yet. Since __nft_rule_del() does not duplicate the
nftnl_rule object when creating the NFT_COMPAT_RULE_DELETE command, this
RULE_ID value is added to both NEWRULE and DELRULE commands - exactly
what is needed to establish the reference.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agotests: shell: Support running for legacy/nft only
Phil Sutter [Wed, 25 Sep 2019 10:54:55 +0000 (12:54 +0200)] 
tests: shell: Support running for legacy/nft only

After some changes, one might want to test a single variant only. Allow
this by supporting -n/--nft and -l/--legacy parameters, each disabling
the other variant.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agotests/shell: Speed up ipt-restore/0004-restore-race_0
Phil Sutter [Tue, 3 Sep 2019 13:22:39 +0000 (15:22 +0200)] 
tests/shell: Speed up ipt-restore/0004-restore-race_0

This test tended to cause quite excessive load on my system, sometimes
taking longer than all other tests combined. Even with the reduced
numbers, it still fails reliably after reverting commit 58d7de0181f61
("xtables: handle concurrent ruleset modifications").

Fixes: 4000b4cf2ea38 ("tests: add test script for race-free restore")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agonft: Get rid of NFT_COMPAT_EXPR_MAX define
Phil Sutter [Tue, 20 Aug 2019 22:28:21 +0000 (00:28 +0200)] 
nft: Get rid of NFT_COMPAT_EXPR_MAX define

Instead simply use ARRAY_SIZE() macro to not overstep supported_exprs
array.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Fix typo in nft_parse_limit() error message
Phil Sutter [Tue, 20 Aug 2019 09:34:58 +0000 (11:34 +0200)] 
nft: Fix typo in nft_parse_limit() error message

Seems like a trivial copy'n'paste bug.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoxtables_error() does not return
Phil Sutter [Tue, 17 Sep 2019 15:53:31 +0000 (17:53 +0200)] 
xtables_error() does not return

It's a define which resolves into a callback which in turn is declared
with noreturn attribute. It will never return, therefore drop all
explicit exit() calls or other dead code immediately following it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>
5 years agonft: Fix add_bitwise_u16() on Big Endian
Phil Sutter [Fri, 20 Sep 2019 09:19:15 +0000 (11:19 +0200)] 
nft: Fix add_bitwise_u16() on Big Endian

Type used for 'mask' and 'xor' parameters was wrong, 'int' is four bytes
on 32 or 64 bit architectures. After casting a uint16_t to int, on Big
Endian the first two bytes of data are (the leading) zero which libnftnl
then copies instead of the actual value.

This problem was noticed when using '--fragment' option:

| # iptables-nft -A FORWARD --fragment -j ACCEPT
| # nft list ruleset | grep frag-off
| ip frag-off & 0 != 0 counter packets 0 bytes 0 accept

With this fix in place, the resulting nft rule is correct:

| ip frag-off & 8191 != 0 counter packets 0 bytes 0 accept

Fixes: 2f1fbab671576 ("iptables: nft: add -f support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft Increase mnl_talk() receive buffer size
Phil Sutter [Wed, 28 Aug 2019 20:10:40 +0000 (22:10 +0200)] 
nft Increase mnl_talk() receive buffer size

This improves cache population quite a bit and therefore helps when
dealing with large rulesets. A simple hard to improve use-case is
listing the last rule in a large chain. These are the average program
run times depending on number of rules:

rule count | legacy | nft old | nft new
---------------------------------------------------------
 50,000 | .052s | .611s | .406s
100,000 | .115s | 2.12s | 1.24s
150,000 | .265s | 7.63s | 4.14s
200,000 | .411s | 21.0s | 10.6s

So while legacy iptables is still magnitudes faster, this simple change
doubles iptables-nft performance in ideal cases.

Note that using a larger buffer than 32KB doesn't further improve
performance since linux kernel won't transmit more data at once. This
limit was set (actually extended from 16KB) in kernel commit
d35c99ff77ecb ("netlink: do not enter direct reclaim from
netlink_dump()").

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Introduce nft_bridge_commit()
Phil Sutter [Fri, 30 Aug 2019 09:47:42 +0000 (11:47 +0200)] 
nft: Introduce nft_bridge_commit()

No need to check family value from nft_commit() if we can have a
dedicated callback for bridge family.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agonft: Use nftnl_*_set_str() functions
Phil Sutter [Tue, 3 Sep 2019 15:46:16 +0000 (17:46 +0200)] 
nft: Use nftnl_*_set_str() functions

Although it doesn't make a difference in practice, they are the correct
API functions to use when assigning string attributes.

While doing so, also drop the needless casts to non-const.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoDEBUG: Print to stderr to not disturb iptables-save
Phil Sutter [Fri, 23 Aug 2019 16:18:33 +0000 (18:18 +0200)] 
DEBUG: Print to stderr to not disturb iptables-save

This way there's at least a chance to get meaningful results from
testsuite with debugging being turned on.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agotests/shell: Make ebtables-basic test more verbose
Phil Sutter [Tue, 14 May 2019 16:26:54 +0000 (18:26 +0200)] 
tests/shell: Make ebtables-basic test more verbose

Print expected entries count if it doesn't match.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agoextensions: fix iptables-{nft,translate} with conntrack EXPECTED
Quentin Armitage [Tue, 17 Sep 2019 18:36:32 +0000 (19:36 +0100)] 
extensions: fix iptables-{nft,translate} with conntrack EXPECTED

iptables-translate -A INPUT -m conntrack --ctstatus EXPECTED,ASSURED
  outputs:
nft add rule ip filter INPUT ct status expected,assured counter
  and
iptables-nft -A INPUT -m conntrack --ctstatus EXPECTED,ASSURED
  produces nft list output:
chain INPUT {
ct status expected,assured counter packets 0 bytes 0 accept
}
which are correct.

However,
iptables-translate -A INPUT -m conntrack --ctstatus EXPECTED
  outputs:
nft # -A INPUT -m conntrack --ctstatus EXPECTED
  and
iptables-nft -A INPUT -m conntrack --ctstatus EXPECTED
  produces nft list output:
chain INPUT {
          counter packets 0 bytes 0 accept
}
neither of which is what is desired.

Commit 6223ead0d - "extensions: libxt_conntrack: Add translation to nft"
included the following code in _conntrack3_mt_xlate():
if (sinfo->match_flags & XT_CONNTRACK_STATUS) {
if (sinfo->status_mask == 1)
return 0;
...

If the intention had been not to produce output when status_mask == 1,
it would have been written as:
                if (sinfo->status_mask == IPS_EXPECTED)
                        return 0;
so it looks as though this is debugging code accidently left in the
original patch.

Removing the lines:
                if (sinfo->status_mask == 1)
                        return 0;
resolves the problems, and
iptables-translate -A INPUT -m conntrack --ctstatus EXPECTED
  outputs:
nft add rule ip filter INPUT ct status expected counter
  and
iptables-nft -A INPUT -m conntrack --ctstatus EXPECTED
  produces nft list output:
chain INPUT {
        ct status expected counter packets 0 bytes 0 accept
}

This commit also includes an additional txlate test to check when
only the status EXPECTED is specified.

Fixes: 6223ead0d06b ("extensions: libxt_conntrack: Add translation to nft")
Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>
5 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>