netlink_delinearize: reverse cross-day meta hour range

f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
reverses the hour range in case that a cross-day range is used, eg.

  meta hour "03:00"-"14:00" counter accept

which results in (Sidney, Australia AEDT time):

  meta hour != "14:00"-"03:00" counter accept

kernel handles time in UTC, therefore, cross-day range may not be
obvious according to local time.

The ruleset listing above is not very intuitive to the reader depending
on their timezone, therefore, complete netlink delinearize path to
reverse the cross-day meta range.

Update manpage to recommend to use a range expression when matching meta
hour range. Recommend range expression for meta time and meta day too.

Extend testcases/listing/meta_time to cover for this scenario.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1737
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

netlink_delinearize: restore binop syntax when listing ruleset for flags

c3d57114f119 ("parser_bison: add shortcut syntax for matching flags
without binary operations") provides a similar syntax to iptables using
a prefix representation for flag matching.

Restore original representation using binop when listing the ruleset.
The parser still accepts the prefix notation for backward compatibility.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

src: do not merge a set with a erroneous one

The included sample causes a crash because we attempt to
range-merge a prefix expression with a symbolic expression.

The first set is evaluated, the symbol expression evaluation fails
and nft queues an error message ("Could not resolve hostname").

However, nft continues evaluation.

nft then encounters the same set definition again and merges the
new content with the preceeding one.

But the first set structure is dodgy, it still contains the
unresolved symbolic expression.

That then makes nft crash (assert) in the set internals.

There are various different incarnations of this issue, but the low
level set processing code does not allow for any partially transformed
expressions to still remain.

Before:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
BUG: invalid range expression type binop
nft: src/expression.c:1479: range_expr_value_low: Assertion `0' failed.

After:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
invalid_range_expr_type_binop:4:18-25: Error: Could not resolve hostname: Name or service not known
elements = { 1&.141.0.1 - 192.168.0.2}
             ^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

doc: libnftables-json: Drop invalid ops from match expression

These make no sense there and are listed again in BINARY OPERATION.

Fixes: 872f373dc50f7 ("doc: Add JSON schema documentation")
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: Fix one json-nft dump for reordered output

Missed this one when regenerating all dumps.

Fixes: 2a0fe52eca32a ("tests: shell: Regenerate all json-nft dumps")
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: Add missing json-nft dumps

Given that a bunch of issues got fixed, add some more dumps.

Also add tests/shell/testcases/owner/dumps/0002-persist.nft while at it,
even though it's really small.

Signed-off-by: Phil Sutter <phil@nwl.cc>

parser: json: Support for synproxy objects

Parsing code was there already, merely the entry in json_parse_cmd_add()
missing.

To support maps with synproxy target, an entry in string_to_nft_object()
is required. While being at it, add other missing entries as well.

Signed-off-by: Phil Sutter <phil@nwl.cc>

json: Support maps with concatenated data

Dump such maps with an array of types in "map" property, make the parser
aware of this.

Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: Regenerate all json-nft dumps

Ordering of 'nft -j list ruleset' output has changed, Regenerate
existing json-nft dumps. No functional change intended, merely the
position of chain objects should have moved up in the "nftables" array.

Signed-off-by: Phil Sutter <phil@nwl.cc>

json: Order output like nft_cmd_expand()

Print empty chain add commands early in list so following verdict maps
and rules referring to them won't cause spurious errors when loading the
resulting ruleset dump.

Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: packetpath/flowtables: Avoid spurious EPERM

On my system for testing, called socat is not allowed to create the pipe
file in local directory (probably due to sshfs). Specify a likely unique
path in /tmp to avoid such problems.

Fixes: 419c0199774c6 ("tests: shell: add test to cover ct offload by using nft flowtables")
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: py: move meter tests to tests/shell

Userspace performs an translation to dynamic set which does not fit well
into tests/py, move them to tests/shell.

Fixes: b8f8ddfff733 ("evaluate: translate meter into dynamic set")
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

src: remove utf-8 character in printf lines

replace "‘" (UTF-8, 0xe280 0x98) with "'" (ASCII 0x27).

Signed-off-by: Florian Westphal <fw@strlen.de>

tests: py: add payload merging test cases

Add a test case that would fail without preceeding fix.

Signed-off-by: Florian Westphal <fw@strlen.de>

nftables: do mot merge payloads on negation

else, a rule like
  tcp sport != 22 tcp dport != 23

will match even if the destination is 23 as long as sport is != 22.
(or vice versa).

Signed-off-by: Sriram Rajagopalan <sriramr@arista.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: translate meter into dynamic set

129f9d153279 ("nft: migrate man page examples with `meter` directive to
sets") already replaced meters by dynamic sets.

This patch removes NFT_SET_ANONYMOUS flag from the implicit set that is
instantiated via meter, so the listing shows a dynamic set instead which
is the recommended approach these days.

Therefore, a batch like this:

 add table t
 add chain t c
 add rule t c tcp dport 80 meter m size 128 { ip saddr timeout 1s limit rate 10/second }

gets translated to a dynamic set:

 table ip t {
        set m {
                type ipv4_addr
                size 128
                flags dynamic,timeout
        }

        chain c {
                tcp dport 80 update @m { ip saddr timeout 1s limit rate 10/second burst 5 packets }
        }
 }

Check for NFT_SET_ANONYMOUS flag is also relaxed for list and flush
meter commands:

 # nft list meter ip t m
 table ip t {
        set m {
                type ipv4_addr
                size 128
                flags dynamic,timeout
        }
 }
 # nft flush meter ip t m

As a side effect the legacy 'list meter' and 'flush meter' commands allow
to flush a dynamic set to retain backward compatibility.

This patch updates testcases/sets/0022type_selective_flush_0 and
testcases/sets/0038meter_list_0 as well as the json output which now
uses the dynamic set representation.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Makefile: mkdir $(builddir}/doc

When building separately from the source tree (as in ../src/configure),
the 'doc' dir is not present from just the source tree. Create the dir
before calling a2x.

Signed-off-by: Neels Hofmeyr <nhofmeyr@sysmocom.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: add test case for named ct objects

Add a dedicated test for named conntrack objects:
timeouts, helpers and expectations.

A json dump file is not added because the json input
code does not support "typeof" declarations for sets/maps.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser: allow to define maps that contain ct helpers

Its currently not possible to use ct helpers in objref maps.

Simply adding "CT HELPER" to "map_block_obj_type" does not work
due to a conflict with the "ct helper" ct_expr block.

  map m {
    type ipv4_addr : ct helper ..

... declares a map storing ip addresses and conntrack helper names
(string type).  This does not make sense, there is no way
to use the associated value (the names) in any sensible way:

ct helper "ftp" - this matches if the packet has a conntrack entry that
was accepted via the "ftp" conntrack helper. In nft vm terms, this is
translated to:

 [ ct load helper => reg 1 ]
 [ cmp eq reg 1 0x00707466 0x00000000 0x00000000 0x00000000 ]

Or one can query a set, e.g. 'ct helper { "ftp", "sip" }'.
"ftp" and "sip" are the kernel-defined names of these connection
tracking helpers.

ct helper set "ftp" is something else, however:

This is used to assign a *userspace defined helper objrect reference*.
Nftables will translate this to:

 [ objref type 3 name ftp ]

.. where "ftp" is a arbitrary user-chosen name.

  ct helper "ftp" {
    type "ftp" protocol tcp
    l3proto ip
  }

IOW, "ct helper" is ambiguous.  Without the "set" keyword (first case),
it places the kernel-defined name of the active connection tracking helper
in the chosen register (or it will cancel rule evaluation if no helper was
active).

With the set keyword (second case), the expected argument is a user-defined
object reference which will then tell the connection tracking engine to
monitor all further packets of the new flow with the given helper template.

This change makes it so that

  map m {
    type ipv4_addr : ct helper ..

declares a map storing ct helper object references suitable for
'ct helper set'.

The better alternative would be to resolve the ambiguity
by adding an additional postfix keyword, for example

 ct helper name (case one)
 ct helper object (case two).

But this needs a kernel change that adds
NFT_CT_HELPER_NAME and NFT_CT_HELPER_OBJECT to enum nft_ct_keys.

While a new kernel could handle old nftables binaries that still use
NFT_CT_HELPER key, new nftables would need to probe support first.

Furthermore,

 ct helper name set "foo"

... would make no sense, as the kernel-defined helper names are
readonly.

 ct helper object "foo"

... would make no sense, unless we extend the kernel to store
the nftables userspace-defined name in a well-known location
in the kernel.  Userdata area cannot work for this, because the
nft conntrack expression in the kernel would need to know how to
retrieve this info again.

Also, I cannot think of a sensible use case for this.
So the only remaining, useful commands are:

ct helper name "ftp"
ct helper object set "foo"

... which is identical to what we already support, just with
extra keyword.

So a much simpler solution that does not need any kernel changes
is make "ct helper" have different meaning depending on wheter it
is placed on the key side, i.e.:

    "typeof ct helper", "typeof ct helper : $value"

versus when its on placed on the data (value) side of maps:

    "typeof $key : ct helper".

Signed-off-by: Florian Westphal <fw@strlen.de>

parser: allow to define maps that contain timeouts and expectations

Its currently not possible to use ct timeouts/expectations/helpers
in objref maps, bison parser lacks the relevant keywords.

This change adds support for timeouts and expectations.
Ct helpers are more problematic, this will come in a different change.

Support is only added for the "typeof" keyword, otherwise we'd
need to add pseudo-datatypes as well, but making "ct expectation"
available as "type" as well might be confusing.

Signed-off-by: Florian Westphal <fw@strlen.de>

rule: fix ASAN errors in chain priority to textual names

ASAN reports several errors when listing this ruleset:

 table ip x {
        chain y {
                type filter hook input priority -2147483648; policy accept;
        }
 }

src/rule.c:1002:8: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
src/rule.c:1001:11: runtime error: signed integer overflow: -2147483648 - 50 cannot be represented in type 'int'

Use int64_t for the offset to avoid an underflow when calculating
closest existing priority definition.

Use llabs() because abs() is undefined with INT32_MIN.

Fixes: c8a0e8c90e2d ("src: Set/print standard chain prios with textual names")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: move test case to "maps" directory

This tests named object maps, so this should reside in maps/
not sets/ directory.

Signed-off-by: Florian Westphal <fw@strlen.de>

tests: maps: add a test case for "limit" objref map

check add, delete and removal operations for objref maps.

Also check type vs. typeof declarations and use both
interval and interval+concatenation (rbtree, pipapo).

Signed-off-by: Florian Westphal <fw@strlen.de>

netlink: allow typeof keywords with objref maps during listing

Without this,

  typeof meta l4proto . ip saddr . tcp sport : limit

... is shown as

  type inet_proto . ipv4_addr . inet_service : limit

The "data" element is a value (the object type number).
It doesn't support userinfo data.

There is no reason to add it, the value is the object type
number that the object-reference map stores.

So, if we have an objref map, DO NOT discard the key part,
as we do for normal maps.

For normal maps, we support either typeof notation, i.e.:

  typeof meta l4proto . ip saddr . tcp sport : ip saddr

or the data type version:
  type inet_proto . ipv4_addr . inet_service : ipv4_addr

... but not a mix, a hyptothetical

  typeof meta l4proto . ip saddr . tcp sport : ipv4_addr

... does not work.

If nft finds no udata attached to the data element, for normal
map case, it has to fall back to the "type" form.

But for objref maps this is expected, udata for key but not for data.
Hence, for objref case, keep the typeof part if its valid.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser: allow typeof in objref maps

Its currently not possible to declare a map that
stores object references with the "typeof" keyword, e.g.

map m {
type ipv4_addr : limit

will work, but

map m {
typeof ip saddr : limit

will give a syntax error ("unexpected limit").
Followup pach will add support for listing side too.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser: compact type/typeof set rules

Set/maps keys can be declared either by 'type' or 'typeof' keyword.

Compact this to use a common block for both cases.

The datatype_set call is redundant, remove it:
at this point $3 == $1->key, so this is a no-op.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser: compact interval typeof rules

There are two nearly identical blocks for typeof maps:
one with INTERVAL keyword present and one without.

Compact this into a single block.

Signed-off-by: Florian Westphal <fw@strlen.de>

src: improve error reporting for destroy command

Example for older kernels (<6.3):
nft destroy table ip missingtable

Before:
Error: Could not process rule: Invalid argument

After:
Error: "destroy" command is not supported, perhaps kernel support is
missing?

Fixes: e1dfd5cc4c46 ("src: add support to command "destroy"")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

parser_json: allow 0 offsets again

Its valid in case of tcp option removal:

[ {
   "reset": {
     "tcp option": {
       "base": 123,
       "len": 0,
       "offset": 0
   }

This makes nft-test.py -j pass again.

Fixes: e08627257ecf ("parser: reject raw payload expressions with 0 length")
Signed-off-by: Florian Westphal <fw@strlen.de>

tests: shell: add more json dumps

Those are expected to be stable, so add them.
Some are not 100% correct, as "typeof" is misprinted as "type" (json output
and input parser lack support for this), but for these files the "type"
is valid too.

This will allow better validation once proper "typeof" support is
added to json.c and json-parser.c.

Signed-off-by: Florian Westphal <fw@strlen.de>

tests: py: add missing json.output data

Fixes: bridge/vlan.t: WARNING: line 56: ...
Fixes: 8b9ae77598b4 ("tests: never merge across non-expression statements redux 2")
Signed-off-by: Florian Westphal <fw@strlen.de>

tests: shell: add regression test for catchall double-delete

Test case for:
 b1db244ffd04 ("netfilter: nf_tables: check if catch-all set element is active in next generation")

Reported-by: lonial con <kongln9170@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

tests: shell: permit use of host-endian constant values in set lookup keys

extend an existing test case with the afl input to cover in/output.

A new test case is added to test linearization, delinearization and
matching

Fixes: c0080feb0d03 ("evaluate: permit use of host-endian constant values in set lookup keys")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

expression: missing line in describe command with invalid expression

Before:

 duh@testbed:~# nft describe blah
 symbol expression, datatype invalid (invalid)duh@testbed:#

After:

 duh@testbed:~# nft describe blah
 symbol expression, datatype invalid (invalid)
 duh@testbed:#

Fixes: 48aca2de80a7 ("iptopt: fix crash with invalid field/type combo")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: permit use of host-endian constant values in set lookup keys

AFL found following crash:

table ip filter {
map ipsec_in {
typeof ipsec in reqid . iif : verdict
flags interval
}

chain INPUT {
type filter hook input priority filter; policy drop;
ipsec in reqid . 100 @ipsec_in
}
}

Which yields:
nft: evaluate.c:1213: expr_evaluate_unary: Assertion `!expr_is_constant(arg)' failed.

All existing test cases with constant values use big endian values, but
"iif" expects host endian values.

As raw values were not supported before, concat byteorder conversion
doesn't handle constants.

Fix this:

1. Add constant handling so that the number is converted in-place,
   without unary expression.

2. Add the inverse handling on delinearization for non-interval set
   types.
   When dissecting the concat data soup, watch for integer constants where
   the datatype indicates host endian integer.

Last, extend an existing test case with the afl input to cover
in/output.

A new test case is added to test linearization, delinearization and
matching.

Based on original patch from Florian Westphal, patch subject and
description wrote by him.

Fixes: b422b07ab2f9 ("src: permit use of constant values in set lookup keys")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

netlink_delinearize: move concat and value postprocessing to helpers

No functional changes intended.

Signed-off-by: Florian Westphal <fw@strlen.de>

cache: Always set NFT_CACHE_TERSE for list cmd with --terse

This fixes at least 'nft -t list table ...' and 'nft -t list set ...'.

Note how --terse handling for 'list sets/maps' remains in place since
setting NFT_CACHE_TERSE does not fully undo NFT_CACHE_SETELEM: setting
both enables fetching of anonymous sets which is pointless for that
command.

Reported-by: anton.khazan@gmail.com
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1735
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>

netlink_linearize: add assertion to catch for buggy byteorder

Add assertion to catch buggy bytecode where unary expression is present
with 1-byte selectors, where no byteorder conversion is required.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: skip byteorder conversion for selector smaller than 2 bytes

Add unary expression to trigger byteorder conversion for host byteorder
selectors only if selectors length is larger or equal than 2 bytes.

 # cat test.nft
 table ip x {
        set test {
                type ipv4_addr . ether_addr . inet_proto
                flags interval
        }

        chain y {
                ip saddr . ether saddr . meta l4proto @test counter
        }
 }

 # nft -f test.nft
 ip x y
  [ meta load iiftype => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ payload load 4b @ network header + 12 => reg 1 ]
  [ payload load 6b @ link header + 6 => reg 9 ]
  [ meta load l4proto => reg 11 ]
  [ byteorder reg 11 = hton(reg 11, 2, 1) ] <--- should not be here
  [ lookup reg 1 set test ]
  [ counter pkts 0 bytes 0 ]

Fixes: 1017d323cafa ("src: support for selectors with different byteorder with interval concatenations")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests/shell: no longer support unprettified ".json-nft" files

By now, all ".json-nft" files are prettified and will be generated in
that form.

Drop the fallback code that accepts them in the previous form.

Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: Pretty-print all *.json-nft dumps

The problem with single line output as produced by 'nft -j list ruleset'
is its incompatibility to unified diff format as any change in this
single line will produce a diff which contains the old and new lines in
total. This is not just unreadable but will blow up patches which may
exceed mailinglists' mail size limits.

Convert them all at once by feeding their contents to
tests/shell/helpers/json-pretty.sh.

Signed-off-by: Phil Sutter <phil@nwl.cc>

tests/shell: have .json-nft dumps prettified to wrap lines

Previously, the .json-nft file in git contains the output of `nft -j
list ruleset`. This is one long line and makes diffs harder to review.

Instead, have the prettified .json-nft file committed to git.

- the diff now operates on the prettified version. That means, it
  compares essentially

     - `nft -j list ruleset | json-sanitize-ruleset.sh | json-pretty.sh`
     - `cat "$TEST.json-nft" | json-pretty.sh`

  The script "json-diff-pretty.sh" is no longer used. It is kept
  however, because it might be a useful for manual comparing files.

  Note that "json-sanitize-ruleset.sh" and "json-pretty.sh" are still
  two separate scripts and called at different times. They also do
  something different. The former mangles the JSON to account for changes
  that are not stable (in the JSON data itself), while the latter only
  pretty prints it.

- when generating a new .json-nft dump file, the file will be updated to
  use the new, prettified format, unless the file is in the old format
  and needs no update. This means, with DUMPGEN=y, old style is preserved
  unless an update becomes necessary.

This requires "json-pretty.sh" having stable output, as those files are
committed to git. This is probably fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: missing auto-merge in json output

Several tests reports DUMP_FAILED because it was missing the auto-merge
flag. That is, the original json dump was not correct. Update tests
accordingly now that json support provides an automerge flag.

Fixes: a4034c66b03e ("json: Support sets' auto-merge option")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

cache: Optimize caching for 'list tables' command

No point in fetching anything other than existing tables from kernel:
'list tables' merely prints existing table names, no contents.

Also populate filter's family field to reduce overhead when listing
tables in one family with many tables in another one. It works without
further adjustments because nftnl_nlmsg_build_hdr() will use the value
for nfgen_family.

Reported-by: anton.khazan@gmail.com
Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1735
Signed-off-by: Phil Sutter <phil@nwl.cc>

Makefile.am: don't silence -Wimplicit-function-declaration

This becomes an error in GCC 14 and Clang 16. It's a common
misconception that these warnings are invalid or simply noise for
Bison/parser files, but even if that were true, we'd need to handle it
somehow anyway. Silencing them does nothing, so stop doing that.

Further, I don't actually get any warnings to fix with bison-3.8.2. This
mirrors changes we've done in other netfilter.org projects.

Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>

evaluate: fix check for unknown in cmd_op_to_name

Example:
nft --debug=all destroy table ip missingtable

Before:
Evaluate unknown

After:
Evaluate destroy

Fixes: e1dfd5cc4c46 ("src: add support to command "destroy"")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>

evaluate: don't assert on net/transport header conflict

before:
nft: evaluate.c:467: conflict_resolution_gen_dependency: Assertion `expr->payload.base == PROTO_BASE_LL_HDR' failed.
Aborted (core dumped)

conflict_resolution_gen_dependency() can only handle linklayer
conflicts, hence the assert.

Rename it accordingly.  Also rename resolve_protocol_conflict, it doesn't
do anything for != PROTO_BASE_LL_HDR and extend the assertion to that
function too.

Callers now enforce PROTO_BASE_LL_HDR prerequisite.

after:
Error: conflicting transport layer protocols specified: comp vs. udp
 ip6 nexthdr comp udp dport 4789
                  ^^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>

datatype: display 0s time datatype

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: shell: cover netns removal for netdev and inet/ingress basechains

Add two tests to exercise netns removal with netdev and inet/ingress
basechains.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: shell: netdevice removal for inet family

cover netdevice removal when such netdevice belongs to basechain.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

json: Support sets' auto-merge option

If enabled, list the option as additional attribute with boolean value.

Fixes: e70354f53e9f6 ("libnftables: Implement JSON output support")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1734
Signed-off-by: Phil Sutter <phil@nwl.cc>

tests: shell: add test to cover ct offload by using nft flowtables

To cover kernel patch ("netfilter: nf_tables: set transport offset from mac header for netdev/egress").

Signed-off-by: Yi Chen <yiche@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: py: remove huge-limit test cases

These tests will fail once the kernel checks for overflow
in the internal token bucken counter, so drop them.

Signed-off-by: Florian Westphal <fw@strlen.de>

rule: fix sym refcount assertion

Scope release must happen last.
afl provided a reproducer where policy is a define, because
scope is released too early we get:
nft: src/rule.c:559: scope_release: Assertion `sym->refcnt == 1' failed.

... because chain->policy is EXPR_SYMBOL.

Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: error out when store needs more than one 128bit register of align fixup

Else this gives:
nft: evaluate.c:2983: stmt_evaluate_payload: Assertion `sizeof(data) * BITS_PER_BYTE >= masklen' failed.

For loads, this is already prevented via expr_evaluate_bits() which has:

  if (masklen > NFT_REG_SIZE * BITS_PER_BYTE)
      return expr_error(ctx->msgs, expr, "mask length %u exceeds allowed maximum of %u\n",
                        masklen, NFT_REG_SIZE * BITS_PER_BYTE);

But for the store path this isn't called.
The reproducer asks to store a 128 bit integer at bit offset 1, i.e.
17 bytes would need to be munged, but we can only handle up to 16 bytes
(one pseudo-register).

Fixes: 78936d50f306 ("evaluate: add support to set IPv6 non-byte header fields")
Signed-off-by: Florian Westphal <fw@strlen.de>

parser: reject raw payload expressions with 0 length

Reject this at parser stage.  Fix up the json input side too, else
reproducer gives:
nft: src/netlink.c:243: netlink_gen_raw_data: Assertion `len > 0' failed.

Signed-off-by: Florian Westphal <fw@strlen.de>

payload: only assert if l2 header base has no length

nftables will assert in some cases because the sanity check is done even
for network and transport header bases.

However, stacked headers are only supported for the link layer.
Move the assertion around and add a test case for this.

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: release mpz type in expr_evaluate_list() error path

Detected when running:

 # nft -f tests/shell/testcases/bogons/nft-f/no_integer_basetype_crash
 ==383222==ERROR: LeakSanitizer: detected memory leaks

 Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fe7b54a9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7fe7b538b9a9 in __gmp_default_allocate (/lib/x86_64-linux-gnu/libgmp.so.10+0xc9a9)

Fixes: 3671c4897003 ("evaluate: guard against NULL basetype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: release key expression in error path of implicit map with unknown datatype

Detected when running:

 # nft -f tests/shell/testcases/bogons/nft-f/mapping_with_invalid_datatype_crash
 ==382584==ERROR: LeakSanitizer: detected memory leaks

 Direct leak of 144 byte(s) in 1 object(s) allocated from:
    #0 0x7fde06ca9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7fde062924af in xmalloc src/utils.c:31
    #2 0x7fde0629266c in xzalloc src/utils.c:70
    #3 0x7fde06167299 in expr_alloc src/expression.c:46
    #4 0x7fde0616b014 in constant_expr_alloc src/expression.c:420
    #5 0x7fde06128e43 in expr_evaluate_map src/evaluate.c:2027
    #6 0x7fde06137b06 in expr_evaluate src/evaluate.c:2891
    #7 0x7fde06132417 in expr_evaluate_relational src/evaluate.c:2497
    #8 0x7fde06137b36 in expr_evaluate src/evaluate.c:2895
    #9 0x7fde06137d5f in stmt_evaluate_expr src/evaluate.c:2914
    #10 0x7fde061524c8 in stmt_evaluate src/evaluate.c:4646
    #11 0x7fde0615c9ee in rule_evaluate src/evaluate.c:5202
    #12 0x7fde061600c7 in cmd_evaluate_add src/evaluate.c:5422

Fixes: 70054e6e1c87 ("evaluate: catch implicit map expressions without known datatype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: bail out if anonymous concat set defines a non concat expression

Iterate over the element list in the anonymous set to validate that all
expressions are concatenations, otherwise bail out.

  ruleset.nft:3:46-53: Error: expression is not a concatenation
               ip protocol . th dport vmap { tcp / 22 : accept, tcp . 80 : drop}
                                             ^^^^^^^^

This is based on a patch from Florian Westphal.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: do not fetch next expression on runaway number of concatenation components

If this is the last expression, then the runaway flag is set on and
evaluation bails in the next iteration, do not fetch next list element
which refers to the list head.

I found this by code inspection, I could not trigger any crash with this
one.

Fixes: ae1d54d1343f ("evaluate: do not crash on runaway number of concatenation components")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: skip anonymous set optimization for concatenations

Concatenation is only supported with sets. Moreover, stripping of the
set leads to broken ruleset listing, therefore, skip this optimization
for the concatenations.

Fixes: fa17b17ea74a ("evaluate: revisit anonymous set with single element optimization")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

evaluate: add missing range checks for dup,fwd and payload statements

Else we assert with:
BUG: unknown expression type range
nft: src/netlink_linearize.c:912: netlink_gen_expr: Assertion `0' failed.

While at it, condense meta and exthdr to reuse the same helper.

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: tproxy: move range error checks after arg evaluation

Testing for range before evaluation will still crash us later during
netlink linearization, prefixes turn into ranges, symbolic expression
might hide a range/prefix.

So move this after the argument has been evaluated.

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: error out when expression has no datatype

add rule ip6 f i rt2 addr . ip6 daddr  { dead:: . dead:: }

... will cause a segmentation fault, we assume expr->dtype is always
set.

rt2 support is incomplete, the template is uninitialised.

This could be fixed up, but rt2 (a subset of the deperecated type 0),
like all other routing headers, lacks correct dependency tracking.

Currently such routing headers are always assumed to be segment routing
headers, we would need to add dependency on 'Routing Type' field in the
routing header, similar to icmp type/code.

Signed-off-by: Florian Westphal <fw@strlen.de>

doc: clarify reject is supported at prerouting stage

It's supported since kernel commit f53b9b0bdc59 ("netfilter: introduce
support for reject at prerouting stage").

Reported-by: Dan Winship <danwinship@redhat.com>
Signed-off-by: Quan Tian <tianquan23@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

doc: incorrect datatype description for icmpv6_type and icmpvx_code

Fix incorrect description in manpage:

 ICMPV6 TYPE TYPE is icmpv6_type
 ICMPVX CODE TYPE is icmpx_code

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: shell: extend coverage for netdevice removal

Add two extra tests to exercise netdevice removal path.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

tests: add a test case for double-flush bug in pipapo

Test for
'netfilter: nft_set_pipapo: skip inactive elements during set walk'.

Reported-by: Xingyuan Mo <hdthky0@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

datatype: do not assert when value exceeds expected width

Inputs:
ip protocol . th dport { tcp / 22,  }'
or
th dport . ip protocol { tcp / 22,  }'

are not rejected at this time. 'list ruleset' yields:
 ip protocol & nft: src/gmputil.c:77: mpz_get_uint8: Assertion `cnt <= 1' failed.
or
 th dport & nft: src/gmputil.c:87: mpz_get_be16: Assertion `cnt <= 1' failed.

While this should be caught at input too, the print path should be more
robust, e.g. when there are direct nfnetlink users.

After this patch, the print functions fall back to
'integer_type_print' which can handle large numbers too.

Note that the output printed this way cannot be read back by nft;
it will dump something like:

  tcp dport & 18446739675663040512 . ip protocol 0 . 0

but thats better than assert().

v2: same problem exists for service too.

Signed-off-by: Florian Westphal <fw@strlen.de>

tests: shell: prefer project nft to system-wide nft

Use $NFT (src/nft, in-tree binary), not the one installed by the distro.
Else we may not find newly added bugs unless user did "make install" or
bug has propagated to release.

Signed-off-by: Florian Westphal <fw@strlen.de>

datatype: Describe rt symbol tables

Implement a symbol_table_print() wrapper for the run-time populated
rt_symbol_tables which formats output similar to expr_describe() and
includes the data source.

Since these tables reside in struct output_ctx there is no implicit
connection between data type and therefore providing callbacks for
relevant datat types which feed the data into said wrapper is a simpler
solution than extending expr_describe() itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>

datatype: Initialize rt_symbol_tables' base field

It is unconditionally accessed in symbol_table_print() so make sure it
is initialized to either BASE_DECIMAL (arbitrary) for empty or
non-existent source files or a proper value depending on entry number
format.

Signed-off-by: Phil Sutter <phil@nwl.cc>

datatype: rt_symbol_table_init() to search for iproute2 configs

There is an ongoing effort among various distributions to tidy up in
/etc. The idea is to reduce contents to just what the admin manually
inserted to customize the system, anything else shall move out to /usr
(or so). The various files in /etc/iproute2 fall in that category as
they are seldomly modified.

The crux is though that iproute2 project seems not quite sure yet where
the files should go. While v6.6.0 installs them into /usr/lib/iproute2,
current mast^Wmain branch uses /usr/share/iproute2. Assume this is going
to stay as /(usr/)lib does not seem right for such files.

Note that rt_symbol_table_init() is not just used for
iproute2-maintained configs but also for connlabel.conf - so retain the
old behaviour when passed an absolute path.

Signed-off-by: Phil Sutter <phil@nwl.cc>

parser_bison: ensure all timeout policy names are released

We need to add a custom destructor for this structure, it
contains the dynamically allocated names.

a:5:55-55: Error: syntax error, unexpected '}', expecting string
policy = { estabQisheestablished : 2m3s, cd : 2m3s, }

==562373==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 160 byte(s) in 2 object(s) allocated from:
    #1 0x5a565b in xmalloc src/utils.c:31:8
    #2 0x5a565b in xzalloc src/utils.c:70:8
    #3 0x3d9352 in nft_parse_bison_filename src/libnftables.c:520:8
[..]

Fixes: c7c94802679c ("src: add ct timeout support")
Signed-off-by: Florian Westphal <fw@strlen.de>

src: do not allow to chain more than 16 binops

netlink_linearize.c has never supported more than 16 chained binops.
Adding more is possible but overwrites the stack in
netlink_gen_bitwise().

Add a recursion counter to catch this at eval stage.

Its not enough to just abort once the counter hits
NFT_MAX_EXPR_RECURSION.

This is because there are valid test cases that exceed this.
For example, evaluation of 1 | 2 will merge the constans, so even
if there are a dozen recursive eval calls this will not end up
with large binop chain post-evaluation.

v2: allow more than 16 binops iff the evaluation function
    did constant-merging.

Signed-off-by: Florian Westphal <fw@strlen.de>

netlink: fix stack overflow due to erroneous rounding

Byteorder switch in this function may undersize the conversion
buffer by one byte, this needs to use div_round_up().

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: don't crash if object map does not refer to a value

Before:
BUG: Value export of 512 bytes would overflownft: src/netlink.c:474: netlink_gen_prefix: Assertion `0' failed.

After:
66: Error: Object mapping data should be a value, not prefix
synproxy name ip saddr map { 192.168.1.0/24 : "v*" }

Signed-off-by: Florian Westphal <fw@strlen.de>

parser_bison: error out on duplicated type/typeof/element keywords

Otherwise nft will leak the previous definition (expressions).
Also remove the nonsensical

   datatype_set($1->key, $3->dtype);

This is a no-op, at this point: $1->key and $3 are identical.

Signed-off-by: Florian Westphal <fw@strlen.de>

tests: shell: add test to cover payload transport match and mangle

Exercise payload transport match and mangle for inet, bridge and netdev
families with IPv4 and IPv6 packets.

To cover kernel patch ("netfilter: nf_tables: set transport offset from
mac header for netdev/egress").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

intervals: BUG on prefix expressions without value

Its possible to end up with prefix expressions that have
a symbolic expression, e.g.:

table t {
        set s {
                type inet_service
                flags interval
                elements = { 172.16.0.0/16 }
        }

        set s {
                type inet_service
                flags interval
                elements = { 0-1024, 8080-8082, 10000-40000 }
        }
}

Without this change, nft will crash.  We end up in setelem_expr_to_range()
with prefix "/16" for the symbolic expression "172.16.0.0".

We than pass invalid mpz_t pointer into libgmp.

This isn't a real fix, but instead of blindly assuming that the attached
expression has a gmp value die with at least some info.

Signed-off-by: Florian Westphal <fw@strlen.de>

tcpopt: don't create exthdr expression without datatype

The reproducer crashes during concat evaluation, as the
exthdr expression lacks a datatype.

This should never happen, i->dtype must be set.

In this case the culprit is tcp option parsing, it will
wire up a non-existent template, because the "nop" option
has no length field (1 byte only).

Signed-off-by: Florian Westphal <fw@strlen.de>

intervals: set_to_range can be static

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: fix stack overflow with huge priority string

Alternative would be to refactor this and move this into the parsers
(bison, json) instead of this hidden re-parsing.

Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>

netlink: fix stack buffer overflow with sub-reg sized prefixes

The calculation of the dynamic on-stack array is incorrect,
the scratch space can be too low which gives stack corruption:

AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdb454f064..
    #1 0x7fabe92aaac4 in __mpz_export_data src/gmputil.c:108
    #2 0x7fabe92d71b1 in netlink_export_pad src/netlink.c:251
    #3 0x7fabe92d91d8 in netlink_gen_prefix src/netlink.c:476

div_round_up() cannot be used here, it fails to account for register
padding.  A 16 bit prefix will need 2 registers (start, end -- 8 bytes
in total).

Remove the dynamic sizing and add an assertion in case upperlayer
ever passes invalid expr sizes down to us.

After this fix, the combination is rejected by the kernel
because of the maps' wrong data size, before the fix userspace
may crash before.

Signed-off-by: Florian Westphal <fw@strlen.de>

src: reject large raw payload and concat expressions

The kernel will reject this too, but unfortunately nft may try
to cram the data into the underlying libnftnl expr.

This causes heap corruption or
BUG: nld buffer overflow: want to copy 132, max 64

After:

Error: Concatenation of size 544 exceeds maximum size of 512
udp length . @th,0,512 . @th,512,512 { 47-63 . 0xe373135363130 . 0x33131303735353203 }
                           ^^^^^^^^^

resp. same warning for an over-sized raw expression.

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: exthdr: statement arg must be not be a range

Else we get:
BUG: unknown expression type range
nft: src/netlink_linearize.c:909: netlink_gen_expr: Assertion `0' failed.

Signed-off-by: Florian Westphal <fw@strlen.de>

netlink: don't crash if prefix for < byte is requested

If prefix is used with a datatype that has less than 8 bits an
assertion is triggered:

src/netlink.c:243: netlink_gen_raw_data: Assertion `len > 0' failed.

This is esoteric, the alternative would be to restrict prefixes
to ipv4/ipv6 addresses.

Simpler fix is to use round_up instead of divide.

Signed-off-by: Florian Westphal <fw@strlen.de>

Revert "evaluate: error out when existing set has incompatible key"

This breaks existing behaviour, add a test case so this is caught in
the future.

The reverted test case will be brought back once a better fix
is available.

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: fix gmp assertion with too-large reject code

Before:
nft: gmputil.c:77: mpz_get_uint8: Assertion `cnt <= 1' failed.
After: Error: reject code must be integer in range 0-255

Signed-off-by: Florian Westphal <fw@strlen.de>

meta: fix tc classid parsing out-of-bounds access

AddressSanitizer: heap-buffer-overflow on address 0x6020000003af ...
  #0 0x7f9a83cbb402 in tchandle_type_parse src/meta.c:89
  #1 0x7f9a83c6753f in symbol_parse src/datatype.c:138

strlen() - 1 can underflow if length was 0.

Simplify the function, there is no need to duplicate the string
while scanning it.

Expect the first strtol to stop at ':', scan for the minor number next.
The second scan is required to stop at '\0'.

Fixes: 6f2eb8548e0d ("src: meta priority support using tc classid")
Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: error out when existing set has incompatible key

Before:
BUG: invalid range expression type symbol
nft: expression.c:1494: range_expr_value_high: Assertion `0' failed.

After:
range_expr_value_high_assert:5:20-27: Error: Could not resolve protocol name
                elements = { 100-11.0.0.0, }
                                 ^^^^^^^^
range_expr_value_high_assert:7:6-7: Error: set definition has conflicting key (ipv4_addr vs inet_proto)

Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: stmt_nat: set reference must point to a map

nat_concat_map() requires a datamap, else we crash:
set->data is dereferenced.

Also update expr_evaluate_map() so that EXPR_SET_REF is checked there
too.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser_bison: fix memory leaks on hookspec error processing

prio_spec may contain an embedded expression, release it.
We also need to release the device expr and the hook string.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser_bison: close chain scope before chain release

cmd_alloc() will free the chain, so we must close the scope opened
in chain_block_alloc beforehand.

The included test file will cause a use-after-free because nft attempts
to search for an identifier in a scope that has been freed:

AddressSanitizer: heap-use-after-free on address 0x618000000368 at pc 0x7f1cbc0e6959 bp 0x7ffd3ccb7850 sp 0x7ffd3ccb7840
    #0 0x7f1cbc0e6958 in symbol_lookup src/rule.c:629
    #1 0x7f1cbc0e66a1 in symbol_get src/rule.c:588
    #2 0x7f1cbc120d67 in nft_parse src/parser_bison.y:4325

Fixes: a66b5ad9540d ("src: allow for updating devices on existing netdev chain")
Signed-off-by: Florian Westphal <fw@strlen.de>

parser_bison: fix ct scope underflow if ct helper section is duplicated

table inet filter {
ct helper sip-5060u {
type "sip" protocol udp
l3proto ip
}5060t {
type "sip" protocol tcp
l3pownerip
}

Will close the 'ct' scope twice, it has to be closed AFTER the separator
has been parsed.

While not strictly needed, also error out if the protocol is already
given, this provides a better error description.

Also make sure we release the string in all error branches.

Signed-off-by: Florian Westphal <fw@strlen.de>

parser_bison: make sure obj_free releases timeout policies

obj_free() won't release them because ->type is still 0 at this
point.

Init this to CT_TIMEOUT.

Signed-off-by: Florian Westphal <fw@strlen.de>

netlink_linearize: avoid strict-overflow warning in netlink_gen_bitwise()

With gcc-13.2.1-1.fc38.x86_64:

  $ gcc -Iinclude -c -o tmp.o src/netlink_linearize.c -Werror -Wstrict-overflow=5 -O3
  src/netlink_linearize.c: In function ‘netlink_gen_bitwise’:
  src/netlink_linearize.c:1790:1: error: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Werror=strict-overflow]
   1790 | }
        | ^
  cc1: all warnings being treated as errors

It also makes more sense this way, where "n" is the hight of the
"binops" stack, and we check for a non-empty stack with "n > 0" and pop
the last element with "binops[--n]".

Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

evaluate: fix bogus assertion failure with boolean datatype

The assertion is too strict, as found by afl++:

typeof iifname . ip saddr . meta ipsec
elements = { "eth0" . 10.1.1.2 . 1 }

meta ipsec is boolean (1 bit), but datasize of 1 is set at 8 bit.

Fixes: 22b750aa6dc9 ("src: allow use of base integer types as set keys in concatenations")
Signed-off-by: Florian Westphal <fw@strlen.de>

netlink: add and use nft_data_memcpy helper

There is a stack overflow somewhere in this code, we end
up memcpy'ing a way too large expr into a fixed-size on-stack
buffer.

This is hard to diagnose, most of this code gets inlined so
the crash happens later on return from alloc_nftnl_setelem.

Condense the mempy into a helper and add a BUG so we can catch
the overflow before it occurs.

->value is too small (4, should be 16), but for normal
cases (well-formed data must fit into max reg space, i.e.
64 byte) the chain buffer that comes after value in the
structure provides a cushion.

In order to have the new BUG() not trigger on valid data,
bump value to the correct size, this is userspace so the additional
60 bytes of stack usage is no concern.

Signed-off-by: Florian Westphal <fw@strlen.de>