We need to restrict this, included bogon asserts with:
BUG: unknown expression type prefix
nft: src/netlink_linearize.c:940: netlink_gen_expr: Assertion `0' failed.
Prefix expressions are only allowed if the concatenation is used within
a set element, not when specifying the lookup key.
For the former, anything that represents a value is allowed.
For the latter, only what will generate data (fill a register) is
permitted.
At this time we do not have an annotation that tells if the expression
is on the left hand side (lookup key) or right hand side (set element).
Add a new list recursion counter for this. If its 0 then we're building
the lookup key, if its the latter the concatenation is the RHS part
of a relational expression and prefix, ranges and so on are allowed.
IOW, we don't really need a recursion counter, another type of annotation
that would tell if the expression is placed on the left or right hand side
of another expression would work too.
v2: explicitly list all 'illegal' expression types instead of
using a default label for them.
This will raise a compiler warning to remind us to adjust the case
labels in case a new expression type gets added in the future.
The existing recursion counter is used by the binop expression to detect
if we've completely followed all the binops.
We can only chain up to NFT_MAX_EXPR_RECURSION binops, but the evaluation
step can perform constant-folding, so we must recurse until we found the
rightmost (last) binop in the chain.
Then we can check the post-eval chain to see if it is something that can
be serialized later (i.e., if we are within the NFT_MAX_EXPR_RECURSION
after constant folding) or not.
Thus we can't reuse the existing ctx->recursion counter for other
expressions; entering the initial expr_evaluate_binop with
ctx->recursion > 0 would break things.
Therefore rename this to an embedded structure.
This allows us to add a new recursion counter in a followup patch.
Use the defined $NFT variable instead of calling the system nft binary directly.
Add a nat_ftp.nodump file to avoid the following check-tree.sh error:
ERR: "tests/shell/testcases/packetpath/nat_ftp" has no "tests/shell/testcases/packetpath/dumps/nat_ftp.{nft,nodump}" file.
Signed-off-by: Yi Chen <yiche@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
If there is no priority.expr set, assume hook.num is bogus, too.
While this is fixing JSON output, it's hard to tell what commit this is
actually fixing: Before commit 627c451b23513 ("src: allow variables in
the chain priority specification"), there was no way to detect
flowtables missing hook specs (e.g. when printing flowtable delete
monitor event).
Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
We can't recover from errors here, but we can abort with a more
precise reason than 'segmentation fault', or stack corruptions
that get caught way later, or not at all.
expr->value is going to be read, we can't cope with other expression
types here.
We will copy to stack buffer of IFNAMSIZ size, abort if we would
overflow.
Check there is a NUL byte present too.
This is a preemptive patch, I've seen one crash in this area but
no reproducer yet.
Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
This patch extends existing flowtable support to improve error
reporting:
# nft add flowtable inet x y '{ devices = { x } ; }'
Error: Could not process rule: No such file or directory
add flowtable inet x y { devices = { x } ; }
^
# nft delete flowtable inet x y '{ devices = { x } ; }'
Error: Could not process rule: No such file or directory
delete flowtable inet x y { devices = { x } ; }
^ Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Prepare for calling netlink_io_error() which needs the context pointer.
Trade this in for the cache pointer since no caller uses a special one.
No functional change intended.
Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Netlink parser tries to keep going despite errors. Faced with an
incompatible ruleset, this is much more user-friendly than exiting the
program upon the first obstacle. This patch fixes three more spots to
support this.
Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Print an error message and discard the object instead of returning it to
the caller. At least when trying to print it, we would hit an assert()
in obj_type_name() anyway.
Fixes: 4756d92e517ae ("src: listing of stateful objects") Signed-off-by: Phil Sutter <phil@nwl.cc>
Whenever a new version adds udata support to an expression, then old
versions of nft will crash when trying to list such a ruleset generated
by a more recent version of nftables.
Make fewer assumptions about the underlying integer type of the enum.
Instead, be clear about where we have an untrusted uint32_t from netlink
and an enum. Rename expr_ops_by_type() to expr_ops_by_type_u32() to make
this clearer. Later we might make the enum as packed, when this starts
to matter more.
Also, only the code path expr_ops() wants strict validation and assert
against valid enum values. Move the assertion out of
__expr_ops_by_type(). Then expr_ops_by_type_u32() does not need to
duplicate the handling of EXPR_INVALID. We still need to duplicate the
check against EXPR_MAX, to ensure that the uint32_t value can be cast to
an enum value.
[ Remove cast on EXPR_MAX. --pablo ]
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
If we have an incomplete rule like "ct original saddr" in inet
family, this function generates an error because it can't determine the required protocol
dependency, hinting at missing ip/ip6 keyword.
We should not go on in this case to avoid a redundant followup error:
nft add rule inet f c ct original saddr 1.2.3.4
Error: cannot determine ip protocol version, use "ip saddr" or "ip6 saddr" instead
add rule inet f c ct original saddr 1.2.3.4
^^^^^^^^^^^^^^^^^
Error: Could not parse symbolic invalid expression
add rule inet f c ct original saddr 1.2.3.4
After this change only the first error is shown.
Fixes: 2b29ea5f3c3e ("src: ct: add eval part to inject dependencies for ct saddr/daddr") Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
When parsing a verdict map json where element jumps to chain represented
as empty string.
internal:0:0-0: Error: Parsing list expression item at index 0 failed.
internal:0:0-0: Error: Invalid set elem at index 0.
internal:0:0-0: Error: Invalid set elem expression.
internal:0:0-0: Error: Parsing command array at index 2 failed.
Just like "ct timeout", "ct expectation" is in need of the same fix,
we get segfault on "nft list ct expectation table t", if table t exists.
This is the exact same pattern as resolved for "ct timeout" in commit 1d2e22fc0521 ("ct timeout: fix 'list object x' vs. 'list objects in table' confusion").
<empty ruleset>
$ nft list ct timeout table t
Error: No such file or directory
list ct timeout table t
^
This is expected to list all 'ct timeout' objects.
The failure is correct, the table 't' does not exist.
But now lets add one:
$ nft add table t
$ nft list ct timeout table t
Segmentation fault (core dumped)
... and thats not expected, nothing should be shown
and nft should exit normally.
Because of missing TIMEOUTS command enum, the backend thinks
it should do an object lookup, but as frontend asked for
'list of objects' rather than 'show this object',
handle.obj.name is NULL, which then results in this crash.
Update the command enums so that backend knows what the
frontend asked for.
The payload statement evaluation pretends that it can handle any
expression for bitfields, but the existing evaluation code only knows
how to handle value expression.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Instead of allocating a lshift expression and relying on the binary
operation transfer propagate this to the mask value, lshift the mask
value immediately.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Blamed commits change output format but did not adjust existing tests:
inet/fib.t: WARNING: line 16: '{"nftables": ..
Fixes: 38f99ee84fe6 ("json: Print single synproxy flags as non-array") Fixes: dbe5c44f2b89 ("json: Print single fib flag as non-array") Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Phil Sutter <phil@nwl.cc>
We can't remove 'meta nfproto' dependencies for all cases.
Its removed for ip/ip6 families, this works fine.
But for others, e.g. inet, removal is not as simple.
For example
meta nfproto ipv4 ct protocol tcp
is listed as 'ct protocol tcp', even when this is uses in the inet
table.
Meta L4PROTO removal checks were correct, but refactor this
into a helper function to split meta/ct checks from the common
calling function.
Ct check was lacking, we need to examine ct keys more closely
to figure out if they need to retain the network protocol depenency
or not. Elide for NFT_CT_SRC/DST and its variants, as those imply
the network protocol to use, all others must keep it as-is.
Revert commit d1a7b9e19fe65 ("tests: py: update netdev reject test
file"), the stored JSON equivalents were correct in that they matched
the standard syntax input.
In fact, we missed a .json.output file recording the expected deviation
in JSON output.
Fixes: d1a7b9e19fe65 ("tests: py: update netdev reject test file") Fixes: 7ca3368cd7575 ("reject: Unify inet, netdev and bridge delinearization") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
When running a test for which no corresponding *.payload file exists,
the *.payload.got file name was incorrectly constructed due to
'payload_path' variable not being set.
Fixes: 2cfab7a3e10fc ("tests/py: Write dissenting payload into the right file") Signed-off-by: Phil Sutter <phil@nwl.cc>
Make sure they match the standard syntax input as much as possible.
For some reason inet/tcp.t.json was using plain arrays in place of
binary OR expressions in many cases. These arrays are interpreted as
list expressions, which seems to be semantically identical but the goal
here is to present an accurate equivalent to the rule in standard
syntax.
tests/py/any/meta.t.json.got: WARNING: line 2: Wrote JSON equivalent for rule meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }
ERROR: did not find JSON equivalent for rule 'meta mark set vlan id map @map1
Fixes: 8d3de823b622 ("evaluate: reset statement length context before evaluating statement") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Originally, the plan was to use tests/shell from git HEAD, but this
does not work well in practise because slight changes in the test
output break checks resulting in lots of [DUMP FAIL] errors.
It is easier to test infrastructure self-contained in this 1.0.6.y
branch.
However, backporting the tests/shell into 1.0.6.y turns out to be more
complicated than expected, so I decided to follow the opposite, which is
to (brute) force a copy of tests/shell from 2a38f458f12bc032dac1b3ba63f95ca5a3c03fbd into this branch.
This also requires a number follow up partial reverts on tests/shell
updates to work with 1.0.6.y.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The bison parser enforces this implicitly by grammar rules.
Because subkeys have to be conatenated via ".", notation, e.g.
"mark . ip saddr", all concatenation expressions always consist of at
least two elements.
But this doesn't apply to the json frontend which just uses an array:
it can be empty or only contain one element.
The included reproducer makes the eval stage set the "concatenation" flag
on the interval set. This prevents the needed conversion code to turn the
element values into ranges from getting run.
Malformed input returns NULL when decoding left/right side of binop.
This causes a NULL dereference in expr_evaluate_binop; left/right must
point to a valid expression.
Fix this in the parser, else would have to sprinkle NULL checks all over
the evaluation code.
After fix, loading the bogon yields:
internal:0:0-0: Error: Malformed object (too many properties): '{}'.
internal:0:0-0: Error: could not decode binop rhs, '<<'.
internal:0:0-0: Error: Invalid mangle statement value
internal:0:0-0: Error: Parsing expr array at index 1 failed.
internal:0:0-0: Error: Parsing command array at index 3 failed.
Fixes: 0ac39384fd9e ("json: Accept more than two operands in binary expressions") Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
After this fix, following errors will be shown:
Error: unqualified type invalid specified in map definition. Try "typeof expression" instead of "type datatype".
map m {
^
map m {
^
Error: map has no mapping data
Fixes: 343a51702656 ("src: store expr, not dtype to track data in sets") Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
typeof_expr allows for symbol, constant and bitwise expressions,
restrict it to selector expressions.
After this patch, input generated by fuzzer is rejected upfront:
# nft -f test.nft
test.nft:3:53-53: Error: syntax error, unexpected number
typeof numgen inc mod 2 : ip daddr . 0
^
test.nft:2:12-13: Error: set definition does not specify key
map t2 {
^^
test.nft:8:65-67: Error: No such file or directory
meta l4proto tcp dnat ip to numgen inc mod 2 map @t2
^^^
test.nft:8:65-67: Error: No such file or directory
meta l4proto tcp dnat ip to numgen inc mod 2 map @t2
^^^
Revisit 4ab1e5e60779 ("src: allow use of 'verdict' in typeof
definitions") to handle verdict as string, later a token can be added
to the scanner and enable it via flex start conditions.
Fixes: 14357cff40ed ("parser: add typeof keyword for declarations") Reported-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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.
Included bogon input generates following Sanitizer splat:
AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7...
WRITE of size 2 at 0x7fffffffcbe4 thread T0
#0 0x0000003a68b8 in __asan_memset (src/nft+0x3a68b8) (BuildId: 3678ff51a5405c77e3e0492b9a985910efee73b8)
#1 0x0000004eb603 in __mpz_export_data src/gmputil.c:108:2
#2 0x0000004eb603 in netlink_export_pad src/netlink.c:256:2
#3 0x0000004eb603 in netlink_gen_range src/netlink.c:471:2
#4 0x0000004ea250 in __netlink_gen_data src/netlink.c:523:10
#5 0x0000004e8ee3 in alloc_nftnl_setelem src/netlink.c:205:3
#6 0x0000004d4541 in mnl_nft_setelem_batch src/mnl.c:1816:11
Problem is that the range end is emitted to the buffer at the *padded*
location (rounded up to next register size), but buffer sizing is
based of the expression length, not the padded length.
Also extend the test script: Capture stderr and if we see
AddressSanitizer warning, make it fail.
Same bug as the one fixed in 600b84631410 ("netlink: fix stack buffer overflow with sub-reg sized prefixes"),
just in a different function.
Apply same fix: no dynamic array + add a range check.
Kernel's flowtable message might not contain the
NFTA_FLOWTABLE_HOOK_DEVS attribute. In that case, nftnl_flowtable_get()
will return NULL for the respective nftnl attribute.
Fixes: db0697ce7f602 ("src: support for flowtable listing") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Code accidentally treats missing NFTNL_FLOWTABLE_PRIO attribute as zero
prio value which may not be correct.
Fixes: db0697ce7f602 ("src: support for flowtable listing") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
The two commits mentioned below resolved null dererence crashes when the
policy resp. priority keyword was missing in the chain/flowtable
specification.
Same issue exists in the json output path, so apply similar fix there
and extend the existing test cases.
Fixes: 5b37479b42b3 ("nftables: don't crash in 'list ruleset' if policy is not set") Fixes: b40bebbcee36 ("rule: do not crash if to-be-printed flowtable lacks priority") Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Phil Sutter <phil@nwl.cc>
-o/--optimize results in EEXIST error when merging two rules that lead
to ambiguous set/map, for instance:
table ip x {
chain v4icmp {}
chain v4icmpc {}
chain y {
ip protocol icmp jump v4icmp
ip protocol icmp goto v4icmpc
}
}
which is not possible because duplicated keys are not possible in
set/map. This is how it shows when running a test:
Merging:
testcases/sets/dumps/sets_with_ifnames.nft:56:3-30: ip protocol icmp jump v4icmp
testcases/sets/dumps/sets_with_ifnames.nft:57:3-31: ip protocol icmp goto v4icmpc
into:
ip protocol vmap { icmp : jump v4icmp, icmp : goto v4icmpc }
internal:0:0-0: Error: Could not process rule: File exists
Add a new step to compare rules that are candidate to be merged to
detect colissions in set/map keys in order to skip them in the next
final merging step.
Check if right hand side of relational is a bitmask, ie.
relational
/ \
... or
/ \
value or
/ \
value value
then, if left hand side is a binop expression, compare left and right
hand sides (not only left hand of this binop expression) to check for
redundant matches in consecutive rules, ie.
relational
/ \
and ...
/ \
payload value
before this patch, only payload in the binop expression was compared.
This allows to compact several rules matching tcp flags in a set/map, eg.
Included test will fail with:
/dev/stdin:8:38-52: Error: Transparent proxy support requires transport protocol match
meta l4proto @protos tproxy to :1088
^^^^^^^^^^^^^^^
Tolerate a set reference too. Because the set can be empty (or there
can be removals later), add a fake 0-rhs value.
This will make pctx_update assign proto_unknown as the transport protocol
in use, Thats enough to avoid 'requires transport protocol' error.
v2: restrict it to meta lhs for now (Pablo Neira Ayuso)
When building each component of the set element key, a late byteorder
switch is performed to ensure that all components in the interval are
represented in big endian, as required by the pipapo backend.
In case that the set element does not fit into the netlink message, the
byteorder switch happens twice, leading to inserting an element with a
bogus component with large sets, so instead:
Note that 16777216 is 0x1000000, which should instead be 0x00000001 to
represent "lo" as u32.
Fix this by switching the value in a temporary variable and use it to
set the set element key attribute in the netlink message.
Later, revisit this to perform this byteorder switch from evaluation
step.
Add tests/shell unit to cover for this bug.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1792 Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
table ip x {
map y {
typeof ip saddr : meta mark
counter
flags interval,timeout
elements = { 1.1.1.1-1.1.1.10 timeout 10m : 20, 2.2.2.2-2.2.2.5 timeout 10m : 30}
}
then, invoking the get element command:
# nft get element x y { 1.1.1.2 }
results in, before (incomplete output):
table ip x {
map y {
type ipv4_addr : mark
flags interval,timeout
elements = { 1.1.1.1 counter packets 0 bytes 0 timeout 10m expires 1m24s160ms : 0x00000014 }
}
}
Note that it displays 1.1.1.1, instead of 1.1.1.1-1.1.1.10.
After this fix:
table ip x {
map y {
type ipv4_addr : mark
flags interval,timeout
elements = { 1.1.1.1-1.1.1.10 counter packets 0 bytes 0 timeout 10m expires 1m24s160ms : 0x00000014 }
}
}
Fixes: a43cc8d53096 ("src: support for get element command") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This uses the wrong length. This must re-use the length of the datatype,
not the string length.
The added test cases will fail without the fix due to erroneous
overlap detection, which in itself is due to incorrect sorting of
the elements.
Example error:
netlink: Error: interval overlaps with an existing one
add element inet testifsets simple_wild { "2-1" } failed.
table inet testifsets {
... elements = { "1-1", "abcdef*", "othername", "ppp0" }
... but clearly "2-1" doesn't overlap with any existing members.
The false detection is because of the "acvdef*" wildcard getting sorted
at the beginning of the list which is because its erronously initialised
as a 64bit number instead of 128 bits (16 bytes / IFNAMSIZ).
Use __datatype_set() to release the existing datatype before assigning
the new one, otherwise ASAN reports the following memleak:
Direct leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7fbc8a2b89cf in __interceptor_malloc ../../../../src/libsa
#1 0x7fbc898c96c2 in xmalloc src/utils.c:31
#2 0x7fbc8971a182 in datatype_clone src/datatype.c:1406
#3 0x7fbc89737c35 in expr_evaluate_unary src/evaluate.c:1366
#4 0x7fbc89758ae9 in expr_evaluate src/evaluate.c:3057
#5 0x7fbc89726bd9 in byteorder_conversion src/evaluate.c:243
#6 0x7fbc89739ff0 in expr_evaluate_bitwise src/evaluate.c:1491
#7 0x7fbc8973b4f8 in expr_evaluate_binop src/evaluate.c:1600
#8 0x7fbc89758b01 in expr_evaluate src/evaluate.c:3059
#9 0x7fbc8975ae0e in stmt_evaluate_arg src/evaluate.c:3198
#10 0x7fbc8975c51d in stmt_evaluate_payload src/evaluate.c:330
Fixes: faa6908fad60 ("evaluate: clone unary expression datatype to deal with dynamic datatype") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
