Máté Eckl [Thu, 17 May 2018 07:36:59 +0000 (09:36 +0200)]
tests: py: print_msg refactor
The errstr attribute was hard-coded to "ERROR:"
errstr has been moved in the parameter list. As print_msg is only
used from the other print_* this is not an issue, and as there is a
print_error function, I don't think that strerr should default to
"ERROR:".
Also this kind of messages now get written to stderr. This can be
beneficial if someone wants to redirect output to a file.
Máté Eckl [Thu, 17 May 2018 07:36:58 +0000 (09:36 +0200)]
tests: py: updated test file structure descripion in README
The order of the table and chain definitions have changed in test files.
Now the name of the chain has to be specified in the definition of the
table, so their order is reverted.
Phil Sutter [Tue, 15 May 2018 09:37:56 +0000 (11:37 +0200)]
libnftables: Move scanner object into struct nft_ctx
The initial approach of keeping as much of lex/yacc-specific data
local to the relevant parsing routines was flawed in that input
descriptors which parsed commands' location information points at were
freed after parsing (in scanner_destroy()) although they were required
later for error reporting in case a command was rejected by the kernel.
To overcome this, keep the scanner pointer in struct nft_ctx so that it
can be kept in place until kernel communication has finished.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 11 May 2018 10:33:46 +0000 (12:33 +0200)]
evaluate: Return ENOENT if rule index is too large
Since EINVAL usually indicates errors from kernel, avoid using it here
too. Instead return ENOENT to indicate there's no entry to append or
prepend the rule to.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 8 May 2018 11:08:45 +0000 (13:08 +0200)]
tests/py: Support testing JSON input and output as well
This extends nft-test.py by optional JSON testing capabilities,
activated via '-j'/'--enable-json' parameter).
JSON testing happens for all rules which are supposed to work: After a
rule has been added and the existing tests (payload, ruleset listing
output) have been performed, basically the same test is done again using
a recorded JSON equivalent and (if necessary) a recorded listing output.
The code tries to ease new test case creation overhead by
auto-generating JSON equivalent input via listing the (non-JSON) rule in
JSON format. Also, differing netlink debug and listing output are stored
in *.got files to assist in analyzing/fixing failing test cases.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 8 May 2018 11:08:43 +0000 (13:08 +0200)]
tests/py: Don't read expected payload for each table
When testing rule adding to different table families, expected payload
was read for each tested family again. Instead, read it just once and
just try to read a family-specific payload for each tested family.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 8 May 2018 11:08:38 +0000 (13:08 +0200)]
libnftables: Implement JSON parser
If JSON output setting is active in current context, try parsing any
input as JSON. If the initial loading of the buffer or filename by
libjansson fails, fall back to regular syntax parser.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 8 May 2018 11:08:37 +0000 (13:08 +0200)]
libnftables: Implement JSON output support
Although technically there already is support for JSON output via 'nft
export json' command, it is hardly useable since it exports all the gory
details of nftables VM. Also, libnftables has no control over what is
exported since the content comes directly from libnftnl.
Instead, implement JSON format support for regular 'nft list' commands.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 8 May 2018 11:08:34 +0000 (13:08 +0200)]
libnftables: Make some arrays globally accessible
This removes static flag and adds declarations in headers for the
following arrays:
* ct_templates from src/ct.c
* mark_tbl from src/datatype.c
* meta_templates and devgroup_tbl from src/meta.c
* table_flags_name from src/rule.c
* set_stmt_op_names from src/statement.c
* tcpopthdr_protocols from src/tcpopt.c
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Combine it with 0002insertposition_0 due to the many similarities,
extend it to test 'handle' and 'index' parameters as well and rename the
testcase accordingly.
Also add a new 0002addinsertlocation_1 which tests that wrong argument
to all of the location parameters fails.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 9 May 2018 14:03:42 +0000 (16:03 +0200)]
Support 'add/insert rule index <IDX>'
Allow to specify an absolute rule position in add/insert commands like
with iptables. The translation to rule handle takes place in userspace,
so no kernel support for this is needed. Possible undesired effects are
pointed out in man page to make users aware that this way of specifying
a rule location might not be ideal.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 9 May 2018 14:03:40 +0000 (16:03 +0200)]
Deprecate add/insert rule 'position' argument
Instead, use 'handle' keyword for the same effect since that is more
consistent with respect to replace/delete commands. The old keyword is
still supported for backwards compatibility and also listed in man page
along with a hint that it shouldn't be used anymore.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 5 May 2018 12:44:53 +0000 (14:44 +0200)]
scanner: Support rfc4291 IPv4-compatible addresses
These are defined in section 2.5.5.1. Although it is stated that they
are deprecated and new implementations are not required to support them,
they occur in ruleset output if an address in the form '::feed:babe' was
given in input. In order to support reinsertion of that rule, we have to
support those deprecated addresses as well.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 5 May 2018 12:44:26 +0000 (14:44 +0200)]
proto: Fix wrong token in proto_icmp6
'token' value of ICMP6HDR_MTU field must be 'mtu', not 'packet-too-big'.
This went unnoticed because rule delinearization for icmp/icmpv6 payload
expressions is problematic anyway in that different fields point to the
same offset and therefore are indistinguishable. In this case, an
expression like e.g. 'icmpv6 mtu 1500' will be printed later as 'icmpv6
parameter-problem 1500'.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Instruct Make to actually install the header to the system, otherwise
users won't see the header in their system after running 'make install'.
Also, export main libnftables header with a proper name, since we have another
private header called 'nftables.h' (i.e, let's be concrete with the naming).
cache_update() needs to accept the full debug mask instead of a boolean of
NFT_DEBUG_NETLINK, because called functions may wish to check other bits
(NFT_DEBUG_MNL in particular).
Phil Sutter [Tue, 24 Apr 2018 09:44:19 +0000 (11:44 +0200)]
rule: Free flowtable in handle_free()
Fixes: db0697ce7f602 ("src: support for flowtable listing") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
src: simplify netlink_get_setelems() and rename it to netlink_list_setelems()
This is called from cache population path, remove netlink_io_error()
call since this is not needed. Rename it for consistency with similar
netlink_list_*() NLM_F_DUMP functions. Get rid of location parameter.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This error path is never entered since mnl_nft_*_batch_{add,del,replace}
calls never fail, and if they ever do fail it will be because we are
hitting OOM, in such case we can display a more generic non-netlink
error.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Legacy tool name is 'brctl' and so the 'br' prefix is already known. If
we use ibrname and obrname it looks consistent with iifname and oifname.
So let's this instead of ibridgename and obridgename since Florian likes
this too.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
expression: fix constant expression allocation on big endian with partial bytes
Commit 5259feeb7cda ("expression: fix constant expression allocation on
big endian") improved constant handling on big endian, but didn't handle
the case of partial bytes correctly.
Currently, constant_data_ptr(val, 6) points to the item after val,
instead of the last byte of val.
Thanks to Stefano for providing the correct expression.
Fixes: 5259feeb7cda ("expression: fix constant expression allocation on big endian") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: reset eval context when evaluating set definitions
David reported nft chokes on this:
nft -f /tmp/A
/tmp/A:9:22-45: Error: datatype mismatch, expected concatenation of (IPv4 address, internet network service, IPv4 address), expression has type concatenation of (IPv4 address, internet network service)
cat /tmp/A
flush ruleset;
table ip filter {
set setA {
type ipv4_addr . inet_service . ipv4_addr
flags timeout
}
set setB {
type ipv4_addr . inet_service
flags timeout
}
}
Problem is we leak set definition details of setA to setB via eval
context, so reset this.
Also add test case for this.
Reported-by: David Fabian <david.fabian@bosson.cz> Signed-off-by: Florian Westphal <fw@strlen.de>
For bridge, iifname is the port name, whereas 'ibrport' is the
logical name of the bridge ("br0") the port ("iifname") is enslaved to.
So, 'ibrport' is a misnomer.
libnftl calls these 'bri_iifname' and 'bri_oifname', which is good
but using 'briiifname' in nft is rather ugly, so use 'ibridgename'
and 'obridgename' instead.
Old names are still recognized, listing shows the new names.
Phil Sutter [Fri, 13 Apr 2018 14:52:34 +0000 (16:52 +0200)]
libnftables: Keep cmds list outside of parser_state
Parser basically turns input into a list of commands and error messages.
Having the commands list being part of struct parser_state does not make
sense from this point of view, also it will have to go away with
upcoming JSON support anyway.
While being at it, change nft_netlink() to take just the list of
commands instead of the whole parser state as parameter, also take care
of command freeing in nft_run_cmd_from_* functions (where the list
resides as auto-variable) instead of from inside nft_run().
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 13 Apr 2018 14:52:32 +0000 (16:52 +0200)]
Review .gitignore files contents
* Move entries belonging to src/ into src/.gitignore.
* Drop lines for files inside build-aux/ since that is ignored already.
* No need to ignore src/Makefile.in, the entry 'Makefile.in' catches
that already.
* Remove entry for '.*.d', (recent?) autotools doesn't create any
matching files.
* Drop entries for non-existent parser.c and parser.h files.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 13 Apr 2018 14:52:29 +0000 (16:52 +0200)]
segtree: Fix for last elem at interval end
Unclosed interval check at end of interval_map_decompose() missed to
check whether interval start is the last possible element in given set
before creating a range expression. This led to the last element
incorrectly printed as range from itself to itself. Fix this by
comparing the upper boundary against the lower one.
In order to keep indenting level low, invert the entry check and jump to
the end if it matches.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 13 Apr 2018 14:52:28 +0000 (16:52 +0200)]
Review raw payload allocation points
In parser_bison.y, call payload_init_raw() instead of assigning all
fields manually. Also drop manual initialization of flags field: it is
not touched in allocation path, so no need for that.
In stmt_evaluate_payload(), setting dtype field is redundant since
payload_init_raw() does that already.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 13 Apr 2018 14:52:27 +0000 (16:52 +0200)]
segtree: Fix memory leaks
This fixes memory leaks in three places:
* set_overlap():
The allocated intervals have to be freed again before returning to
caller. While being at it, reduce indenting level in said function to
stay below 80 columns boundary.
* range_is_prefix():
* interval_map_decompose():
GMP documentation suggests to call mpz_clear() for all mpz_t type
variables once they are not used anymore to free the space they occupy.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Fri, 13 Apr 2018 14:52:24 +0000 (16:52 +0200)]
erec: Review erec_print()
A new requirement to erec for the upcoming JSON support is printing
records with file input descriptors without open stream. The approach is
to treat 'name' field as file name, open it, extract the offending line
and close it again.
Further changes to libnftables input parsing routines though have shown
that the whole concept of file pointer reuse in erec is tedious and not
worth keeping:
* Closed files are to be supported as well, so there needs to be
fallback code for opening the file anyway.
* When input descriptor is duplicated from parser state into an error
record, the file pointer is copied as well. Therefore care has to be
taken to not free the parser state before any error records have been
printed. This is the only point where old and duplicated input
descriptors are connected.
Therefore drop struct input_descriptor's 'fp' field and just always open
the file by name. This way also the old stream offset doesn't have to be
restored after reading.
While being at it, this patch fixes two other (potential) problems:
* If the offending line from input contains tabs, add them at the right
position in the marker buffer as well to avoid misalignment.
* The input file may not be seekable (/dev/stdin for instance), so skip
printing of offending line and markers if it couldn't be read
properly.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Wed, 11 Apr 2018 08:21:35 +0000 (10:21 +0200)]
cli: Drop String termination workaround
This spot was missed by commit 2b3f18e0cf7a7 ("libnftables: Fix for
input without trailing newline") - since line termination is now added
in nft_run_cmd_from_buffer(), cli is relieved from doing so.
Fixes: 2b3f18e0cf7a7 ("libnftables: Fix for input without trailing newline") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 10 Apr 2018 17:00:25 +0000 (19:00 +0200)]
tests/py: Review print statements in nft-test.py
Make use of format strings as they are easier to read than manual string
concatenation.
Also use class Table's __str__ method instead of printing the 'name'
attribute. This changes the output in that table names are prepended by
their family, but the extra information may come in handy when analyzing
issues.
Since class Chain's __str__ method returns just the 'name' attribute
content, it may be used as synonym.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 10 Apr 2018 17:00:24 +0000 (19:00 +0200)]
tests/py: Use libnftables instead of calling nft binary
This adds a simple nftables Python class in py/nftables.py which gives
access to libnftables API via ctypes module.
nft-test.py is extended to make use of the above class instead of
calling nft binary. Since command line formatting had to be touched
anyway, this patch also streamlines things a bit by introducing
__str__ methods to classes Table and Chain and making extensive use of
format strings instead of onerously adding all string parts together.
Since the called commands don't see a shell anymore, all shell meta
character escaping done in testcases is removed.
The visible effects of this change are:
* Four new warnings in ip/flowtable.t due to changing objref IDs (will
be addressed later in a patch to libnftnl).
* Reported command line in warning and error messages changed slightly
for obvious reasons.
* Reduction of a full test run's runtime by a factor of four. Status
diff after running with 'time':
Phil Sutter [Tue, 10 Apr 2018 17:00:23 +0000 (19:00 +0200)]
libnftables: Simplify cookie integration
This increases the size of struct output_ctx quite a bit, but allows to
simplify internal functions dealing with the cookies mainly because
output_fp becomes accessible from struct cookie.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 10 Apr 2018 17:00:22 +0000 (19:00 +0200)]
libnftables: Support buffering output and error
When integrating libnftables into Python code using ctypes module,
having to use a FILE pointer for output becomes a show-stopper.
Therefore make Python hackers' lives (a little) less painful by
providing convenience functions to setup buffering output and error
streams using fopencookie() and retrieving the buffers.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Tue, 10 Apr 2018 17:00:20 +0000 (19:00 +0200)]
libnftables: Fix for input without trailing newline
Input parser implementation requires a newline at end of input,
otherwise the last pattern may not be recognized correctly.
If input comes from a file, the culprit was YY_INPUT macro not expecting
the last line not ending with a newline, so the last word wasn't
accepted. This is easily fixed by checking for feof(yyin) in there. A
simple test case for that is:
| echo -en "table ip t {\nchain c {\n}\n}" >/tmp/foo
| nft -f /tmp/foo
Input from a string buffer is a bit more tricky: The culprit here is
that detection of classid pattern is done by checking the character
following it which makes it impossible to sit right at end of input and
I haven't found an alternative to that. After dropping the manual
newline appending when combining argv into a single buffer in main(),
a rule like this won't be recognized anymore:
| nft add rule ip t c meta priority feed:babe
Since a direct call to run_cmd_from_buffer() via libnftables bypasses
the sanitizing done in main() entirely, it has to happen in libnftables
instead which means creating a newline-terminated duplicate of the input
buffer.
Note that main() created a buffer one byte longer than needed since it
accounts for whitespace at end of each argv but doesn't add it to the
buffer for the last one, so buffer length is reduced by two bytes
instead of just one although only one less character is printed into it.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>