On Sun, Sep 03, 2023 at 04:12:35AM +0200, Alexandr Nedvedicky wrote: > in my opinion is to fix pf_match_rule() function, so ICMP error message > will no longer match 'keep state' rule. Diff below is for IPv4. I still > need to think of more about IPv6. My gut feeling is it will be very similar.
Thanks for the detailed analysis. You proposed fix means that our default pf would block icmp error packets now. block return # block stateless traffic pass # establish keep-state To have the old behaviour one would write block return # block stateless traffic pass no state # allow all packets pass # establish keep-state if suitable The default rule, that is used with an empty pf.conf, still passes all packets, as it does not keep state. I think this makes sense. Passing icmp error packets without error should not be the default. If someone needs it, it is possible. The implicit default rule is still as dumb as possible. OK bluhm@ > --------8<---------------8<---------------8<------------------8<-------- > diff --git a/sys/net/pf.c b/sys/net/pf.c > index 4f0fc3f91a9..0993aed85fb 100644 > --- a/sys/net/pf.c > +++ b/sys/net/pf.c > @@ -4148,6 +4148,9 @@ enter_ruleset: > (r->rule_flag & PFRULE_STATESLOPPY) == 0 && > ctx->icmp_dir != PF_IN), > TAILQ_NEXT(r, entries)); > + /* icmp packet must match existing state */ > + PF_TEST_ATTRIB(r->keep_state && ctx->state_icmp, > + TAILQ_NEXT(r, entries); > break; > > case IPPROTO_ICMPV6:
