Re: b118509076b3 (probably) breaks my firewall

Re: b118509076b3 (probably) breaks my firewall

[Chris Clayton]

[the address I used for regressions was bounced. Using the right one...]

On 09/09/2022 19:21, Chris Clayton wrote:
> 
> 
> On 09/09/2022 11:19, Pablo Neira Ayuso wrote:
>> On Thu, Sep 08, 2022 at 11:48:59PM +0200, Florian Westphal wrote:
>>> Chris Clayton <chris2553@googlemail.com> wrote:
>>>
>>> [ CC Pablo ]
>>>
>>>> On 08/09/2022 20:19, Florian Westphal wrote:
>>>>> Chris Clayton <chris2553@googlemail.com> wrote:
>>>>>> Just a heads up and a question...
>>>>>>
>>>>>> I've pulled the latest and greatest from Linus' tree and built and installed the kernel. git describe gives
>>>>>> v6.0-rc4-126-g26b1224903b3.
>>>>>>
>>>>>> I find that my firewall is broken because /proc/sys/net/netfilter/nf_conntrack_helper no longer exists. It existed on an
>>>>>> -rc4 kernel. Are changes like this supposed to be introduced at this stage of the -rc cycle?
>>>>>
>>>>> The problem is that the default-autoassign (nf_conntrack_helper=1) has
>>>>> side effects that most people are not aware of.
>>>>>
>>>>> The bug that propmpted this toggle from getting axed was that the irc (dcc) helper allowed
>>>>> a remote client to create a port forwarding to the local client.
>>>>
>>>>
>>>> Ok, but I still think it's not the sort of change that should be introduced at this stage of the -rc cycle.
>>>> The other problem is that the documentation (Documentation/networking/nf_conntrack-sysctl.rst) hasn't been updated. So I
>>>> know my firewall is broken but there's nothing I can find that tells me how to fix it.
>>>
>>> Pablo, I don't think revert+move the 'next' will avoid this kinds of
>>> problems, but at least the nf_conntrack-sysctl.rst should be amended to
>>> reflect that this was removed.
>>
>> I'll post a patch to amend the documentation.
>>
>>> I'd keep it though because people that see an error wrt. this might be
>>> looking at nf_conntrack-sysctl.rst.
>>>
>>> Maybe just a link to
>>> https://home.regit.org/netfilter-en/secure-use-of-helpers/?
>>>
> but
> I'm afraid that document isn't much use to a "Joe User" like me. It's written by people who know a lot about the subject
> matter to be read by other people who know a lot about the subject matter.
> 
>>> What do you think?
>>
>> I'll update netfilter.org to host a copy of the github sources.
>>
>> We have been announcing this going deprecated for 10 years...
> 
> 
> That may be the case, it should be broken before -rc1 is released. Breaking it at -rc4+ is, I think, a regression!
> Adding Thorsten Leemuis to cc list

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

[Pablo Neira Ayuso]

On Fri, Sep 09, 2022 at 07:21:47PM +0100, Chris Clayton wrote:
> On 09/09/2022 11:19, Pablo Neira Ayuso wrote:
> > On Thu, Sep 08, 2022 at 11:48:59PM +0200, Florian Westphal wrote:
> >> Chris Clayton <chris2553@googlemail.com> wrote:
> >>
> >> [ CC Pablo ]
> >>
> >>> On 08/09/2022 20:19, Florian Westphal wrote:
> >>>> Chris Clayton <chris2553@googlemail.com> wrote:
> >>>>> Just a heads up and a question...
> >>>>>
> >>>>> I've pulled the latest and greatest from Linus' tree and built and installed the kernel. git describe gives
> >>>>> v6.0-rc4-126-g26b1224903b3.
> >>>>>
> >>>>> I find that my firewall is broken because /proc/sys/net/netfilter/nf_conntrack_helper no longer exists. It existed on an
> >>>>> -rc4 kernel. Are changes like this supposed to be introduced at this stage of the -rc cycle?
> >>>>
> >>>> The problem is that the default-autoassign (nf_conntrack_helper=1) has
> >>>> side effects that most people are not aware of.
> >>>>
> >>>> The bug that propmpted this toggle from getting axed was that the irc (dcc) helper allowed
> >>>> a remote client to create a port forwarding to the local client.
> >>>
> >>>
> >>> Ok, but I still think it's not the sort of change that should be introduced at this stage of the -rc cycle.
> >>> The other problem is that the documentation (Documentation/networking/nf_conntrack-sysctl.rst) hasn't been updated. So I
> >>> know my firewall is broken but there's nothing I can find that tells me how to fix it.
> >>
> >> Pablo, I don't think revert+move the 'next' will avoid this kinds of
> >> problems, but at least the nf_conntrack-sysctl.rst should be amended to
> >> reflect that this was removed.
> > 
> > I'll post a patch to amend the documentation.
> > 
> >> I'd keep it though because people that see an error wrt. this might be
> >> looking at nf_conntrack-sysctl.rst.
> >>
> >> Maybe just a link to
> >> https://home.regit.org/netfilter-en/secure-use-of-helpers/?
>
> but
> I'm afraid that document isn't much use to a "Joe User" like me. It's written by people who know a lot about the subject
> matter to be read by other people who know a lot about the subject matter.

This is always an issue: deprecating stuff is problematic. After
finally removing this toggle, there are chances that more users come
to complain at the flag day to say they did not have enough time to
update their setup to enable conntrack helpers by policy as the
document recommends.

This is the history behind this toggle:

- In 2012, the documentation above is released and a toggle is added
  to disable the existing behaviour.

- In 2016, this toggle is set off by default, _that was already
  breaking existing setups_ as a way to attract users' attention on
  this topic. Yes, that was a tough way to attract attention on this
  topic.

  Moreover, this warning message was also available via dmesg:

        nf_conntrack: default automatic helper assignment
                      has been turned off for security reasons and CT-based
                      firewall rule not found. Use the iptables CT target
                      to attach helpers instead.

  There was a simple way to restore the previous behaviour
  by simply:

        echo 1 > /proc/sys/net/netfilter/nf_conntrack_helper

  Still, maybe not many people look at this warning message.

- In 2022, the toggle is removed. There is still a way to restore your
  setup, which is to enable conntrack helpers via policy. Yes, it
  requires a bit more effort, but there is documentation available on
  how to do this.

  Why at -rc stage? Someone reported a security issue related to
  one of the conntrack helpers, and the reporter claims many users
  still rely on the insecure configuration. This attracted again
  our attention on this toggle, and we decided it was a good idea to
  finally remove it, the sooner the better.

> >> What do you think?
> > 
> > I'll update netfilter.org to host a copy of the github sources.
> > 
> > We have been announcing this going deprecated for 10 years...
> 
> That may be the case, it should be broken before -rc1 is released. Breaking it at -rc4+ is, I think, a regression!
> Adding Thorsten Leemuis to cc list

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.

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

[Willy Tarreau]

Hi Pablo!

On Sat, Sep 10, 2022 at 04:02:18AM +0200, Pablo Neira Ayuso wrote:
> This is always an issue: deprecating stuff is problematic. After
> finally removing this toggle, there are chances that more users come
> to complain at the flag day to say they did not have enough time to
> update their setup to enable conntrack helpers by policy as the
> document recommends.

netfilter is particular in that it often runs on completely headless
systems, and many users do not even watch logs so there are limited
ways to communicate to them.

> This is the history behind this toggle:
> 
> - In 2012, the documentation above is released and a toggle is added
>   to disable the existing behaviour.
> 
> - In 2016, this toggle is set off by default, _that was already
>   breaking existing setups_ as a way to attract users' attention on
>   this topic. Yes, that was a tough way to attract attention on this
>   topic.
> 
>   Moreover, this warning message was also available via dmesg:
> 
>         nf_conntrack: default automatic helper assignment
>                       has been turned off for security reasons and CT-based
>                       firewall rule not found. Use the iptables CT target
>                       to attach helpers instead.

FWIW when you're just a user, that message isn't particularly clear
about what the user must do. An example of rule for each helper found
could have been useful (typically a match on dport 21 for ftp).

The other problem is to try to force users to notice this message when
they simply upgrade a kernel on their headless firewall. Among the
things users detect on headless systems are:
  - long pause before first starting the service (e.g. 30s). That could
    be sufficient for the user to log into the firewall and try to figure
    what is happening.
  - refusal to load a configuration so that it doesn't work *at all*.
    It might not be easy with firewall rules since any config is valid.

Configs that seem to work when doing a few tests are the hardest ones
to troubleshoot because exhaustive tests are needed and any users are
not interested in running them and often don't know at all how to do
that..

>   There was a simple way to restore the previous behaviour
>   by simply:
> 
>         echo 1 > /proc/sys/net/netfilter/nf_conntrack_helper
> 
>   Still, maybe not many people look at this warning message.

Definitely, and it's not clear that this is a temporary switch nor
that it does have negative impacts. Most users just copy-paste random
advices found in forums and blogs. I like to name switches in a way that
make people think twice such as "nf_conntrack_enable_insecure_helpers". 
Of course it's easy to say after the problem happens, I'm just saying
this to try to improve the situation in the future.

> - In 2022, the toggle is removed. There is still a way to restore your
>   setup, which is to enable conntrack helpers via policy. Yes, it
>   requires a bit more effort, but there is documentation available on
>   how to do this.
> 
>   Why at -rc stage? Someone reported a security issue related to
>   one of the conntrack helpers, and the reporter claims many users
>   still rely on the insecure configuration. This attracted again
>   our attention on this toggle, and we decided it was a good idea to
>   finally remove it, the sooner the better.

I agree. In addition, breakage is always possible when upgrading a
kernel and users have to check. Of course we never like it when it
happens but we've seen plenty of other changes in the past, including
some features that used to be configured as module arguments and that
ended up in /sys (e.g. bonding), or new inter-module dependencies that
cause breakage because the final image is missing them, etc.

> > > We have been announcing this going deprecated for 10 years...
> > 
> > That may be the case, it should be broken before -rc1 is released. Breaking it at -rc4+ is, I think, a regression!
> > Adding Thorsten Leemuis to cc list
> 
> 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.

And in addition by then there will be even more such users. Deprecation
is not rocket science, if it doesn't work in 10 years there's something
wrong, either an important feature is being removed that users heavily
depend on, or a message is not seen or not understood. And in both cases,
postponing without changing anything doesn't help the problem go away
but makes it worse.

Just my two cents,
Willy

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

[Jakub Kicinski]

On Sat, 10 Sep 2022 04:02:18 +0200 Pablo Neira Ayuso wrote:
> > > I'll update netfilter.org to host a copy of the github sources.
> > > 
> > > We have been announcing this going deprecated for 10 years...  
> > 
> > That may be the case, it should be broken before -rc1 is released. Breaking it at -rc4+ is, I think, a regression!
> > Adding Thorsten Leemuis to cc list  
> 
> 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?
Perhaps we can add the file back but have it do nothing?

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

[Florian Westphal]

Jakub Kicinski <kuba@kernel.org> wrote:
> On Sat, 10 Sep 2022 04:02:18 +0200 Pablo Neira Ayuso wrote:
> > > > I'll update netfilter.org to host a copy of the github sources.
> > > > 
> > > > We have been announcing this going deprecated for 10 years...  
> > > 
> > > That may be the case, it should be broken before -rc1 is released. Breaking it at -rc4+ is, I think, a regression!
> > > Adding Thorsten Leemuis to cc list  
> > 
> > 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.

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.

> 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.

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

[Jakub Kicinski]

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.

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

[Florian Westphal]

Jakub Kicinski <kuba@kernel.org> wrote:
> 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...

Well, we can restore the knob and some strongly worded printk.
(or even tain/warn_on_once/whatever).

So its not like we no options, but autoassign=1 is a
problematic configuration and so it would prefer to finally get rid
of it.

> 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,

This isn't 100% correct either, because its not necessarliy about
a security bug.  Helpers (by design) make things reachable that
otherwise would not be, e.g. ftp with 'loose=1' modparam adds a
'from anywhere to x:y' reverse forward, so if client is behind nat
(and the helper is active) this can be used to expose a service to
a 3rd party (granted, this is unlikely, given its off by default).

> and (b) not loading
> the module doesn't necessarily mean removing the file (at least IMHO).

We did not disable module load, but loading a connection tracking
module has no effect anymore without the needed iptables (or nftables)
rules to tell the conntrack engine which connections need to be
monitored by which helper.

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

[Chris Clayton]

On 19/09/2022 21:57, Jakub Kicinski wrote:
> 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?
> 

It's a script executed at boot via sysv init. I wrote the script myself following a HOWTO that I found somewhere on the
net. I very rarely run an ftp server on my laptop but I do occasionally need to get files from a remote ftp server.

I eventually figured out what needed to be done to restore my firewall to working order. I had no clue that the change
was coming. I built my system using the Linux From Scratch recipes in 2017. I update the software I have installed
whenever newreleases become available so it's like my own rolling release. But it is very stable. I inspect the output
from the boot log and dmesg fairly regularly (at least once a week), but had never seen anything about this deprecation
until my firewall failed to load when the write to the now-removed variable was attempted.

So I guess I'm an unusual case in that I don't rely on distro maintainers to fix up stuff like this on the rare
occasions it comes along. On reflection, I'd say leave it be - as I said earlier, it just seemed rather late in the 6.0
development cycle for this to pop up.
> 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.

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

[Thorsten Leemhuis]

Chris, thx for CCing the regression list, I've been watching this thread.

On 20.09.22 08:49, Chris Clayton wrote:
>
> So I guess I'm an unusual case in that I don't rely on distro maintainers to fix up stuff like this on the rare
> occasions it comes along. On reflection, I'd say leave it be

Okay. With a bit of luck only very few users are affected by this; if
not we might need to revisit this.

> - as I said earlier, it just seemed rather late in the 6.0
> development cycle for this to pop up.

With security fixes that can happen, as delaying the fix might the
inferior of two choices. :-/

Ciao, Thorsten