Hello,
Dilli Paudel in Oracle was playing with PF enough to find funny glitch.
He used rule as follows:
block in on vnic4 from 192.168.1.0/24 to any route-to 172.16.1.1@vnic5
Many people expect the route-to action is somewhat futile as 'block' action
takes precedence here, so packet gets always dropped. Well the reality is
very different (and still makes a sort of sense) from PF point of view.
The snippet comes from pf_test():
6586 switch (action) {
6587 case PF_SYNPROXY_DROP:
....
6593 case PF_DIVERT:
6594 switch (pd.af) {
6595 case AF_INET:
....
6610 case PF_AFRT:
6611 if (pf_translate_af(&pd)) {
....
6624 #endif /* INET6 */
6625 default:
6626 /* pf_route can free the mbuf causing *m0 to become
NULL */
6627 if (r->rt) {
6628 switch (pd.af) {
6629 case AF_INET:
6630 pf_route(m0, r, pd.dir,
pd.kif->pfik_ifp, s);
6631 break;
the action comes from matching rule. It's PF_DROP in case of Dilli's rule. As
you can see there is no case-branch for PF_DROP in switch statement at line
6586, so a default: is executed. For route-to action the r->rt is set and PF
executes the route_to*().
Dilli suggests to introduce PF_DROP case branch to switch() at line 6586 Issue
has been also discussed on Friday over icb, where Mr. bluhm further suggested
we should try to add some sanity check to parse.y.
As a side effect the patch breaks block rules with dup-to action. dup-to
action as a part of block rule might make some sense... So if there is
someone, who really needs block ... dup-to he should opt for equivalent
rule using pass ... route-to
Also there is one more question:
shall we implement similar sanity checks for nat-to/rdr-to/... actions?
no one should expect those in block rule, so making pfctl to refuse such rules
loudly sounds like a right thing to do...
regards
sasha
--------8<---------------8<---------------8<------------------8<--------
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.648
diff -u -p -r1.648 parse.y
--- sbin/pfctl/parse.y 21 Apr 2015 16:34:59 -0000 1.648
+++ sbin/pfctl/parse.y 31 Aug 2015 20:30:56 -0000
@@ -3997,8 +3997,11 @@ rule_consistent(struct pf_rule *r, int a
problems++;
}
- /* match rules rules */
- if (r->action == PF_MATCH) {
+ /*
+ * Basic rule sanity check.
+ */
+ switch (r->action) {
+ case PF_MATCH:
if (r->divert.port) {
yyerror("divert is not supported on match rules");
problems++;
@@ -4016,6 +4019,15 @@ rule_consistent(struct pf_rule *r, int a
yyerror("af-to is not supported on match rules");
problems++;
}
+ break;
+ case PF_DROP:
+ if (r->rt) {
+ yyerror("route-to, reply-to and dup-to "
+ "must not be used on block rules");
+ problems++;
+ }
+ break;
+ default:;
}
return (-problems);
}
Index: sys/net/pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.936
diff -u -p -r1.936 pf.c
--- sys/net/pf.c 19 Aug 2015 21:22:41 -0000 1.936
+++ sys/net/pf.c 31 Aug 2015 20:31:02 -0000
@@ -6622,6 +6622,10 @@ done:
action = PF_PASS;
break;
#endif /* INET6 */
+ case PF_DROP:
+ m_freem(*m0);
+ *m0 = NULL;
+ break;
default:
/* pf_route can free the mbuf causing *m0 to become NULL */
if (r->rt) {
