The conditional around json_unpack() was meant to accept a missing
policy attribute. But the accidentally inverted check made the function
either ignore a given policy or access uninitialized memory.
Parts of the code were unsafe (parsing 'I' format into uint32_t), the
rest just plain wrong (parsing 'o' format into char *tmp). Introduce a
temporary int variable to parse into.
The statement happily accepted any valid expression as payload and
assumed it to be a tcpopt expression (actually, a special case of
exthdr). Add a check to make sure this is the case.
Standard syntax does not provide this flexibility, so no need to have
the check there as well.
3975430b12d9 ("src: expand table command before evaluation") moved
ruleset expansion before evaluation, except for sets and maps. For
sets and maps there is still a post_expand() phase.
This patch moves sets and map expansion to allocate an independent
CMD_OBJ_SETELEMS command to add elements to named set and maps which is
evaluated, this consolidates the ruleset expansion to happen always
before the evaluation step for all objects, except for anonymous sets
and maps.
This approach avoids an interference with the set interval code which
detects overlaps and merges of adjacents ranges. This set interval
routine uses set->init to maintain a cache of existing elements. Then,
the post_expand() phase incorrectly expands set->init cache and it
triggers a bogus ENOENT errors due to incorrect bytecode (placing
element addition before set creation) in combination with user declared
sets using the flat syntax notation.
Since the evaluation step (coming after the expansion) creates
implicit/anonymous sets and maps, those are not expanded anymore. These
anonymous sets still need to be evaluated from set_evaluate() path and
the netlink bytecode generation path, ie. do_add_set(), needs to deal
with anonymous sets.
Note that, for named sets, do_add_set() does not use set->init. Such
content is part of the existing cache, and the CMD_OBJ_SETELEMS command
is responsible for adding elements to named sets.
Fixes: 3975430b12d9 ("src: expand table command before evaluation") Reported-by: Jann Haber <jannh@selfnet.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Test `./tests/shell/run-tests.sh -V tests/shell/testcases/maps/nat_addr_port`
fails:
==118== 195 (112 direct, 83 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==118== at 0x484682C: calloc (vg_replace_malloc.c:1554)
==118== by 0x48A39DD: xmalloc (utils.c:37)
==118== by 0x48A39DD: xzalloc (utils.c:76)
==118== by 0x487BDFD: datatype_alloc (datatype.c:1205)
==118== by 0x487BDFD: concat_type_alloc (datatype.c:1288)
==118== by 0x488229D: stmt_evaluate_nat_map (evaluate.c:3786)
==118== by 0x488229D: stmt_evaluate_nat (evaluate.c:3892)
==118== by 0x488229D: stmt_evaluate (evaluate.c:4450)
==118== by 0x488328E: rule_evaluate (evaluate.c:4956)
==118== by 0x48ADC71: nft_evaluate (libnftables.c:552)
==118== by 0x48AEC29: nft_run_cmd_from_buffer (libnftables.c:595)
==118== by 0x402983: main (main.c:534)
I think the reference handling for datatype is wrong. It was introduced
by commit 01a13882bb59 ('src: add reference counter for dynamic
datatypes').
We don't notice it most of the time, because instances are statically
allocated, where datatype_get()/datatype_free() is a NOP.
Fix and rework.
- Commit 01a13882bb59 comments "The reference counter of any newly
allocated datatype is set to zero". That seems not workable.
Previously, functions like datatype_clone() would have returned the
refcnt set to zero. Some callers would then then set the refcnt to one, but
some wouldn't (set_datatype_alloc()). Calling datatype_free() with a
refcnt of zero will overflow to UINT_MAX and leak:
if (--dtype->refcnt > 0)
return;
While there could be schemes with such asymmetric counting that juggle the
appropriate number of datatype_get() and datatype_free() calls, this is
confusing and error prone. The common pattern is that every
alloc/clone/get/ref is paired with exactly one unref/free.
Let datatype_clone() return references with refcnt set 1 and in
general be always clear about where we transfer ownership (take a
reference) and where we need to release it.
- set_datatype_alloc() needs to consistently return ownership to the
reference. Previously, some code paths would and others wouldn't.
given:
table ip filter {
set test {
type ipv4_addr . ether_addr . mark
flags interval
elements = { 198.51.100.0/25 . 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00 . 0x0000006f, }
}
}
We get lookup failure:
nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f }
Error: Could not process rule: No such file or directory
Its possible to work around this via dummy range somewhere in the key, e.g.
nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f-0x6f }
but that shouldn't be needed, so make sure the INTERVAL flag is enabled
for the queried element if the set is of interval type.
This patch reworks it to perform this optimization from the evaluation
step of the relational expression. Hence, when optimizing for protocol
flags, use OP_EQ instead of OP_IMPLICIT, that is:
tcp flags { syn }
becomes (to represent an exact match):
tcp flags == syn
given OP_IMPLICIT and OP_EQ are not equivalent for flags.
01167c393a12 ("evaluate: do not remove anonymous set with protocol flags
and single element") disabled this optimization, which is enabled again
after this patch.
Fixes: 01167c393a12 ("evaluate: do not remove anonymous set with protocol flags and single element") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
nft_gmp_print() passes the format string and arguments to
gmp_vfprintf(). Note that the format string is then interpreted
by gmp, which also understand special specifiers like "%Zx".
Note that with clang we get various compiler warnings:
gcc doesn't warn, because to gcc 'Z' is a deprecated alias for 'z' and
because the 3rd argument of the attribute((format())) is zero (so gcc
doesn't validate the arguments). But Z specifier in gmp expects a
"mpz_t" value and not a size_t. It's really not the same thing.
The correct solution is not to mark the function to accept a printf format
string.
Fixes: 2535ba7006f2 ('src: get rid of printf') Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Otherwise, nft crashes with prefix longer than 127 bytes:
# nft add rule x y log prefix \"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\"
==159385==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffed5bf4a10 at pc 0x7f3134839269 bp 0x7ffed5bf48b0 sp 0x7ffed5bf4060
WRITE of size 129 at 0x7ffed5bf4a10 thread T0
#0 0x7f3134839268 in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:778
#1 0x7f3133e3074e in __mpz_export_data /tmp/nftables/src/gmputil.c:110
#2 0x7f3133d21d3c in expr_to_string /tmp/nftables/src/expression.c:192
#3 0x7f3133ded103 in netlink_gen_log_stmt /tmp/nftables/src/netlink_linearize.c:1148
#4 0x7f3133df33a1 in netlink_gen_stmt /tmp/nftables/src/netlink_linearize.c:1682
[...]
Fixes: e76bb3794018 ('src: allow for variables in the log prefix string') Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The old "plain" numbers are still accepted (and assumed to be in
seconds), but output will use units which is unexpected due to 'O -'.
Adjust input instead of adding this subtly different output line.
Fixes: 5c25c5a35cbd2 ("parser: allow ct timeouts to use time_spec values") Signed-off-by: Phil Sutter <phil@nwl.cc>
Since commit 343a51702656a ("src: store expr, not dtype to track data in
sets"), set->data is allocated for object maps in set_evaluate(), all
other map types have set->data initialized by the parser already,
set_evaluate() also checks that.
Drop the confusing check, later in the function set->data is
dereferenced unconditionally.
Fixes: 343a51702656a ("src: store expr, not dtype to track data in sets") Signed-off-by: Phil Sutter <phil@nwl.cc>
On a slow system, one of the 'delete element' commands would
occasionally fail. Assuming it can only happen if the 2s timeout passes
"too quickly", work around it by adding elements with a 2m timeout
instead and when wanting to test the element expiry just drop and add
the element again with a short timeout.
Fixes: 6231d3fa4af1e ("tests: shell: Fix for unstable sets/0043concatenated_ranges_0") Signed-off-by: Phil Sutter <phil@nwl.cc>
Its legal to DNAT in output and SNAT in input chain, so don't test
for that being illegal.
Fixes: 8beafab74c39 ("rule: allow src/dstnat prios in input and output") Fixes: 34ce4e4a7bb6 ("test: shell: Test cases for standard chain prios") Signed-off-by: Florian Westphal <fw@strlen.de>
Extend e0aace943412 ("libnftables: Drop cache in error case") to also
drop the cache with -c/--check, this is a dry run mode and kernel does
not get any update.
This fixes a bug with -o/--optimize, which first runs in an implicit
-c/--check mode to validate that the ruleset is correct, then it
provides the proposed optimization. In this case, if the cache is not
emptied, old objects in the cache refer to scanner data that was
already released, which triggers BUG like this:
Make "nft list sets" include set elements in listing by default.
In nftables 1.0.0, "nft list sets" did not include the set elements,
but with "--json" they were included.
1.0.1 and newer never include them.
This causes a problem for people updating from 1.0.0 and relying
on the presence of the set elements.
Change nftables to always include the set elements.
The "--terse" option is honored to get the "no elements" behaviour.
nft reports EEXIST when reading an existing set whose NFT_SET_EVAL has
been previously inferred from the ruleset.
# cat test.nft
table ip test {
set dlist {
type ipv4_addr
size 65535
}
chain output {
type filter hook output priority filter; policy accept;
udp dport 1234 update @dlist { ip daddr } counter packets 0 bytes 0
}
}
# nft -f test.nft
# nft -f test.nft
test.nft:2:6-10: Error: Could not process rule: File exists
set dlist {
^^^^^
Phil Sutter says:
In the first call, the set lacking 'dynamic' flag does not exist
and is therefore added to the cache. Consequently, both the 'add set'
command and the set statement point at the same set object. In the
second call, a set with same name exists already, so the object created
for 'add set' command is not added to cache and consequently not updated
with the missing flag. The kernel thus rejects the NEWSET request as the
existing set differs from the new one.
Set on the NFT_SET_EVAL flag if the existing set sets it on.
Fixes: 8d443adfcc8c1 ("evaluate: attempt to set_eval flag if dynamic updates requested") Tested-by: Eric Garver <eric@garver.life> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
fee6bda06403 ("evaluate: remove anon sets with exactly one element")
introduces an optimization to remove use of sets with single element.
Skip this optimization if set element contains stateful statements.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
On my (slow?) testing VM, The test tends to fail when doing a full run
(i.e., calling run-test.sh without arguments) and tends to pass when run
individually.
The problem seems to be the 1s element timeout which in some cases may
pass before element deletion occurs. Simply fix this by doubling the
timeout. It has to pass just once, so shouldn't hurt too much.
Fixes: 618393c6b3f25 ("tests: Introduce test for set with concatenated ranges") Reviewed-by: Stefano Brivio <sbrivio@redhat.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
This patch reverts 403b46ada490 ("netlink_delinearize: kill dependency
before eval of 'redirect' stmt"). Since ("evaluate: bogus missing
transport protocol"), this workaround is not required anymore.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Users have to specify a transport protocol match such as
meta l4proto tcp
before the redirect statement, even if the redirect statement already
implicitly refers to the transport protocol, for instance:
test.nft:3:16-53: Error: transport protocol mapping is only valid after transport protocol match
redirect to :tcp dport map { 83 : 8083, 84 : 8084 }
~~~~~~~~ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Evaluate the redirect expression before the mandatory check for the
transport protocol match, so protocol context already provides a
transport protocol.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The redirect and masquerade statements can be handled as verdicts:
- if redirect statement specifies no ports.
- masquerade statement, in any case.
Exceptions to the rule: If redirect statement specifies ports, then nat
map transformation can be used iif both statements specify ports.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1668 Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
If xtables support was compiled in but the required libxtables DSO is
not found, nft prints an error message and leaks memory:
| counter packets 0 bytes 0 XT target MASQUERADE not found
This is not as bad as it seems, the output combines stdout and stderr.
Dropping stderr produces an incomplete ruleset listing, though. While
this seemingly inline output can't easily be avoided, fix a few things:
* Respect octx->error_fp, libnftables might have been configured to
redirect stderr somewhere else.
* Align error message formatting with others.
* Don't return immediately, but free allocated memory and fall back to
printing the expression in "untranslated" form.
Fixes: 5c30feeee5cfe ("xt: Delay libxtables access until translation") Signed-off-by: Phil Sutter <phil@nwl.cc>
Round up expression lengths when determining whether to insert a
byte-order conversion. For example, if one is masking a network header
which spans a byte boundary, the mask will span two bytes and so it will
need to be in NBO.
Fixes: bb03cbcd18a1 ("evaluate: no need to swap byte-order for values of fewer than 16 bits.") Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Yet another Bison workaround: Instead of the fancy error message, an
incomprehensible syntax error is emitted:
| # iptables-nft -A FORWARD -p tcp -m osf --genre linux
| # nft list ruleset | nft -f -
| # Warning: table ip filter is managed by iptables-nft, do not touch!
| /dev/stdin:4:29-31: Error: syntax error, unexpected osf, expecting string
| meta l4proto tcp xt match osf counter packets 0 bytes 0
| ^^^
Avoid this by quoting the extension name when printing:
| # nft list ruleset | sudo ./src/nft -f -
| # Warning: table ip filter is managed by iptables-nft, do not touch!
| /dev/stdin:4:20-33: Error: unsupported xtables compat expression, use iptables-nft with this ruleset
| meta l4proto tcp xt match "osf" counter packets 0 bytes 0
| ^^^^^^^^^^^^^^
Fixes: 79195a8cc9e9d ("xt: Rewrite unsupported compat expression dumping") Fixes: e41c53ca5b043 ("xt: Fall back to generic printing from translation") Signed-off-by: Phil Sutter <phil@nwl.cc>
The nested syntax notation results in one single table command which
includes all other objects. This differs from the flat notation where
there is usually one command per object.
This patch adds a previous step to the evaluation phase to expand the
objects that are contained in the table into independent commands, so
both notations have similar representations.
Remove the code to evaluate the nested representation in the evaluation
phase since commands are independently evaluated after the expansion.
The commands are expanded after the set element collapse step, in case
that there is a long list of singleton element commands to be added to
the set, to shorten the command list iteration.
This approach also avoids interference with the object cache that is
populated in the evaluation, which might refer to objects coming in the
existing command list that is being processed.
There is still a post_expand phase to detach the elements from the set
which could be consolidated by updating the evaluation step to handle
the CMD_OBJ_SETELEMS command type.
This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
contains rules") which broke rule addition/insertion by index because
the expansion code after the evaluation messes up the cache.
Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
If the data in the mapping contains a range, then upgrade value to range.
Otherwise, the following error is displayed:
/dev/stdin:11:57-75: Error: Could not process rule: Invalid argument
dnat ip to iifname . ip saddr map { enp2s0 . 10.1.1.136 : 1.1.2.69, enp2s0 . 10.1.1.1-10.1.1.135 : 1.1.2.66-1.84.236.78 }
^^^^^^^^^^^^^^^^^^^
The kernel rejects this command because userspace sends a single value
while the kernel expects the range that represents the min and the max
IP address to be used for NAT. The upgrade is also done when concatenation
with intervals is used in the rhs of the mapping.
For anonymous sets, expansion cannot be done from expr_evaluate_mapping()
because the EXPR_F_INTERVAL flag is inferred from the elements. For
explicit sets, this can be done from expr_evaluate_mapping() because the
user already specifies the interval flag in the rhs of the map definition.
Update tests/shell and tests/py to improve testing coverage in this case.
Fixes: 9599d9d25a6b ("src: NAT support for intervals in maps") Fixes: 66746e7dedeb ("src: support for nat with interval concatenation") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The -o/--optimize feature only cares about linear rulesets at this
stage, but do not hit assert() in this case.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1656 Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Select the merge criteria based on the statements that are used
in the candidate rules, instead of using the list of statements
in the given chain.
Update tests to include a rule with a verdict, which triggers
the bug described in the bugzilla ticket.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1657 Fixes: 0a6dbfce6dc3 ("optimize: merge nat rules with same selectors into map") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Move code to build concatenations into helper function, this routine
includes support for expansion of implicit sets containing singleton
values. This is preparation work to reuse existing code in a follow up
patch.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Eric reports that nft asserts when using integer basetype constants with
'typeof' sets. Example:
table netdev t {
set s {
typeof ether saddr . vlan id
flags dynamic,timeout
}
chain c { }
}
loads fine. But adding a rule with add/update statement fails:
nft 'add rule netdev t c set update ether saddr . 0 @s'
nft: netlink_linearize.c:867: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.
When the 'ether saddr . 0' concat expression is processed, there is
no set definition available anymore to deduce the required size of the
integer constant.
nft eval step then derives the required length using the data types.
'0' has integer basetype, so the deduced length is 0.
The assertion triggers because serialization step finds that it
needs one more register.
2 are needed to store the ethernet address, another register is
needed for the vlan id.
Update eval step to make the expression context store the set key
information when processing the preceeding set reference, then
let stmt_evaluate_set() preserve the existing context instead of
zeroing it again via stmt_evaluate_arg().
This makes concat expression evaluation compute the total size
needed based on the sets key definition.
Reported-by: Eric Garver <eric@garver.life> Signed-off-by: Florian Westphal <fw@strlen.de>
If input does not contain a single 'add' command (unusual, but
possible), 'ret' value was not initialized by nft_optimize() before
returning its value.
If auto-merge is enabled, skip check for element mismatch introduced by 6d1ee9267e7e ("intervals: check for EXPR_F_REMOVE in case of element
mismatch"), which is only relevant to sets with no auto-merge.
The interval adjustment routine for auto-merge already checks for
unexisting intervals in that case.
Uncovered via ASAN:
==11946==ERROR: AddressSanitizer: heap-use-after-free on address
0x60d00000021c at pc 0x559ae160d5b3 bp 0x7ffc37bcb800 sp 0x7ffc37bcb7f8
READ of size 4 at 0x60d00000021c thread T0
#0 0x559ae160d5b2 in 0? /builddir/build/BUILD/nftables-1.0.6/src/intervals.c:424
#1 0x559ae15cb05a in interval_set_eval.lto_priv.0 (/usr/lib64/libnftables.so.1+0xaf05a)
#2 0x559ae15e1c0d in setelem_evaluate.lto_priv.0 (/usr/lib64/libnftables.so.1+0xc5c0d)
#3 0x559ae166b715 in nft_evaluate (/usr/lib64/libnftables.so.1+0x14f715)
#4 0x559ae16749b4 in nft_run_cmd_from_buffer (/usr/lib64/libnftables.so.1+0x1589b4)
#5 0x559ae20c0e7e in main (/usr/bin/nft+0x8e7e)
#6 0x559ae1341146 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#7 0x559ae1341204 in __libc_start_main_impl ../csu/libc-start.c:381
#8 0x559ae20c1420 in _start ../sysdeps/x86_64/start.S:115
0x60d00000021c is located 60 bytes inside of 144-byte region [0x60d0000001e0,0x60d000000270) freed by thread T0 here:
#0 0x559ae18ea618 in __interceptor_free ../../../../gcc-12.2.0/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x559ae160c315 in 4 /builddir/build/BUILD/nftables-1.0.6/src/intervals.c:349
#2 0x559ae160c315 in 0? /builddir/build/BUILD/nftables-1.0.6/src/intervals.c:420
previously allocated by thread T0 here:
#0 0x559ae18eb927 in __interceptor_calloc ../../../../gcc-12.2.0/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x559ae15c5076 in set_elem_expr_alloc (/usr/lib64/libnftables.so.1+0xa9076)
Fixes: 6d1ee9267e7e ("intervals: check for EXPR_F_REMOVE in case of element mismatch") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
If memory allocation fails, calloc() returns NULL which was not checked
for. The code seems to expect zero array size though, so simply
replacing this call by one of the x*calloc() ones won't work. So guard
the call also by a check for 'len'.
Fixes: db0697ce7f602 ("src: support for flowtable listing") Signed-off-by: Phil Sutter <phil@nwl.cc>
Arguments passed to sizeof() where deemed suspicious by covscan due to
the different type. Consistently specify size of an array 'a' using
'sizeof(*a) * nmemb'.
For the statement arrays in stmt_matrix, even use xzalloc_array() since
the item count is fixed and therefore can't be zero.
table ip foo {
map pinned {
typeof ip daddr . ct original proto-dst : ip daddr . tcp dport
size 65535
flags dynamic,timeout
timeout 6m
}
chain pr {
meta l4proto tcp update @pinned { ip saddr . ct original proto-dst timeout 1m30s : ip daddr . tcp dport }
}
}
resulting in the following misleading error:
map-broken.nft:10:51-82: Error: datatype mismatch: expected concatenation of (IPv4 address), expression has type concatenation of (IPv4 address, internet network service)
meta l4proto tcp update @pinned { ip saddr . ct original proto-dst timeout 1m30s : ip daddr . tcp dport }
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
If `yytext` begins with '0', it will be parsed as octal. However, this
has unexpected consequences if the token contains non-octal characters.
`09` will be parsed as 0; `0308` will be parsed as 24, because
`strtoull` and its siblings stop parsing as soon as they reach a
character in the input which is not valid for the base.
Replace the `{numberstring}` match with separate `{hexstring}` and
`{decstring}` matches. For `{decstring}` set the base to 8 if the
leading character is '0', and handle an incompletely parsed token in
the same way as one that causes `strtoull` to set `errno`.
Thus, instead of:
$ sudo nft -f - <<<'
table x {
chain y {
ip saddr 0308 continue comment "parsed as 0.0.0.24/32"
}
}
'
$ sudo nft list chain x y
table ip x {
chain y {
ip saddr 0.0.0.24 continue comment "parsed as 0.0.0.24/32"
}
}
We get:
$ sudo ./src/nft -f - <<<'
> table x {
> chain y {
> ip saddr 0308 continue comment "error"
> }
> }
> '
/dev/stdin:4:14-17: Error: Could not resolve hostname: Name or service not known
ip saddr 0308 continue comment "error"
^^^^
Phil Sutter [Thu, 24 Nov 2022 15:16:41 +0000 (16:16 +0100)]
xt: Fall back to generic printing from translation
If translation is not available or fails, print the generic format
instead of calling the print callback (which does not respect
output_fp) or silently failing.
Phil Sutter [Thu, 24 Nov 2022 13:17:17 +0000 (14:17 +0100)]
xt: Rewrite unsupported compat expression dumping
Choose a format which provides more information and is easily parseable.
Then teach parsers about it and make it explicitly reject the ruleset
giving a meaningful explanation. Also update the man pages with some
more details.
Phil Sutter [Thu, 10 Nov 2022 17:44:43 +0000 (18:44 +0100)]
xt: Delay libxtables access until translation
There is no point in spending efforts setting up the xt match/target
when it is not printed afterwards. So just store the statement data from
libnftnl in struct xt_stmt and perform the extension lookup from
xt_stmt_xlate() instead.
This means some data structures are only temporarily allocated for the
sake of passing to libxtables callbacks, no need to drag them around.
Also no need to clone the looked up extension, it is needed only to call
the functions it provides.
While being at it, select numeric output in xt_xlate_*_params -
otherwise there will be reverse DNS lookups which should not happen by
default.
Set pointer to list of expression to NULL and check that it is set on
before using it.
In function ‘expr_evaluate_concat’,
inlined from ‘expr_evaluate’ at evaluate.c:2488:10:
evaluate.c:1338:20: warning: ‘expressions’ may be used uninitialized [-Wmaybe-uninitialized]
1338 | if (runaway) {
| ^
evaluate.c: In function ‘expr_evaluate’:
evaluate.c:1321:33: note: ‘expressions’ was declared here
1321 | const struct list_head *expressions;
| ^~~~~~~~~~~
Reported-by: Florian Westphal <fw@strlen.de> Fixes: 508f3a270531 ("netlink: swap byteorder of value component in concatenation of intervals") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
netlink: unfold function to generate concatenations for keys and data
Add a specific function to generate concatenation with and without
intervals in maps. This restores the original function added by 8ac2f3b2fca3 ("src: Add support for concatenated set ranges") which is
used by 66746e7dedeb ("src: support for nat with interval
concatenation") to generate the data concatenations in maps.
Only the set element key requires the byteswap introduced by 1017d323cafa
("src: support for selectors with different byteorder with interval
concatenations"). Therefore, better not to reuse the same function for
key and data as the future might bring support for more kind of
concatenations in data maps.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: do not crash on runaway number of concatenation components
Display error message in case user specifies more data components than
those defined by the concatenation of selectors.
# cat example.nft
table ip x {
chain y {
type filter hook prerouting priority 0; policy drop;
ip saddr . meta mark { 1.2.3.4 . 0x00000100 . 1.2.3.6-1.2.3.8 } accept
}
}
# nft -f example.nft
example.nft:4:3-22: Error: too many concatenation components
ip saddr . meta mark { 1.2.3.4 . 0x00000100 . 1.2.3.6-1.2.3.8 } accept
~~~~~~~~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Without this patch, nft crashes:
==464771==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d000000418 at pc 0x7fbc17513aa5 bp 0x7ffc73d33c90 sp 0x7ffc73d33c88
READ of size 8 at 0x60d000000418 thread T0
#0 0x7fbc17513aa4 in expr_evaluate_concat /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:1348
#1 0x7fbc1752a9da in expr_evaluate /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:2476
#2 0x7fbc175175e2 in expr_evaluate_set_elem /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:1504
#3 0x7fbc1752aa22 in expr_evaluate /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:2482
#4 0x7fbc17512cb5 in list_member_evaluate /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:1310
#5 0x7fbc17518ca0 in expr_evaluate_set /home/pablo/devel/scm/git-netfilter/nftables/src/evaluate.c:1590
[...]
Fixes: 64bb3f43bb96 ("src: allow to use typeof of raw expressions in set declaration") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
netlink: swap byteorder of value component in concatenation of intervals
Commit 1017d323cafa ("src: support for selectors with different byteorder with
interval concatenations") was incomplete.
Switch byteorder of singleton values in a set that contains
concatenation of intervals. This singleton value is actually represented
as a range in the kernel.
After this patch, if the set represents a concatenation of intervals:
- EXPR_F_INTERVAL denotes the lhs of the interval.
- EXPR_F_INTERVAL_END denotes the rhs of the interval (this flag was
already used in this way before this patch).
If none of these flags are set on, then the set contains concatenations
of singleton values (no interval flag is set on), in such case, no
byteorder swap is required.
Update tests/shell and tests/py to cover the use-case breakage reported
by Eric.
Fixes: 1017d323cafa ("src: support for selectors with different byteorder with interval concatenations") Reported-by: Eric Garver <eric@garver.life> Tested-by: Eric Garver <eric@garver.life> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
tests: py: missing json for different byteorder selector with interval concatenation
Add missing json output, otherwise -j reports an error.
Fixes: 1017d323cafa ("src: support for selectors with different byteorder with interval concatenations") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Munch lines full comment lines, regular expression matches lines that
start by space or tab, then # follows, finally anything including one
single line break.
Call reset_pos() to ensure error reporting location is not puzzled.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1196 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Harald Welte [Tue, 6 Dec 2022 14:03:33 +0000 (15:03 +0100)]
doc: payload-expression.txt: Mention that 'ih' exists
Back in commit b67abc51ba6f ("src: raw payload match and mangle on inner
header / payload data") a new payload expression 'ih' was added, but the
documentation wasn't updated accordingly.
Let's at least mention in the man page that it exists at all.
Signed-off-by: Harald Welte <laforge@gnumonks.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
src: support for selectors with different byteorder with interval concatenations
Assuming the following interval set with concatenation:
set test {
typeof ip saddr . meta mark
flags interval
}
then, the following rule:
ip saddr . meta mark @test
requires bytecode that swaps the byteorder for the meta mark selector in
case the set contains intervals and concatenations.
inet x y
[ meta load nfproto => reg 1 ]
[ cmp eq reg 1 0x00000002 ]
[ payload load 4b @ network header + 12 => reg 1 ]
[ meta load mark => reg 9 ]
[ byteorder reg 9 = hton(reg 9, 4, 4) ] <----- this is required !
[ lookup reg 1 set test dreg 0 ]
This patch updates byteorder_conversion() to add the unary expression
that introduces the byteorder expression.
Moreover, store the meta mark range component of the element tuple in
the set in big endian as it is required for the range comparisons. Undo
the byteorder swap in the netlink delinearize path to listing the meta
mark values accordingly.
Update tests/py to validate that byteorder expression is emitted in the
bytecode. Update tests/shell to validate insertion and listing of a
named map declaration.
A similar commit 806ab081dc9a ("netlink: swap byteorder for
host-endian concat data") already exists in the tree to handle this for
strings with prefix (e.g. eth*).
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 11 Oct 2022 16:46:55 +0000 (18:46 +0200)]
Warn for tables with compat expressions in rules
While being able to "look inside" compat expressions using nft is a nice
feature, it is also (yet another) pitfall for unaware users, deceiving
them into assuming interchangeability (or at least compatibility)
between iptables-nft and nft.
In reality, which involves 'nft list ruleset | nft -f -', any correctly
translated compat expressions will turn into native nftables ones not
understood by (the version of) iptables-nft which created them in the
first place. Other compat expressions will vanish, potentially
compromising the firewall ruleset.
Emit a warning (as comment) to give users a chance to stop and
reconsider before shooting their own foot.
monitor: missing cache and set handle initialization
This leads to a crash when adding stateful expressions to sets:
netlink.c:928:38: runtime error: member access within null pointer of type 'struct nft_ctx'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==13781==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000d0 (pc 0x7fc96fc2b6b2 bp 0x7ffc0e26b080 sp 0x7ffc0e26b020 T0)
==13781==The signal is caused by a READ memory access.
==13781==Hint: address points to the zero page.
#0 0x7fc96fc2b6b2 in table_cache_find /home/pablo/devel/scm/git-netfilter/nftables/src/cache.c:456
#1 0x7fc96fd244d4 in netlink_parse_set_expr /home/pablo/devel/scm/git-netfilter/nftables/src/netlink_delinearize.c:1857
#2 0x7fc96fcf1b4d in netlink_delinearize_set /home/pablo/devel/scm/git-netfilter/nftables/src/netlink.c:928
#3 0x7fc96fd41966 in netlink_events_cache_addset /home/pablo/devel/scm/git-netfilter/nftables/src/monitor.c:649
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
