Project

General

Profile

Bug #10716

Policy routing rules are not written correctly for a down gateway

Added by Steve Wheeler about 1 month ago. Updated 24 days ago.

Status:
Feedback
Priority:
Normal
Category:
Rules / NAT
Target version:
Start date:
06/30/2020
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.4.x
Affected Architecture:
All

Description

There are two options for the behaviour of policy routing rules that reference a gateway that is down.
The default behaviour is to omit the gateway from the rule but the rule itself in every other way remains.

The alternative behaviour, which is selected by enabling 'Skip rules when gateway is down', is to omit the rule entirely from the ruleset when the gateway is down.

Both are currently broken.
In the default mode when the gateway is down due to 100% packet loss it still appears as:

# Gateways
GWGW2 = " route-to ( vtnet0 172.21.16.141 ) " 

If it is marked down it appears as:
# Gateways
GWGW2 = "  " 

In both cases a rule referencing it still contain the bad gateway, for example:
pass  in  quick  on $LAN  $GWGW2 inet proto tcp  from any to 9.9.9.9 tracker 1593470028 flags S/SA keep state  label "USER_RULE" 

In the alternative mode it is different. If the gateway is down due to 100% packet loss it still appears as:

GWGW2 = " route-to ( vtnet0 172.21.16.141 ) " 

And the rule referencing it is the same:
pass  in  quick  on $LAN  $GWGW2 inet proto tcp  from any to 9.9.9.9 tracker 1593470028 flags S/SA keep state  label "USER_RULE" 

However when the gateway is marked down the gateway no appears inthe ruleset at all and the rule referencing it is replaced with:
# rule  disabled because gateway GW2 is down  label "USER_RULE" 

That look like the expected behaviour but it is only triggered by manually marking the gateway down which is of limited use.
I suspect the code added here for that particular case is still in play but all other situations are no longer triggering anything.
https://github.com/pfsense/pfsense/pull/1594

Associated revisions

Revision 9544cf66 (diff)
Added by Viktor Gurov about 1 month ago

Gateway down policy rule fix. Issue #10716

History

#1 Updated by Jim Pingle about 1 month ago

Having $GWGW2 on the rule when the content of $GWGW2 is empty is fine. That effectively means the gateway is not being placed on the rule, it just saves logic by not having to have the test both on the code making the rule and the code making the contents of the gateway macro.

The other parts do appear to be problematic, however.

Might be worth checking if https://github.com/pfsense/pfsense/pull/4336 already fixes this.

#2 Updated by Viktor Gurov about 1 month ago

Jim Pingle wrote:

Having $GWGW2 on the rule when the content of $GWGW2 is empty is fine. That effectively means the gateway is not being placed on the rule, it just saves logic by not having to have the test both on the code making the rule and the code making the contents of the gateway macro.

The other parts do appear to be problematic, however.

Might be worth checking if https://github.com/pfsense/pfsense/pull/4336 already fixes this.

This is a different issue/feature

Fix:
https://github.com/pfsense/pfsense/pull/4383

It also fixes fw rule creation for the gateways group (down or force_down state)

#3 Updated by Steve Wheeler about 1 month ago

Tested this patch. It corrects the 'Skip rules when gateway is down' setting but not the default 'omit gateway from the rule' setting.

#4 Updated by Steve Wheeler about 1 month ago

But it sets the value of the gateway variable to 'empty' as Jim commented above so that's OK.

#5 Updated by Steve Wheeler 29 days ago

Tested the new patch with gateway group rules again 2.4.5p1. Seems to work as expected in all conditions.

Tested gateway: UP, manually marked down, down due 100% loss, down due to high packet loss in a group, down due to high latency in a group.

#6 Updated by Jim Pingle 27 days ago

  • Status changed from New to Pull Request Review

#7 Updated by Renato Botelho 24 days ago

  • Status changed from Pull Request Review to Feedback
  • Assignee set to Renato Botelho
  • % Done changed from 0 to 100

PR has been merged. Thanks!

Also available in: Atom PDF