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>
Needed by followup patch. EXPR_SET_REF handling is bonkers, it
"works" when using { key : value } because ->key and ->left are aliased
in struct expr to the same location.
evaluate: propagate binop_transfer() adjustment to set key size
The right shift transfer may be result in adjusting the set key size,
eg. ip6 dscp results in fetching 6 bits that are splitted between two
bytes, hence the set element ends up being 16 bytes long.
Reported-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
when explicitly filtering icmp-in-ipv6 and icmp6-in-ip don't remove the
required l3 protocol dependency, else "nft list ruleset" can't be read
via nft -f anymore.
Florian Westphal [Tue, 27 Mar 2018 07:29:54 +0000 (09:29 +0200)]
src: avoid errouneous assert with map+concat
Phil reported following assert:
add rule ip6 f o mark set ip6 saddr . ip6 daddr . tcp dport \
map { dead::beef . f00::. 22 : 1 }
nft: netlink_linearize.c:655: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.
This happens because "mark set" will allocate one register (the dreg),
but netlink_gen_concat_expr will populate a lot more register space if
the concat expression strings a lot of expressions together.
As the assert is useful pseudo-reserve the register space as per
concat->len and undo after generating the expressions.
Reported-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Duncan Roe [Tue, 27 Mar 2018 04:17:01 +0000 (15:17 +1100)]
doc: nft.8 more spelling fixes
I ran the following command:
ispell -p ./ispell_nft -H nft.xml
to create the local dictionary ispell_nft.
ispell_nft contains almost every special word in nft.xml.
The idea is that anyone can run ispell the same way and only have to accept:
- alpha strings in hexadecimal numbers
- "FIXME" : that has to be fixed eventually
- "differv" : I don't know what that is or whether it's correct
You need to use the English (i.e. American) dictionary, and you want the screen
to be about 100 chars wide (at least).
The patch enforces consistent capitalisation of words, e.g. IPv4 is always that
way but ipv4_addr stays as before. The existing dictionary suggested capital
Ethernet so that is in there too.
Current libnftables API should be stable enough to release it into the
public, and after 4aba100e593f ("rule: reset cache iff there is an
existing cache") we have a simple way to batch commands through this
API.
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:07 +0000 (18:02 +0100)]
tests/shell: Allow to specify multiple testcases
Extend run-tests.sh a bit so that all remaining arguments after option
parsing are treated as filenames to test and complain if one doesn't
seem like such. This allows for doing stuff like:
| ./run-tests.sh testcases/include/000*
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:06 +0000 (18:02 +0100)]
tests/shell: Fix sporadic fail of include/0007glob_double_0
Since ruleset listing shows tables sorted by handle (which in turn
depends on table creation ordering), using random filenames here
guarantees to make the test fail randomly.
Since the include files reside in a temporary directory anyway, there is
no need to randomize their names so simplify the whole test a bit.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:05 +0000 (18:02 +0100)]
flowtable: Make parsing a little more robust
It was surprisingly easy to crash nft with invalid syntax in 'add
flowtable' command. Catch at least three possible ways (illustrated in
provided test case) by making evaluation phase survive so that bison
gets a chance to complain.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:04 +0000 (18:02 +0100)]
tests/shell: Fix flowtable test cases
The major problem here was that existence of network interfaces 'eth0'
and 'wlan0' was assumed. Overcome this by just using 'lo' instead, which
exists even in newly created netns by default.
Another minor issue was false naming of 0004delete_after_add0 - the
expected return code is supposed to be separated by '_' from the
remaining filename.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:03 +0000 (18:02 +0100)]
tests/shell: Fix dump of chains/0016delete_handle_0
The purpose of this test is to delete some chains by their handle and
that is supposed to succeed. So the respective dump should not contain
them anymore.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Mon, 19 Mar 2018 17:02:02 +0000 (18:02 +0100)]
Support 'nft -f -' to read from stdin
In libnftables, detect if given filename is '-' and treat it as the
common way of requesting to read from stdin, then open /dev/stdin
instead. (Calling 'nft -f /dev/stdin' worked before as well, but this
makes it official.)
With this in place and bash's support for here strings, review all tests
in tests/shell for needless use of temp files. Note that two categories
of test cases were intentionally left unchanged:
- Tests creating potentially large rulesets to avoid running into shell
parameter length limits.
- Tests for 'include' directive for obvious reasons.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Phil Sutter [Sat, 17 Mar 2018 09:39:27 +0000 (10:39 +0100)]
Combine redir and masq statements into nat
All these statements are very similar, handling them with the same code
is obvious. The only thing required here is a custom extension of enum
nft_nat_types which is used in nat_stmt to distinguish between snat and
dnat already. Though since enum nft_nat_types is part of kernel uAPI,
create a local extended version containing the additional fields.
Note that nat statement printing got a bit more complicated to get the
number of spaces right for every possible combination of attributes.
Note also that there wasn't a case for STMT_MASQ in
rule_parse_postprocess(), which seems like a bug. Since STMT_MASQ became
just a variant of STMT_NAT, postprocessing will take place for it now
anyway.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Unlike plain "ip dscp { 4, 63 }", we don't have a relational operation in
case of vmap. Binop fixups need to be done when evaluating map statements.
This patch is incomplete. 'ip dscp' works, but this won't:
nft add rule --debug=netlink ip6 test-ip6 input ip6 dscp vmap { 0x04 : accept, 0x3f : continue }
The generated expressions look sane, however there is disagreement on
the sets key size vs. the sizes of the individual elements in the set.
This is because ip6 dscp spans a byte boundary.
Key set size is still set to one byte (dscp type is 6bits).
However, binop expansion requirements result in 2 byte loads, i.e.
set members will be 2 bytes in size as well.
This can hopefully get addressed in an incremental patch.
Florian Westphal [Thu, 11 Jan 2018 15:30:22 +0000 (16:30 +0100)]
evaluate: handle binop adjustment recursively
currently this is fine, but a followup commit will add
EXPR_SET_ELEM handling.
And unlike RANGE we cannot assume the key is a value.
Therefore make binop_can_transfer and binop_transfer_one handle
right hand recursively if needed. For RANGE, call it again with
from/to.
For future SET_ELEM, we can then just call the function recursively
again with right->key as new RHS.
Florian Westphal [Thu, 11 Jan 2018 15:30:20 +0000 (16:30 +0100)]
src: segtree: use value expression length
In case of EXPR_MAPPING, expr->len is 0, we need to use
the length of the key instead.
Without this we can get assertion failure later on:
nft: netlink_delinearize.c:1484: binop_adjust_one: Assertion `value->len >= binop->right->len' failed.