Project

General

Profile

Bug #3560

Disabled Static Route not fully disabled

Added by Phillip Davis about 3 years ago. Updated 3 months ago.

Status:
Resolved
Priority:
Low
Category:
Routing
Target version:
Start date:
03/31/2014
Due date:
% Done:

100%

Affected version:
All
Affected Architecture:

Description

Add a gateway to an internal router behind LAN. Add a static route to some private IPv4 subnet behind that gateway. Automatic Outbound NAT is enabled. The system defines a route in the routing table and includes that subnet in "tonatsubnets" list in the pf rule set, and correctly NATs that subnet out on WAN good stuff.
Now edit the static route, select "Disabled" and save, apply. The route is removed from the routing table - good. But "tonatsubnets" still contains the subnet and thus there are still NAT rule/s outbound for that subnet - should not be like that.
It looks like many bits of code do not check and process only the "enabled" static routes.
/etc/inc/util.inc:function get_staticroutes() returns all routes, enabled or disabled.
For validation, that is probably good - e.g. when deleting an alias it is good to check if the alias is used even in a disabled static route.
But for live implementation code, just the enabled static routes should be returned for the caller to process and put into conf file, pf rule set...
Perhaps add a parameter so the caller can decide if they want all, or just the enabled static routes. Then adjust the existing calls to get_staticroutes().

Also, /etc/inc/services.inc function services_dhcrelay6_configure() does some processing of $config['staticroutes']['route'] without ever filtering out "disabled" static routes. So that code is going to process static routes that are disabled, whatever side-effect that will have.

I looked at this because of forum https://forum.pfsense.org/index.php?topic=74348.msg406471#msg406471 where @dayid noted that he resolved his issue by actually deleting a static route that had been just disabled. I expect that a review of the calls to get_staticroutes() as proposed above will result in this forum issue being fixed also.

Associated revisions

Revision cf08b49e
Added by Phillip Davis 4 months ago

Fix #3560 correctly handle disabled static routes

1) util.inc - add parameter to get_staticroutes() so the caller can
choose to see all static routes or only the ones that are currently
enabled.
2) filter.inc - just process enabled static routes when making direct
networks list, tonathosts etc.
3) services.inc - only include enabled static routes when making confogs
for DHCP Relay.
4) unbound.inc - only include enable static routes in
unbound_acls_config
5) rc.newroutedns - only trigger if there is an enabled static route.

Note: GUI validation has been left as-is. e.g. in system_gateways we don
not allow to delete a gateway if there is a disabled static route using
it... If people want to delete "higher level" stuff, then they need to
first delete the disabled static route(s). Otherwise it will get rather
"risky" having disabled static routes in the config that refer to
gateways that no longer exist, or have a subnet range that now matches a
local interafce or...

Revision 25e5d826
Added by Phillip Davis 4 months ago

Fix #3560 correctly handle disabled static routes

1) util.inc - add parameter to get_staticroutes() so the caller can
choose to see all static routes or only the ones that are currently
enabled.
2) filter.inc - just process enabled static routes when making direct
networks list, tonathosts etc.
3) services.inc - only include enabled static routes when making confogs
for DHCP Relay.
4) unbound.inc - only include enable static routes in
unbound_acls_config
5) rc.newroutedns - only trigger if there is an enabled static route.

Note: GUI validation has been left as-is. e.g. in system_gateways we don
not allow to delete a gateway if there is a disabled static route using
it... If people want to delete "higher level" stuff, then they need to
first delete the disabled static route(s). Otherwise it will get rather
"risky" having disabled static routes in the config that refer to
gateways that no longer exist, or have a subnet range that now matches a
local interafce or...

(cherry picked from commit cf08b49e20810a0aa953561892b1d5bee353957e)

History

#1 Updated by Phillip Davis about 3 years ago

I didn't bother putting a target version on this, IMHO I wouldn't hold up any release for this! The simple workaround is just to actually delete the static route, rather than only disabling it.

#2 Updated by Phillip Davis about 3 years ago

After doing testing, I deleted my static route. But there was no subsystem-dirty prompt to apply the change. The pf rule set "tonatsubnets" list still contained the subnet from the static route. After deleting the gateway, I got the apply button, and that rebuilt the pf rule set and "tonatsubnets" correctly lost the deleted static route.

#3 Updated by Chris Buechler over 1 year ago

  • Status changed from New to Confirmed
  • Priority changed from Normal to Low
  • Affected version changed from 2.1 to All

#5 Updated by Renato Botelho 4 months ago

  • Status changed from Confirmed to Feedback
  • Assignee set to Renato Botelho
  • Target version set to 2.4.0
  • % Done changed from 0 to 100

PR has been merged, thanks!

#6 Updated by Jim Pingle 4 months ago

  • Status changed from Feedback to Resolved

Works

#7 Updated by Jim Pingle 3 months ago

  • Target version changed from 2.4.0 to 2.3.3

Also available in: Atom PDF