Phil Sutter [Tue, 14 Jan 2020 15:50:35 +0000 (16:50 +0100)]
tests: shell: Search diff tool once and for all
Instead of calling 'which diff' over and over again, just detect the
tool's presence in run-tests.sh and pass $DIFF to each testcase just
like with nft binary.
Fall back to using 'true' command to avoid the need for any conditional
calling in test cases.
While being at it, unify potential diff calls so that a string
comparison in shell happens irrespective of diff presence.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 13 Jan 2020 13:53:24 +0000 (14:53 +0100)]
monitor: Fix output for ranges in anonymous sets
Previous fix for named interval sets was simply wrong: Instead of
limiting decomposing to anonymous interval sets, it effectively disabled
it entirely.
Since code needs to check for both interval and anonymous bits
separately, introduce set_is_interval() helper to keep the code
readable.
Also extend test case to assert ranges in anonymous sets are correctly
printed by echo or monitor modes. Without this fix, range boundaries are
printed as individual set elements.
Fixes: 5d57fa3e99bb9 ("monitor: Do not decompose non-anonymous sets") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 9 Jan 2020 16:43:11 +0000 (17:43 +0100)]
monitor: Fix for use after free when printing map elements
When populating the dummy set, 'data' field must be cloned just like
'key' field.
Fixes: 343a51702656a ("src: store expr, not dtype to track data in sets") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 9 Jan 2020 12:34:20 +0000 (13:34 +0100)]
monitor: Do not decompose non-anonymous sets
They have been decomposed already, trying to do that again causes a
segfault. This is a similar fix as in commit 8ecb885589591 ("src:
restore --echo with anonymous sets").
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Mon, 6 Jan 2020 22:35:10 +0000 (22:35 +0000)]
evaluate: fix expr_set_context call for shift binops.
expr_evaluate_binop calls expr_set_context for shift expressions to set
the context data-type to `integer`. This clobbers the byte-order of the
context, resulting in unexpected conversions to NBO. For example:
$ sudo nft flush ruleset
$ sudo nft add table t
$ sudo nft add chain t c '{ type filter hook output priority mangle; }'
$ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0x10 | 0xe'
$ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0xf << 1'
$ sudo nft list table t
table ip t {
chain c {
type filter hook output priority mangle; policy accept;
oif "lo" tcp dport 22 ct mark set 0x0000001e
oif "lo" tcp dport 22 ct mark set 0x1e000000
}
}
Replace it with a call to __expr_set_context and set the byteorder to
that of the left operand since this is the value being shifted.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
scanner: incorrect error reporting after file inclusion
scanner_pop_buffer() incorrectly sets the current input descriptor. The
state->indesc_idx field actually stores the number of input descriptors
in the stack, decrement it and then update the current input descriptor
accordingly.
Florian Westphal [Tue, 10 Dec 2019 14:23:35 +0000 (15:23 +0100)]
evaluate: print a hint about 'typeof' syntax on 0 keylen
If user says
'type integer; ...' in a set definition, don't just throw an error --
provide a hint that the typeof keyword can be used to provide
the needed size information.
Florian Westphal [Fri, 16 Aug 2019 12:22:01 +0000 (14:22 +0200)]
tests: add typeof test cases
Add sets using unspecific string/integer types, one with
osf name, other with vlan id. Neither type can be used directly,
as they lack the type size information.
This patch adds two new expression operations to build and to parse the
userdata area that describe the set key and data typeof definitions.
For maps, the grammar enforces either
"type data_type : data_type" or or "typeof expression : expression".
Check both key and data for valid user typeof info first.
If they check out, flag set->key_typeof_valid as true and use it for
printing the key info.
This patch comes with initial support for using payload expressions
with the 'typeof' keyword, followup patches will add support for other
expressions as well.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fw@strlen.de>
This will be needed once we add support for the 'typeof' keyword to
handle maps that could e.g. store 'ct helper' "type" values.
Instead of:
set foo {
type ipv4_addr . mark;
this would allow
set foo {
typeof(ip saddr) . typeof(ct mark);
(exact syntax TBD).
This would be needed to allow sets that store variable-sized data types
(string, integer and the like) that can't be used at at the moment.
Adding special data types for everything is problematic due to the
large amount of different types needed.
For anonymous sets, e.g. "string" can be used because the needed size can
be inferred from the statement, e.g. 'osf name { "Windows", "Linux }',
but in case of named sets that won't work because 'type string' lacks the
context needed to derive the size information.
With 'typeof(osf name)' the context is there, but at the moment it won't
help because the expression is discarded instantly and only the data
type is retained.
segtree: don't remove nul-root element from interval set
Check from the delinearize set element path if the nul-root element
already exists in the interval set. Hence, the element insertion path
skips the implicit nul-root interval insertion.
Under some circunstances, nft bogusly fails to delete the last element
of the interval set and to create an element in an existing empty
internal set. This patch includes a test that reproduces the issue.
Fixes: 4935a0d561b5 ("segtree: special handling for the first non-matching segment") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Duncan Roe [Fri, 6 Dec 2019 02:37:12 +0000 (13:37 +1100)]
doc: Clarify conditions under which a reject verdict is permissible
A phrase like "input chain" is a throwback to xtables documentation.
In nft, chains are containers for rules. They do have a type, but what's
important here is which hook each uses.
netlink: off-by-one write in netdev chain device array
==728473== Invalid write of size 8
==728473== at 0x48960F2: netlink_delinearize_chain (netlink.c:422)
==728473== by 0x4896252: list_chain_cb (netlink.c:459)
==728473== by 0x4896252: list_chain_cb (netlink.c:441)
==728473== by 0x4F2C654: nftnl_chain_list_foreach (chain.c:1011)
==728473== by 0x489629F: netlink_list_chains (netlink.c:478)
==728473== by 0x4882303: cache_init_objects (rule.c:177)
==728473== by 0x4882303: cache_init (rule.c:222)
==728473== by 0x4882303: cache_update (rule.c:272)
==728473== by 0x48A7DCE: nft_evaluate (libnftables.c:408)
==728473== by 0x48A86D9: nft_run_cmd_from_buffer (libnftables.c:449)
==728473== by 0x10A5D6: main (main.c:338)
Fixes: 3fdc7541fba0 ("src: add multidevice support for netdev chain") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Sat, 30 Nov 2019 11:30:57 +0000 (11:30 +0000)]
doc: fix inconsistency in set statement documentation.
The description of the set statement asserts that the set must have been
created with the "dynamic" flag. However, this is not the case, and it
is contradicted by the following example in which the "dynamic" flag
does not appear.
In fact, one or both of the "dynamic" or the "timeout" flags need to be
used, depending on what the set statement contains. Amend the
description to explain this more accurately.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Eric Jallot [Fri, 29 Nov 2019 14:30:39 +0000 (15:30 +0100)]
scanner: fix out-of-bound memory write in include_file()
Before patch:
# echo 'include "/tmp/rules.nft"' > /tmp/rules.nft
# nft -f /tmp/rules.nft
In file included from /tmp/rules.nft:1:1-25:
from /tmp/rules.nft:1:1-25:
[snip]
from /tmp/rules.nft:1:1-25:
/tmp/rules.nft:1:1-25: Error: Include nested too deeply, max 16 levels
include "/tmp/rules.nft"
^^^^^^^^^^^^^^^^^^^^^^^^^
double free or corruption (out)
Aborted (core dumped)
valgrind reports:
==8856== Invalid write of size 8
==8856== at 0x4E8FCAF: include_file (scanner.l:718)
==8856== by 0x4E8FEF6: include_glob (scanner.l:793)
==8856== by 0x4E9985D: scanner_include_file (scanner.l:875)
==8856== by 0x4E89D7A: nft_parse (parser_bison.y:828)
==8856== by 0x4E765E1: nft_parse_bison_filename (libnftables.c:394)
==8856== by 0x4E765E1: nft_run_cmd_from_filename (libnftables.c:497)
==8856== by 0x40172D: main (main.c:340)
So perform bounds checking on MAX_INCLUDE_DEPTH before writing.
After patch:
# nft -f /tmp/rules.nft
In file included from /tmp/rules.nft:1:1-25:
from /tmp/rules.nft:1:1-25:
[snip]
from /tmp/rules.nft:1:1-25:
/tmp/rules.nft:1:1-25: Error: Include nested too deeply, max 16 levels
include "/tmp/rules.nft"
^^^^^^^^^^^^^^^^^^^^^^^^^
# echo $?
1
Also:
Update scanner_push_file() function definition accordingly.
Fixes: 32325e3c3fab4 ("libnftables: Store top_scope in struct nft_ctx") Signed-off-by: Eric Jallot <ejallot@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 21 Nov 2019 10:41:24 +0000 (11:41 +0100)]
segtree: Fix add and delete of element in same batch
The commit this fixes accidentally broke a rather exotic use-case which
is but used in set-simple.t of tests/monitor:
| # nft 'add element t s { 22-25 }; delete element t s { 22-25 }'
Since ranges are now checked for existence in userspace before delete
command is submitted to kernel, the second command above was rejected
because the range in question wasn't present in cache yet. Fix this by
adding new interval set elements to cache after creating the batch job
for them.
Fixes: decc12ec2dc31 ("segtree: Check ranges when deleting elements") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 16 Nov 2019 21:32:18 +0000 (22:32 +0100)]
tests/py: Set a fixed timezone in nft-test.py
Payload generated for 'meta time' matches depends on host's timezone and
DST setting. To produce constant output, set a fixed timezone in
nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
the remaining two tests.
Fixes: 0518ea3f70d8c ("tests: add meta time test cases") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Ander Juaristi <a@juaristi.eus> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 27 Nov 2019 12:58:00 +0000 (13:58 +0100)]
nft.8: Fix nat family spec position
In inet family nat statements, ip/ip6 keyword must come before 'to'
keyword, not after.
Fixes: fbe27464dee45 ("src: add nat support for the inet family") Cc: Florian Westphal <fw@strlen.de> Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
src: add ability to set/get secmarks to/from connection
Labeling established and related packets requires the secmark to be stored in the connection.
Add the ability to store and retrieve secmarks like:
...
chain input {
...
# label new incoming packets
ct state new meta secmark set tcp dport map @secmapping_in
# add label to connection
ct state new ct secmark set meta secmark
# set label for est/rel packets from connection
ct state established,related meta secmark set ct secmark
...
}
...
chain output {
...
# label new outgoing packets
ct state new meta secmark set tcp dport map @secmapping_out
# add label to connection
ct state new ct secmark set meta secmark
# set label for est/rel packets from connection
ct state established,related meta secmark set ct secmark
...
}
...
This patch also disallow constant value on the right hand side.
# nft add rule x y meta secmark 12
Error: Cannot be used with right hand side constant value
add rule x y meta secmark 12
~~~~~~~~~~~~ ^^
# nft add rule x y ct secmark 12
Error: Cannot be used with right hand side constant value
add rule x y ct secmark 12
~~~~~~~~~~ ^^
# nft add rule x y ct secmark set 12
Error: ct secmark must not be set to constant value
add rule x y ct secmark set 12
^^^^^^^^^^^^^^^^^
This patch improves 3bc84e5c1fdd ("src: add support for setting secmark").
Signed-off-by: Christian Göttsche <cgzones@googlemail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Michal Rostecki [Thu, 21 Nov 2019 12:33:32 +0000 (13:33 +0100)]
mnl: Fix -Wimplicit-function-declaration warnings
This change fixes the following warnings:
mnl.c: In function ‘mnl_nft_flowtable_add’:
mnl.c:1442:14: warning: implicit declaration of function ‘calloc’ [-Wimplicit-function-declaration]
dev_array = calloc(len, sizeof(char *));
^~~~~~
mnl.c:1442:14: warning: incompatible implicit declaration of built-in function ‘calloc’
mnl.c:1442:14: note: include ‘<stdlib.h>’ or provide a declaration of ‘calloc’
mnl.c:1449:2: warning: implicit declaration of function ‘free’ [-Wimplicit-function-declaration]
free(dev_array);
^~~~
mnl.c:1449:2: warning: incompatible implicit declaration of built-in function ‘free’
mnl.c:1449:2: note: include ‘<stdlib.h>’ or provide a declaration of ‘free’
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org> Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
ct state new meta secmark set tcp dport map @secmapping_in
becomes
ct state new secmark name tcp dport map @secmapping_in
which is not correct.
Fixes: 3bc84e5c1fdd ("src: add support for setting secmark") Signed-off-by: Christian Göttsche <cgzones@googlemail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 15 Nov 2019 18:11:32 +0000 (19:11 +0100)]
scanner: Introduce numberstring
This token combines decstring and hexstring. The latter two had
identical action blocks (which were not completely trivial), this allows
to merge them.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 6 Nov 2019 20:59:07 +0000 (21:59 +0100)]
files: Drop shebangs from config files
These are not meant to be executed as is but instead loaded via
'nft -f' - all-in-one.nft even points this out in header comment.
While being at it, drop two spelling mistakes found along the way.
Consequently remove executable bits - being registered in automake as
dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.
Also there is obviously no need for replacement of nft binary path
anymore, drop that bit from Makefile.am.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 16 Nov 2019 20:02:07 +0000 (21:02 +0100)]
parser_bison: Avoid set references in odd places
With set references being recognized by symbol_expr and that being part
of primary_expr as well as primary_rhs_expr, they could basically occur
anywhere while in fact they are allowed only in quite a few spots.
Untangle things a bit by introducing set_ref_expr and adding that only
in places where it is needed to pass testsuites.
Make sure users can define variables as set references, eg.
define xyz = @setref
And allow to use them from set expressions and statements too.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 14 Nov 2019 14:45:07 +0000 (15:45 +0100)]
cache: Reduce caching for get command
Introduce a function to distinguish which command object was given and
request only the necessary bits to have sets and their elements
available for 'get element' command.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 14 Nov 2019 13:41:50 +0000 (14:41 +0100)]
segtree: Fix get element for little endian ranges
This fixes get element command for interval sets with host byte order
data type, like e.g. mark. During serializing of the range (or element)
to query, data was exported in wrong byteorder and consequently not
found in kernel.
The mystery part is that code seemed correct: When calling
constant_expr_alloc() from set_elem_add(), the set key's byteorder was
passed with correct value of BYTEORDER_HOST_ENDIAN.
Comparison with delete/add element code paths though turned out that in
those use-cases, constant_expr_alloc() is called with BYTEORDER_INVALID:
- seg_tree_init() takes byteorder field value of first element in
init->expressions (i.e., the elements requested on command line) and
assigns that to tree->byteorder
- tree->byteorder is passed to constant_expr_alloc() in
set_insert_interval()
- the elements' byteorder happens to be the default value
This patch may not fix the right side, but at least it aligns get with
add/delete element codes.
Fixes: a43cc8d53096d ("src: support for get element command") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 12 Nov 2019 19:00:15 +0000 (20:00 +0100)]
segtree: Check ranges when deleting elements
Make sure any intervals to delete actually exist, otherwise reject the
command. Without this, it is possible to mess up rbtree contents:
| # nft list ruleset
| table ip t {
| set s {
| type ipv4_addr
| flags interval
| auto-merge
| elements = { 192.168.1.0-192.168.1.254, 192.168.1.255 }
| }
| }
| # nft delete element t s '{ 192.168.1.0/24 }'
| # nft list ruleset
| table ip t {
| set s {
| type ipv4_addr
| flags interval
| auto-merge
| elements = { 192.168.1.255-255.255.255.255 }
| }
| }
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 12 Nov 2019 18:28:37 +0000 (19:28 +0100)]
meta: Rewrite hour_type_print()
There was no point in this recursively called __hour_type_print_r() at
all, it takes only four lines of code to split the number of seconds
into hours, minutes and seconds.
While being at it, inverse the conditional to reduce indenting for the
largest part of the function's body. Also introduce SECONDS_PER_DAY
macro to avoid magic numbers.
Fixes: f8f32deda31df ("meta: Introduce new conditions 'time', 'day' and 'hour'") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 30 Oct 2019 20:45:39 +0000 (21:45 +0100)]
libnftables: Store top_scope in struct nft_ctx
Allow for interactive sessions to make use of defines. Since parser is
initialized for each line, top scope defines didn't persist although
they are actually useful for stuff like:
| # nft -i
| define goodports = { 22, 23, 80, 443 }
| add rule inet t c tcp dport $goodports accept
| add rule inet t c tcp sport $goodports accept
While being at it, introduce scope_alloc() and scope_free().
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 5 Nov 2019 21:40:26 +0000 (21:40 +0000)]
src: add and use `set_is_meter` helper
The sets constructed for meters are flagged as anonymous and dynamic.
However, in some places there are only checks that they are dynamic,
which can lead to normal sets being classified as meters.
For example:
# nft add table t
# nft add set t s { type ipv4_addr; size 256; flags dynamic,timeout; }
# nft add chain t c
# nft add rule t c tcp dport 80 meter m size 128 { ip saddr limit rate 10/second }
# nft list meters
table ip t {
set s {
type ipv4_addr
size 256
flags dynamic,timeout
}
meter m {
type ipv4_addr
size 128
flags dynamic
}
}
# nft list meter t m
table ip t {
meter m {
type ipv4_addr
size 128
flags dynamic
}
}
# nft list meter t s
Error: No such file or directory
list meter t s
^
Add a new helper `set_is_meter` and use it wherever there are checks for
meters.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Eric Jallot [Wed, 30 Oct 2019 17:19:17 +0000 (18:19 +0100)]
doc: fix missing family in plural forms list command.
Fixes: 067ac215e93f ("doc: update nft list plural form parameters") Signed-off-by: Eric Jallot <ejallot@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This patch allows you to specify multiple netdevices to be bound to the
netdev basechain, eg.
# nft add chain netdev x y { \
type filter hook ingress devices = { eth0, eth1 } priority 0\; }
json codebase has been updated to support for one single device with the
existing representation, no support for multidevice is included in this
patch.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 29 Oct 2019 11:20:18 +0000 (12:20 +0100)]
tests/py: Fix test script for Python3 tempfile
When instantiating a temporary file using tempfile's TemporaryFile()
constructor, the resulting object's 'name' attribute is of type int.
This in turn makes print_msg() puke while trying to concatenate string
and int using '+' operator.
Fix this by using format strings consequently, thereby cleaning up code
a bit.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 22 Oct 2019 20:58:55 +0000 (21:58 +0100)]
main: remove duplicate output flag assignment.
`NFT_CTX_OUTPUT_NUMERIC_TIME` is implicit in
`NFT_CTX_OUTPUT_NUMERIC_ALL`: there are is no need explicitly to OR it
into output_flags when `--numeric` is passed.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Given that this change breaks typical commands like
'nft list ruleset -a' while on the other hand escaping of semicolons and
(depending on shell) curly braces is still required, decision was made
to not go with this solution.
Jeremy Sowden [Mon, 21 Oct 2019 21:49:22 +0000 (22:49 +0100)]
src: add --terse to suppress output of set elements.
Listing an entire ruleset or a table with `nft list` prints the elements
of all set definitions within the ruleset or table. Seeing the full set
contents is not often necessary especially when requesting to see
someone's ruleset for help and support purposes. Add a new option '-t,
--terse' options to suppress the output of set contents.
Jeremy Sowden [Mon, 21 Oct 2019 21:49:21 +0000 (22:49 +0100)]
src: use `-T` as the short option for `--numeric-time`.
A new `--terse` option will be introduced in a later patch. Change the
short option used for `--numeric-time` from `-t` to `-T` in order to
leave `-t` free.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 21 Oct 2019 16:51:14 +0000 (18:51 +0200)]
main: Fix for misleading error with negative chain priority
getopt_long() would try to parse the negative priority as an option and
return -1 as it is not known:
| # nft add chain x y { type filter hook input priority -30\; }
| nft: invalid option -- '3'
Fix this by prefixing optstring with a plus character. This instructs
getopt_long() to not collate arguments but just stop after the first
non-option, leaving the rest for manual handling. In fact, this is just
what nft desires: mixing options with nft syntax leads to confusive
command lines anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 21 Oct 2019 14:29:03 +0000 (16:29 +0200)]
tproxy: Add missing error checking when parsing from netlink
netlink_get_register() may return NULL and every other caller checks
that. Assuming this situation is not expected, just jump to 'err' label
without queueing an explicit error message.
Fixes: 2be1d52644cf7 ("src: Add tproxy support") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
If --echo is passed, then the cache already contains the commands that
have been sent to the kernel. However, anonymous sets are an exception
since the cache needs to be updated in this case.
Remove the old cache logic from the monitor code that has been replaced
by 01e5c6f0ed03 ("src: add cache level flags").
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Acked-by: Phil Sutter <phil@nwl.cc>
Eric Jallot [Thu, 17 Oct 2019 11:08:36 +0000 (13:08 +0200)]
flowtable: fix memleak in exit path
Add missing loop in table_free().
Free all objects in flowtable_free() and add conditions in case of error recovery
in the parser (See commit 4be0a3f922a29).
Also, fix memleak in the parser.
This fixes the following memleak:
# valgrind --leak-check=full nft add flowtable inet raw f '{ hook ingress priority filter; devices = { eth0 }; }'
==15414== Memcheck, a memory error detector
==15414== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==15414== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==15414== Command: nft add flowtable inet raw f {\ hook\ ingress\ priority\ filter;\ devices\ =\ {\ eth0\ };\ }
==15414==
==15414==
==15414== HEAP SUMMARY:
==15414== in use at exit: 266 bytes in 4 blocks
==15414== total heap usage: 55 allocs, 51 frees, 208,105 bytes allocated
==15414==
==15414== 5 bytes in 1 blocks are definitely lost in loss record 2 of 4
==15414== at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==15414== by 0x5C64AA9: strdup (strdup.c:42)
==15414== by 0x4E705ED: xstrdup (utils.c:75)
==15414== by 0x4E93F01: nft_lex (scanner.l:648)
==15414== by 0x4E85C1C: nft_parse (parser_bison.c:5577)
==15414== by 0x4E75A07: nft_parse_bison_buffer (libnftables.c:375)
==15414== by 0x4E75A07: nft_run_cmd_from_buffer (libnftables.c:443)
==15414== by 0x40170F: main (main.c:326)
==15414==
==15414== 261 (128 direct, 133 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
==15414== at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==15414== by 0x4E705AD: xmalloc (utils.c:36)
==15414== by 0x4E705AD: xzalloc (utils.c:65)
==15414== by 0x4E560B6: expr_alloc (expression.c:45)
==15414== by 0x4E56288: symbol_expr_alloc (expression.c:286)
==15414== by 0x4E8A601: nft_parse (parser_bison.y:1842)
==15414== by 0x4E75A07: nft_parse_bison_buffer (libnftables.c:375)
==15414== by 0x4E75A07: nft_run_cmd_from_buffer (libnftables.c:443)
==15414== by 0x40170F: main (main.c:326)
Fixes: 92911b362e906 ("src: add support to add flowtables") Signed-off-by: Eric Jallot <ejallot@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 16 Oct 2019 22:20:59 +0000 (00:20 +0200)]
rule: Fix for single line ct timeout printing
Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout")
removed an extra semicolon at end of line, but thereby broke single line
output. The correct fix is to use opts->stmt_separator which holds
either newline or semicolon chars depending on output mode.
Fixes: 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Wed, 16 Oct 2019 21:46:10 +0000 (23:46 +0200)]
tests/monitor: Fix for changed ct timeout format
Commit a9b0c385a1d5e ("rule: print space between policy and timeout")
changed spacing in ct timeout objects but missed to adjust related test
case.
Fixes: a9b0c385a1d5e ("rule: print space between policy and timeout") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Tue, 15 Oct 2019 13:58:13 +0000 (15:58 +0200)]
mnl: Don't use nftnl_set_set()
The function is unsafe to use as it effectively bypasses data length
checks. Instead use nftnl_set_set_str() which at least asserts a const
char pointer is passed.
Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Eric Jallot [Tue, 8 Oct 2019 13:47:24 +0000 (15:47 +0200)]
obj: fix memleak in parser_bison.y
Each object (secmark, synproxy, quota, limit, counter) is dynamically allocated
by the parser and not freed at exit.
However, there is no need to use dynamic allocation here because struct obj
already provides the required storage. Update the grammar to ensure that
obj_alloc() is called before config occurs.
This fixes the following memleak (secmark as example):
# valgrind --leak-check=full nft add secmark inet raw ssh \"system_u:object_r:ssh_server_packet_t:s0\"
==14643== Memcheck, a memory error detector
==14643== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14643== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==14643== Command: nft add secmark inet raw ssh "system_u:object_r:ssh_server_packet_t:s0"
==14643==
==14643==
==14643== HEAP SUMMARY:
==14643== in use at exit: 256 bytes in 1 blocks
==14643== total heap usage: 41 allocs, 40 frees, 207,809 bytes allocated
==14643==
==14643== 256 bytes in 1 blocks are definitely lost in loss record 1 of 1
==14643== at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==14643== by 0x4E72074: xmalloc (utils.c:36)
==14643== by 0x4E72074: xzalloc (utils.c:65)
==14643== by 0x4E89A31: nft_parse (parser_bison.y:3706)
==14643== by 0x4E778E7: nft_parse_bison_buffer (libnftables.c:375)
==14643== by 0x4E778E7: nft_run_cmd_from_buffer (libnftables.c:443)
==14643== by 0x40170F: main (main.c:326)
Fixes: f44ab88b1088e ("src: add synproxy stateful object support") Fixes: 3bc84e5c1fdd1 ("src: add support for setting secmark") Fixes: c0697eabe832d ("src: add stateful object support for limit") Fixes: 4d38878b39be4 ("src: add/create/delete stateful objects") Signed-off-by: Eric Jallot <ejallot@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Eric Jallot [Mon, 30 Sep 2019 08:38:23 +0000 (10:38 +0200)]
src: obj: fix memleak in handle_free()
Using limit object as example:
# valgrind --leak-check=full nft list ruleset
==9937== Memcheck, a memory error detector
==9937== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9937== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==9937== Command: nft list ruleset
==9937==
table inet raw {
limit lim1 {
rate 1/second
}
}
==9937==
==9937== HEAP SUMMARY:
==9937== in use at exit: 5 bytes in 1 blocks
==9937== total heap usage: 50 allocs, 49 frees, 212,065 bytes allocated
==9937==
==9937== 5 bytes in 1 blocks are definitely lost in loss record 1 of 1
==9937== at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==9937== by 0x5C65AA9: strdup (strdup.c:42)
==9937== by 0x4E720A3: xstrdup (utils.c:75)
==9937== by 0x4E660FF: netlink_delinearize_obj (netlink.c:972)
==9937== by 0x4E6641C: list_obj_cb (netlink.c:1064)
==9937== by 0x50E8993: nftnl_obj_list_foreach (object.c:494)
==9937== by 0x4E664EA: netlink_list_objs (netlink.c:1085)
==9937== by 0x4E4FE82: cache_init_objects (rule.c:188)
==9937== by 0x4E4FE82: cache_init (rule.c:221)
==9937== by 0x4E4FE82: cache_update (rule.c:271)
==9937== by 0x4E7716E: nft_evaluate (libnftables.c:406)
==9937== by 0x4E778F7: nft_run_cmd_from_buffer (libnftables.c:447)
==9937== by 0x40170F: main (main.c:326)
Fixes: 4756d92e517ae ("src: listing of stateful objects") Signed-off-by: Eric Jallot <ejallot@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
libnftables: memleak when list of commands is empty
==9946== 200,807 (40 direct, 200,767 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
==9946== at 0x4837B65: calloc (vg_replace_malloc.c:762)
==9946== by 0x4F28216: nftnl_batch_alloc (batch.c:66)
==9946== by 0x48A33E8: mnl_batch_init (mnl.c:164)
==9946== by 0x48A736F: nft_netlink.isra.0 (libnftables.c:29)
==9946== by 0x48A7D03: nft_run_cmd_from_filename (libnftables.c:508)
==9946== by 0x10A621: main (main.c:328)
Fixes: fc6d0f8b0cb1 ("libnftables: get rid of repeated initialization of netlink_ctx") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
