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>
src: get rid of nft_ctx_output_{get,set}_numeric()
This patch adds NFT_CTX_OUTPUT_NUMERIC_SYMBOL, which replaces the last
client of the numeric level approach.
This patch updates `-n' option semantics to display all output
numerically.
Note that monitor code was still using the -n option to skip printing
the process name, this patch updates that path too to print it
inconditionally to simplify things.
Given the numeric levels have no more clients after this patch, remove
that code.
Update several tests/shell not to use -nn.
This patch adds NFT_CTX_OUTPUT_NUMERIC_ALL which enables all flags to
provide a fully numerical output.
Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This is a partial revert of b0f6a45b25dd1 ("src: add --literal option")
which was added during the development cycle before 0.9.1 is released.
After looking at patch: https://patchwork.ozlabs.org/patch/969864/ that
allows to print priority, uid, gid and protocols as numerics, I decided
to revisit this to provide individual options to turn on literal
printing.
What I'm proposing is to provide a good default for everyone, and
provide options to turn on literal/numeric printing.
This patch adds nft_ctx_output_{set,get}_flags() and define two flags to
enable reverse DNS lookups and to print ports as service names.
This patch introduces -S/--services, to print service names as per
/etc/services.
Acked-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 26 Oct 2018 13:01:38 +0000 (15:01 +0200)]
JSON: Add support for echo option
The basic principle is to not return a JSON object freshly created from
netlink responses, but just update the existing user-provided one to
make sure callers get back exactly what they expect.
To achieve that, keep the parsed JSON object around in a global variable
('cur_root') and provide a custom callback to insert handles into it
from received netlink messages. The tricky bit here is updating rules
since unique identification is problematic. Therefore drop possibly
present handles from input and later assume updates are received in
order so the first rule not having a handle set is the right one.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 26 Oct 2018 09:42:05 +0000 (11:42 +0200)]
tests/shell: Add testcase for cache update problems
The first test in there shows how the current cache update strategy
causes trouble. The second test shows that proposed "locking" of cache
when local entries are added is flawed, too.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 24 Oct 2018 19:14:37 +0000 (21:14 +0200)]
json: Work around segfault when encountering xt stmt
When trying to convert an xt stmt into JSON, print() callback was
called. Though the code in src/xt.c does not respect output_fp,
therefore buffer wasn't filled as expected making libjansson to puke:
| # nft -j list ruleset
| warning: stmt ops xt have no json callback
| nft: json.c:169: stmt_print_json: Assertion `__out' failed.
| Aborted (core dumped)
Avoid this by detecting xt stmt ops and returning a stub.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 24 Oct 2018 10:35:03 +0000 (12:35 +0200)]
json: Fix osf ttl support
Having to use numerical values for ttl property in JSON is not
practical as these values are arbitrary and meaningful only in
netfilter. Instead align JSON output/input with standard API, accepting
names for TTL matching strategy.
Also add missing documentation in libnftables-json man page and fix JSON
equivalent in tests/py.
Fixes: 03eafe098d5ee ("osf: add ttl option support") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 22 Oct 2018 13:45:09 +0000 (15:45 +0200)]
tests: shell: Extend get element test
Despite the recent fixes, the test still fails. While trying to address
the remaining issues, I found more potentially problematic inputs so
extend the test by those.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 12 Oct 2018 15:23:24 +0000 (17:23 +0200)]
parser_json: Fix for ineffective family value checks
Since handle->family is unsigned, checking for value < 0 never yields
true. Overcome this by changing parse_family() to return an error code
and write the parsed family value into a pointer passed as parameter.
The above change required a bit more cleanup to avoid passing pointers
to signed variables to the function. Also leverage json_parse_family() a
bit more to reduce code side.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 12 Oct 2018 10:54:09 +0000 (12:54 +0200)]
Fix memleak in netlink_parse_fwd() error path
Make sure allocated 'stmt' is freed before returning to caller.
Fixes: 30d45266bf38b ("expr: extend fwd statement to support address and family") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 11 Oct 2018 15:48:59 +0000 (17:48 +0200)]
monitor: Fix printing of ct objects
Monitor output is supposed to be single lined without tabs, but ct
object were printed with newlines and tabs hard-coded. Fixing this
wasn't too hard given that there is 'stmt_separator' to also include
semi-colons where required if newline was removed.
A more obvious mistake was position of object type in monitor output:
Like with other object types, it has to occur between command and table
spec. As a positive side-effect, this aligns ct objects better with
others (see obj_type_name_array for instance).
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Thu, 11 Oct 2018 15:48:54 +0000 (17:48 +0200)]
tests/py: Add missing JSON bits for inet/meta.t
Those were forgotten when renaming meta secpath to meta ipsec.
Fixes: 8f55ed41d0070 ("src: rename meta secpath to meta ipsec") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The netlink layer sits in between the mnl and the rule layers, remove
it. We can remove alloc_nftnl_chain() and consolidate infrastructure in
the src/mnl.c file.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>