On Tue, May 03, 2022 at 12:26:52AM +0200, Alexandr Nedvedicky wrote: > OK ? or should I also drop a check for link-local source address > in IPv6?
The link-local check makes sense. > --------8<---------------8<---------------8<------------------8<-------- > diff --git a/sys/net/pf.c b/sys/net/pf.c > index f15e1ead8c0..2187d895749 100644 > --- a/sys/net/pf.c > +++ b/sys/net/pf.c > @@ -6456,8 +6456,15 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, > u_short *reason) > pd->off += hlen; > pd->proto = h->ip_p; > /* IGMP packets have router alert options, allow them */ > - if (pd->proto == IPPROTO_IGMP) > - CLR(pd->badopts, PF_OPT_ROUTER_ALERT); > + if (pd->proto == IPPROTO_IGMP) { > + /* According to RFC 1112 ttl must be set to 1. */ > + if (h->ip_ttl != 1) { > + DPFPRINTF(LOG_NOTICE, "TTL in IGMP must be 1"); > + REASON_SET(reason, PFRES_IPOPTIONS); > + return (PF_DROP); > + } else > + CLR(pd->badopts, PF_OPT_ROUTER_ALERT); You return in the if block. No need for else. > + } > /* stop walking over non initial fragments */ > if ((h->ip_off & htons(IP_OFFMASK)) != 0) > return (PF_PASS); > @@ -6698,7 +6705,21 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr > *h, u_short *reason) > case MLD_LISTENER_REPORT: > case MLD_LISTENER_DONE: > case MLDV2_LISTENER_REPORT: > - CLR(pd->badopts, PF_OPT_ROUTER_ALERT); > + /* > + * According to RFC 2710 all MLD messages are > + * sent with hop-limit (ttl) set to 1, and link > + * local source address. If either one is > + * missing then MLD message is invalid and > + * should be discarded. > + */ > + if ((h->ip6_hlim == 1) && > + IN6_IS_ADDR_LINKLOCAL(&h->ip6_src)) > + CLR(pd->badopts, PF_OPT_ROUTER_ALERT); > + else { > + DPFPRINTF(LOG_NOTICE, "invalid MLD"); > + REASON_SET(reason, PFRES_IPOPTIONS); > + return (PF_DROP); > + } Can you turn around the logic? if (something bad) return clear badopts > > break; > } > return (PF_PASS);