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
