core icon indicating copy to clipboard operation
core copied to clipboard

Anitlockout rule opens ports on WAN interface

Open Curly060 opened this issue 10 months ago • 12 comments

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

  • [X] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
  • [X] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Describe the bug

Under some circumstances the anti lockout firewall rule chooses a WAN interface and then WebGUI (ports 80,443) and SSH (port 22, if enabled) are accessible on the WAN! That is for sure not desired behaviour!

To Reproduce

Steps to reproduce the behavior:

  1. install a fresh OPNsense on a host with 4 NICs
  2. create a 2nd WAN interface (WAN_NEW)
  3. create a 2nd LAN interface (LAN_NEW) and an appropriate "allow" firewall rule
  4. at this point verify that both WANs have no open ports
  5. delete the old WAN and LAN interfaces
  6. reload all services
  7. check the WAN again for open ports

=> Now port 80 and 443 are open on WAN (and 22 if SSH access was enabled)

Note: The order of creation of WAN_NEW and LAN_NEW is important: WAN_NEW first, then LAN_NEW

Expected behavior The anti lockout rule should never apply to a WAN interface.

Describe alternatives you considered Enabling the setting "Disable anti-lockout" in Firewall/Settings/Advanced prevents this from happening, of course. This will be my default setting from now on.

Environment

Software version used and hardware type if relevant, e.g.: OPNsense 24.1.3_1-amd64 FreeBSD 13.2-RELEASE-p10

Curly060 avatar Apr 10 '24 15:04 Curly060

The machine simply can't know you intend to use that port as wan.... disable the option if you don't need it, you can find the setting in "Firewall: Settings: Advanced". If there's only a single interface, people expect the anti-lockout to apply, otherwise its the first interface it can find (lan, optX, wan).

(see also https://github.com/opnsense/core/commit/dee270a851944da598f7606c96b2162b80382c50)

AdSchellevis avatar Apr 10 '24 16:04 AdSchellevis

And that is exactly why the machine shouldn't guess!

I found out the hard way, suddenly my firewall was open, even though I did not make any changes to the Firewall rules. It was simply because I made changes to my interfaces and then a default(!) setting kicked in opening ports on WAN.

That is quite unexpected. Choosing any interface is actually unexpected. The setting could easily be made more robust: Instead of a checkbox it could be a select box where one can select the interface that the rule shall apply for. By default it is LAN, nothing selected means disabled and if someone tries to delete the selected interface a warning could appear thatg this will disable the anti lockout rule.

No more guessing, no surprises. So what do you think?

I know I can disable it (I wrote that) and I have now. But this is a default setting and it was in its default state all those years I have been using OPNsense...

P.S.: What does the "support" label mean?

Curly060 avatar Apr 11 '24 06:04 Curly060

If we change the behaviour a lot more complaints of people being locked out will follow. I don’t see a way out of this as you try to indicate.

fichtner avatar Apr 11 '24 06:04 fichtner

As all rules that apply are visible on the interface in question, it shouldn't be needed to change anything here. Offering people a choice to which interface it applies basically is the same as offering the option to disable it, with the added bonus of having people locked out before they're able to change the setting.

I don't mind extending the help text if it's not clear enough how this works now, the feature itself is there for good reasons (I don't hear anybody volunteering offering free support to all people who managed to lock themselves out).

AdSchellevis avatar Apr 11 '24 07:04 AdSchellevis

I cannot believe you two guys. You worry more about locked out complaining users than an open port on the WAN side? And as a workaround you suggest to me to switch off that very rule so that exactly that what you fear may happen: I may eventually lock myself out and complain?

I never questioned the sense of the anti lockout rule, in fact, I want it on. But I do not want it to randomly open ports on WAN. The rule clearly has a bug. Switching the rule off is not a solution. A better help text also not, because both do not fix the root cause of the bug. Fixing the bug is the solution.

So I suggested a way to fix it. But I did not expect we end up discussing the sense of the rule. I expected to discuss possible solutions to the problem and how we can make the rule work 100% correctly at all times. I believe this is possible without too much effort. We are talking about router/firewall software here and a randomly open port is completely unacceptable. Being locked out is also quite uncool.

So: If you are worried about users being locked out by accident, prevent that from happening. Prevent the removal of the interface that is currently selected by the anti lockout rule and force the user to select a different interface before being able to delete it. Problem solved, no random open ports, no accidentally locked out users and an always properly working anti-lockout rule. Just how it should be.

That is my suggestion and I believe it will work. If not, please explain me exactly why it will not work. It is very well possible that I am not aware of side effects. But please, let's not discuss the sense of this rule, let's discuss possible solutions to a critical bug of that rule.

So up to you:

  1. Ignore the bug and keep things as they are.
  2. Find a solution to the bug. In that case I am willing to have a go at fixing it or at least support with testing.

Awaiting your choice.

Curly060 avatar Apr 11 '24 15:04 Curly060

Just open a PR and I’ll happily guide you through all the cans of worms you’re going to hit eventually. Maybe that’s more productive than warning you about introducing regressions that are somebody else‘s problem from your POV.

Cheers, Franco

fichtner avatar Apr 11 '24 15:04 fichtner

I dont get the point, you install opnsense and in interface assignment via CLI you are asked about lan and wan interface. The LAN gets anti lockout, the WAN doesnt. This should be intuitive for everyone?

mimugmail avatar Apr 11 '24 15:04 mimugmail

The edge case presented here is complete reassignment through GUI. The lowest OPT1 will be the lockout interface in that case. LAN and WAN only create from console and factory reset.

fichtner avatar Apr 11 '24 17:04 fichtner

How about the anti lockout rule deactivates on interfaces that have a non-RFC-1918 address or a GUA? (Or a Gateway assigned?)

Monviech avatar Apr 11 '24 17:04 Monviech

I have created a PR as requested:

  • the anti lockout interface is now a select box
  • the interface can no longer be an inactive interface (the old logic didn't filter for active interfaces and thus may have selected an inactive one...)
  • the code prevents the user from unassigning/disabling/deleting the interface that is currently being used by the anti lockout rule
  • the old configuration is being migrated to the new setting (*)

I have successfully tested the following scenarios:

  • Reset to factory defaults => will choose LAN interface
  • migrate old configuration with both, antilockout enabled or disabled -- if it was enabled, the same logic as now chooses an interface (**) -- if it was disabled, it will stay disabled
  • delete the selected interface => not possible, error message shows the problem
  • create bridge, VLAN and GRE, assign and enable them, then choose as antilockout interface, then try to delete it => not possible, because the current logic already detects that the VLAN, bridge, GRE is already in use
  • create PPPoE interface, assign and enable it, try to delete it => it is possible to delete the PPPoE interface, the current logic does not prevent to remove an assigned PPPoE interface. Could this be related to https://github.com/opnsense/core/issues/7349?

* The place of the migration code seems wrong. I have seen that it is possible to do config migrations, but I couldn't find the "hook" for the system/webgui model (I also didn't spend much time looking for it...) ** the code is almost the same. I have added a filter for enabled interfaces so that only enabled interfaces will be found.

Please have a look and feel free to make the necessary changes. As you can see I have done quite a bit of testing and so far it works pretty well.

Curly060 avatar Apr 14 '24 11:04 Curly060

Thanks, at first glance this likely inhibits scope creep and room for regressions going over multiple files and trying to accomplish migration and backwards compatibility but maybe that’s just me. I’ll take a closer look tomorrow.

fichtner avatar Apr 14 '24 12:04 fichtner

Simply that being said a more elegant solution would be to generate anti-lockout rules per interface setting although from past experiences rule toggles in interface code have their downsides as well. It would also require fudging values into setup wizards and changing the current anti-lockout behaviour to off by default which… bears the risk ok further lockouts. 😉

fichtner avatar Apr 14 '24 12:04 fichtner

This issue has been automatically timed-out (after 180 days of inactivity).

For more information about the policies for this repository, please read https://github.com/opnsense/core/blob/master/CONTRIBUTING.md for further details.

If someone wants to step up and work on this issue, just let us know, so we can reopen the issue and assign an owner to it.

OPNsense-bot avatar Oct 07 '24 14:10 OPNsense-bot