On Fri, Feb 24, 2023 at 08:42:29AM +0100, Luca Di Gregorio wrote: > I would implement this logic: > > If the IP Destination Address is 224.0.0.0/4, then the TTL should be 1. > If the IP Destination Address is not 224.0.0.0/4, then no restrictions on > TTL. > > In your code, I would do this modification: > > - if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) { > - DPFPRINTF(LOG_NOTICE, "Invalid IGMP"); > - REASON_SET(reason, PFRES_IPOPTIONS); > - return (PF_DROP); > - } > > + if ((h->ip_ttl != 1) && IN_MULTICAST(h->ip_dst.s_addr)) { > + DPFPRINTF(LOG_NOTICE, "Invalid IGMP"); > + REASON_SET(reason, PFRES_IPOPTIONS); > + return (PF_DROP); > + }
Sounds resonable. > Anyway, I'm ok if you revert your commit from May 3rd. If this will be the > case, I expect (please correct me if I'm wrong) that in /etc/pf.conf there > must be the line: > pass proto igmp allow-opts > Otherwise the packet with Router Alert will be discarded. No, the router alert and ttl commits are distinct. ---------------------------- revision 1.1128 date: 2022/05/03 13:32:47; author: sashan; state: Exp; lines: +22 -2; commitid: A2jgIEZZkDP6Qtya; Make pf(4) more paranoid about IGMP/MLP messages. MLD/IGMP messages with ttl other than 1 will be discarded. Also MLD messages with other than link-local source address will be discarded. IGMP messages with destination address other than multicast class will be discarded. feedback and OK bluhm@, cluadio@ ---------------------------- revision 1.1127 date: 2022/04/29 08:58:49; author: bluhm; state: Exp; lines: +115 -21; commitid: KJ50nLH6n2yUzUKe; IGMP and ICMP6 MLD packets always have the router alert option set. pf blocked IPv4 options and IPv6 option header by default. This forced users to set allow-opts in pf rules. Better let multicast work by default. Detect router alerts by parsing IP options and hop by hop headers. If the packet has only this option and is a multicast control packet, do not block it due to bad options. tested by otto@; OK sashan@ ---------------------------- Sasha suggested to revert only revision 1.1128. The automatic allow-opts is revision 1.1127. We keep that. > Regarding MLD, I can't say anything because I've never tested multicast > routing with IP6. We should figure out what RFC says about IPv6 MLD. If we use Luca's smarter logic for IPv4, we should also fix IPv6. bluhm