| Message ID | 20221121111932.18222-5-fw@strlen.de |
|---|---|
| State | RFC, archived |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | update iptables-nft dissector | expand |
On Mon, Nov 21, 2022 at 12:19:31PM +0100, Florian Westphal wrote: > Before, nft fails to restore some rules because it sees: > insert rule ip filter INPUT iifname iifname ip ... > > Add extra escaping for " so that the shell won't remove it and > nft will see 'iifname "iifname"'. This is fixing up the wrong side, see: struct xt_xlate_{mt,tg}_params::escape_quotes this is set if iptables-translate was called and unset if iptables-restore-translate was called. I didn't invent this, but the logic seems to be escape quotes when printing a command, don't when printing a dump file content. I have a patch in my queue which extends the conditional quoting to interface names. Will submit it later today along with other fixes in that corner. Cheers, Phil
Phil Sutter <phil@nwl.cc> wrote: > On Mon, Nov 21, 2022 at 12:19:31PM +0100, Florian Westphal wrote: > > Before, nft fails to restore some rules because it sees: > > insert rule ip filter INPUT iifname iifname ip ... > > > > Add extra escaping for " so that the shell won't remove it and > > nft will see 'iifname "iifname"'. > > This is fixing up the wrong side, see: Not sure what you mean here. The quotes ARE printed, but the shell strips them away. > struct xt_xlate_{mt,tg}_params::escape_quotes Ick. > this is set if iptables-translate was called and unset if > iptables-restore-translate was called. I didn't invent this, but the > logic seems to be escape quotes when printing a command, don't when > printing a dump file content. > > I have a patch in my queue which extends the conditional quoting to > interface names. Will submit it later today along with other fixes in > that corner. I would prefer to rip this out, I don't think any of the tools should print '\"' instead of '"'.
On Tue, Nov 22, 2022 at 05:01:28PM +0100, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > On Mon, Nov 21, 2022 at 12:19:31PM +0100, Florian Westphal wrote: > > > Before, nft fails to restore some rules because it sees: > > > insert rule ip filter INPUT iifname iifname ip ... > > > > > > Add extra escaping for " so that the shell won't remove it and > > > nft will see 'iifname "iifname"'. > > > > This is fixing up the wrong side, see: > > Not sure what you mean here. > > The quotes ARE printed, but the shell strips them away. > > > struct xt_xlate_{mt,tg}_params::escape_quotes > > Ick. > > > this is set if iptables-translate was called and unset if > > iptables-restore-translate was called. I didn't invent this, but the > > logic seems to be escape quotes when printing a command, don't when > > printing a dump file content. > > > > I have a patch in my queue which extends the conditional quoting to > > interface names. Will submit it later today along with other fixes in > > that corner. > > I would prefer to rip this out, I don't think any of the tools should > print '\"' instead of '"'. Either way is fine with me. See how I explicitly call 'echo "<input>" | nft -f -' in xlate-test.py to force evaluation by the shell - an earlier version of that code would break since nft saw the escapes. So *we* don't need them, but one could argue it educates users that they'll have to escape the quotes if they specify them on command line. Cheers, Phil
Phil Sutter <phil@nwl.cc> wrote: > On Tue, Nov 22, 2022 at 05:01:28PM +0100, Florian Westphal wrote: > > Phil Sutter <phil@nwl.cc> wrote: > > > On Mon, Nov 21, 2022 at 12:19:31PM +0100, Florian Westphal wrote: > > > > Before, nft fails to restore some rules because it sees: > > > > insert rule ip filter INPUT iifname iifname ip ... > > > > > > > > Add extra escaping for " so that the shell won't remove it and > > > > nft will see 'iifname "iifname"'. > > > > > > This is fixing up the wrong side, see: > > > > Not sure what you mean here. > > > > The quotes ARE printed, but the shell strips them away. > > > > > struct xt_xlate_{mt,tg}_params::escape_quotes > > > > Ick. > > > > > this is set if iptables-translate was called and unset if > > > iptables-restore-translate was called. I didn't invent this, but the > > > logic seems to be escape quotes when printing a command, don't when > > > printing a dump file content. > > > > > > I have a patch in my queue which extends the conditional quoting to > > > interface names. Will submit it later today along with other fixes in > > > that corner. > > > > I would prefer to rip this out, I don't think any of the tools should > > print '\"' instead of '"'. > > Either way is fine with me. See how I explicitly call 'echo "<input>" | > nft -f -' in xlate-test.py to force evaluation by the shell - an earlier > version of that code would break since nft saw the escapes. So *we* > don't need them, but one could argue it educates users that they'll have > to escape the quotes if they specify them on command line. What if we replace: iptables-translate -A INPUT -j LOG --log-prefix "foo bar" nft add rule ip filter INPUT counter log prefix \"foo bar\" With nft add rule ip filter INPUT 'counter log prefix "foo bar"' IOW, get rid of all escaped_quotes code and change: - printf("%s\n", xt_xlate_rule_get(xl)); + + if (cs->restore) + printf("%s\n", xt_xlate_rule_get(xl)); + else + printf("'%s'\n", xt_xlate_rule_get(xl)); ... this would always place everything rule-related printed by xtables-translate in single quotes while leaving iptables-restore-translate alone. What do you think?
On Wed, Nov 23, 2022 at 10:31:54AM +0100, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > On Tue, Nov 22, 2022 at 05:01:28PM +0100, Florian Westphal wrote: > > > Phil Sutter <phil@nwl.cc> wrote: > > > > On Mon, Nov 21, 2022 at 12:19:31PM +0100, Florian Westphal wrote: > > > > > Before, nft fails to restore some rules because it sees: > > > > > insert rule ip filter INPUT iifname iifname ip ... > > > > > > > > > > Add extra escaping for " so that the shell won't remove it and > > > > > nft will see 'iifname "iifname"'. > > > > > > > > This is fixing up the wrong side, see: > > > > > > Not sure what you mean here. > > > > > > The quotes ARE printed, but the shell strips them away. > > > > > > > struct xt_xlate_{mt,tg}_params::escape_quotes > > > > > > Ick. > > > > > > > this is set if iptables-translate was called and unset if > > > > iptables-restore-translate was called. I didn't invent this, but the > > > > logic seems to be escape quotes when printing a command, don't when > > > > printing a dump file content. > > > > > > > > I have a patch in my queue which extends the conditional quoting to > > > > interface names. Will submit it later today along with other fixes in > > > > that corner. > > > > > > I would prefer to rip this out, I don't think any of the tools should > > > print '\"' instead of '"'. > > > > Either way is fine with me. See how I explicitly call 'echo "<input>" | > > nft -f -' in xlate-test.py to force evaluation by the shell - an earlier > > version of that code would break since nft saw the escapes. So *we* > > don't need them, but one could argue it educates users that they'll have > > to escape the quotes if they specify them on command line. > > What if we replace: > iptables-translate -A INPUT -j LOG --log-prefix "foo bar" > nft add rule ip filter INPUT counter log prefix \"foo bar\" > > With > nft add rule ip filter INPUT 'counter log prefix "foo bar"' > > IOW, get rid of all escaped_quotes code and change: > > - printf("%s\n", xt_xlate_rule_get(xl)); > + > + if (cs->restore) > + printf("%s\n", xt_xlate_rule_get(xl)); > + else > + printf("'%s'\n", xt_xlate_rule_get(xl)); > > > ... this would always place everything rule-related printed by xtables-translate in > single quotes while leaving iptables-restore-translate alone. > > What do you think? This looks like a very nice solution to eliminate the conditional escaping all over the place! Obviously, it means a mass-update of all txlate files. Thanks, Phil
diff --git a/xlate-test.py b/xlate-test.py index f3fcd797af90..5711a6427d78 100755 --- a/xlate-test.py +++ b/xlate-test.py @@ -104,7 +104,7 @@ def test_one_replay(name, sourceline, expected, result): "flush ruleset", "add table " + fam + table_name, "add chain " + fam + table_name + " " + chain_name - ] + [ l.removeprefix("nft ") for l in expected.split("\n") ] + ] + [ l.removeprefix("nft ") for l in expected.replace('\"', '\\"', -1).split("\n") ] # feed input via the pipe to make sure the shell "does its thing" cmd = "echo \"" + "\n".join(nft_input) + "\" | " + args.nft + " -f -"
Before, nft fails to restore some rules because it sees: insert rule ip filter INPUT iifname iifname ip ... Add extra escaping for " so that the shell won't remove it and nft will see 'iifname "iifname"'. Signed-off-by: Florian Westphal <fw@strlen.de> --- xlate-test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)