| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Adding elements to a set or map with an invalid definition causes nft to
segfault. The following nftables.conf triggers the crash:
flush ruleset
create table inet filter
set inet filter foo {}
add element inet filter foo { foobar }
Simply parsing and checking the config will trigger it:
$ nft -c -f nftables.conf.crash
Segmentation fault
The error in the set/map definition is correctly caught and queued, but
because the set is invalid and does not contain a key type, adding to it
causes a NULL pointer dereference of set->key within setelem_evaluate().
I don't think it's necessary to queue another error since the underlying
problem is correctly detected and reported when parsing the definition
of the set. Simply checking the validity of set->key before using it
seems to fix it, causing the error in the definition of the set to be
reported properly. The element type error isn't caught, but that seems
reasonable since the key type is invalid or unknown anyway:
$ ./nft -c -f ~/nftables.conf.crash
/home/pti/nftables.conf.crash:3:21-21: Error: set definition does not specify key
set inet filter foo {}
^
[ Add tests to cover this case --pablo ]
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1597
Signed-off-by: Peter Tirsek <peter@tirsek.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Store set element location in the per-command netlink error location
array. This allows for fine grain error reporting when adding and
deleting elements.
# nft -f test.nft
test.nft:5:4-20: Error: Could not process rule: File exists
00:01:45:09:0b:26 : drop,
^^^^^^^^^^^^^^^^^
test.nft contains a large map with one redundant entry.
Thus, users do not have to find the needle in the stack.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Set might have more than 16 elements, use a runtime array to store
netlink error location.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
opt_newline causes interfere since it points to the previous line.
Refer to set element key for error reporting.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
If auto-merge is disable and element to be deleted finds no exact
matching, then bail out.
Fixes: 3e8d934e4f72 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
The set deletion routine expects an initialized set, otherwise it crashes.
Fixes: 3e8d934e4f72 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
netlink_get_register() clones the expression in the register,
release after using it.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
Otherwise bogus variable redefinition are reported via -o/--optimize:
redefinition.conf:5:8-21: Error: redefinition of symbol 'interface_inet'
define interface_inet = enp5s0
^^^^^^^^^^^^^^
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Revert support for limit statement, the limit statement is stateful and
it applies a ratelimit per rule, transformation for merging rules with
the limit statement needs to use anonymous sets with statements.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
| |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Add test to cover this case.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Do not try to merge rules with unsupported statements. This patch adds a
dummy unsupported statement which is included in the statement
collection and the rule vs statement matrix.
When looking for possible rule mergers, rules using unsupported
statements are discarded, otherwise bogus rule mergers might occur.
Note that __stmt_type_eq() already returns false for unsupported
statements.
Add a test using meta mark statement, which is not yet supported.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Extend expr_cmp() to compare hash expressions used in relational.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Extend expr_cmp() to compare numgen expressions used in relational.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Do recursive call using left expression in the binop expression tree to
search for the primary expression.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Extend expr_cmp() to compare fib expressions used in relational.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Extend expr_cmp() to compare xfrm expressions used in relational.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Extend expr_cmp() to compare osf expressions used in relational.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
Skip comparison when collecting the statement and building the rule vs
statement matrix. Compare verdict type when merging rules.
When infering rule mergers, honor the STMT_VERDICT with map (ie. vmap).
Fixes: 561aa3cfa8da ("optimize: merge verdict maps with same lookup key")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Add missing code to the statement collection routine. Compare reject
expressions when available. Add tests/shell.
Fixes: fb298877ece2 ("src: add ruleset optimization infrastructure")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Remove rule comment after merging rules, let the user decide if they want
to reintroduce the comment in the ruleset file.
Update optimizations/merge_stmt test.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Do not print stateful information such as counters which are likely set
to zero.
Before this patch:
Merging:
packets.conf:10:3-29: ip protocol 4 counter drop
packets.conf:11:3-29: ip protocol 41 counter drop
packets.conf:12:3-29: ip protocol 47 counter drop
into:
ip protocol { 4, 41, 47 } counter packets 0 bytes 0 drop
^^^^^^^^^^^^^^^^^
After:
Merging:
packets.conf:10:3-29: ip protocol 4 counter drop
packets.conf:11:3-29: ip protocol 41 counter drop
packets.conf:12:3-29: ip protocol 47 counter drop
into:
ip protocol { 4, 41, 47 } counter drop
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Otherwise set reference ends up included in an anonymous set, as an
element, which is not supported.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
| |
When building the statement matrix, do not compare expression right hand
side, otherwise bogus mismatches might occur.
The fully compared flag is set on when comparing rules to look for
possible mergers.
Fixes: 3f36cc6c3dcd ("optimize: do not merge unsupported statement expressions")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When adding element(s) to a non-empty set, code merged the two lists and
sorted the result. With many individual 'add element' commands this
causes substantial overhead. Make use of the fact that
existing_set->init is sorted already, sort only the list of new elements
and use list_splice_sorted() to merge the two sorted lists.
Add set_sort_splice() and use it for set element overlap detection and
automerge.
A test case adding ~25k elements in individual commands completes in
about 1/4th of the time with this patch applied.
Joint work with Pablo.
Fixes: 3da9643fb9ff9 ("intervals: add support to automerge with kernel elements")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Translate set element to range and sort in maps for the NFT_SET_MAP
case, which does not support for automerge yet.
Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Two new elements that represent an exact overlap should not trigger an error.
add table t
add set t s { type ipv4_addr; flags interval; }
add element t s { 1.0.1.0/24 }
...
add element t s { 1.0.1.0/24 }
result in a bogus error.
# nft -f set.nft
set.nft:1002:19-28: Error: conflicting intervals specified
add element t s { 1.0.1.0/24 }
^^^^^^^^^^
Fixes: 3da9643fb9ff ("intervals: add support to automerge with kernel elements")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Robots might generate a long list of singleton element commands such as:
add element t s { 1.0.1.0/24 }
...
add element t s { 1.0.2.0/23 }
collapse them into one single command before the evaluation step, ie.
add element t s { 1.0.1.0/24, ..., 1.0.2.0/23 }
this speeds up overlap detection and set element automerge operations in
this worst case scenario.
Since 3da9643fb9ff9 ("intervals: add support to automerge with kernel
elements"), the new interval tracking relies on mergesort. The pattern
above triggers the set sorting for each element.
This patch adds a list to cmd objects that store collapsed commands.
Moreover, expressions also contain a reference to the original command,
to uncollapse the commands after the evaluation step.
These commands are uncollapsed after the evaluation step to ensure error
reporting works as expected (command and netlink message are mapped
1:1).
For the record:
- nftables versions <= 1.0.2 did not perform any kind of overlap
check for the described scenario above (because set cache only contained
elements in the kernel in this case). This is a problem for kernels < 5.7
which rely on userspace to detect overlaps.
- the overlap detection could be skipped for kernels >= 5.7.
- The extended netlink error reporting available for set elements
since 5.19-rc might allow to remove the uncollapse step, in this case,
error reporting does not rely on the netlink sequence to refer to the
command triggering the problem.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Excess nesting of scanner scopes is very fragile and error prone:
rule `iif != lo ip daddr 127.0.0.1/8 counter limit rate 1/second log flags all prefix "nft_lo4 " drop`
fails with `Error: No symbol type information` hinting at `prefix`
Problem is that we nest via:
counter
limit
log
flags
By the time 'prefix' is scanned, state is still stuck in 'counter' due
to this nesting. Working around "prefix" isn't enough, any other
keyword, e.g. "level" in 'flags all level debug' will be parsed as 'string' too.
So, revert this.
Fixes: a16697097e2b ("scanner: flags: move to own scope")
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Otherwise bogus error reports on set datatype mismatch might occur, such as:
Error: datatype mismatch, expected Internet protocol, expression has type IPv4 address
meta l4proto { tcp, udp } th dport 443 dnat to 10.0.0.1
~~~~~~~~~~~~ ^^^^^^^^^^^^
with an unrelated set declaration.
table ip test {
set set_with_interval {
type ipv4_addr
flags interval
}
chain prerouting {
type nat hook prerouting priority dstnat; policy accept;
meta l4proto { tcp, udp } th dport 443 dnat to 10.0.0.1
}
}
This bug has been introduced in the evaluation step.
Reported-by: Roman Petrov <nwhisper@gmail.com>
Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge)"
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Call xfree() instead since stmt_alloc() does not initialize the
statement type fields.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1609
Fixes: ea1f1c9ff608 ("optimize: memleak in statement matrix")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
use the chain_cache_find function for faster lookup of chain instead of
iterating over all chains in table
Signed-off-by: ChanderG <mail@chandergovind.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use pr_gmp_debug() instead to compile with minigmp.
intervals.c: In function ‘set_delete’:
intervals.c:489:25: warning: implicit declaration of function ‘gmp_printf’; did you mean ‘gmp_vfprintf’? [-Wimplicit-function-declaration]
489 | gmp_printf("remove: [%Zx-%Zx]\n",
| ^~~~~~~~~~
| gmp_vfprintf
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently the json output of `nft -j list ruleset` interleaves rules
with chains.
As reported in this bug:
https://bugzilla.netfilter.org/show_bug.cgi?id=1580
the json cannot be fed into `nft -j -f <file>` since rules may
reference chains that are created later
Instead create rules after all chains are output.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1580
Signed-off-by: ChanderG <mail@chandergovind.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
netlink_release_registers() does not release the expression in the last
32-bit register.
struct netlink_parse_ctx {
...
struct expr *registers[MAX_REGS + 1];
This array is MAX_REGS + 1 (verdict register + 16 32-bit registers).
Fixes: 371c3a0bc3c2 ("netlink_delinearize: release expressions in context registers")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
For concat ranges, something like 'ppp*' is translated as a range
from 'ppp\0\0\0...' to 'ppp\ff\ff\ff...'.
In order to display this properly, check for presence of string base
type and convert to symbolic expression, with appended '*' character.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
All data must be passed in network byte order, else matching
won't work respectively kernel will reject the interval because
it thinks that start is after end
This is needed to allow use of 'ppp*' in interval sets with
concatenations.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Do not remove the range if it does not exists yet in the kernel, adjust it
instead. Uncovered by use-after-free error.
==276702==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d00190663c at pc 0x7ff310ab526f bp 0x7fffeb76f750 sp 0x7fffeb76f748 READ of size 4 at 0x60d00190663c thread T0
#0 0x7ff310ab526e in __adjust_elem_right .../nftables/src/intervals.c:300
#1 0x7ff310ab59a7 in adjust_elem_right .../nftables/src/intervals.c:311
#2 0x7ff310ab6daf in setelem_adjust .../nftables/src/intervals.c:354
#3 0x7ff310ab783a in setelem_delete .../nftables/src/intervals.c:411
#4 0x7ff310ab80e6 in __set_delete .../nftables/src/intervals.c:451
Fixes: 3e8d934e4f72 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Release clone object in case this statement is not supported.
Fixes: 743b0e81371f ("optimize: do not clone unsupported statement")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Verdict and nat are mutually exclusive, no need to support for this
combination.
# cat ruleset.nft
table ip x {
chain y {
type nat hook postrouting priority srcnat; policy drop;
ip saddr 1.1.1.1 tcp dport 8000 snat to 4.4.4.4:80
ip saddr 2.2.2.2 tcp dport 8001 snat to 5.5.5.5:90
}
}
# nft -o -c -f ruleset.nft
Merging:
ruleset.nft:4:3-52: ip saddr 1.1.1.1 tcp dport 8000 snat to 4.4.4.4:80
ruleset.nft:5:3-52: ip saddr 2.2.2.2 tcp dport 8001 snat to 5.5.5.5:90
into:
snat to ip saddr . tcp dport map { 1.1.1.1 . 8000 : 4.4.4.4 . 80, 2.2.2.2 . 8001 : 5.5.5.5 . 90 }
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Skip unsupported statements when building the statement matrix,
otherwise clone remains uninitialized.
Fixes: fb298877ece2 ("src: add ruleset optimization infrastructure")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Keep inspecting rule verdicts before assuming they are equal. Update
existing test to catch this bug.
Fixes: 1542082e259b ("optimize: merge same selector with different verdict into verdict map")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
assert(1) is a no-op, this should be assert(0). Use BUG() instead.
Add missing CATCHALL to avoid BUG().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
So follow up command in this batch that update the set assumes this
element is already in the kernel.
Fixes: 3da9643fb9ff ("intervals: add support to automerge with kernel elements")
Fixes: 3ed9fadaab95 ("intervals: build list of elements to be added from cache")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
"typeof ip saddr . ipsec in reqid" won't work because reqid uses
integer type, i.e. dtype->size is 0.
With "typeof", the size can be derived from the expression length,
via set->key.
This computes the concat length based either on dtype->size or
expression length.
It also updates concat evaluation to permit a zero datatype size
if the subkey expression has nonzero length (i.e., typeof was used).
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
Loop over the set cache and add elements that have no EXPR_F_KERNEL,
meaning that these are new elements in the set that have resulted
from adjusting/split existing ranges.
This fixes several partial deletions of the same interval in one
command.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Iterate over the list of elements to be deleted, then splice one
EXPR_F_REMOVE element at a time to update the list of existing sets
incrementally.
Fixes: 3e8d934e4f722 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Do not add elements to purge list which are not in the kernel,
otherwise, bogus ENOENT is reported.
Fixes: 3e8d934e4f722 ("intervals: support to partial deletion with automerge")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
| |
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Since setelem_delete() assigns to 'prev' pointer only if it doesn't have
EXPR_F_REMOVE flag set, there is no need to check that flag in called
functions.
Fixes: 3e8d934e4f722 ("intervals: support to partial deletion with automerge")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|