Hello On Tue, May 03, 2022 at 10:44:48AM +0200, Claudio Jeker wrote: </snip> > > The RFC does not use the usual MUST to enforce any of this. > So yes, we should probably not be too strict because there is no way to > force accept the packet when pf_walk_header() returns PF_DROP. > > I agree that the TTL MUST be 1. Also the destination address must be a > multicast address (IGMP to a unicast IP makes no sense). >
updated diff below requires destination address to be multicast for IGMP. thanks and regards sashan --------8<----------------8<----------------8<----------------8<-------- diff --git a/sys/net/pf.c b/sys/net/pf.c index f15e1ead8c0..feb7965c427 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) + if (pd->proto == IPPROTO_IGMP) { + /* According to RFC 1112 ttl must be set to 1. */ + if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) { + DPFPRINTF(LOG_NOTICE, "TTL in IGMP must be 1"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } CLR(pd->badopts, PF_OPT_ROUTER_ALERT); + } /* stop walking over non initial fragments */ if ((h->ip_off & htons(IP_OFFMASK)) != 0) return (PF_PASS); @@ -6698,6 +6705,19 @@ 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: + /* + * 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)) { + DPFPRINTF(LOG_NOTICE, "invalid MLD"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } CLR(pd->badopts, PF_OPT_ROUTER_ALERT); break; }