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;