Hello Luca,
On Thu, Feb 23, 2023 at 09:22:07AM +0100, Luca Di Gregorio wrote:
> Synopsis: PF still blocks IGMP multicast control packets
> Category: system
> Environment:
> System : OpenBSD 7.2
> Details : OpenBSD 7.2 (GENERIC) #6: Sat Jan 21 01:01:28 MST 2023
> r...@syspatch-72-amd64.openbsd.org:
> /usr/src/sys/arch/amd64/compile/GENERIC
>
> Architecture: OpenBSD.amd64
> Machine : amd64
>
> Description:
> In https://www.openbsd.org/plus72.html it is stated that:
> "Changed pf(4) handling of IGMP and ICMP6 MLD packets to allow multicast
> control
> packets to work by default."
> But, with PF enabled, igmp dvmrp Prune messages between two mrouted's are
> still blocked.
>
> Tests can be done with the default lines in /etc/pf.conf:
>
> set skip on lo
> block return
> pass
> block return in on ! lo0 proto tcp to port 6000:6010
> block return out log proto {tcp udp} user _pbuild
>
</snip>>
I went through the mail thread here. I'd like to check if
I got the problem right.
IGMP/mroute is busted when pf(4) is enabled. pf(4) just
assumes all IGMP packets with TTL other than 1 are illegal
and should be discarded right away without even checking
the rules.
the idea in pf_walk_header() is to make sure IGMP packet with
router alert option has TTL 1 and expected destination address.
if legitimate IGMP packet gets dropped here in pf_walk_header()
then the code does not work as intended (and I'm very sorry for that)
can you give a try diff below? it just reverts my commit from May 3rd.
I would like to know if it helps. It should let IGMP packet further up
in pf(4) to state/rule check.
thanks a lot for your help.
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 8cb1326a160..2f941aaea88 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -6845,15 +6845,8 @@ 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) {
- /* 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, "Invalid IGMP");
- REASON_SET(reason, PFRES_IPOPTIONS);
- return (PF_DROP);
- }
+ if (pd->proto == IPPROTO_IGMP)
CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
- }
/* stop walking over non initial fragments */
if ((h->ip_off & htons(IP_OFFMASK)) != 0)
return (PF_PASS);
@@ -7094,19 +7087,6 @@ 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;
}
