Phil Sutter [Thu, 25 Apr 2019 12:56:54 +0000 (14:56 +0200)]
src: use UDATA defines from libnftnl
Userdata attribute names have been added to libnftnl, use them instead
of the local copy.
While being at it, rename udata_get_comment() in netlink_delinearize.c
and the callback it uses since the function is specific to rules. Also
integrate the existence check for NFTNL_RULE_USERDATA into it along with
the call to nftnl_rule_get_data().
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Eric Garver [Wed, 1 May 2019 16:34:45 +0000 (12:34 -0400)]
parser_json: fix crash on add rule to bad references
Pass the location via the handle so the error leg in
rule_translate_index() can reference it. Applies to invalid references
to tables, chains, and indexes.
Fixes: 586ad210368b ("libnftables: Implement JSON parser") Signed-off-by: Eric Garver <eric@garver.life> Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Upcoming dscp codepoint for background traffic of low precendence
such as bulk data transfers with low priority in time, non time-critical
backups, larger software updates, web search engines while gathering
information from web servers and so on.
Signed-off-by: Loganaden Velvindron <logan@cyberstorm.mu> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 25 Apr 2019 12:59:41 +0000 (14:59 +0200)]
tests/py: Fix error messages in chain_delete()
Adding string and chain object is an illegal operation in Python.
Instead concatenate with cmd string since that contains all required
information already.
Fixes: 820fd08b5f1d4 ("tests/py: Review print statements in nft-test.py") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 25 Apr 2019 12:59:40 +0000 (14:59 +0200)]
tests: monitor: Adjust to changed events ordering
When replacing a rule, kernel nowadays seems to report rule add event
before rule delete one. Since both events belong to the same
transaction, this is harmless per definition and merely needs adjustment
in expected output.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 11 Apr 2019 10:38:51 +0000 (12:38 +0200)]
parser_json: Disallow ct helper as type to map to
When creating a map, users may either map dtype:dtype or dtype:object.
In the second case, only counter, quota, limit and secmark is allowed by
bison, but JSON parser wasn't as strict, allowing ct helper as well.
Remove that to avoid undefined behaviour.
Fixes: 586ad210368b7 ("libnftables: Implement JSON parser") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Program received signal SIGSEGV, Segmentation fault.
#1 0x00007ffff7f734f9 in string_to_nft_object (str=0x55555555f410
"mark") at parser_json.c:2513
2513 if (!strcmp(str, obj_tbl[i]))
The obj_tbl array is allocated with the maximum element index even
if lower indexes are not populated, so it produces null pointer
items.
This patch ensures that the maximum number of possible indexes
but also the element is not comparing a null pointer.
Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Do not inconditionally hold reference to start interval.
The handling depends on what kind of range expression we need to build,
either no range at all, a prefix or a plain range. Depending on the
case, we need to partially clone what we need from the expression to
avoid use-after-free.
This fixes valgrind reports that look like this, when listing rulesets:
==30018== 2,057,984 (1,028,992 direct, 1,028,992 indirect) bytes in 8,039 blocks are definitely lost in loss record 76 of 83
==30018== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==30018== by 0x4E75978: xmalloc (utils.c:36)
==30018== by 0x4E75A5D: xzalloc (utils.c:65)
==30018== by 0x4E5CEC0: expr_alloc (expression.c:45)
==30018== by 0x4E5D610: mapping_expr_alloc (expression.c:985)
==30018== by 0x4E6A068: netlink_delinearize_setelem (netlink.c:810)
==30018== by 0x5B51320: nftnl_set_elem_foreach (set_elem.c:673)
==30018== by 0x4E6A2D5: netlink_list_setelems (netlink.c:864)
==30018== by 0x4E56C76: cache_init_objects (rule.c:166)
==30018== by 0x4E56C76: cache_init (rule.c:216)
==30018== by 0x4E56C76: cache_update (rule.c:243)
==30018== by 0x4E64530: cmd_evaluate_list (evaluate.c:3503)
==30018== by 0x4E64530: cmd_evaluate (evaluate.c:3880)
==30018== by 0x4E7D12F: nft_parse (parser_bison.y:798)
==30018== by 0x4E7AB56: nft_parse_bison_buffer (libnftables.c:349)
==30018== by 0x4E7AB56: nft_run_cmd_from_buffer (libnftables.c:394)
Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
table ip6 nat { chain output {
type nat hook output priority 0; policy accept;
dnat to dead:2::99
}
Now consider same ruleset, but using 'table inet nat':
nft now lacks context to determine address family to parse 'to $address'.
This adds code to make the following work:
table inet nat { [ .. ]
# detect af from network protocol context:
ip6 daddr dead::2::1 dnat to dead:2::99
# use new dnat ip6 keyword:
dnat ip6 to dead:2::99
}
On list side, the keyword is only shown in the inet family, else the
short version (dnat to ...) is used as the family is redundant when the
table already mandates the ip protocol version supported.
Address mismatches such as
table ip6 { ..
dnat ip to 1.2.3.4
are detected/handled during the evaluation phase.
Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fingerprints need to be unique to make this fit into the set/map
infrastructure for exact matches. Having multiples fingerprints with
same signature is a problem, since it forces users to add multiple
rules.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
==6297== 24 bytes in 3 blocks are definitely lost in loss record 2 of 13
==6297== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==6297== by 0x56193B9: strdup (strdup.c:42)
==6297== by 0x4E758BD: xstrdup (utils.c:75)
==6297== by 0x4E7F9D3: nft_parse (parser_bison.y:1895)
==6297== by 0x4E7AAE1: nft_parse_bison_filename (libnftables.c:370)
==6297== by 0x4E7AAE1: nft_run_cmd_from_filename (libnftables.c:438)
==6297== by 0x109A33: main (main.c:310)
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: improve error reporting in tproxy with inet family
# nft add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
Error: conflicting protocols specified: ip vs. unknown. You must specify ip or ip6 family in tproxy statement
add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
~~~~~~~~ ^^^^^^^^^^^^^^^
instead of:
# nft add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
Error: Conflicting network layer protocols.
add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
^^^^^^^^^^^^^^^
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1310 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
parser_bison: missing tproxy syntax with port only for inet family
# nft add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy ip to :2000
Error: syntax error, unexpected colon
add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy ip to :2000
^
Syntax with no protocol for tproxy complains with:
# nft add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
Error: Conflicting network layer protocols.
add rule inet filter divert ip daddr 0.0.0.0/0 meta l4proto tcp tproxy to :2000
^^^^^^^^^^^^^^^
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1310 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 2 Apr 2019 13:34:43 +0000 (15:34 +0200)]
parser_json: Rewrite echo support
Instead of guessing which object to update with retrieved handle,
introduce a list containing struct cmd <-> json_t associations. Upon
batch commit, allocated cmd objects are assigned a unique netlink
sequence number. Monitor events contain that number as well, so they may
be associated to the cmd object which triggered them. Using
json_cmd_assoc list the event may in turn be associated to the input's
JSON object which should receive the handle value.
This also fixes incorrect behaviour if JSON input contained "insert"
commands.
Fixes: bb32d8db9a125 ("JSON: Add support for echo option") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Luis Ressel [Sun, 17 Mar 2019 18:24:22 +0000 (19:24 +0100)]
configure.ac: Clean up AC_ARG_{WITH, ENABLE} invocations, s/==/=/
* AC_ARG_ENABLE implicitly defines enable_debug; there's no point in
performing extra work just to define with_debug with an identical
value.
* The same applies to with_xtables and with_libxtables.
* The AS_IF block in the `AC_ARG_ENABLE([man-doc], ...` invocation is
essentially a noop. All it does is to set enable_man_doc to `yes` if
has a value that matches neither `yes` nor `no`. (This could happen if
a user calls `configure --enable-man-doc=foo`, but that'd be a user
error which we don't need to handle.)
* The correct operator for equality tests in `test` is `=`. Some
implementations also support `==`, but this is not portable.
Signed-off-by: Luis Ressel <aranea@aixah.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Luis Ressel [Sun, 17 Mar 2019 17:19:11 +0000 (18:19 +0100)]
configure.ac: Fix a2x check
* If enable_man_doc is set, but a2x can't be found, configure should
fail instead of silently disabling man page creation.
* The AS_IF block checking $need_a2x is never active (need_a2x has been
removed from configure.ac in 13e44a608 and a277479dc).
* AC_CHECK_PROG(VAR, ...) is a noop if VAR is already set, allowing the
user to explicitly specify the (path to the) binary in VAR. Adjust the
AS_IF check to account for this.
Signed-off-by: Luis Ressel <aranea@aixah.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
parser_bison: no need for statement separator for ct object commands
Otherwise, this forces user to place a double semi-colon to skip a
parser error in a multi-line commands:
# nft add "ct helper ip filter test { type \"ftp\" protocol tcp; };add rule filter test ct helper set \"ftp\""
Error: syntax error, unexpected add, expecting end of file or newline or semicolon
add ct helper ip filter test { type "ftp" protocol tcp; };add rule filter test ct helper set "ftp"
^^^
Reported-by: Laura Garcia <nevola@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: misleading error reporting with sets and maps
When trying to list a map content, if set is used, nft reports:
# nft list set filter group_7933
Error: No such file or directory; did you mean set ‘group_7933’ in table ip ‘filter’?
list set filter group_7933
^^^^^^^^^^
Which is confusing in case user wants to list an existing map:
# nft list map filter group_7933
table ip filter {
map group_7933 {
type ipv4_addr : classid
flags interval
elements = { 10.4.22.0/24 : 1:c7cb }
}
}
Instead, give a hint to user that probably wants to list a map, not a set:
# nft list set filter group_7933
Error: No such file or directory; did you mean map ‘group_7933’ in table ip ‘filter’?
list set filter group_7933
^^^^^^^^^^
Fixes: 285bb67a11ad ("src: introduce simple hints on incorrect set") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
segtree: add missing non-matching segment to set in flat representation
# cat test.nft
add set x y { type ipv4_addr; }
add element x y { 10.0.24.0/24 }
# nft -f test.nft
# nft delete element x y { 10.0.24.0/24 }
bogusly returns -ENOENT. The non-matching segment (0.0.0.0 with end-flag
set on) is not added to the set in the example above.
This patch also adds a test to cover this case.
Fixes: 4935a0d561b5 ("segtree: special handling for the first non-matching segment") Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 26 Feb 2019 21:13:41 +0000 (22:13 +0100)]
json: Fix memleaks in echo support
When extracting netlink message data for populating JSON objects with
handles, allocated nftnl objects were not freed. Though since freeing
these objects also frees retrieved string attributes, copy them using
strdupa() which takes care of memory deallocation upon function return.
This is ideal since these strings are used only to find the right JSON
object to insert the handle into.
Fixes: bb32d8db9a125 ("JSON: Add support for echo option") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 26 Feb 2019 21:13:39 +0000 (22:13 +0100)]
parser_json: Duplicate chain name when parsing jump verdict
Since verdict expression frees the chain name, pass a newly allocated
string to it. Otherwise double free happens because json_decref() frees
the string property value as well.
Fixes: d1057a5feb5fd ("JSON: Simplify verdict statement parsing") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 26 Feb 2019 21:13:38 +0000 (22:13 +0100)]
libnftables: Print errors before freeing commands
Commands may contain data printed by an error record, so make sure
cmd_free() is not called before erec_print_list() has returned.
Fixes: 778de37d82e7b ("libnftables: Keep cmds list outside of parser_state") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
size of struct expr changes from 144 to 128 bytes on x86_64.
This doesn't look like much, but large rulesets can have tens of thousands
of expressions (each set element is represented by an expression).
Temporary kludge to remove all the expr->ops->type == ... patterns.
Followup patch will remove expr->ops, and make expr_ops() lookup
the correct expr_ops struct instead to reduce struct expr size.
Florian Westphal [Wed, 16 Jan 2019 12:54:13 +0000 (13:54 +0100)]
mnl: name is ignored when deleting a table
nlt is reallocated, leaking first allocation and also removing
the table name/handle that was set on nlt object.
Add a test case for this as well, the batch is supposed to fail
when trying to delete a non-existant table, rather than wiping
all tables in the same address family.
nf_tables can handle payload exprs for sizes <= sizeof(u32) via a direct
operation from the eval loop, rather than a a call to the payload
expression. Two loads for four byte quantities are thus faster than a
single load for an 8 byte load.
ip saddr 1.2.3.4 ip daddr 2.3.4.5
is faster with this applied, even though it involves two payload and two
two compare expressions, just because all can be handled from the main
loop without any calls to expression ops.
Keep merging for linklayer and when at least one of the expressions
already exceeded the 4 byte "limit" anyway.
Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
test files are located via find + a pattern search that looks for
_[0-9]. Previous change makes all test scripts return 0 when the
test case is supposed to pass, so the foo_$retval name is no longer
needed.
Update script to look for all executeables in the 'testcases' directory.
This makes it necessary to make two dump-files non-executeable.
The shell-based tests currently encode a return value in the
file name, i.e. foo_1 expects that the script should return '1'
for the test case to pass.
This is very error prone, and one test case is even broken (i.e.,
it returns 1, but because of a different, earlier error).
do_something || exit 1
or
'set -e'
are both pretty common patterns, in both cases tests should fail.
In those test-cases that deliberately test for an error,
nft something_should_fail || exit 0
nft something_should_fail && exit 1
or a similar constructs should be used.
This initial commit modififies all '_1' scripts to return 0 on
success, usually via 'nft wrong || exit 0'.
All tests pass, except the one broken test case that hasn't worked
before either, but where 'set -e' use made it pass (the failing command
is supposed to work, and the command that is supposed to fail is never
run).
tests: shell: add test case for leaking of stateful object refcount
Taehee Yoo fixed a bug in error path handling for object refcounts.
Quoting patch description:
| $nft add table ip filter
| $nft add counter ip filter c1
| $nft add map ip filter m1 { type ipv4_addr : counter \;}
| $nft add element ip filter m1 { 1 : c1 }
| $nft add element ip filter m1 { 1 : c1 }
| $nft delete element ip filter m1 { 1 }
| $nft delete counter ip filter c1
|Result:
| Error: Could not process rule: Device or resource busy
| delete counter ip filter c1
|
|At the second 'nft add element ip filter m1 { 1 : c1 }', the reference
|count of the 'c1' is increased then it tries to insert into the 'm1'. but
|the 'm1' already has same element so it returns -EEXIST.
|But it doesn't decrease the reference count of the 'c1' in the error path.
|Due to a leak of the reference count of the 'c1', the 'c1' can't be
|removed by 'nft delete counter ip filter c1'.
Florian Westphal [Sat, 29 Dec 2018 20:29:00 +0000 (21:29 +0100)]
xt: fix build with --with-xtables
The previous change is bonkers, it fixes build when libxtables isn't
even installed, but broke build when --with-xtables is provided to configure.
Reason is that the include guard comes too early, so xtables.h is never
included, causing build to fail because no libxtables function prototypes
and definitions are available.
Fixes: 9e84f3f083bb ("xt: fix build when libxtables is not installed") Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Mon, 17 Dec 2018 15:29:56 +0000 (16:29 +0100)]
src: Reject 'export vm json' command
Since libnftnl recently dropped JSON output support, this form of JSON
export is not available anymore. Point at 'nft -j list ruleset' command
for a replacement in error message.
Since 'export' command is not useable anymore, remove it from
documentation. Instead point out that 'list ruleset' command serves well
for dumping and later restoring.
To not cause pointless inconvenience for users wishing to store their
ruleset in JSON format, make JSON parser fallback to CMD_ADD if no
recognized command property was found. This allows to feed the output of
'nft -j list ruleset' into 'nft -f' without any modification.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
src: introduce simple hints on incorrect identifier
# cat test.nft
define test = "1.2.3.4"
table ip x {
chain y {
ip saddr $text
}
}
# nft -f test.nft
test.nft:5:13-16: Error: unknown identifier 'text'; did you mean identifier ‘test’?
ip saddr $text
^^^^
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
src: provide suggestion for misspelled object name
Use this from the lookup path, to check for misspellings:
# nft add table filter
# nft add chain filtre test
Error: No such file or directory; did you mean table ‘filter’ in family ip?
add chain filtre test
^^^^^^
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Florian Westphal [Wed, 28 Nov 2018 10:44:59 +0000 (11:44 +0100)]
tests: add test case for rule replacement expression deactivation
When a rule is replaced, all of the expressions that are in use need to
be deactivated.
Kernel missed to do this on replace. In this example, this would
result in a leak of the chain use counter ("jump" expression
deactivation is not called). This then either resulted in a BUG or
WARN, depending on kernel version.
This patch adds simple infrastructure to provide a hints to user on
references to incorrect table. While at it, remove "Could not process
rule:" which I think it is implicit in the error.
# nft rule x y ip saddr @y
Error: No such file or directory; did you mean table ‘x’ in family inet?
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 12 Nov 2018 14:02:52 +0000 (15:02 +0100)]
nft.8: Clarify 'index' option of add rule command
Documentation for add rule command might trick readers into believing
the optional 'index' argument does not need to be that of an existing
rule. This false assumption is fueled by the fact that iptables allows
to insert with last rule number + 1 to actually append to a chain.
Change the relevant sentence to clarify that.
While being at it, drop the deprecated 'position' option from
documentation - since this will likely go away at some point, don't
encourage users to use it although they should notice that they
shoudn't.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 31 Oct 2018 22:16:56 +0000 (23:16 +0100)]
doc: Fix for make distcheck
When building from a separate build directory, a2x did not find the
source file nft.txt. Using '$<' instead fixes this.
Fixes: 3bacae9e4a1e3 ("doc: Review man page building in Makefile.am") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 31 Oct 2018 12:53:16 +0000 (13:53 +0100)]
py: Adjust Nftables class to output flags changes
Introduce setter/getter methods for each introduced output flag. Ignore
NFT_CTX_OUTPUT_NUMERIC_ALL for now since it's main purpose is for
internal use.
Adjust the script in tests/py accordingly: Due to the good defaults,
only numeric proto output has to be selected - this is not a must, but
allows for the test cases to remain unchanged.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
json: fix json_events_cb() declaration when libjansson is not present
When nftables is configured without libjansson support, the following
compilation error is shown:
monitor.c: In function ‘netlink_echo_callback’:
monitor.c:910:10: error: too many arguments to function ‘json_events_cb’
return json_events_cb(nlh, &echo_monh);
^~~~~~~~~~~~~~
This patch makes a declaration of the json_events_cb() function
consistent.
Fixes: bb32d8db9a12 ("JSON: Add support for echo option") Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
expression: always print range expression numerically
Otherwise we end up displaying things that we cannot parse as input.
Moreover, in a range, it's relevant to the user the values that are
enclosed in the range, so let's print this numerically.
Fixes: baa4e0e3fa5f ("src: add NFT_CTX_OUTPUT_NUMERIC_PROTO") Reported-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
