Re: [iptables PATCH] iptables-nft: fix basechain policy configuration

From: Jozsef Kadlecsik <kadlec@netfilter.org>
To: Phil Sutter <phil@nwl.cc>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
	Arturo Borrero Gonzalez <arturo@netfilter.org>,
	netfilter-devel@vger.kernel.org
Subject: Re: [iptables PATCH] iptables-nft: fix basechain policy configuration
Date: Fri, 9 Oct 2020 21:05:23 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.23.453.2010092056500.19307@blackhole.kfki.hu> (raw)
In-Reply-To: <20201009120455.GJ13016@orbyte.nwl.cc>

Hi Phil,

On Fri, 9 Oct 2020, Phil Sutter wrote:

> On Fri, Oct 09, 2020 at 12:37:25PM +0200, Jozsef Kadlecsik wrote:
> [...]
> > I know lots of effort went into backward compatibility, this should be 
> > included there too.
> 
> Certainly doable. Some hacking turned into quite a mess, though:
> 
> When restoring without '--noflush', a chain cache is needed - simply 
> doable by treating NFT_CL_FAKE differently. Reacting upon a chain policy 
> of '-' is easy, just lookup the existing chain's policy from cache and 
> use that. Things then become ugly for not specified chains: 
> 'flush_table' callback really deletes the table. So one has to gather 
> the existing builtin chains first, check if their policy is non-default 
> and restore those. If they don't exist though, one has to expect for 
> them to occur when refreshing the transaction (due to concurrent ruleset 
> change). So the batch jobs have to be created either way and just set to 
> 'skip' if either table or chain doesn't exist or the policy is ACCEPT.

I think the main problem is the difference between nft and iptables when 
printing the base chains and their policy, as you wrote:

> But that is a significant divergence between legacy and nft:
> 
> | # iptables -P FORWARD DROP
> | # iptables-restore <<EOF
> | *filter
> | COMMIT
> | EOF
> | # iptables-save
>
> With legacy, the output is:
> 
> | *filter
> | :INPUT ACCEPT [0:0]
> | :FORWARD DROP [0:0]
> | :OUTPUT ACCEPT [0:0]
> | COMMIT
> 
> With nft, there's no output at all. What do you think, should we fix
> that? If so, which side?

It looks as nft would loose the DROP policy of FORWARD! That looks like 
definitely wrong. It was explicitly set, so it should be printed/saved. 

Also, if nft in >legacy mode< would print the base chains with their 
policy (regardless of the value), couldn't then restore mode handle those 
properly?

> But first I need more coffee. %)

Me too... :-)

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
          H-1525 Budapest 114, POB. 49, Hungary