]> git.ipfire.org Git - thirdparty/nftables.git/log
thirdparty/nftables.git
4 years agosrc: enable json echo output when reading native syntax
Jose M. Guisado Gomez [Tue, 4 Aug 2020 10:38:46 +0000 (12:38 +0200)] 
src: enable json echo output when reading native syntax

This patch fixes a bug in which nft did not print any output when
specifying --echo and --json and reading nft native syntax.

This patch respects behavior when input is json, in which the output
would be the identical input plus the handles.

Adds a json_echo member inside struct nft_ctx to build and store the json object
containing the json command objects, the object is built using a mock
monitor to reuse monitor json code. This json object is only used when
we are sure we have not read json from input.

[ added json_alloc_echo() to compile without json support --pablo ]

Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1446
Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Tested-by: Eric Garver <eric@garver.life>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agojson: echo: Speedup seqnum_to_json()
Phil Sutter [Fri, 20 Nov 2020 19:01:59 +0000 (20:01 +0100)] 
json: echo: Speedup seqnum_to_json()

Derek Dai reports:
"If there are a lot of command in JSON node, seqnum_to_json() will slow
down application (eg: firewalld) dramatically since it iterate whole
command list every time."

He sent a patch implementing a lookup table, but we can do better: Speed
this up by introducing a hash table to store the struct json_cmd_assoc
objects in, taking their netlink sequence number as key.

Quickly tested restoring a ruleset containing about 19k rules:

| # time ./before/nft -jeaf large_ruleset.json >/dev/null
| 4.85user 0.47system 0:05.48elapsed 97%CPU (0avgtext+0avgdata 69732maxresident)k
| 0inputs+0outputs (15major+16937minor)pagefaults 0swaps

| # time ./after/nft -jeaf large_ruleset.json >/dev/null
| 0.18user 0.44system 0:00.70elapsed 89%CPU (0avgtext+0avgdata 68484maxresident)k
| 0inputs+0outputs (15major+16645minor)pagefaults 0swaps

Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1479
Reported-by: Derek Dai <daiderek@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agotests: py: update format of registers in bitwise payloads.
Jeremy Sowden [Sun, 15 Nov 2020 15:11:47 +0000 (15:11 +0000)] 
tests: py: update format of registers in bitwise payloads.

libnftnl has been changed to bring the format of registers in bitwise
dumps in line with those in other types of expression.  Update the
expected output of Python test-cases.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoproto: Fix ARP header field ordering
Phil Sutter [Tue, 10 Nov 2020 12:07:49 +0000 (13:07 +0100)] 
proto: Fix ARP header field ordering

In ARP header, destination ether address sits between source IP and
destination IP addresses. Enum arp_hdr_fields had this wrong, which
in turn caused wrong ordering of entries in proto_arp->templates. When
expanding a combined payload expression, code assumes that template
entries are ordered by header offset, therefore the destination ether
address match was printed as raw if an earlier field was matched as
well:

| arp saddr ip 192.168.1.1 arp daddr ether 3e:d1:3f:d6:12:0b

was printed as:

| arp saddr ip 192.168.1.1 @nh,144,48 69068440080907

Note: Although strictly not necessary, reorder fields in
proto_arp->templates as well to match their actual ordering, just to
avoid confusion.

Fixes: 4b0f2a712b579 ("src: support for arp sender and target ethernet and IPv4 addresses")
Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agotests: py: remove duplicate payloads.
Jeremy Sowden [Mon, 9 Nov 2020 18:07:10 +0000 (18:07 +0000)] 
tests: py: remove duplicate payloads.

nft-test.py only needs one payload per rule, but a number of rules have
duplicates, typically one per address family, so just keep the last
payload for rules listed more than once.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agojson: tcp: add raw tcp option match support
Florian Westphal [Tue, 3 Nov 2020 11:04:20 +0000 (12:04 +0100)] 
json: tcp: add raw tcp option match support

To similar change as in previous one, this time for the
jason (de)serialization.

Re-uses the raw payload match syntax, i.e. base,offset,length.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotcp: add raw tcp option match support
Florian Westphal [Mon, 2 Nov 2020 19:10:25 +0000 (20:10 +0100)] 
tcp: add raw tcp option match support

tcp option @42,16,4 (@kind,offset,length).

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotcpopt: allow to check for presence of any tcp option
Florian Westphal [Wed, 21 Oct 2020 21:54:17 +0000 (23:54 +0200)] 
tcpopt: allow to check for presence of any tcp option

nft currently doesn't allow to check for presence of arbitrary tcp options.
Only known options where nft provides a template can be tested for.

This allows to test for presence of raw protocol values as well.

Example:

tcp option 42 exists

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotcpopt: split tcpopt_hdr_fields into per-option enum
Florian Westphal [Mon, 2 Nov 2020 14:22:40 +0000 (15:22 +0100)] 
tcpopt: split tcpopt_hdr_fields into per-option enum

Currently we're limited to ten template fields in exthdr_desc struct.
Using a single enum for all tpc option fields thus won't work
indefinitely (TCPOPTHDR_FIELD_TSECR is 9) when new option templates get
added.

Fortunately we can just use one enum per tcp option to avoid this.
As a side effect this also allows to simplify the sack offset
calculations.  Rather than computing that on-the-fly, just add extra
fields to the SACK template.

expr->exthdr.offset now holds the 'raw' value, filled in from the option
template. This would ease implementation of 'raw option matching'
using offset and length to load from the option.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotcpopt: rename noop to nop
Florian Westphal [Mon, 2 Nov 2020 13:58:41 +0000 (14:58 +0100)] 
tcpopt: rename noop to nop

'nop' is the tcp padding "option". "noop" is retained for compatibility
on parser side.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotcpopts: clean up parser -> tcpopt.c plumbing
Florian Westphal [Mon, 2 Nov 2020 13:53:26 +0000 (14:53 +0100)] 
tcpopts: clean up parser -> tcpopt.c plumbing

tcpopt template mapping is asymmetric:
one mapping is to match dumped netlink exthdr expression to the original
tcp option template.

This struct is indexed by the raw, on-write kind/type number.

The other mapping maps parsed options to the tcp option template.
Remove the latter.  The parser is changed to translate the textual
option name, e.g. "maxseg" to the on-wire number.

This avoids the second mapping, it will also allow to more easily
support raw option matching in a followup patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agoparser: merge sack-perm/sack-permitted and maxseg/mss
Florian Westphal [Sun, 1 Nov 2020 23:27:04 +0000 (00:27 +0100)] 
parser: merge sack-perm/sack-permitted and maxseg/mss

One was added by the tcp option parsing ocde, the other by synproxy.

So we have:
synproxy ... sack-perm
synproxy ... mss

and

tcp option maxseg
tcp option sack-permitted

This kills the extra tokens on the scanner/parser side,
so sack-perm and sack-permitted can both be used.

Likewise, 'synproxy maxseg' and 'tcp option mss size 42' will work too.
On the output side, the shorter form is now preferred, i.e. sack-perm
and mss.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agojson: fix ip6 dnat test case after range to prefix transformation change
Florian Westphal [Sat, 7 Nov 2020 13:26:34 +0000 (14:26 +0100)] 
json: fix ip6 dnat test case after range to prefix transformation change

Tests currently fail with
ip6/dnat.t: WARNING: line 8: ... because test still expects a range expression.

Fixes: ee4391d0ac1e7 ("nat: transform range to prefix expression when possible")
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agojson: add missing nat_type flag and netmap nat flag
Florian Westphal [Tue, 3 Nov 2020 13:32:12 +0000 (14:32 +0100)] 
json: add missing nat_type flag and netmap nat flag

JSON in/output doesn't know about nat_type and thus cannot save/restore
nat mappings involving prefixes or concatenations because the snat
statement lacks the prefix/concat/interval type flags.

Furthermore, bison parser was extended to support netmap.
This is done with an internal 'netmap' flag that is passed to the
kernel.  We need to dump/restore that as well.

Also make sure ip/snat.t passes in json mode.

Fixes: 35a6b10c1bc4 ("src: add netmap support")
Fixes: 9599d9d25a6b ("src: NAT support for intervals in maps")
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotests: avoid warning and add missing json test cases
Florian Westphal [Tue, 3 Nov 2020 12:48:20 +0000 (13:48 +0100)] 
tests: avoid warning and add missing json test cases

make dnat.t pass in json mode.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotests: json: add missing test case output
Florian Westphal [Tue, 3 Nov 2020 10:12:50 +0000 (11:12 +0100)] 
tests: json: add missing test case output

Fix warnings and errors when running nf-test.py -j due to missing json test case updates.
This also makes bridge/reject.t pass in json mode.

No code changes.

Fixes: 8615ed93f6e4c4 ("evaluate: enable reject with 802.1q")
Fixes: fae0a0972d7a71 ("tests: py: Enable anonymous set rule with concatenated ranges in inet/sets.t")
Fixes: 2a20b5bdbde8a1 ("datatype: add frag-needed (ipv4) to reject options")
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agosrc: Optimize prefix matches on byte-boundaries
Phil Sutter [Tue, 27 Oct 2020 16:33:15 +0000 (17:33 +0100)] 
src: Optimize prefix matches on byte-boundaries

If a prefix expression's length is on a byte-boundary, it is sufficient
to just reduce the length passed to "cmp" expression. No need for
explicit bitwise modification of data on LHS. The relevant code is
already there, used for string prefix matches. There is one exception
though, namely zero-length prefixes: Kernel doesn't accept zero-length
"cmp" expressions, so keep them in the old code-path for now.

This patch depends upon the previous one to correctly parse odd-sized
payload matches but has to extend support for non-payload LHS as well.
In practice, this is needed for "ct" expressions as they allow matching
against IP address prefixes, too.

Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agosrc: Support odd-sized payload matches
Phil Sutter [Tue, 27 Oct 2020 16:05:25 +0000 (17:05 +0100)] 
src: Support odd-sized payload matches

When expanding a payload match, don't disregard oversized templates at
the right offset. A more flexible user may extract less bytes from the
packet if only parts of a field are interesting, e.g. only the prefix of
source/destination address. Support that by using the template, but fix
the length. Later when creating a relational expression for it, detect
the unusually small payload expression length and turn the RHS value
into a prefix expression.

Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agotests: py: add netdev folder and reject.t icmp cases
Jose M. Guisado Gomez [Thu, 22 Oct 2020 19:43:55 +0000 (21:43 +0200)] 
tests: py: add netdev folder and reject.t icmp cases

Add unit tests for the use of reject with icmp inside netdev family.

reject.t from inet family couldn't be reused because it was using
meta nfproto which is not supported inside netdev.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: add netdev support for reject default
Jose M. Guisado Gomez [Thu, 22 Oct 2020 19:43:54 +0000 (21:43 +0200)] 
evaluate: add netdev support for reject default

Enables not specifying any icmp type and code when using reject inside
netdev.

This patch completely enables using reject for the netdev family.

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agodoc: correct chain name in example of adding a rule
Jeremy Sowden [Sun, 1 Nov 2020 19:33:13 +0000 (19:33 +0000)] 
doc: correct chain name in example of adding a rule

The example adds a rule to the `output` chain, not the `input` chain.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agotests/shell: Improve fix in sets/0036add_set_element_expiration_0
Phil Sutter [Thu, 29 Oct 2020 12:35:55 +0000 (13:35 +0100)] 
tests/shell: Improve fix in sets/0036add_set_element_expiration_0

Explicitly eliminate the newgen message from output instead of just the
last line to make sure no other output is dropped by accident. This also
allows the test to pass in unpatched kernels which do not emit the
newgen message despite NLM_F_ECHO if no netlink listeners are present.

Fixes: 46b54fdcf266d ("Revert "monitor: do not print generation ID with --echo"")
Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agotests: shell: exercise validation with nft -c
Pablo Neira Ayuso [Fri, 30 Oct 2020 19:36:22 +0000 (20:36 +0100)] 
tests: shell: exercise validation with nft -c

Using oif in fib from prerouting is not support, make sure -c reports an
error.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agobuild: Bump version to v0.9.7 v0.9.7
Pablo Neira Ayuso [Mon, 26 Oct 2020 13:40:40 +0000 (14:40 +0100)] 
build: Bump version to v0.9.7

Update release name based on the Fearless Fosdick series: Anyface.

Bump dependencies on libnftnl.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoRevert "monitor: do not print generation ID with --echo"
Pablo Neira Ayuso [Thu, 22 Oct 2020 20:34:17 +0000 (22:34 +0200)] 
Revert "monitor: do not print generation ID with --echo"

Revert 0e258556f7f3 ("monitor: do not print generation ID with --echo").

There is actually a kernel bug which is preventing from displaying
this generation ID message.

Update the tests/shell to remove the last line of the --echo output
which displays the generation ID once the "netfilter: nftables: fix netlink
report logic in flowtable and genid" kernel fix is applied.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agomonitor: do not print generation ID with --echo
Pablo Neira Ayuso [Tue, 20 Oct 2020 19:48:16 +0000 (21:48 +0200)] 
monitor: do not print generation ID with --echo

This fixes testcases/sets/0036add_set_element_expiration_0

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agojson: Fix memleak in set_dtype_json()
Phil Sutter [Thu, 8 Oct 2020 17:10:13 +0000 (19:10 +0200)] 
json: Fix memleak in set_dtype_json()

Turns out json_string() already dups the input, so the temporary dup
passed to it is lost.

Fixes: e70354f53e9f6 ("libnftables: Implement JSON output support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosegtree: UAF in interval_map_decompose()
Pablo Neira Ayuso [Tue, 20 Oct 2020 19:24:36 +0000 (21:24 +0200)] 
segtree: UAF in interval_map_decompose()

reported by tests/monitor# bash run-tests.sh
...
SUMMARY: AddressSanitizer: heap-use-after-free /home/pablo/devel/scm/git-netfilter/nftables/src/expression.c:1385 in expr_ops

Due to incorrect structure layout when calling interval_expr_copy().

Fixes: c1f0476fd590 ("segtree: copy expr data to closing element")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: improve rule error reporting
Pablo Neira Ayuso [Mon, 19 Oct 2020 12:46:22 +0000 (14:46 +0200)] 
src: improve rule error reporting

Kernel provides information regarding expression since
83d9dcba06c5 ("netfilter: nf_tables: extended netlink error reporting for
expressions").

A common mistake is to refer a chain which does not exist, e.g.

 # nft add rule x y jump test
 Error: Could not process rule: No such file or directory
 add rule x y jump test
                   ^^^^

Use the existing netlink extended error reporting infrastructure to
provide better error reporting as in the example above.

Requires Linux kernel patch 83d9dcba06c5 ("netfilter: nf_tables:
extended netlink error reporting for expressions").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: constify location parameter in cmd_add_loc()
Pablo Neira Ayuso [Mon, 19 Oct 2020 21:51:16 +0000 (23:51 +0200)] 
src: constify location parameter in cmd_add_loc()

Constify pointer to location object to compile check for unintentional
updates.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agorule: larger number of error locations
Pablo Neira Ayuso [Mon, 19 Oct 2020 12:45:48 +0000 (14:45 +0200)] 
rule: larger number of error locations

Statically store up to 32 locations per command, if the number of
locations is larger than 32, then skip rather than hit assertion.

Revisit this later to dynamically store location per command using a
hashtable.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agodoc: nft.8: describe inet ingress hook
Pablo Neira Ayuso [Wed, 14 Oct 2020 19:02:57 +0000 (21:02 +0200)] 
doc: nft.8: describe inet ingress hook

Available since Linux kernel >= 5.10.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosegtree: copy expr data to closing element
Florian Westphal [Thu, 15 Oct 2020 14:47:21 +0000 (16:47 +0200)] 
segtree: copy expr data to closing element

When last expr has no closing element we did not propagate
expr properties such as comment or expire date to the newly
allocated set elem.

Before:
nft create table t
nft 'add set t s { type ipv4_addr; flags interval; timeout 60s; }'
nft add element t s { 224.0.0.0/3 }
nft list set t s | grep -o 'elements.*'
elements = { 224.0.0.0-255.255.255.255 }

nft flush set t s
nft add element t s { 224.0.0.0/4, 240.0.0.0/4 }
nft list set t s | grep -o 'elements.*'
elements = { 224.0.0.0/4 expires 55s152ms, 240.0.0.0-255.255.255.255 }

nft delete set t s
nft 'add set t s { type ipv4_addr; flags interval; auto-merge; timeout 60s; }'
nft add element t s { 224.0.0.0/4, 240.0.0.0/4 }
nft list set t s | grep -o 'elements.*'
elements = { 224.0.0.0-255.255.255.255 }

After:
elements = { 224.0.0.0-255.255.255.255 expires 58s515ms }
elements = { 224.0.0.0/4 expires 54s622ms, 240.0.0.0-255.255.255.255 expires 54s622ms }
elements = { 224.0.0.0-255.255.255.255 expires 57s92ms }

Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1454
Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agoproto: add sctp crc32 checksum fixup
Florian Westphal [Tue, 6 Oct 2020 21:16:32 +0000 (23:16 +0200)] 
proto: add sctp crc32 checksum fixup

Stateless SCTP header mangling doesn't work reliably.
This tells the kernel to update the checksum field using
the sctp crc32 algorithm.

Note that this needs additional kernel support to work.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agosrc: ingress inet support
Pablo Neira Ayuso [Tue, 13 Oct 2020 10:35:47 +0000 (12:35 +0200)] 
src: ingress inet support

Add support for inet ingress chains.

 table inet filter {
        chain ingress {
                type filter hook ingress device "veth0" priority filter; policy accept;
        }
chain input {
type filter hook input priority filter; policy accept;
}
chain forward {
type filter hook forward priority filter; policy accept;
}
 }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: py: add missing test JSON output for TCP flag tests.
Jeremy Sowden [Sun, 11 Oct 2020 19:23:24 +0000 (20:23 +0100)] 
tests: py: add missing test JSON output for TCP flag tests.

Fixes: 3926a3369bb5 ("mergesort: unbreak listing with binops")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: py: correct order of set elements in test JSON output.
Jeremy Sowden [Sun, 11 Oct 2020 19:23:23 +0000 (20:23 +0100)] 
tests: py: correct order of set elements in test JSON output.

Fixes: 741a06ac15d2 ("mergesort: find base value expression type via recursion")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: py: add missing JSON output for ct test.
Jeremy Sowden [Sun, 11 Oct 2020 19:23:22 +0000 (20:23 +0100)] 
tests: py: add missing JSON output for ct test.

Fixes: dcec7d57559a ("ct: Add support for the 'id' key")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoSolves Bug 1462 - `nft -j list set` does not show counters
Gopal Yadav [Wed, 7 Oct 2020 14:03:37 +0000 (19:33 +0530)] 
Solves Bug 1462 - `nft -j list set` does not show counters

Element counters reside in 'stmt' field as counter statement. Append
them to 'elem' object as additional 'counter' property, generated by
counter_stmt_json().

Signed-off-by: Gopal Yadav <gopunop@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agoevaluate: Reject quoted strings containing only wildcard
Phil Sutter [Thu, 24 Sep 2020 15:38:45 +0000 (17:38 +0200)] 
evaluate: Reject quoted strings containing only wildcard

Fix for an assertion fail when trying to match against an all-wildcard
interface name:

| % nft add rule t c iifname '"*"'
| nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
| zsh: abort      nft add rule t c iifname '"*"'

Fix this by detecting the string in expr_evaluate_string() and returning
an error message:

| % nft add rule t c iifname '"*"'
| Error: All-wildcard strings are not supported
| add rule t c iifname "*"
|                      ^^^

While being at it, drop the 'datalen >= 1' clause from the following
conditional as together with the added check for 'datalen == 0', all
possible other values have been caught already.

4 years agonft: migrate man page examples with `meter` directive to sets
Devin Bayer [Thu, 1 Oct 2020 09:30:27 +0000 (11:30 +0200)] 
nft: migrate man page examples with `meter` directive to sets

this updates the two examples in the man page that use the obsolete `meter` to
use sets. I also fixed a bit of formatting for the conntrack expressions.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add comment support for chains
Jose M. Guisado Gomez [Mon, 21 Sep 2020 13:28:23 +0000 (15:28 +0200)] 
src: add comment support for chains

This patch enables the user to specify a comment when adding a chain.

Relies on kernel space supporting userdata for chains.

> nft add table ip filter
> nft add chain ip filter input { comment "test"\; type filter hook input priority 0\; policy accept\; }
> list ruleset

table ip filter {
chain input {
comment "test"
type filter hook input priority filter; policy accept;
}
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agojson: Combining --terse with --json has no effect
Gopal Yadav [Tue, 22 Sep 2020 08:25:33 +0000 (13:55 +0530)] 
json: Combining --terse with --json has no effect

--terse with --json is ignored, fix this. This patch also includes a test.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1388
Signed-off-by: Gopal Yadav <gopunop@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: context tracking for multiple transport protocols
Pablo Neira Ayuso [Mon, 14 Sep 2020 18:51:20 +0000 (20:51 +0200)] 
src: context tracking for multiple transport protocols

This patch extends the protocol context infrastructure to track multiple
transport protocols when they are specified from sets.

This removes errors like:

 "transport protocol mapping is only valid after transport protocol match"

when invoking:

 # nft add rule x z meta l4proto { tcp, udp } dnat to 1.1.1.1:80

This patch also catches conflicts like:

 # nft add rule x z ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
 Error: conflicting protocols specified: udp vs. tcp
 add rule x z ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
                                       ^^^^^^^^^
and:

 # nft add rule x z meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
 Error: conflicting protocols specified: udp vs. tcp
 add rule x z meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
                                        ^^^^^^^^^
Note that:

- the singleton protocol context tracker is left in place until the
  existing users are updated to use this new multiprotocol tracker.
  Moving forward, it would be good to consolidate things around this new
  multiprotocol context tracker infrastructure.

- link and network layers are not updated to use this infrastructure
  yet. The code that deals with vlan conflicts relies on forcing
  protocol context updates to the singleton protocol base.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: remove one indent level in __expr_evaluate_payload()
Pablo Neira Ayuso [Mon, 14 Sep 2020 14:59:17 +0000 (16:59 +0200)] 
evaluate: remove one indent level in __expr_evaluate_payload()

If there is protocol context for this base, just return from function
to remove one level of indentation. This patch is cleanup.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: py: flush log file output before running each command
Pablo Neira Ayuso [Mon, 14 Sep 2020 19:10:13 +0000 (21:10 +0200)] 
tests: py: flush log file output before running each command

If nft crashes or hits an assertion, the last command run shows in the
/tmp/nftables-test.log file.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agolibnftables: avoid repeated command list traversal on errors
Jindrich Makovicka [Sat, 2 May 2020 11:52:06 +0000 (13:52 +0200)] 
libnftables: avoid repeated command list traversal on errors

Because the command seqnums are monotonic, repeated traversals
of the cmds list from the beginning are not necessary as long as
the error seqnums are also monotonic.

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agomnl: larger receive socket buffer for netlink errors
Pablo Neira Ayuso [Sun, 13 Sep 2020 16:05:08 +0000 (18:05 +0200)] 
mnl: larger receive socket buffer for netlink errors

Assume each error in the batch will result in a 1k notification for the
non-echo flag set on case as described in 860671662d3f ("mnl: fix --echo
buffer size again").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoparser_bison: fail when specifying multiple comments
Jose M. Guisado Gomez [Thu, 10 Sep 2020 16:40:20 +0000 (18:40 +0200)] 
parser_bison: fail when specifying multiple comments

Before this patch grammar supported specifying multiple comments, and
only the last value would be assigned.

This patch adds a function to test if an attribute is already assigned
and, if so, calls erec_queue with this attribute location.

Use this function in order to check for duplication (or more) of comments
for actions that support it.

> nft add table inet filter { flags "dormant"\; comment "test"\; comment "another"\;}

Error: You can only specify this once. This statement is duplicated.
add table inet filter { flags dormant; comment test; comment another;}
                                                     ^^^^^^^^^^^^^^^^

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add comment support for objects
Jose M. Guisado Gomez [Thu, 3 Sep 2020 09:16:06 +0000 (11:16 +0200)] 
src: add comment support for objects

Enables specifying an optional comment when declaring named objects. The
comment is to be specified inside the object's block ({} block)

Relies on libnftnl exporting nftnl_obj_get_data and kernel space support
to store the comments.

For consistency, this patch makes the comment be printed first when
listing objects.

Adds a testcase importing all commented named objects except for secmark,
although it's supported.

Example: Adding a quota with a comment

> add table inet filter
> nft add quota inet filter q { over 1200 bytes \; comment "test_comment"\; }
> list ruleset

table inet filter {
quota q {
comment "test_comment"
over 1200 bytes
}
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agomergesort: find base value expression type via recursion
Pablo Neira Ayuso [Thu, 3 Sep 2020 10:33:21 +0000 (12:33 +0200)] 
mergesort: find base value expression type via recursion

Sets that store flags might contain a mixture of values and binary
operations. Find the base value type via recursion to compare the
expressions.

Make sure concatenations are listed in a deterministic way via
concat_expr_msort_value() which builds a mpz value with the tuple.

Adjust a few tests after this update since listing differs after this
update.

Fixes: 14ee0a979b62 ("src: sort set elements in netlink_get_setelems()")
Fixes: 3926a3369bb5 ("mergesort: unbreak listing with binops")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: allow tests/monitor to use a custom nft executable
Balazs Scheidler [Sat, 29 Aug 2020 07:04:05 +0000 (09:04 +0200)] 
tests: allow tests/monitor to use a custom nft executable

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: added "socket wildcard" testcases
Balazs Scheidler [Sat, 29 Aug 2020 07:04:04 +0000 (09:04 +0200)] 
tests: added "socket wildcard" testcases

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agodoc: added documentation on "socket wildcard"
Balazs Scheidler [Sat, 29 Aug 2020 07:04:03 +0000 (09:04 +0200)] 
doc: added documentation on "socket wildcard"

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc/scanner.l: fix whitespace issue for the TRANSPARENT keyword
Balazs Scheidler [Sat, 29 Aug 2020 07:04:02 +0000 (09:04 +0200)] 
src/scanner.l: fix whitespace issue for the TRANSPARENT keyword

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosocket: add support for "wildcard" key
Balazs Scheidler [Sat, 29 Aug 2020 07:04:01 +0000 (09:04 +0200)] 
socket: add support for "wildcard" key

iptables had a "-m socket --transparent" which didn't match sockets that are
bound to all addresses (e.g.  0.0.0.0 for ipv4, and ::0 for ipv6).  It was
possible to override this behavior by using --nowildcard, in which case it
did match zero bound sockets as well.

The issue is that nftables never included the wildcard check, so in effect
it behaved like "iptables -m socket --transparent --nowildcard" with no
means to exclude wildcarded listeners.

This is a problem as a user-space process that binds to 0.0.0.0:<port> that
enables IP_TRANSPARENT would effectively intercept traffic going in _any_
direction on the specific port, whereas in most cases, transparent proxies
would only need this for one specific address.

The solution is to add "socket wildcard" key to the nft_socket module, which
makes it possible to match on the wildcardness of a socket from
one's ruleset.

This is how to use it:

table inet haproxy {
chain prerouting {
         type filter hook prerouting priority -150; policy accept;
socket transparent 1 socket wildcard 0 mark set 0x00000001
}
}

This patch effectively depends on its counterpart in the kernel.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add comment support when adding tables
Jose M. Guisado Gomez [Fri, 21 Aug 2020 16:40:30 +0000 (18:40 +0200)] 
src: add comment support when adding tables

Adds userdata building logic if a comment is specified when creating a
new table. Adds netlink userdata parsing callback function.

Relies on kernel supporting userdata for nft_table.

Example:

> nft add table ip x { comment "test"\; }
> nft list ruleset

table ip x {
comment "test"
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add chain hashtable cache
Pablo Neira Ayuso [Fri, 21 Aug 2020 10:04:12 +0000 (12:04 +0200)] 
src: add chain hashtable cache

This significantly improves ruleset listing time with large rulesets
(~50k rules) with _lots_ of non-base chains.

 # time nft list ruleset &> /dev/null

Before this patch:

real    0m11,172s
user    0m6,810s
sys     0m4,220s

After this patch:

real    0m4,747s
user    0m0,802s
sys     0m3,912s

This patch also removes list_bindings from netlink_ctx since there is no
need to keep a temporary list of chains anymore.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add expression handler hashtable
Pablo Neira Ayuso [Thu, 20 Aug 2020 16:21:37 +0000 (18:21 +0200)] 
src: add expression handler hashtable

netlink_parsers is actually small, but update this code to use a
hashtable instead since more expressions may come in the future.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: sets: Check rbtree overlap detection after tree rotations
Stefano Brivio [Wed, 19 Aug 2020 22:00:18 +0000 (00:00 +0200)] 
tests: sets: Check rbtree overlap detection after tree rotations

Ticket https://bugzilla.netfilter.org/show_bug.cgi?id=1449 showed
an issue with rbtree overlap detection coming from the fact that,
after tree rotations performed as part of tree rebalancing, caused
by deletions, end elements are not necessarily descendants of their
corresponding start elements.

Add single-sized elements, delete every second one of them, and
re-add them (they will always be full overlaps) in order to check
overlap detection after tree rotations.

Port indices used in the sets are pseudo-random numbers generated
with Marsaglia's Xorshift algorithm with triplet (5, 3, 1), chosen
for k-distribution over 16-bit periods, which gives a good
statistical randomness and forces 201 rebalancing operations out of
250 deletions with the chosen seed (1).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agonftables: dump raw element info from libnftnl when netlink debugging is on
Florian Westphal [Thu, 20 Aug 2020 15:23:57 +0000 (17:23 +0200)] 
nftables: dump raw element info from libnftnl when netlink debugging is on

Example: nft --debug=netlink list ruleset
inet firewall @knock_candidates_ipv4
        element 0100007f 00007b00  : 0 [end]
        element 0200007f 0000f1ff  : 0 [end]
        element 0100007f 00007a00  : 0 [end]
inet firewall @__set0
        element 00000100  : 0 [end]
        element 00000200  : 0 [end]
inet firewall knock-input 3
  [ meta load l4proto => reg 1 ]
  ...

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agomergesort: unbreak listing with binops
Pablo Neira Ayuso [Wed, 19 Aug 2020 23:05:04 +0000 (01:05 +0200)] 
mergesort: unbreak listing with binops

tcp flags == {syn, syn|ack}
tcp flags & (fin|syn|rst|psh|ack|urg) == {ack, psh|ack, fin, fin|psh|ack}

results in:

BUG: Unknown expression binop
nft: mergesort.c:47: expr_msort_cmp: Assertion `0' failed.
Aborted (core dumped)

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add comment support for map too
Pablo Neira Ayuso [Mon, 17 Aug 2020 10:12:23 +0000 (12:12 +0200)] 
src: add comment support for map too

Extend and slightly rework tests/shell to cover this case too.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: add comment support for set declarations
Jose M. Guisado Gomez [Tue, 11 Aug 2020 14:27:20 +0000 (16:27 +0200)] 
src: add comment support for set declarations

Allow users to add a comment when declaring a named set.

Adds set output handling the comment in both nftables and json
format.

$ nft add table ip x
$ nft add set ip x s {type ipv4_addr\; comment "some_addrs"\; elements = {1.1.1.1, 1.2.3.4}}

$ nft list ruleset
table ip x {
set s {
type ipv4_addr;
comment "some_addrs"
elements = { 1.1.1.1, 1.2.3.4 }
}
}

$ nft --json list ruleset
{
    "nftables": [
        {
            "metainfo": {
                "json_schema_version": 1,
                "release_name": "Capital Idea #2",
                "version": "0.9.6"
            }
        },
        {
            "table": {
                "family": "ip",
                "handle": 4857,
                "name": "x"
            }
        },
        {
            "set": {
                "comment": "some_addrs",
                "elem": [
                    "1.1.1.1",
                    "1.2.3.4"
                ],
                "family": "ip",
                "handle": 1,
                "name": "s",
                "table": "x",
                "type": "ipv4_addr"
            }
        }
    ]
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: cache gets out of sync in interactive mode
Pablo Neira Ayuso [Thu, 6 Aug 2020 10:52:00 +0000 (12:52 +0200)] 
src: cache gets out of sync in interactive mode

Since 94a945ffa81b ("libnftables: Get rid of explicit cache flushes"),
the cache logic checks for the generation number to refresh the cache.

This breaks interactive mode when listing stateful objects though. This
patch adds a new flag to force a cache refresh when the user requests a
ruleset listing.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosegtree: memleaks in interval_map_decompose()
Pablo Neira Ayuso [Tue, 4 Aug 2020 20:12:12 +0000 (22:12 +0200)] 
segtree: memleaks in interval_map_decompose()

mpz_init_bitmask() overrides the existing memory area:

==19179== 8 bytes in 1 blocks are definitely lost in loss record 1 of 1
==19179==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==19179==    by 0x489C718: xmalloc (utils.c:36)
==19179==    by 0x4B825C5: __gmpz_init2 (in /usr/lib/x86_64-linux-g nu/libgmp.so.10.3.2)                                               f
==19179==    by 0x4880239: constant_expr_alloc (expression.c:400)
==19179==    by 0x489B8A1: interval_map_decompose (segtree.c:1098)
==19179==    by 0x489017D: netlink_list_setelems (netlink.c:1220)
==19179==    by 0x48779AC: cache_init_objects (rule.c:170)         5
==19179==    by 0x48779AC: cache_init (rule.c:228)
==19179==    by 0x48779AC: cache_update (rule.c:279)
==19179==    by 0x48A21AE: nft_evaluate (libnftables.c:406)

left-hand side of the interval is leaked when building the range:

==25835== 368 (128 direct, 240 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 5
==25835==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==25835==    by 0x489B628: xmalloc (utils.c:36)
==25835==    by 0x489B6F8: xzalloc (utils.c:65)
==25835==    by 0x487E176: expr_alloc (expression.c:45)
==25835==    by 0x487F960: mapping_expr_alloc (expression.c:1149)
==25835==    by 0x488EC84: netlink_delinearize_setelem (netlink.c:1166)
==25835==    by 0x4DC6928: nftnl_set_elem_foreach (set_elem.c:725)
==25835==    by 0x488F0D5: netlink_list_setelems (netlink.c:1215)
==25835==    by 0x487695C: cache_init_objects (rule.c:170)
==25835==    by 0x487695C: cache_init (rule.c:228)
==25835==    by 0x487695C: cache_update (rule.c:279)
==25835==    by 0x48A10BE: nft_evaluate (libnftables.c:406)
==25835==    by 0x48A19B6: nft_run_cmd_from_buffer (libnftables.c:451)
==25835==    by 0x10A8E1: main (main.c:487)

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: 0044interval_overlap_0: Repeat insertion tests with timeout
Stefano Brivio [Mon, 3 Aug 2020 14:06:21 +0000 (16:06 +0200)] 
tests: 0044interval_overlap_0: Repeat insertion tests with timeout

Mike Dillinger reported issues with insertion of entries into sets
supporting intervals that were denied because of false conflicts with
elements that were already expired. Partial failures would occur to,
leading to the generation of new intervals the user didn't specify,
as only the opening or the closing elements wouldn't be inserted.

The reproducer provided by Mike looks like this:

  #!/bin/bash
  nft list set ip filter blacklist4-ip-1m
  for ((i=1;i<=10;i++)); do
        nft add element filter blacklist4-ip-1m {$i.$i.$i.$i}
        sleep 1
  done
  nft list set ip filter blacklist4-ip-1m

which, run in a loop at different intervals, show the different kind
of failures.

Extend the existing test case for overlapping and non-overlapping
intervals to systematically cover sets with a configured timeout.

As reported by Pablo, the test would fail if we keep a one-second
timeout if it runs on a "slow" kernel (e.g. with KASan), using the
libtool wrapper in src/nft as $NFT, because we can't issue 218
commands within one second. To avoid that, introduce an adaptive
timeout based on how many times we can list a single entry with a
fixed one-second timeout.

On a single 2.9GHz AMD Epyc 7351 thread:
                                     test run   nft commands/s  timeout
- src/nft libtool wrapper, KASan:       68.4s          10         32s
- nft binary, KASan:                     5.1s         168          2s
- src/nft libtool wrapper, w/o KASan:   18.3s          37          8s
- nft binary, w/o KASan:                 2.4s         719          1s

While at it, fix expectation for insertion of '15-20 . 50-60' (it's
expected to succeed, given the list), and the reason why I didn't
notice: a simple command preceded by ! won't actually result in
the shell exiting, even if it fails. Add some clearer failure reports
too.

v2:
  - adjust set timeouts to nft commands/s
  - fix checks on expected outcome of insertions and reports

Reported-by: Mike Dillinger <miked@softtalker.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: 0043concatenated_ranges_0: Fix checks for add/delete failures
Stefano Brivio [Mon, 3 Aug 2020 14:06:39 +0000 (16:06 +0200)] 
tests: 0043concatenated_ranges_0: Fix checks for add/delete failures

The test won't stop if we simply precede commands expected to fail
by !. POSIX.1-2017 says:

  -e
      When this option is on, if a simple command fails for any of
      the reasons listed in Consequences of Shell Errors or returns
      an exit status value >0, and is not part of the compound list
      following a while, until or if keyword, and is not a part of
      an AND or OR list, and is not a pipeline preceded by the "!"
      reserved word, then the shell will immediately exit.

...but I didn't care about the last part.

Replace those '! nft ...' commands by 'nft ... && exit 1' to actually
detect failures.

As a result, I didn't notice that now, correctly, inserting elements
into a set that contains the same exact element doesn't actually
fail, because nft doesn't pass NLM_F_EXCL on a simple 'add'. Drop
re-insertions from the checks we perform here, overlapping elements
are already covered by other tests.

Fixes: 618393c6b3f2 ("tests: Introduce test for set with concatenated ranges")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: fix obj list output when reset command
Jose M. Guisado Gomez [Sat, 1 Aug 2020 21:30:10 +0000 (23:30 +0200)] 
src: fix obj list output when reset command

This patch enables json output when doing a reset command.

Previously do_list_obj was called at the end of do_command_reset to
list the named object affected by the reset, this function
is for nft output only.

Listing affected objects using do_command_list ensures
output flags will be honored.

Eg: For a ruleset like

table inet x {
counter user123 {
packets 12 bytes 1433
}

counter user321 {
packets 0 bytes 0
}

quota user123 {
over 2000 bytes
}

quota user124 {
over 2000 bytes
}

set y {
type ipv4_addr
}

...
}

{
    "nftables": [
        {
            "metainfo": {
                "json_schema_version": 1,
                "release_name": "Capital Idea #2",
                "version": "0.9.6"
            }
        },
        {
            "counter": {
                "bytes": 0,
                "family": "inet",
                "handle": 3,
                "name": "user321",
                "packets": 0,
                "table": "x"
            }
        },
        {
            "counter": {
                "bytes": 1433,
                "family": "inet",
                "handle": 2,
                "name": "user123",
                "packets": 12,
                "table": "x"
            }
        }
    ]
}

Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1336
Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: disregard ct address matching without family
Pablo Neira Ayuso [Fri, 31 Jul 2020 18:10:11 +0000 (20:10 +0200)] 
evaluate: disregard ct address matching without family

The following rule:

 # nft add rule ip x y ct original daddr @servers

breaks with:

 # nft list ruleset
nft: netlink_delinearize.c:124: netlink_parse_concat_expr: Assertion `consumed > 0' failed.
Aborted

Bail out if this syntax is used, instead users should rely on:

 # nft add rule ip x y ct original ip daddr @servers
                                   ~~

which uses NFT_CT_{SRC,DST}_{IP,IP6} in the bytecode generation.

This issue is described in 7f742d0a9071 ("ct: support for
NFT_CT_{SRC,DST}_{IP,IP6}").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agonetlink_delinearize: transform binary operation to prefix only with values
Pablo Neira Ayuso [Wed, 29 Jul 2020 17:40:02 +0000 (19:40 +0200)] 
netlink_delinearize: transform binary operation to prefix only with values

The following rule:

 nft add rule inet filter input ip6 saddr and ffff:ffff:ffff:ffff:: @allowable counter

when listing the ruleset becomes:

 ip6 saddr @allowable/64 counter packets 3 bytes 212

This transformation is unparseable, allow prefix transformation only for
values.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: UAF in hook priority expression
Pablo Neira Ayuso [Tue, 28 Jul 2020 17:49:26 +0000 (19:49 +0200)] 
evaluate: UAF in hook priority expression

Release priority expression right before assigning the constant
expression that results from the evaluation.

Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: memleak in invalid default policy definition
Pablo Neira Ayuso [Tue, 28 Jul 2020 17:39:12 +0000 (19:39 +0200)] 
evaluate: memleak in invalid default policy definition

Release the clone expression from the exit path.

Fixes: 5173151863d3 ("evaluate: replace variable expression by the value expression")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoparser_bison: memleak symbol redefinition
Pablo Neira Ayuso [Tue, 28 Jul 2020 17:36:57 +0000 (19:36 +0200)] 
parser_bison: memleak symbol redefinition

Missing expr_free() from the error path.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: remove table from cache on delete table
Pablo Neira Ayuso [Tue, 28 Jul 2020 17:32:44 +0000 (19:32 +0200)] 
evaluate: remove table from cache on delete table

The following ruleset crashes nft if loaded twice, via nft -ef:

 add table inet filter
 delete table inet filter

 table inet filter {
        chain input {
                type filter hook input priority filter; policy drop;
                iifname { "eth0" } counter accept
        }
 }

If the table contains anonymous sets, such as __set0, then delete + add
table might result in nft reusing the existing stale __set0 in the cache.
The problem is that nft gets confused and it reuses the existing stale
__set0 instead of the new anonymous set __set0 with the same name.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: remove cache lookups after the evaluation phase
Pablo Neira Ayuso [Tue, 28 Jul 2020 15:57:20 +0000 (17:57 +0200)] 
src: remove cache lookups after the evaluation phase

This patch adds a new field to the cmd structure for elements to store a
reference to the set. This saves an extra lookup in the netlink bytecode
generation step.

This patch also allows to incrementally update during the evaluation
phase according to the command actions, which is required by the follow
up ("evaluate: remove table from cache on delete table") bugfix patch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: flush set cache from the evaluation phase
Pablo Neira Ayuso [Tue, 28 Jul 2020 10:44:20 +0000 (12:44 +0200)] 
evaluate: flush set cache from the evaluation phase

This patch reworks 40ef308e19b6 ("rule: flush set cache before flush
command"). This patch flushes the set cache earlier, from the command
evaluation step.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agonft: rearrange help output to group related options together
Arturo Borrero Gonzalez [Thu, 23 Jul 2020 10:41:31 +0000 (12:41 +0200)] 
nft: rearrange help output to group related options together

It has been reported that nft options are a bit chaotic. With a growing list of options for the nft
CLI, we can do better when presenting them to the user who requests help.

This patch introduces a textual output grouping for options, in 4 groups:
  * Options (general)     -- common Unix utility options
  * Options (operative)   -- the options that modify the operative behaviour of nft
  * Options (translation) -- output text modifiers for data translation
  * Options (parsing)     -- output text modifiers for parsing and other operations

There is no behavior change in this patch, is mostly a cosmetic change in the hope that users will
find the nft tool a bit less confusing to use.

After this patch, the help output is:

=== 8< ===
% nft --help
Usage: nft [ options ] [ cmds... ]

Options (general):
  -h, help                      Show this help
  -v, version                   Show version information
  -V                            Show extended version information

Options (ruleset input handling):
  -f, file <filename>           Read input from <filename>
  -i, interactive               Read input from interactive CLI
  -I, includepath <directory>   Add <directory> to the paths searched for include files. Defaul[..]
  -c, check                     Check commands validity without actually applying the changes.

Options (ruleset list formatting):
  -a, handle                    Output rule handle.
  -s, stateless                 Omit stateful information of ruleset.
  -t, terse                     Omit contents of sets.
  -S, service                   Translate ports to service names as described in /etc/services.
  -N, reversedns                Translate IP addresses to names.
  -u, guid                      Print UID/GID as defined in /etc/passwd and /etc/group.
  -n, numeric                   Print fully numerical output.
  -y, numeric-priority          Print chain priority numerically.
  -p, numeric-protocol          Print layer 4 protocols numerically.
  -T, numeric-time              Print time values numerically.

Options (command output format):
  -e, echo                      Echo what has been added, inserted or replaced.
  -j, json                      Format output in JSON
  -d, debug <level [,level...]> Specify debugging level (scanner, parser, eval, netlink, mnl, p[..]
=== 8< ===

While at it, refresh the man page to better reflex this new grouping, and add some missing options.

Joint work with Pablo.

Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agojson: Expect refcount increment by json_array_extend()
Phil Sutter [Wed, 29 Jul 2020 12:21:12 +0000 (14:21 +0200)] 
json: Expect refcount increment by json_array_extend()

This function is apparently not "joining" two arrays but rather copying
all items from the second array to the first, leaving the original
reference in place. Therefore it naturally increments refcounts, which
means if used to join two arrays caller must explicitly decrement the
second array's refcount.

Fixes: e70354f53e9f6 ("libnftables: Implement JSON output support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
4 years agoevaluate: bail out with concatenations and singleton values
Pablo Neira Ayuso [Wed, 22 Jul 2020 15:24:34 +0000 (17:24 +0200)] 
evaluate: bail out with concatenations and singleton values

The rule:

 # nft add rule x y iifname . oifname p . q

is equivalent to:

 # nft add rule x y iifname p oifname q

Bail out with:

 Error: Use concatenations with sets and maps, not singleton values
 add rule x y iifname . oifname p . q
              ^^^^^^^^^^^^^^^^^ ~~~~~

instead of:

 BUG: invalid expression type concat
 nft: evaluate.c:1916: expr_evaluate_relational: Assertion `0' failed.
 Aborted

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: extend 0043concatenated_ranges_0 to cover maps too
Florian Westphal [Wed, 22 Jul 2020 11:51:26 +0000 (13:51 +0200)] 
tests: extend 0043concatenated_ranges_0 to cover maps too

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agonetlink: fix concat range expansion in map case
Florian Westphal [Wed, 22 Jul 2020 11:51:25 +0000 (13:51 +0200)] 
netlink: fix concat range expansion in map case

Maps with range + concatenation do not work:

Input to nft -f:
        map map_test_concat_interval {
                type ipv4_addr . ipv4_addr : mark
                flags interval
                elements = { 192.168.0.0/24 . 192.168.0.0/24 : 1,
                     192.168.0.0/24 . 10.0.0.1 : 2,
                             192.168.1.0/24 . 10.0.0.1 : 3,
                             192.168.0.0/24 . 192.168.1.10 : 4,
                }
        }

nft list:
        map map_test_concat_interval {
                type ipv4_addr . ipv4_addr : mark
                flags interval
                elements = { 192.168.0.0 . 192.168.0.0-10.0.0.1 : 0x00000002,
                             192.168.1.0-192.168.0.0 . 10.0.0.1-192.168.1.10 : 0x00000004 }
        }

This is not a display bug, nft sends broken information
to kernel.  Use the correct key expression to fix this.

Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
4 years agosrc: allow for negative value in variable definitions
Pablo Neira Ayuso [Tue, 21 Jul 2020 13:00:24 +0000 (15:00 +0200)] 
src: allow for negative value in variable definitions

Extend test to cover for negative value in chain priority definition.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: replace variable expression by the value expression
Pablo Neira Ayuso [Tue, 21 Jul 2020 16:09:28 +0000 (18:09 +0200)] 
evaluate: replace variable expression by the value expression

The variable expression provides the binding between the variable
dereference and the value expression. Replace the variable expression by
the real value expression after the evaluation.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: permit get element on maps
Florian Westphal [Tue, 21 Jul 2020 11:10:17 +0000 (13:10 +0200)] 
evaluate: permit get element on maps

Its possible to add an element to a map, but you can't read it back:

before:
nft add element inet filter test "{ 18.51.100.17 . ad:c1:ac:c0:ce:c0 . 3761 : 0x42 }"
nft get element inet filter test "{ 18.51.100.17 . ad:c1:ac:c0:ce:c0 . 3761 : 0x42 }"
Error: No such file or directory; did you mean map ‘test’ in table inet ‘filter’?
get element inet filter test { 18.51.100.17 . ad:c1:ac:c0:ce:c0 . 3761 : 0x42 }
                        ^^^^
after:
nft get element inet filter test "{ 18.51.100.17 . ad:c1:ac:c0:ce:c0 . 3761 : 0x42 }"
table inet filter {
        map test {
                type ipv4_addr . ether_addr . inet_service : mark
                flags interval,timeout
                elements = { 18.51.100.17 . ad:c1:ac:c0:ce:c0 . 3761 : 0x00000042 }
        }
}

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agorule: missing map command expansion
Pablo Neira Ayuso [Tue, 21 Jul 2020 00:11:56 +0000 (02:11 +0200)] 
rule: missing map command expansion

Maps also need to be split in two commands for proper error reporting.

Fixes: c9eae091983a ("src: add CMD_OBJ_SETELEMS")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agorule: flush set cache before flush command
Pablo Neira Ayuso [Mon, 20 Jul 2020 23:50:06 +0000 (01:50 +0200)] 
rule: flush set cache before flush command

Flush the set cache before adding the flush command to the netlink batch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: shell: remove check for reject from prerouting
Pablo Neira Ayuso [Thu, 16 Jul 2020 17:13:22 +0000 (19:13 +0200)] 
tests: shell: remove check for reject from prerouting

It reports a failure with the following kernel patch:

 commit f53b9b0bdc59c0823679f2e3214e0d538f5951b9
 Author: Laura Garcia Liebana <nevola@gmail.com>
 Date:   Sun May 31 22:26:23 2020 +0200

    netfilter: introduce support for reject at prerouting stage

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoevaluate: use evaluate_expr_variable() for chain policy evaluation
Pablo Neira Ayuso [Thu, 16 Jul 2020 17:12:43 +0000 (19:12 +0200)] 
evaluate: use evaluate_expr_variable() for chain policy evaluation

evaluate_policy() is very similar to evaluate_expr_variable(), replace it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: allow to use variables in flowtable and chain devices
Pablo Neira Ayuso [Thu, 16 Jul 2020 12:36:28 +0000 (14:36 +0200)] 
src: allow to use variables in flowtable and chain devices

This patch adds support for using variables for devices in the chain and
flowtable definitions, eg.

 define if_main = lo

 table netdev filter1 {
    chain Main_Ingress1 {
        type filter hook ingress device $if_main priority -500; policy accept;
    }
 }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: shell: chmod 755 testcases/chains/0030create_0
Pablo Neira Ayuso [Thu, 16 Jul 2020 16:30:38 +0000 (18:30 +0200)] 
tests: shell: chmod 755 testcases/chains/0030create_0

Update permissions in this test script.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agotests: extend existing dormat test case to catch a kernel bug
Florian Westphal [Tue, 14 Jul 2020 16:34:59 +0000 (18:34 +0200)] 
tests: extend existing dormat test case to catch a kernel bug

This is a test case for the kernel bug fixed by:
  netfilter: nf_tables: fix nat hook table deletion

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agomonitor: print "dormant" flag in monitor mode
Florian Westphal [Tue, 14 Jul 2020 16:33:59 +0000 (18:33 +0200)] 
monitor: print "dormant" flag in monitor mode

This distinction is important: a table with this flag is inert -- all
base chains are unregistered and see no traffic.

Signed-off-by: Florian Westphal <fw@strlen.de>
4 years agoevaluate: UAF in stmt_evaluate_log_prefix()
Pablo Neira Ayuso [Wed, 15 Jul 2020 20:02:18 +0000 (22:02 +0200)] 
evaluate: UAF in stmt_evaluate_log_prefix()

Release existing list expression including variables after creating the
prefix string.

Fixes: 96c909ef46f0 ("src: allow for variables in the log prefix string")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agoparser_bison: memleak in log prefix string
Pablo Neira Ayuso [Wed, 15 Jul 2020 19:39:39 +0000 (21:39 +0200)] 
parser_bison: memleak in log prefix string

Release the string after creating the constant expression.

Fixes: 96c909ef46f0 ("src: allow for variables in the log prefix string")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agosrc: support for implicit chain bindings
Pablo Neira Ayuso [Sat, 4 Jul 2020 00:43:44 +0000 (02:43 +0200)] 
src: support for implicit chain bindings

This patch allows you to group rules in a subchain, e.g.

 table inet x {
        chain y {
                type filter hook input priority 0;
                tcp dport 22 jump {
                        ip saddr { 127.0.0.0/8, 172.23.0.0/16, 192.168.13.0/24 } accept
                        ip6 saddr ::1/128 accept;
                }
        }
 }

This also supports for the `goto' chain verdict.

This patch adds a new chain binding list to avoid a chain list lookup from the
delinearize path for the usual chains. This can be simplified later on with a
single hashtable per table for all chains.

From the shell, you have to use the explicit separator ';', in bash you
have to escape this:

 # nft add rule inet x y tcp dport 80 jump { ip saddr 127.0.0.1 accept\; ip6 saddr ::1 accept \; }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
4 years agodatatype: convert chain name from gmp value to string
Pablo Neira Ayuso [Sat, 4 Jul 2020 00:43:40 +0000 (02:43 +0200)] 
datatype: convert chain name from gmp value to string

Add expr_chain_export() helper function to convert the chain name that
is stored in a gmp value variable to string.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agosrc: allow for variables in the log prefix string
Pablo Neira Ayuso [Tue, 7 Jul 2020 15:42:37 +0000 (17:42 +0200)] 
src: allow for variables in the log prefix string

For example:

 define test = "state"
 define foo = "match"

 table x {
        chain y {
                ct state invalid log prefix "invalid $test $foo:"
        }
 }

This patch scans for variables in the log prefix string. The log prefix
expression is a list of constant and variable expression that are
converted into a constant expression from the evaluation phase.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agosrc: use expression to store the log prefix
Pablo Neira Ayuso [Tue, 7 Jul 2020 12:31:33 +0000 (14:31 +0200)] 
src: use expression to store the log prefix

Intsead of using an array of char.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
5 years agosegtree: zap element statement when decomposing interval
Pablo Neira Ayuso [Mon, 6 Jul 2020 08:48:16 +0000 (10:48 +0200)] 
segtree: zap element statement when decomposing interval

Otherwise, interval sets do not display element statement such as
counters.

Fixes: 6d80e0f15492 ("src: support for counter in set definition")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>