summaryrefslogtreecommitdiffstats
path: root/iptables/tests/shell/testcases/ipt-restore
Commit message (Collapse)AuthorAgeFilesLines
* Revert "xshared: Print protocol numbers if --numeric was given"Phil Sutter2024-02-071-1/+1
| | | | | | | | | | | | | | This reverts commit da8ecc62dd765b15df84c3aa6b83dcb7a81d4ffa. The patch's original intention is not entirely clear anymore. If it was to reduce delays involved by calling getprotobynumber() though, commit b6196c7504d4d ("xshared: Prefer xtables_chain_protos lookup over getprotoent") avoids those if --numeric flag was given already. Also, this numeric protocol output did not cover iptables-save which is a more relevant candidate for such optimizations anyway. Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1729 Signed-off-by: Phil Sutter <phil@nwl.cc>
* *tables-restore: Enforce correct counters syntax if presentPhil Sutter2023-07-281-0/+7
| | | | | | | | | If '--counters' option was not given, restore parsers would ignore anything following the policy word. Make them more strict, rejecting anything in that spot which does not look like counter values even if not restoring counters. Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft-restore: Fix for deletion of new, referenced rulePhil Sutter2023-03-011-0/+16
| | | | | | | | | | | | | | | | | | Combining multiple corner-cases here: * Insert a rule before another new one which is not the first. Triggers NFTNL_RULE_ID assignment of the latter. * Delete the referenced new rule in the same batch again. Causes overwriting of the previously assigned RULE_ID. Consequently, iptables-nft-restore fails during *insert*, because the reference is dangling. Reported-by: Eric Garver <eric@garver.life> Fixes: 760b35b46e4cc ("nft: Fix for add and delete of same rule in single batch") Signed-off-by: Phil Sutter <phil@nwl.cc> Tested-by: Eric Garver <eric@garver.life>
* xshared: Print protocol numbers if --numeric was givenPhil Sutter2022-07-291-1/+1
| | | | | | | | | | This is much trickier than expected: On one hand, proto_to_name() is used to lookup protocol extensions so must resolve despite FMT_NUMERIC being set. On the other, --verbose implies --numeric but changing the output there is probably a bad idea. Luckily the latter situation is identified by FMT_NOTABLE bit. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Fix testcases for changed ip6tables opts outputPhil Sutter2022-07-291-1/+1
| | | | | | | | Adjust captured output, ip6tables prints '--' instead of spaces since the commit in Fixes: tag. Fixes: 6e41c2d8747b2 ("iptables: xshared: Ouptut '--' in the opt field in ipv6's fake mode") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xshared: Implement xtables lock timeout using signalsJethro Beekman2022-02-151-2/+1
| | | | | | | | | | | | | | | | | | | | | | | Previously, if a lock timeout is specified using `-wN `, flock() is called using LOCK_NB in a loop with a sleep. This results in two issues. The first issue is that the process may wait longer than necessary when the lock becomes available. For this the `-W` option was added, but this requires fine-tuning. The second issue is that if lock contention is high, invocations using `-w` (without a timeout) will always win lock acquisition from invocations that use `-w N`. This is because invocations using `-w` are actively waiting on the lock whereas those using `-w N` only check from time to time whether the lock is free, which will never be the case. This patch removes the sleep loop and deprecates the `-W` option (making it non-functional). Instead, flock() is always called in a blocking fashion, but the alarm() function is used with a non-SA_RESTART signal handler to cancel the system call. Signed-off-by: Jethro Beekman <jethro@fortanix.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* iptables-restore: Support for extra debug outputPhil Sutter2022-02-041-3/+6
| | | | | | | | | | | | | | Treat --verbose just like iptables itself, increasing debug level with number of invocations. To propagate the level into do_command() callback, insert virtual '-v' flags into rule lines. The only downside of this is that simple verbose output is changed and now also prints the rules as they are added - which would be useful if the lines contained the chain they apply to. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Drop any dump sorting in placePhil Sutter2020-12-212-3/+3
| | | | | | | With iptables-nft-save output now sorted just like legacy one, no sorting to unify them is needed anymore. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests/shell: Test for fixed extension registrationPhil Sutter2020-12-071-0/+25
| | | | | | | | | | Use strace to look at iptables-restore behaviour with typically problematic input (conntrack revision 0 is no longer supported by current kernels) to make sure the fix in commit a1eaaceb0460b ("libxtables: Simplify pending extension registration") is still effective. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Improve concurrent noflush restore test a bitPhil Sutter2020-10-271-0/+14
| | | | | | | | | | | | The described issue happens only if chain FOO does not exist at program start so flush the ruleset after each iteration to make sure this is the case. Sadly the bug is still not 100% reproducible on my testing VM. While being at it, add a paragraph describing what exact situation the test is trying to provoke. Fixes: dac904bdcd9a1 ("nft: Fix for concurrent noflush restore calls") Signed-off-by: Phil Sutter <phil@nwl.cc>
* nft: Fix for concurrent noflush restore callsPhil Sutter2020-10-131-0/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Transaction refresh was broken with regards to nft_chain_restore(): It created a rule flush batch object only if the chain was found in cache and a chain add object only if the chain was not found. Yet with concurrent ruleset updates, one has to expect both situations: * If a chain vanishes, the rule flush job must be skipped and instead the chain add job become active. * If a chain appears, the chain add job must be skipped and instead rules flushed. Change the code accordingly: Create both batch objects and set their 'skip' field depending on the situation in cache and adjust both in nft_refresh_transaction(). As a side-effect, the implicit rule flush becomes explicit and all handling of implicit batch jobs is dropped along with the related field indicating such. Reuse the 'implicit' parameter of __nft_rule_flush() to control the initial 'skip' field value instead. A subtle caveat is vanishing of existing chains: Creating the chain add job based on the chain in cache causes a netlink message containing that chain's handle which the kernel dislikes. Therefore unset the chain's handle in that case. Fixes: 58d7de0181f61 ("xtables: handle concurrent ruleset modifications") Signed-off-by: Phil Sutter <phil@nwl.cc>
* xtables-restore: Fix verbose mode table flushingPhil Sutter2020-06-091-0/+76
| | | | | | | | | | When called with --verbose mode, iptables-nft-restore did not print anything when flushing the table. Fix this by adding a "manual" mode to nft_cmd_table_flush(), turning it into a wrapper around '-F' and '-X' commands, which is exactly what iptables-legacy-restore does to flush a table. This though requires a real cache, so don't set NFT_CL_FAKE then. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Fix syntax in ipt-restore/0010-noflush-new-chain_0Phil Sutter2020-05-291-0/+1
| | | | | | | | The here-doc statement missed the final delimiter. Worked anyways because end-of-file would do the trick. Fixes: a103fbfadf4c1 ("xtables-restore: Fix parser feed from line buffer") Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Add test for nfbz#1391Phil Sutter2020-04-281-0/+7
| | | | | | | | Problem is fixed since commit c550c81fd373e ("nft: cache: Fix nft_release_cache() under stress"), looks like another case of use-after-free. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Test -F in dump filesPhil Sutter2020-04-231-0/+12
| | | | | | | While not really useful, iptables-nft-restore shouldn't segfault either. This tests the problem described in nfbz#1407. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Extend ipt-restore/0004-restore-race_0Phil Sutter2020-04-231-3/+2
| | | | | | | | | Add a second table to dump/restore. This triggers failures after reverting c550c81fd373e ("nft: cache: Fix nft_release_cache() under stress"), hence acts as a reproducer for the bug fixed by that commit as well. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Improve ipt-restore/0001load-specific-table_0 a bitPhil Sutter2020-04-231-1/+1
| | | | | | | | Instead of reading from stdin, pass dump file as regular parameter. This way dump file name occurs in 'bash -x' output which helps finding out where things fail. Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: Fix skip checks with --host modePhil Sutter2020-02-141-1/+1
| | | | | | | | | | 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-121-0/+16
| | | | | | | | | | | | | | | | | | | | | | 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-restore: Fix parser feed from line bufferPhil Sutter2019-12-041-0/+10
| | | | | | | | | | | | 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>
* 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: Unbreak *tables-restorePhil Sutter2019-10-231-2/+19
| | | | | | | | | | | | | | | | | Commit 3dc433b55bbfa ("xtables-restore: Fix --table parameter check") installed an error check which evaluated true in all cases as all callers of do_command callbacks pass a pointer to a table name already. Attached test case passed as it tested error condition only. Fix the whole mess by introducing a boolean to indicate whether a table parameter was seen already. Extend the test case to cover positive as well as negative behaviour and to test ebtables-restore and ip6tables-restore as well. Also add the required checking code to the latter since the original commit missed it. Fixes: 3dc433b55bbfa ("xtables-restore: Fix --table parameter check") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables-restore: Fix --table parameter checkPhil Sutter2019-10-211-0/+13
| | | | | | | | | | | | | | | | | | | | | Xtables-restore tries to reject rule commands in input which contain a --table parameter (since it is adding this itself based on the previous table line). The manual check was not perfect though as it caught any parameter starting with a dash and containing a 't' somewhere, even in rule comments: | *filter | -A FORWARD -m comment --comment "- allow this one" -j ACCEPT | COMMIT Instead of error-prone manual checking, go a much simpler route: All do_command callbacks are passed a boolean indicating they're called from *tables-restore. React upon this when handling a table parameter and error out if it's not the first one. Fixes: f8e5ebc5986bf ("iptables: Fix crash on malformed iptables-restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
* xtables-restore: Introduce rule counter tokenizer functionPhil Sutter2019-10-181-0/+22
| | | | | | | | | | | | | | The same piece of code appears three times, introduce a function to take care of tokenizing and error reporting. Pass buffer pointer via reference so it can be updated to point to after the counters (if found). While being at it, drop pointless casting when passing pcnt/bcnt to add_argv(). Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* nft: Fix for add and delete of same rule in single batchPhil Sutter2019-09-301-5/+13
| | | | | | | | | | | | | | | Another corner-case found when extending restore ordering test: If a delete command in a dump referenced a rule added earlier within the same dump, kernel would reject the resulting NFT_MSG_DELRULE command. Catch this by assigning the rule to delete a RULE_ID value if it doesn't have a handle yet. Since __nft_rule_del() does not duplicate the nftnl_rule object when creating the NFT_COMPAT_RULE_DELETE command, this RULE_ID value is added to both NEWRULE and DELRULE commands - exactly what is needed to establish the reference. Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
* tests/shell: Speed up ipt-restore/0004-restore-race_0Phil Sutter2019-09-261-2/+2
| | | | | | | | | | | This test tended to cause quite excessive load on my system, sometimes taking longer than all other tests combined. Even with the reduced numbers, it still fails reliably after reverting commit 58d7de0181f61 ("xtables: handle concurrent ruleset modifications"). Fixes: 4000b4cf2ea38 ("tests: add test script for race-free restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Florian Westphal <fw@strlen.de>
* restore legacy behaviour of iptables-restore when rules start with -4/-6Adel Belhouane2019-07-292-0/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | v2: moved examples to testcase files Legacy implementation of iptables-restore / ip6tables-restore allowed to insert a -4 or -6 option at start of a rule line to ignore it if not matching the command's protocol. This allowed to mix specific ipv4 and ipv6 rules in a single file, as still described in iptables 1.8.3's man page in options -4 and -6. The implementation over nftables doesn't behave correctly in this case: iptables-nft-restore accepts both -4 or -6 lines and ip6tables-nft-restore throws an error on -4. There's a distribution bug report mentioning this problem: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925343 Restore the legacy behaviour: - let do_parse() return and thus not add a command in those restore special cases - let do_commandx() ignore CMD_NONE instead of bailing out I didn't attempt to fix all minor anomalies, but just to fix the regression. For example in the line below, iptables should throw an error instead of accepting -6 and then adding it as ipv4: % iptables-nft -6 -A INPUT -p tcp -j ACCEPT Signed-off-by: Adel Belhouane <bugs.a.b@free.fr> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: Fix ipt-restore/0004-restore-race_0 testcasePhil Sutter2019-05-151-4/+3
| | | | | | | | | | | | | | | Two issues fixed: * XTABLES_LIBDIR was set wrong (CWD is not topdir but tests/). Drop the export altogether, the testscript does this already. * $LINES is a variable set by bash, so initial dump sanity check failed all the time complaining about a spurious initial dump line count. Use $LINES1 instead. Fixes: 4000b4cf2ea38 ("tests: add test script for race-free restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: add test script for race-free restoreFlorian Westphal2019-04-271-0/+119
| | | | | | | | | | | | | | | | | | | | | | | | | | xtables-nft-restore ignores -w, check that we don't add duplicate rules when parallel restores happen. With a slightly older iptables-nft version this ususally fails with: I: [EXECUTING] iptables/tests/shell/testcases/ipt-restore/0004-restore-race_0 iptables-restore v1.8.2 (nf_tables): line 5: CHAIN_USER_ADD failed (File exists): chain UC-0 line 6: CHAIN_USER_ADD failed (File exists): chain UC-1 W: [FAILED] ipt-restore/0004-restore-race_0: expected 0 but got 4 or I: [EXECUTING] iptables/tests/shell/testcases/ipt-restore/0004-restore-race_0 iptables-restore v1.8.2 (nf_tables): line 1: TABLE_FLUSH failed (No such file or directory): table filter or /tmp/tmp.SItN4URxxF /tmp/tmp.P1y4LIxhTl differ: byte 7159, line 137 As the legacy version should not have such race (due to nature of full-table-replace), only do one iteration for legacy case. Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables: Fix for inserting rule at wrong positionPhil Sutter2019-01-281-0/+117
| | | | | | | | | | | | | | | | | | | iptables-restore allows to insert rules at a certain position which is problematic for iptables-nft to realize since rule position is not determined by number but handle of previous or following rule and in case the rules surrounding the new one are new as well, they don't have a handle to refer to yet. Fix this by making use of NFTNL_RULE_POSITION_ID attribute: When inserting before a rule which does not have a handle, refer to it using its NFTNL_RULE_ID value. If the latter doesn't exist either, assign a new one to it. The last used rule ID value is tracked in a new field of struct nft_handle which is incremented before each use. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xtables: Accept --wait in iptables-nft-restorePhil Sutter2018-09-101-0/+21
| | | | | | | | | | | Passing --wait option to iptables-nft-restore led to program abort because the flag parameter was not skipped. Mimick iptables-restore behaviour when encountering --wait or --wait-interval options (but still ignore the parameter). Fixes: b9d7b49d84bc2 ("xtables-compat: restore: sync options with iptables-restore") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
* iptables-restore: free the table lock when skipping a tableJoel Goguen2018-07-263-0/+101
Currently, when running `iptables-restore --table=X`, where `X` is not the first table in the rules dump, the restore will fail when parsing the second table: - a lock is acquird when parsing the first table name - the table name does not match the parameter to `--table` so processing continues until the next table - when processing the next table a lock is acquired, which fails because a lock is already held Another app is currently holding the xtables lock. Perhaps you want to use the -w option? This will release the lock as soon as it's decided the current table won't be used. Signed-off-by: Joel Goguen <contact+netfilter@jgoguen.ca> Signed-off-by: Florian Westphal <fw@strlen.de>