summaryrefslogtreecommitdiffstats
path: root/iptables
Commit message (Collapse)AuthorAgeFilesLines
* nft: cache: Fix iptables-save segfault under stressHEADmasterPhil Sutter2020-03-161-14/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If kernel ruleset is constantly changing, code called by nft_is_table_compatible() may crash: For each item in table's chain list, nft_is_chain_compatible() is called. This in turn calls nft_build_cache() to fetch chain's rules. Though if kernel genid has changed meanwhile, cache is flushed and rebuilt from scratch, thereby freeing table's chain list - the foreach loop in nft_is_table_compatible() then operates on freed memory. A simple reproducer (may need a few calls): | RULESET='*filter | :INPUT ACCEPT [10517:1483527] | :FORWARD ACCEPT [0:0] | :OUTPUT ACCEPT [1714:105671] | COMMIT | ' | | for ((i = 0; i < 100; i++)); do | iptables-nft-restore <<< "$RULESET" & | done & | iptables-nft-save To fix the problem, basically revert commit ab1cd3b510fa5 ("nft: ensure cache consistency") so that __nft_build_cache() no longer flushes the cache. Instead just record kernel's genid when fetching for the first time. If kernel rule set changes until the changes are committed, the commit simply fails and local cache is being rebuilt. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: cache: Fix for unused variable warningsPhil Sutter2020-03-161-3/+1
| | | | | | | Loop index variable was left in place after removing the loops. Fixes: 39ec645093baa ("nft: cache: Simplify chain list allocation") Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: cache: Review flush_cache()Phil Sutter2020-03-061-9/+11
| | | | | | | | | While fixing for iptables-nft-restore under stress, I managed to hit NULL-pointer deref in flush_cache(). Given that nftnl_*_list_free() functions are not NULL-pointer tolerant, better make sure such are not passed by accident. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: cache: Simplify chain list allocationPhil Sutter2020-03-061-27/+19
| | | | | | | | | | Allocate chain lists right after fetching table cache, regardless of whether partial cache is fetched or not. Chain list pointers reside in struct nft_cache's table array and hence are present irrespective of actual tables in kernel. Given the small number of tables, there wasn't much overhead avoided by the conditional in fetch_chain_cache(). Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: cache: Make nft_rebuild_cache() respect fake cachePhil Sutter2020-03-062-4/+10
| | | | | | | | | | | | If transaction needed a refresh in nft_action(), restore with flush would fetch a full cache instead of merely refreshing table list contained in "fake" cache. To fix this, nft_rebuild_cache() must distinguish between fake cache and full rule cache. Therefore introduce NFT_CL_FAKE to be distinguished from NFT_CL_RULES. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: cache: Fix nft_release_cache() under stressPhil Sutter2020-03-061-2/+8
| | | | | | | | | | | | | | | | | iptables-nft-restore calls nft_action(h, NFT_COMPAT_COMMIT) for each COMMIT line in input. When restoring a dump containing multiple large tables, chances are nft_rebuild_cache() has to run multiple times. If the above happens, consecutive table contents are added to __cache[1] which nft_rebuild_cache() then frees, so next commit attempt accesses invalid memory. Fix this by making nft_release_cache() (called after each successful commit) return things into pre-rebuild state again, but keeping the fresh cache copy. Fixes: f6ad231d698c7 ("nft: keep original cache in case of ERESTART") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables: Review nft_init()Phil Sutter2020-02-2410-42/+24
| | | | | | | | | | | | | | | | | Move common code into nft_init(), such as: * initial zeroing nft_handle fields * family ops lookup and assignment to 'ops' field * setting of 'family' field This requires minor adjustments in xtables_restore_main() so extra field initialization doesn't happen before nft_init() call. As a side-effect, this fixes segfaulting xtables-monitor binary when printing rules for trace event as in that code-path 'ops' field wasn't initialized. Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables: Drop -4 and -6 support from xtables-{save,restore}Phil Sutter2020-02-242-18/+2
| | | | | | Legacy tools don't support those options, either. Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables: Align effect of -4/-6 options with legacyPhil Sutter2020-02-242-13/+96
| | | | | | | | | | Legacy iptables doesn't accept -4 or -6 if they don't match the symlink's native family. The only exception to that is iptables-restore which simply ignores the lines introduced by non-matching options, which is useful to create combined dump files for feeding into both iptables-restore and ip6tables-restore. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: Drop pointless assignmentPhil Sutter2020-02-181-1/+0
| | | | | | | | No need to set 'i' to zero here, it is not used before the next assignment. Fixes: 77e6a93d5c9dc ("xtables: add and set "implict" flag on transaction objects") Signed-off-by: Phil Sutter <phil@nwl.cc>
* ebtables: among: Support mixed MAC and MAC/IP entriesPhil Sutter2020-02-182-5/+19
| | | | | | | | | | | | | | Powered by Stefano's support for concatenated ranges, a full among match replacement can be implemented. The trick is to add MAC-only elements as a concatenation of MAC and zero-length prefix, i.e. a range from 0.0.0.0 till 255.255.255.255. Although not quite needed, detection of pure MAC-only matches is left in place. For those, no implicit 'meta protocol' match is added (which is required otherwise at least to keep nft output correct) and no concat type is used for the set. Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables-translate: Fix for iface++Phil Sutter2020-02-141-3/+3
| | | | | | | | | In legacy iptables, only the last plus sign remains special, any previous ones are taken literally. Therefore xtables-translate must not replace all of them with asterisk but just the last one. Fixes: e179e87a1179e ("xtables-translate: Fix for interface name corner-cases") Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Fix skip checks with --host modePhil Sutter2020-02-1413-84/+88
| | | | | | | | | | When testing host binaries, XT_MULTI variable contains just the program name without path component which most skip checks didn't expect. Fix them, and while being at it also reduce indenting level in two scripts by moving the skip check up front with an early exit call. Fixes: 416898e335322 ("tests/shell: Support testing host binaries") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables-restore: fix for --noflush and empty linesPhil Sutter2020-02-122-3/+21
| | | | | | | | | | | | | | | | | | | | | | Lookahead buffer used for cache requirements estimate in restore --noflush separates individual lines with nul-chars. Two consecutive nul-chars are interpreted as end of buffer and remaining buffer content is skipped. Sadly, reading an empty line (i.e., one containing a newline character only) caused double nul-chars to appear in buffer as well, leading to premature stop when reading cached lines from buffer. To fix that, make use of xtables_restore_parse_line() skipping empty lines without calling strtok() and just leave the newline character in place. A more intuitive approach, namely skipping empty lines while buffering, is deliberately not chosen as that would cause wrong values in 'line' variable. Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1400 Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
* xtables-translate: Fix for interface name corner-casesPhil Sutter2020-02-111-5/+28
| | | | | | | | | | | | | | | | | | There are two special situations xlate_ifname() didn't cover for: * Interface name containing '*': This went unchanged, creating a command nft wouldn't accept. Instead translate into '\*' which doesn't change semantics. * Interface name being '+': Can't translate into nft wildcard character as nft doesn't accept asterisk-only interface names. Instead decide what to do based on 'invert' value: Skip match creation if false, match against an invalid interface name if true. Also add a test to make sure future changes to this behaviour are noticed. Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables-translate: Guard strcpy() call in xlate_ifname()Phil Sutter2019-12-061-3/+2
| | | | | | | | The function potentially fed overlong strings to strcpy(). Given that everything needed to avoid this is there, reorder code a bit to prevent those inputs, too. Fixes: 0ddd663e9c167 ("iptables-translate: add in/out ifname wildcard match translation to nft")
* xtables-restore: Avoid access of uninitialized dataPhil Sutter2019-12-061-1/+1
| | | | | | | When flushing, 'buffer' is not written to prior to checking its first byte's value. Therefore it needs to be initialized upon declaration. Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation")
* iptables-apply: script and manpage updategw.2010@tnode.com2019-12-052-106/+242
| | | | | | | | | | | | | | | | | | | This is GW's update to iptables-apply. It does a code cleanup and adds two options: one runs a command and the other writes the sucessful rules file. I modified the script to use mktemp instead of tempfile. I also fixed a couple of hyphens in the man page addition. Arturo says: I'm not a strong supporter of this script, but there are many users of it, so better do things right and add this patch that should produce no harm anyway. This patch is forwarded from the iptables Debian package, where it has been around for many years now. Signed-off-by: GW <gw.2010@tnode.com> Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* iptables: mention iptables-apply(8) in manpagesLaurence J. Lane2019-12-052-2/+2
| | | | | | | | | | | | Add iptables-apply(8) to the SEE ALSO section of *-save(8) and *-restore(8). Arturo says: This patch is forwarded from the iptables Debian package, where it has been around for many years now. Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* iptables: cleanup "allows to" usageLaurence J. Lane2019-12-051-2/+2
| | | | | | | | | | | | Gramatical cleanup. Arturo says: This patch is forwarded from the iptables Debian package, where it has been around for many years now. Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* iptables: install iptables-apply script and manpageLaurence J. Lane2019-12-052-1/+7
| | | | | | | | | | | | | | | We have the iptables-apply script in the tree (and in the release tarball), but is not being installed anywhere. Same for the manpage. Arturo says: I'm not a strong supporter of this script, but there are many users of it, so better do things right and do a proper installation. This patch is forwarded from the iptables Debian package, where it has been around for many years now. Signed-off-by: Laurence J. Lane <ljlane@debian.org> Signed-off-by: Arturo Borrero Gonzalez <arturo@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Fix parser feed from line bufferPhil Sutter2019-12-042-1/+13
| | | | | | | | | | | | When called with --noflush, xtables-restore would trip over chain lines: Parser uses strtok() to separate chain name, policy and counters which inserts nul-chars into the source string. Therefore strlen() can't be used anymore to find end of line. Fix this by caching line length before calling xtables_restore_parse_line(). Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* Fix DEBUG buildPhil Sutter2019-12-041-1/+1
| | | | | | | | | Fixed commit missed to update this conditional call to nft_rule_print_save(). Fixes: 1e8ef6a584754 ("nft: family_ops: Pass nft_handle to 'rule_to_cs' callback") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: bridge: Rudimental among extension supportPhil Sutter2019-11-255-31/+451
| | | | | | | | | Support among match as far as possible given the limitations of nftables sets, namely limited to homogeneous MAC address only or MAC and IP address only matches. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Support parsing lookup expressionPhil Sutter2019-11-253-1/+13
| | | | | | | | Add required glue code to support family specific lookup expression parsers implemented as family_ops callback. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Embed rule's table name in nft_xt_ctxPhil Sutter2019-11-252-0/+2
| | | | | | | | Down to the point where expression parsing happens, the rule's table is not known anymore but relevant if set lookups are required. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Bore up nft_parse_payload()Phil Sutter2019-11-252-1/+11
| | | | | | | | | Allow for closer inspection by storing payload expression's base and length values. Also facilitate for two consecutive payload expressions as LHS of a (cmp/lookup) statement as used with concatenations. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Support NFT_COMPAT_SET_ADDPhil Sutter2019-11-251-0/+58
| | | | | | | | Implement the required infrastructure to create sets as part of a batch job commit. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Introduce NFT_CL_SETS cache levelPhil Sutter2019-11-253-9/+200
| | | | | | | | | | In order to support anonymous sets, introduce an intermediate cache level between NFT_CL_CHAINS and NFT_CL_RULES. Actually chains are not needed to fetch sets, but given that sets are only needed for rules, put it late to not slow down fetching chains. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Eliminate pointless calls to nft_family_ops_lookup()Phil Sutter2019-11-251-10/+5
| | | | | | | | | If nft_handle is available, use its 'ops' field instead of performing a new lookup. For the same reason, there is no need to pass ops pointer to __nft_print_header(). Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Keep nft_handle pointer in nft_xt_ctxPhil Sutter2019-11-252-27/+15
| | | | | | | | | Instead of carrying the family value, carry the handle (which contains the family value) and relieve expression parsers from having to call nft_family_ops_lookup(). Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: family_ops: Pass nft_handle to 'rule_to_cs' callbackPhil Sutter2019-11-2510-26/+43
| | | | | | | | | | | This is the actual callback used to parse nftables rules. Pass nft_handle to it so it can access the cache (and possible sets therein). Having to pass nft_handle to nft_rule_print_save() allows to simplify it a bit since no family ops lookup has to be done anymore. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: family_ops: Pass nft_handle to 'print_rule' callbackPhil Sutter2019-11-256-18/+20
| | | | | | | | Prepare for 'rule_to_cs' callback to receive nft_handle pointer so it is able to access cache for set lookups. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: family_ops: Pass nft_handle to 'rule_find' callbackPhil Sutter2019-11-255-11/+10
| | | | | | | | | In order to prepare for rules containing set references, nft handle has to be passed to nft_rule_to_iptables_command_state() in order to let it access the set in cache. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: family_ops: Pass nft_handle to 'add' callbackPhil Sutter2019-11-257-12/+14
| | | | | | | | | In order for add_match() to create anonymous sets when converting xtables matches it needs access to nft handle. So pass it along from callers of family ops' add callback. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Fix -Z for rules with NFTA_RULE_COMPATPhil Sutter2019-11-151-0/+39
| | | | | | | | | | | | | | | | | | | | | The special nested attribute NFTA_RULE_COMPAT holds information about any present l4proto match (given via '-p' parameter) in input. The match is contained as meta expression as well, but some xtables extensions explicitly check it's value (see e.g. xt_TPROXY). This nested attribute is input only, the information is lost after parsing (and initialization of compat extensions). So in order to feed a rule back to kernel with zeroed counters, the attribute has to be reconstructed based on the rule's expressions. Other code paths are not affected since rule_to_cs() callback will populate respective fields in struct iptables_command_state and 'add' callback (which is the inverse to rule_to_cs()) calls add_compat() in any case. Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: CMD_ZERO needs a rule cachePhil Sutter2019-11-152-0/+3
| | | | | | | | | | | In order to zero rule counters, they have to be fetched from kernel. Fix this for both standalone calls as well as xtables-restore --noflush. Fixes: b5cb6e631c828 ("nft-cache: Fetch only chains in nft_chain_list_get()") Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation") Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: Add ipt-restore/0007-flush-noflush_0Phil Sutter2019-11-061-0/+42
| | | | | | | | Simple test to make sure iptables-restore does not touch tables it is not supposed to. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Improve performance of --noflush operationPhil Sutter2019-11-061-7/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | The reason for that full cache fetching when called with --noflush even before looking at any input data was that there might be a command requiring a rule cache following some rule add/insert ones which don't. At that point one needs to fetch rules from kernel and try to insert the local ones at the right spot which is non-trivial. At the same time there is a performance-critical use-case for --noflush, namely fast insertion of a bunch of rules in one go, avoiding the process spawn overhead. Optimize for this use-case by preloading input into a 64KB buffer to see if it fits. If so, search for commands requiring a rule cache. If there are none, skip initial full cache fetching. The above algorithm may abort at any point, so actual input parsing must happen in three stages: 1) parse all preloaded lines from 64KB buffer 2) parse any leftover line in line buffer (happens if input exceeds the preload buffer size) 3) parse remaining input from input file pointer Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Allow lines without trailing newline characterPhil Sutter2019-11-062-1/+7
| | | | | | | | | | | | | | | Old code in add_param_to_argv() assumed the input line would always end with a newline character. Without it, the last word of input wasn't recognized. Fix this by adding a final check for param.len (indicating leftover data in buffer). In line parsing code itself, only COMMIT line check required presence of trailing newline. The replaced conditional is not 100% accurate as it allows for characters after newline to be present, but since fgets() is used this shouldn't happen anyway. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Remove some pointless linebreaksPhil Sutter2019-11-061-15/+10
| | | | | | | | Due to reduced indenting level, some linebreaks are no longer needed. OTOH, strings should not be split to aid in grepping for error output. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Introduce line parsing functionPhil Sutter2019-11-061-170/+177
| | | | | | | | | | Move the loop code parsing a distinct line of input into a dedicated function as a preparation for changing input sources. Since loop code either calls continue or exit() directly, there is no need for a return code to indicate failure. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Introduce struct nft_xt_restore_statePhil Sutter2019-11-061-31/+35
| | | | | | | | | | | | This data structure holds parser state information. A follow-up patch will extract line parsing code into a separate function which will need a place to persistently store this info in between calls. While being at it, make 'in_table' variable boolean and drop some extra braces in conditionals checking its value. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Integrate restore callbacks into struct nft_xt_restore_parsePhil Sutter2019-11-063-16/+21
| | | | | | | | There's really no point in passing those as separate parameter. While being at it, make them static const everywhere. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft-arp: Use xtables_print_mac_and_mask()Phil Sutter2019-10-301-27/+4
| | | | | | | | | | This libxtables function does exactly what the local implementation did. The only noteworthy difference is that it assumes MAC/mask lengths, but the local implementation was passed ETH_ALEN in each invocation, so no practical difference. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-arp: Use xtables_parse_interface()Phil Sutter2019-10-301-44/+6
| | | | | | | | | The local implementation differs just slightly but libxtables version seems more correct (no needless memsetting of mask, more relevant illegal character checking) so use that one. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-arp: Drop some unused variablesPhil Sutter2019-10-301-17/+0
| | | | | Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-arp: Integrate OPT_* defines into xshared.hPhil Sutter2019-10-302-29/+21
| | | | | | | | | | | | | | | | | | These defines are internal use only, so their actual value doesn't matter as long as they're unique and inverse_for_options array items match: When negating a given option, the corresponding OPT_* value's bit is used as an index into inverse_for_options to retrieve the corresponding invflag. If zero, either negating or the option itself is not supported. (In practice, a lookup for unsupported option won't happen as those are caught by getopt_long()). Since xtables-arp's OPT_* values change, adjust the local inverse_for_options array accordingly. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* Replace TRUE/FALSE with true/falsePhil Sutter2019-10-304-42/+12
| | | | | | | And drop the conditional defines. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-arp: Drop generic_opt_check()Phil Sutter2019-10-301-66/+0
| | | | | | | | With all fields in commands_v_options[][] being whitespace, the function is effectively a noop. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>