Project

General

Profile

Bug #7299

Error loading rules for old rule with ICMP type specified

Added by Phillip Davis over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Rules / NAT
Target version:
Start date:
02/22/2017
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.3.3
Affected Architecture:

Description

1) Have an old config with a rule that specifies Protocol ICMP and ICMP type "Echo Request" (for example)
The old rule should be from a previous version where:
<ipprotocol>inet</ipprotocol>
was not stored in the rule config XML.
The rule must not have a gateway specified, and must not be on a WAN-interface that has an IPv4 address (if either of these conditions are true, then the rule is written with an IPv4 gateway specified, or a reply-to clause with an IPv4 address - which seems to allow pf to deduce that it is an "inet" rule)
2) Use the rule in a system upgraded to 2.3.3

Errors are reported like:
There were error(s) loading the rules: /tmp/rules.debug:247: must indicate address family with icmp-type/code - The line in question reads [247]: pass in quick on $WANIF proto icmp from $ahRemoteManagement to $ahWanVip icmp-type echoreq tracker 1463665353 keep state label "USER_RULE: ICMP monitoring"
@ 2017-02-22 16:22:43

See forum https://forum.pfsense.org/index.php?topic=126031.0

Associated revisions

Revision da57defa (diff)
Added by Phillip Davis over 2 years ago

Fix #7299 and other stuff

As far as I can see, filter_generate_user_rule() is always supposed to be called with 'ipprotocol' set to 'inet' or 'inet6'. The cases of rules for both ('inet46') are handled by calling filter_generate_user_rule() twice, passing 'inet' then 'inet6'.

So at this point, if 'ipprotocol' is blank, then it is from an old rule, and it [can|should|must] default to 'inet'.

This would provide a generic fix for old rules that do not have 'ipprotocol' specified.

The other thing that could be done is make some upgrade code that fills in 'ipprotocol' on old rules at upgrade.

Revision 568b607a (diff)
Added by Phillip Davis over 2 years ago

Fix #7299 and other stuff

As far as I can see, filter_generate_user_rule() is always supposed to be called with 'ipprotocol' set to 'inet' or 'inet6'. The cases of rules for both ('inet46') are handled by calling filter_generate_user_rule() twice, passing 'inet' then 'inet6'.

So at this point, if 'ipprotocol' is blank, then it is from an old rule, and it [can|should|must] default to 'inet'.

This would provide a generic fix for old rules that do not have 'ipprotocol' specified.

The other thing that could be done is make some upgrade code that fills in 'ipprotocol' on old rules at upgrade.

Revision 61ea29be (diff)
Added by Phillip Davis over 2 years ago

Fix #7299 and other stuff

As far as I can see, filter_generate_user_rule() is always supposed to be called with 'ipprotocol' set to 'inet' or 'inet6'. The cases of rules for both ('inet46') are handled by calling filter_generate_user_rule() twice, passing 'inet' then 'inet6'.

So at this point, if 'ipprotocol' is blank, then it is from an old rule, and it [can|should|must] default to 'inet'.

This would provide a generic fix for old rules that do not have 'ipprotocol' specified.

The other thing that could be done is make some upgrade code that fills in 'ipprotocol' on old rules at upgrade.

History

#1 Updated by Phillip Davis over 2 years ago

https://github.com/pfsense/pfsense/pull/3571 for minimal fix to this particular problem.

#2 Updated by Phillip Davis over 2 years ago

https://github.com/pfsense/pfsense/pull/3572 has a more general fix that should catch any other ways that rules from old configs can generate pf rules that are missing the 'inet' keyword and then cause problems.
Of course this more general fix needs some thought about if there is some other corner case that will now cause a problem.

#3 Updated by Grischa Zengel over 2 years ago

The worst thing is that there are no rules loaded and the pfsense is unusable.

#4 Updated by Phillip Davis over 2 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100

#5 Updated by Renato Botelho over 2 years ago

  • Status changed from Feedback to Resolved

Also available in: Atom PDF