Another process might race to add chains after chain_cache_init().
The generation check does not help since it comes after cache_init().
NLM_F_DUMP_INTR only guarantees consistency within one single netlink
dump operation, so it does not help either (cache population requires
several netlink dump commands).
Let's be safe and do not assume the chain exists in the cache when
populating the rule cache.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
- Chains that reside in the cache are stored in the new
tables->cache_chain and tables->cache_chain_ht. The hashtable chain
cache allows for fast chain lookups.
- Chains that defined via command line / ruleset file reside in
tables->chains.
Note that chains in the cache (already in the kernel) are not placed in
the table->chains.
By keeping separated lists, chains defined via command line / ruleset
file can be added to cache.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Florian Westphal [Tue, 30 Mar 2021 23:26:19 +0000 (01:26 +0200)]
netlink: don't crash when set elements are not evaluated as expected
define foo = 2001:db8:123::/48
table inet filter {
set foo {
typeof ip6 saddr
elements = $foo
}
}
gives crash. This now exits with:
stdin:1:14-30: Error: Unexpected initial set type prefix
define foo = 2001:db8:123::/48
^^^^^^^^^^^^^^^^^
For literals, bison parser protects us, as it enforces
'elements = { 2001:... '.
For 'elements = $foo' we can't detect it at parsing stage as the '$foo'
symbol might as well evaluate to "{ 2001, ...}" (i.e. we can't do a
set element allocation).
As an alternative to print the datatype values when no symbol table is
available. Use it to print protocols available via getprotobynumber()
which actually refers to /etc/protocols.
Not very efficient, getprotobynumber() causes a series of open()/close()
calls on /etc/protocols, but this is called from a non-critical path.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1503 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Simon Ruderich [Tue, 9 Mar 2021 10:53:30 +0000 (11:53 +0100)]
doc: use symbolic names for chain priorities
This replaces the numbers with the matching symbolic names with one
exception: The NAT example used "priority 0" for the prerouting
priority. This is replaced by "dstnat" which has priority -100 which is
the new recommended priority.
Also use spaces instead of tabs for consistency in lines which require
updates.
Signed-off-by: Simon Ruderich <simon@ruderich.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Florian Westphal [Tue, 16 Mar 2021 23:40:34 +0000 (00:40 +0100)]
scanner: add support for scope nesting
Adding a COUNTER scope introduces parsing errors. Example:
add rule ... counter ip saddr 1.2.3.4
This is supposed to be
COUNTER IP SADDR SYMBOL
but it will be parsed as
COUNTER IP STRING SYMBOL
... and rule fails with unknown saddr.
This is because IP state change gets popped right after it was pushed.
bison parser invokes scanner_pop_start_cond() helper via
'close_scope_counter' rule after it has processed the entire 'counter' rule.
But that happens *after* flex has executed the 'IP' rule.
IOW, the sequence of events is not the exepcted
"COUNTER close_scope_counter IP SADDR SYMBOL close_scope_ip", it is
"COUNTER IP close_scope_counter".
close_scope_counter pops the just-pushed SCANSTATE_IP and returns the
scanner to SCANSTATE_COUNTER, so next input token (saddr) gets parsed
as a string, which gets then rejected from bison.
To resolve this, defer the pop operation until the current state is done.
scanner_pop_start_cond() already gets the scope that it has been
completed as an argument, so we can compare it to the active state.
If those are not the same, just defer the pop operation until the
bison reports its done with the active flex scope.
This leads to following sequence of events:
1. flex switches to SCANSTATE_COUNTER
2. flex switches to SCANSTATE_IP
3. bison calls scanner_pop_start_cond(SCANSTATE_COUNTER)
4. flex remains in SCANSTATE_IP, bison continues
5. bison calls scanner_pop_start_cond(SCANSTATE_IP) once the entire
ip rule has completed: this pops both IP and COUNTER.
Florian Westphal [Thu, 11 Mar 2021 13:23:02 +0000 (14:23 +0100)]
scanner: ct: move to own scope
This allows moving multiple ct specific keywords out of INITIAL scope.
Next few patches follow same pattern:
1. add a scope_close_XXX rule
2. add a SCANSTATE_XXX & make flex switch to it when
encountering XXX keyword
3. make bison leave SCANSTATE_XXXX when it has seen the complete
expression.
nftables: xt: fix misprint in nft_xt_compatible_revision
The rev variable is used here instead of opt obviously by mistake.
Please see iptables:nft_compatible_revision() for an example how it
should be.
This breaks revision compatibility checks completely when reading
compat-target rules from nft utility. That's why nftables can't work on
"old" kernels which don't support new revisons. That's a problem for
containers.
E.g.: 0 and 1 is supported but not 2:
https://git.sw.ru/projects/VZS/repos/vzkernel/browse/net/netfilter/xt_nat.c#111
Reproduce of the problem on Virtuozzo 7 kernel
3.10.0-1160.11.1.vz7.172.18 in centos 8 container:
iptables-nft -t nat -N TEST
iptables-nft -t nat -A TEST -j DNAT --to-destination 172.19.0.2
nft list ruleset > nft.ruleset
nft -f - < nft.ruleset
#/dev/stdin:19:67-81: Error: Range has zero or negative size
# meta l4proto tcp tcp dport 81 counter packets 0 bytes 0 dnat to 3.0.0.0-0.0.0.0
# ^^^^^^^^^^^^^^^
But nft reads this as rev 2 format (nf_nat_range2) which does not have
rangesize, and thus flugs 3 is treated as ip 3.0.0.0, which is wrong and
can't be restored later.
(Should probably be the same on Centos 7 kernel 3.10.0-1160.11.1)
Fixes: fbc0768cb696 ("nftables: xt: don't use hard-coded AF_INET") Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 4 Feb 2021 01:20:23 +0000 (02:20 +0100)]
mnl: Set NFTNL_SET_DATA_TYPE before dumping set elements
In combination with libnftnl's commit "set_elem: Fix printing of verdict
map elements", This adds the vmap target to netlink dumps. Adjust dumps
in tests/py accordingly.
Simon Ruderich [Sun, 7 Mar 2021 09:51:35 +0000 (10:51 +0100)]
doc: remove duplicate tables in synproxy example
The "outcome ruleset" is the same as the two tables in the example.
Don't duplicate this information which just wastes space in the
documentation and can confuse the reader (it took me a while to realize
the tables are the same).
In addition, use the same table name for both tables to make it clear
that they can be the same. They will be merged in the resulting ruleset.
Signed-off-by: Simon Ruderich <simon@ruderich.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
nft_mnl_socket_reopen() was introduced to deal with the EINTR case.
By reopening the netlink socket, pending netlink messages that are part of
a stale netlink dump are implicitly drop. This patch replaces the
nft_mnl_socket_reopen() strategy by pulling out all of the remaining
netlink message to restart in a clean state.
This is implicitly fixing up a bug in the table ownership support, which
assumes that the netlink socket remains open until nft_ctx_free() is
invoked.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Add new flag to allow userspace process to own tables: Tables that have
an owner can only be updated/destroyed by the owner. The table is
destroyed either if the owner process calls nft_ctx_free() or owner
process is terminated (implicit table release).
The ruleset listing includes the program name that owns the table:
nft> list ruleset
table ip x { # progname nft
flags owner
chain y {
type filter hook input priority filter; policy accept;
counter packets 1 bytes 309
}
}
Original code to pretty print the netlink portID to program name has
been extracted from the conntrack userspace utility.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 719e44277f8e ("main: use one data-structure to initialize getopt_long(3) arguments and help.") Cc: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Štěpán Němec <snemec@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 17 Feb 2021 11:38:42 +0000 (12:38 +0100)]
monitor: Don't print newgen message with JSON output
Iff this should be printed, it must adhere to output format settings. In
its current form it breaks JSON syntax, so skip it for non-default
output formats.
Fixes: cb7e02f44d6a6 ("src: enable json echo output when reading native syntax") Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 29 Dec 2020 18:33:44 +0000 (19:33 +0100)]
tests/py: Add a test sanitizer and fix its findings
This is just basic housekeeping:
- Remove duplicate tests in any of the *.t files
- Remove explicit output if equal to command itself in *.t files
- Remove duplicate payload records in any of the *.t.payload* files
- Remove stale payload records (for which no commands exist in the
respective *.t file
- Remove duplicate/stale entries in any of the *.t.json files
In some cases, tests were added instead of removing a stale payload
record if it fit nicely into the sequence of tests.
Phil Sutter [Tue, 15 Dec 2020 12:52:47 +0000 (13:52 +0100)]
tests/py: Write dissenting payload into the right file
The testsuite supports diverging payloads depending on table family.
This is necessary since for some families, dependency matches are
created.
If a payload mismatch happens, record it into a "got"-file which matches
the family-specific payload file, not the common one. This eases use of
diff-tools a lot as the extra other families' payloads confuse the
tools.
payload: check icmp dependency before removing previous icmp expression
nft is too greedy when removing icmp dependencies.
'icmp code 1 type 2' did remove the type when printing.
Be more careful and check that the icmp type dependency of the
candidate expression (earlier icmp payload expression) has the same
type dependency as the new expression.
Reported-by: Eric Garver <eric@garver.life> Reported-by: Michael Biebl <biebl@debian.org> Tested-by: Eric Garver <eric@garver.life> Fixes: d0f3b9eaab8d77e ("payload: auto-remove simple icmp/icmpv6 dependency expressions") Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 26 Jan 2021 17:37:12 +0000 (18:37 +0100)]
reject: Unify inet, netdev and bridge delinearization
Postprocessing for inet family did not attempt to kill any existing
payload dependency, although it is perfectly fine to do so. The mere
culprit is to not abbreviate default code rejects as that would drop
needed protocol info as a side-effect. Since postprocessing is then
almost identical to that of bridge and netdev families, merge them.
While being at it, extend tests/py/netdev/reject.t by a few more tests
taken from inet/reject.t so this covers icmpx rejects as well.
Cc: Jose M. Guisado Gomez <guigom@riseup.net> Signed-off-by: Phil Sutter <phil@nwl.cc>
Phil Sutter [Tue, 26 Jan 2021 16:06:33 +0000 (17:06 +0100)]
reject: Fix for missing dependencies in netdev family
Like with bridge family, rejecting with either icmp or icmpv6 must
create a dependency match on meta protocol. Upon delinearization, treat
netdev reject identical to bridge as well so no family info is lost.
This makes reject statement in netdev family fully symmetric so fix
the tests in tests/py/netdev/reject.t, adjust the related payload dumps
and add JSON equivalents which were missing altogether.
Fixes: 0c42a1f2a0cc5 ("evaluate: add netdev support for reject default") Fixes: a51a0bec1f698 ("tests: py: add netdev folder and reject.t icmp cases") Cc: Jose M. Guisado Gomez <guigom@riseup.net> Signed-off-by: Phil Sutter <phil@nwl.cc>
Florian Westphal [Tue, 26 Jan 2021 14:45:47 +0000 (15:45 +0100)]
json: ct: add missing test input
ERROR: did not find JSON equivalent for rule 'meta mark set ct original ip saddr . meta mark map { 1.1.1.1 . 0x00000014 : 0x0000001e }'
ERROR: did not find JSON equivalent for rule 'ct original ip saddr . meta mark { 1.1.1.1 . 0x00000014 }'
Florian Westphal [Thu, 21 Jan 2021 15:46:27 +0000 (16:46 +0100)]
json: icmp: move expected parts to json.output
Phil Sutter says:
In general, *.t.json files should contain JSON equivalents for rules as
they are *input* into nft. So we want them to be as close to the
introductory standard syntax comment as possible.
Undo earlier change and place the expected dependency added by
nft internals to json.output rather than icmp.t.json.
evaluate: disallow ct original {s,d}ddr from concatenations
Extend 8b043938e77b ("evaluate: disallow ct original {s,d}ddr from
maps") to cover concatenations too.
Error: specify either ip or ip6 for address matching
add rule x y meta mark set ct original saddr . meta mark map { 1.1.1.1 . 20 : 30 }
^^^^^^^^^^^^^^^^^
The old syntax for ct original saddr without either ip or ip6 results
in unknown key size, which breaks the listing. The old syntax is only
allowed in simple rules for backward compatibility.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1489 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
test.nft:6:55-71: Error: specify either ip or ip6 for address matching
add rule ip mangle manout ct direction reply mark set ct original daddr map { $ext1_ip : 0x11, $ext2_ip : 0x12 }
^^^^^^^^^^^^^^^^^
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1489 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>