Hello,

the issue has been discovered and analyzed by Luca di Gregorio
on bugs@ [1]. The thread [1] covers all details. People who
wish to read brief summary can skip to Luca's email [2].

To wrap it up the current handling IGMP/MLD messages in pf(4)
is exact opposite. I failed to translate English words from
RFC standards into correct C code. Patch below are two one-liners
which make multicast for IPv4/IPv6 to work again with pf(4) enabled.

Let me quote Luca's summary for IPv6  here:

    If the IP Destination Address is multicast, then the TTL
    should be 1.

    If the IP Destination Address is not multicast, then
    no restrictions on TTL.

and Luca's summary for IPv4:

    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.

As I've said all details and references to RFCs can be found
in Luca's emails on bugs@ [1].

Diff below to fix IGMP/MLD handling has been essentially proposed
by Luca [3]. OK to commit?

thanks and
regards
sashan

[1] https://marc.info/?t=167714059600002&r=1&w=2

[2] https://marc.info/?l=openbsd-bugs&m=167727244015783&w=2

[3] https://marc.info/?l=openbsd-bugs&m=167722460220004&w=2

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 8cb1326a160..c328109026c 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -6847,7 +6847,7 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short 
*reason)
        /* 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)) {
+               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);
@@ -7101,8 +7101,8 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, 
u_short *reason)
                                 * missing then MLD message is invalid and
                                 * should be discarded.
                                 */
-                               if ((h->ip6_hlim != 1) ||
-                                   !IN6_IS_ADDR_LINKLOCAL(&h->ip6_src)) {
+                               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);

Reply via email to