Catchall elements coming from the parser provide expr->len == 0.
However, the existing mergesort implementation requires expr->len to be
set up to the length of the set key to properly sort elements.
In particular, set element deletion leverages such list sorting to find
if elements exists in the set.
Fixes: 419d19688688 ("src: add set element catch-all support") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This is because we pass the EXPR_SET_ELEM expr to the register allocation,
which will make it reserve 1 128 bit register / 16 bytes.
This happens to be enough for most cases, but its not for ipv6 concat data.
Pass the actual key and data instead: This will reserve enough space to
hold a possible concat expression.
Also add test cases.
Signed-off-by: Son Dinh <dinhtrason@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de>
cmd: skip variable set elements when collapsing commands
ASAN reports an issue when collapsing commands that represent an element
through a variable:
include/list.h:60:13: runtime error: member access within null pointer of type 'struct list_head'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==11398==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7ffb77cf09c2 bp 0x7ffc818267c0 sp 0x7ffc818267a0 T0)
==11398==The signal is caused by a WRITE memory access.
==11398==Hint: address points to the zero page.
#0 0x7ffb77cf09c2 in __list_add include/list.h:60
#1 0x7ffb77cf0ad9 in list_add_tail include/list.h:87
#2 0x7ffb77cf0e72 in list_move_tail include/list.h:169
#3 0x7ffb77cf86ad in nft_cmd_collapse src/cmd.c:478
#4 0x7ffb77da9f16 in nft_evaluate src/libnftables.c:531
#5 0x7ffb77dac471 in __nft_run_cmd_from_filename src/libnftables.c:720
#6 0x7ffb77dad703 in nft_run_cmd_from_filename src/libnftables.c:807
Skip such commands to address this issue.
This patch also extends tests/shell to cover for this bug.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1754 Fixes: 498a5f0c219d ("rule: collapse set element commands") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
inet_pton() allows for broader IPv4-Mapped IPv6 address syntax than
those specified by rfc4291 Sect.2.5.5. This patch extends the scanner to
support them for compatibility reasons. This allows to represent the
last 4 bytes of an IPv6 address as an IPv4 address.
NFT_CACHE_REFRESH is set on inconditionally by ruleset list commands to
deal with stateful information in this ruleset. This flag results in
dropping the existing cache and fully fetching all objects from the
kernel.
Set on this flag for reset commands too, this is missing.
List/reset commands allow for filtering by specific family and object,
therefore, NFT_CACHE_REFRESH also signals that the cache is partially
populated.
Check if this flag is requested by the current list/reset command, as
well as cache->flags which represents the cache after the _previous_
list of commands.
A follow up patch allows to recycle the existing cache if the flags
report that the same objects are already available in the cache,
NFT_CACHE_REFRESH is useful to report that cache cannot be recycled.
Fixes: 407c54f71255 ("src: cache gets out of sync in interactive mode") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
# nft add rule netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321
fails with:
Error: conflicting link layer protocols specified: ether vs. vlan
netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321
^^^^^^^
Users can work around this issue by prepending an explicit match for
vlan ethertype field, that is:
ether type vlan ip saddr 10.1.1.1 ...
^-------------^
but nft should really handle this itself.
The error above is triggered by the following check in
resolve_ll_protocol_conflict():
/* This payload and the existing context don't match, conflict. */
if (pctx->protocol[base + 1].desc != NULL)
return 1;
This check was added by 39f15c243912 ("nft: support listing expressions
that use non-byte header fields") and f7d5590688a6 ("tests: vlan tests")
to deal with conflicting link layer protocols, for instance:
ether type ip vlan id 1
this is matching ethertype ip at offset 12, but then it matches for vlan
id at offset 14 which is not present given the previous check.
One possibility is to remove such check, but nft does not bail out for
the example above and it results in bytecode that never matches:
# nft --debug=netlink netdev x y ether type ip vlan id 10
netdev x y
[ meta load iiftype => reg 1 ]
[ cmp eq reg 1 0x00000001 ]
[ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
[ cmp eq reg 1 0x00000008 ] <---- ip
[ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
[ cmp eq reg 1 0x00000081 ] <---- vlan
[ payload load 2b @ link header + 14 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
[ cmp eq reg 1 0x00000a00 ]
This is due to resolve_ll_protocol_conflict() which deals with the
conflict by updating protocol context and emitting an implicit
dependency, but there is already an explicit match coming from the user.
This patch adds a new helper function to check if an implicit dependency
clashes with an existing statement, which results in:
# nft add rule netdev x y ether type ip vlan id 1
Error: conflicting statements
add rule netdev x y ether type ip vlan id 1
^^^^^^^^^^^^^ ~~~~~~~
Theoretically, no duplicated implicit dependency should ever be emitted
if protocol context is correctly handled.
Only implicit payload expressions are considered at this stage for this
conflict check, this patch can be extended to deal with other dependency
types.
Fixes: 39f15c243912 ("nft: support listing expressions that use non-byte header fields") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Lacking an explicit description of possible hook values, emphasising the
word in the description text should draw readers' attention in the right
direction.
When merging the JSON arrays generated for LHS and RHS of nested binop
expressions, the emptied array objects leak if their reference is not
decremented.
Fix this and tidy up other spots which did it right already by
introducing a json_array_extend wrapper.
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org> Fixes: 0ac39384fd9e4 ("json: Accept more than two operands in binary expressions") Signed-off-by: Phil Sutter <phil@nwl.cc>
In corner cases, expr_msort_cmp() may return 0 for two non-identical
elements. An example are ORed tcp flags: 'syn' and 'syn | ack' are
considered the same value since expr_msort_value() reduces the latter to
its LHS.
Keeping the above in mind and looking at how list_expr_sort() works: The
list in 'head' is cut in half, the first half put into the temporary
list 'list' and finally 'list' is merged back into 'head' considering
each element's position. Shall expr_msort_cmp() return 0 for two
elements, the one from 'list' ends up after the one in 'head', thus
reverting their previous ordering.
The practical implication is that output never matches input for the
sample set '{ syn, syn | ack }' as the sorting after delinearization in
netlink_list_setelems() keeps swapping the elements. Out of coincidence,
the commit this fixes itself illustrates the use-case this breaks,
namely tracking a ruleset in git: Each ruleset reload will trigger an
update to the stored dump.
This change breaks interval set element deletion because __set_delete()
implicitly relies upon this reordering of duplicate entries by inserting
a clone of the one to delete into the start (via list_move()) and after
sorting assumes the clone will end up right behind the original. Fix
this by calling list_move_tail() instead.
Fixes: 14ee0a979b622 ("src: sort set elements in netlink_get_setelems()") Signed-off-by: Phil Sutter <phil@nwl.cc>
Currently, ICMP{v4,v6,inet} code datatypes only describe those that are
supported by the reject statement, but they can also be used for icmp
code matching. Moreover, ICMP code types go hand-to-hand with ICMP
types, that is, ICMP code symbols depend on the ICMP type.
Thus, the output of:
nft describe icmp_code
look confusing because that only displays the values that are supported
by the reject statement.
Disentangle this by adding internal datatypes for the reject statement
to handle the ICMP code symbol conversion to value as well as ruleset
listing.
The existing icmp_code, icmpv6_code and icmpx_code remain in place. For
backward compatibility, a parser function is defined in case an existing
ruleset relies on these symbols.
As for the manpage, move existing ICMP code tables from the DATA TYPES
section to the REJECT STATEMENT section, where this really belongs to.
But the icmp_code and icmpv6_code table stubs remain in the DATA TYPES
section because that describe that this is an 8-bit integer field.
f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
reverses a cross-day range expressed as "22:00"-"02:00" UTC time into
!= "02:00"-"22:00" so meta hour ranges works.
Listing is however confusing, hence, 44d144cd593e ("netlink_delinearize:
reverse cross-day meta hour range") introduces code to reverse a cross-day.
However, it also adds code to reverse a range in == to-from form
(assuming OP_IMPLICIT) which is never exercised from the listing path
because the range expression is not currently used, instead two
instructions (cmp gte and cmp lte) are used to represent the range.
Remove this branch otherwise a reversed notation will be used to display
meta hour ranges once the range instruction is to represent this.
Add test for cross-day scenario in EADT timezone.
Fixes: 44d144cd593e ("netlink_delinearize: reverse cross-day meta hour range") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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>
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>
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>
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.
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:
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:
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.
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>
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>
This fails because the relational expression first evaluates
the left hand side, so when concat evaluation sees '1.2.3.4'
no key context is available.
Check if the RHS is a set reference, and, if so, evaluate
the right hand side.
This sets a pointer to the set key in the evaluation context
structure which then makes the concat evaluation step parse
1.2.3.4 and 80 as ipv4 address and 16bit port number.
On delinearization, extend relop postprocessing to
copy the datatype from the rhs (set reference, has
proper datatype according to set->key) to the lhs (concat
expression).
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.
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>
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.
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>
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> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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>
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>
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.
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>
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>
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>
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>
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.
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.
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>
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.
Bail out if flowtable declaration is missing and no devices are
specified.
Otherwise, this reports a bogus error when adding new devices to an
existing flowtable.
# nft -v
nftables v1.0.9 (Old Doc Yak #3)
# ip link add dummy1 type dummy
# ip link set dummy1 up
# nft 'create flowtable inet filter f1 { hook ingress priority 0; counter }'
# nft 'add flowtable inet filter f1 { devices = { dummy1 } ; }'
Error: missing hook and priority in flowtable declaration
add flowtable inet filter f1 { devices = { dummy1 } ; }
^^^^^^^^^^^^^^^^^^^^^^^^
Fixes: 5ad475fce5a1 ("evaluate: bail out if new flowtable does not specify hook and priority") Reported-by: Martin Gignac <martin.gignac@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Use the key from the evaluation context to perform the byteorder
conversion in case that this expression is used for lookups and updates
on explicit sets.
Simple matches on values and implicit sets rely on the binary transfer
mechanism to propagate the shift to the constant, no explicit byteorder
is required in such case.
Fixes: 668c18f67203 ("evaluate: place byteorder conversion before rshift in payload statement") Reported-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
map expression (which is used a key to look up for the mapping) needs to
consider the statement length context, otherwise incorrect bytecode is
generated when {ct,meta} statement is generated.
# nft -f - <<EOF
add table ip6 t
add chain ip6 t c
add map ip6 t mapv6 { typeof ip6 dscp : meta mark; }
EOF
# nft -d netlink add rule ip6 t c meta mark set ip6 dscp map @mapv6
ip6 t c
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
... missing byteorder conversion here before shift ...
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ lookup reg 1 set mapv6 dreg 1 ]
[ meta set mark with reg 1 ]
Reset statement length context only for the mapping side for the
elements in the set.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand") Reported-by: Brian Davidson <davidson.brian@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
For bitfield that spans more than one byte, such as ip6 dscp, byteorder
conversion needs to be done before rshift. Add unary expression for this
conversion only in the case of meta and ct statements.
Before this patch:
# nft --debug=netlink add rule ip6 x y 'meta mark set ip6 dscp'
ip6 x y
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ byteorder reg 1 = ntoh(reg 1, 2, 2) ] <--------- incorrect
[ meta set mark with reg 1 ]
After this patch:
# nft --debug=netlink add rule ip6 x y 'meta mark set ip6 dscp'
ip6 x y
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
[ byteorder reg 1 = ntoh(reg 1, 2, 2) ] <-------- correct
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ meta set mark with reg 1 ]
For the matching case, binary transfer already deals with the rshift to
adjust left and right hand side of the expression, the unary conversion
is not needed in such case.
Fixes: 8221d86e616b ("tests: py: add test-cases for ct and packet mark payload expressions") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
$ 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>
table ip x {
chain y {
meta iifname { abcde*, xyz }
}
}
triggers the following memleak:
==6871== 16 bytes in 1 blocks are definitely lost in loss record 1 of 1
==6871== at 0x483877F: malloc (vg_replace_malloc.c:307)
==6871== by 0x48AD898: xmalloc (utils.c:37)
==6871== by 0x4BC8B22: __gmpz_init2 (in /usr/lib/x86_64-linux-gnu/libgmp.so.10.4.1)
==6871== by 0x4887E67: constant_expr_alloc (expression.c:424)
==6871== by 0x488EF1F: expr_evaluate_prefix (evaluate.c:1138)
==6871== by 0x488EF1F: expr_evaluate (evaluate.c:2725)
==6871== by 0x488E76D: expr_evaluate_set_elem (evaluate.c:1662)
==6871== by 0x488E76D: expr_evaluate (evaluate.c:2739)
==6871== by 0x4891033: list_member_evaluate (evaluate.c:1454)
==6871== by 0x488E2B6: expr_evaluate_set (evaluate.c:1757)
==6871== by 0x488E2B6: expr_evaluate (evaluate.c:2737)
==6871== by 0x48910D0: elems_evaluate (evaluate.c:4605)
==6871== by 0x4891432: set_evaluate (evaluate.c:4711)
==6871== by 0x48915BC: implicit_set_declaration (evaluate.c:122)
==6871== by 0x488F18A: expr_evaluate_relational (evaluate.c:2503)
==6871== by 0x488F18A: expr_evaluate (evaluate.c:2745)
expr_evaluate_prefix() calls constant_expr_alloc() which have already
called mpz_init2(), the second call to mpz_init2() overlaps the existing
mpz_t data memory area.
Remove extra mpz_init2() call to fix this memleak.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
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.
When using ip dscp in combination with bitwise operation:
# nft --debug=netlink add rule ip x y 'ct mark set ip dscp | 0x4'
ip x y
[ payload load 1b @ network header + 1 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
[ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
[ bitwise reg 1 = ( reg 1 & 0xfffffffb ) ^ 0x00000004 ]
[ ct set mark with reg 1 ]
the listing is showing in the incorrect byteorder:
# nft list ruleset
table ip x {
chain y {
ct mark set ip dscp | 0x4000000
}
}
handle and and or operations in host byteorder.
The following command:
# nft --debug=netlink add rule ip6 x y 'ct mark set ip6 dscp | 0x4'
ip6 x y
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
[ bitwise reg 1 = ( reg 1 & 0xfffffffb ) ^ 0x00000004 ]
[ ct set mark with reg 1 ]
works fine (without requiring this patch) because there is an explicit
byteorder expression.
However, ip dscp takes only 1-byte, so it does not require the byteorder
expression. Use host byteorder if the rhs of bitwise AND OR is larger
than lhs payload expression and such expression is equal or less than
1-byte.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Get length from statement, instead infering it from the expression that
is used to set the value. In the particular case of {ct|meta} mark, this
is 32 bits.
note that mask 0xffffffef is used instead of 0x00000fef.
Patch ("evaluate: support shifts larger than the width of the left operand")
provides the statement length through eval context. Use it to evaluate the
bitwise expression accordingly, otherwise bytecode is incorrect:
# nft --debug=netlink add rule ip x y 'ct mark set ip dscp & 0x0f << 1 | 0xff000000'
ip x y
[ payload load 1b @ network header + 1 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
[ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
[ bitwise reg 1 = ( reg 1 & 0x1e000000 ) ^ 0x000000ff ] <-- incorrect byteorder for OR
[ byteorder reg 1 = ntoh(reg 1, 4, 4) ] <-- no needed for single ip dscp byte
[ ct set mark with reg 1 ]
Correct bytecode:
# nft --debug=netlink add rule ip x y 'ct mark set ip dscp & 0x0f << 1 | 0xff000000
ip x y
[ payload load 1b @ network header + 1 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
[ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
[ bitwise reg 1 = ( reg 1 & 0x0000001e ) ^ 0xff000000 ]
[ ct set mark with reg 1 ]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Otherwise expr_evaluate_value() fails with invalid datatype:
# nft --debug=netlink add rule ip x y 'ct mark set ip dscp & 0x0f << 1'
BUG: invalid basetype invalid
nft: evaluate.c:440: expr_evaluate_value: Assertion `0' failed.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
