Re: removing conntrack helper toggle to enable auto-assignment [was Re: b118509076b3 (probably) breaks my firewall]

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 19 Sep 2022 22:23:10 +0200 Florian Westphal wrote:
> Jakub Kicinski <kuba@kernel.org> wrote:
> > On Sat, 10 Sep 2022 04:02:18 +0200 Pablo Neira Ayuso wrote:  
> > > Disagreed, reverting and waiting for one more release cycle will just
> > > postpone the fact that users must adapt their policies, and that they
> > > rely on a configuration which is not secure.  
> > 
> > What are the chances the firewall actually needs the functionality?  
> 
> Unknown, there is no way to tell.

Chris, is your firewall based on some project or a loose bunch of
scripts you wrote?


I had little exposure to NF/conntrack in my career but I was guessing 
for most users one of the two cases:

 - the system is professionally (i.e. someone is paid) maintained, 
   so they should have noticed the warning and fixed in the last 10 yrs

 - the system is a basic SOHO setup which is highly unlikely to see much
   more than TLS or QUIC these days

IOW the intersection of complex traffic and lack of maintenance is
small.

> In old times, it was enough (not tested, just for illustration):
> 
> iptables -A FORWARD -p tcp -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
> 
> and load nf_conntrack_ftp (or whatever).  Module will auto-snoop traffic
> on tcp port 21 for ftp commands, if it finds some, it auto-installs dynamic
> 'expectation entries', so when data connection comes it will hit RELATED rule
> above.
> 
> This stopped working years ago, unless you did set the (now removed)
> knob back to 1.
> 
> Assuming iptables, users would need to do something like
> iptables -t raw -A PREROUTING -p tcp --dport 21 -d $ftpaddr -j CT --helper "ftp"
> 
> to tell that packets/connections on tcp:21 need to be examined for ftp commands.

Thanks for the explainer! 

> > Perhaps we can add the file back but have it do nothing?  
> 
> I think its even worse, users would think that auto-assign is enabled.

Well, users should do the bare minimum of reading kernel logs :(

I think we should do _something_ because we broke so many things 
in this release if we let this rot until its smell reaches Linus -
someone is getting yelled at...

Now, Linus is usually okay with breaking uAPI if there is no other 
way of preventing a security issue. But (a) we break autoload of
all helpers and we only have security issue in one, and (b) not loading
the module doesn't necessarily mean removing the file (at least IMHO).
We have a bunch of dead files in proc already, although perhaps the 
examples I can think of are tunables.