Hello,

> 
> Well, not entirely (:  I did it while exploring the code and sent
> out to provoke further discussion.  Today I've talked to reyk@ and
> we think that it's better to go down a different road: make sure we
> don't create states on reply packets in the first place.
> 
that's actually very wise approach as replies can be spoofed...

> I've tested this with ICMP, ICMPv6 and NAT64 (slightly).  Any OKs?
> Objections?

I have no objections, just a small wish, can you set icmp_dir to -1,
if we are not dealing with ICMP? there is a tool we use in Solaris,
which yells on us because of uninitialized variable. I know it's
false positive, but I've gave up on explaining...

patch below combines your fix with my 'wish'.

thanks a lot
regards
sasha

Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.913
diff -u -r1.913 pf.c
--- pf.c        11 May 2015 12:22:14 -0000      1.913
+++ pf.c        21 May 2015 18:59:29 -0000
@@ -3124,6 +3124,8 @@
                }
                break;
 #endif /* INET6 */
+       default:
+               icmp_dir = -1;
        }
 
        ruleset = &pf_main_ruleset;
@@ -3206,6 +3208,11 @@
                                TAILQ_NEXT(r, entries));
                        /* icmp only. type always 0 in other cases */
                        PF_TEST_ATTRIB((r->code && r->code != icmpcode + 1),
                                TAILQ_NEXT(r, entries));
+                       /* icmp only. don't create states on replies */
+                       PF_TEST_ATTRIB((r->keep_state && !state_icmp &&
+                           (r->rule_flag & PFRULE_STATESLOPPY) == 0 &&
+                           icmp_dir != PF_IN),
                                TAILQ_NEXT(r, entries));
                        break;

Reply via email to