From: Florian Westphal <fw@strlen.de>
To: Phil Sutter <phil@nwl.cc>,
Pablo Neira Ayuso <pablo@netfilter.org>,
Florian Westphal <fw@strlen.de>, Jan Engelhardt <jengelh@inai.de>,
netfilter-devel@vger.kernel.org
Subject: Re: [PATCH iptabes-nft] iptables-nft: allow removal of empty builtin chains
Date: Mon, 13 Sep 2021 18:02:20 +0200 [thread overview]
Message-ID: <20210913160220.GQ23554@breakpoint.cc> (raw)
In-Reply-To: <20210913154611.GB22465@orbyte.nwl.cc>
Phil Sutter <phil@nwl.cc> wrote:
> > Ok, so I will just send a simplified version of this patch that
> > will remove all empty basechains for -X too.
>
> I believe there was a misunderstanding: How I read Pablo's comments, he
> was walking about '-X' with base-chain name explicitly given. If a user
> calls e.g. 'iptables-nft -X FORWARD', it is clear that the new behaviour
> is intended and dropping any non-standard policy is not a surprise. The
> code right now though behaves unexpectedly:
>
> | # nft flush ruleset
> | # ./install/sbin/iptables-nft -P FORWARD DROP
> | # ./install/sbin/iptables-nft -X
> | # nft list ruleset
> | table ip filter {
> | }
>
> So forward DROP policy is lost even though the user just wanted to make
> sure any user-defined chains are gone. But things are worse in practice:
>
> | # iptables -A FORWARD -d 10.0.0.1 -j ACCEPT
> | # iptables -P FORWARD DROP
> | # iptables -X
>
> With iptables-nft, the last command above fails (EBUSY). I expect users
> to be pedantic when it comes to unexpected firewall openings or bogus
> errors in iptables-wrapping scripts.
>
> IMHO we're fine if chains with non-standard policy stay in place. Yet
> this might be racey because IIRC we don't have a "delete chain only if
> policy is accept" command flavour in kernel. This would be interesting,
> because we could drop a base chain also when it's flushed - just
> ignoring a rejected delete if it happens to be non-standard policy.
>
> The safe option should be to delete base chains only if given
> explicitly, as suggested by Pablo already I suppose.
No idea, I won't change anything. V1 kept '-X' behaviour as-is:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210814174643.130760-1-fw@strlen.de/
see the "don't delete built-in chain" comment, the reject-check was kept
in place for the case where iptables-nft is iterating over all the
chains; explict '-X $NAME' was required.
So I don't know what I should change now. Feel free to update
as you see fit, including a revert.
prev parent reply other threads:[~2021-09-13 16:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-14 17:46 [PATCH iptabes-nft] iptables-nft: allow removal of empty builtin chains Florian Westphal
2021-08-14 20:18 ` Jan Engelhardt
2021-08-14 20:53 ` Florian Westphal
2021-08-15 13:12 ` Pablo Neira Ayuso
2021-08-15 13:27 ` Florian Westphal
2021-08-15 13:49 ` Pablo Neira Ayuso
2021-08-15 14:14 ` Florian Westphal
2021-08-15 14:27 ` Pablo Neira Ayuso
2021-08-15 14:36 ` Florian Westphal
2021-09-13 15:46 ` Phil Sutter
2021-09-13 16:02 ` Florian Westphal [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210913160220.GQ23554@breakpoint.cc \
--to=fw@strlen.de \
--cc=jengelh@inai.de \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=phil@nwl.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.