netdev - FW: IGMP messages leading to MRT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Wed, 14 Apr 2021 09:50:03 +0000
From:   Sietse van Zanen <sietse@...dom.nu>
To:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: FW: IGMP messages leading to MRT_NOCACHE upcalls

Hi,

IGMP packets are eligible to be forwarded by the kernel, leading to MRT_NOCACHE upcalls to userspace. I decided to check into this, because there was a notable difference in multicast mrt between FreeBSD (which "arguably" has the better network stack) and Linux using the exact same user space daemon on the exact same network.

I decided to quickly patch ipmr.c to log what is eligible to be forwarded (in ip_mr_input()):
   if (ip_hdr(skb)->ttl <= 1 || (ntohl(ip_hdr(skb)->daddr) & IGMP_LOCAL_GROUP_MASK)
                                  == IGMP_LOCAL_GROUP) {
        pr_err("ipmr: packet from 0x%x with ttl %u to 0x%x proto %d\n", ntohl(ip_hdr(skb)->saddr), ip_hdr(skb)->ttl, ntohl(ip_hdr(skb)->daddr), ip_hdr(skb)->protocol);
    }

And surely:
[ 4621.488211] ipmr: packet from 0xc0a80190 with ttl 1 to 0xeffffffa proto 17
[ 4621.514365] ipmr: packet from 0xa001464 with ttl 1 to 0xeffffffa proto 17
[ 4621.692470] ipmr: packet from 0xc0a80190 with ttl 1 to 0xeffffffa proto 17
[ 4621.767651] ipmr: packet from 0xc0a80190 with ttl 1 to 0xeffffffa proto 17
[ 4625.497651] ipmr: packet from 0xc0a8018f with ttl 1 to 0xeffffffa proto 2
[ 4626.021713] ipmr: packet from 0xc0a8018f with ttl 1 to 0xe1004701 proto 2
[ 4650.288509] ipmr: packet from 0xa001401 with ttl 1 to 0xe1004701 proto 2
[ 4651.288419] ipmr: packet from 0xa001401 with ttl 1 to 0xe1004701 proto 2

Looking at the FreeBSD source code (from sys/netinet/ip_mroute.c X_ip_mforward()), here these packets are rightly and correctly discarded even before an mrt cache lookup is made.
/*
   * Don't forward a packet with time-to-live of zero or one,
     * or a packet destined to a local-only group.
     */
    if (ip->ip_ttl <= 1 || IN_LOCAL_GROUP(ntohl(ip->ip_dst.s_addr))) {
       MFC_UNLOCK();
       VIF_UNLOCK();
       return 0;

The following comment in ipmr.c also makes absolutely no sense to me:
            /* IGMPv1 (and broken IGMPv2 implementations sort of
             * Cisco IOS <= 11.2(8)) do not put router alert
             * option to IGMP packets destined to routable
            * groups. It is very bad, because it means
             * that we can forward NO IGMP messages.

IGMP messages, as per rfc1112, rfc2236 and rfc3376 are sent with ttl of 1 and hence must not be forwarded. IGMP pertains to the local network only, and will be seriously broken if routed. Also multicast forwarding rules clearly specify that packets should only be forwarded if their ttl is greater than the interface threshold, which is impossible for packets with ttl of 1. 
Network trace shows that these packets are not actually forwarded, only marked as eligible in ip_mr_input().

The following questions therefor deserve an answer:
1. Why are packets with a ttl of 1 eligible to be forwarded and not immediately discarded?
2. Why are IGMP packets eligible to be forwarded?
3. Why are IGMP packets resulting in MRT_NOCACHE upcalls at all? 
Userspace daemon is already receiving the IGMP packets themselves and has no way for discerning the resulting upcalls leading to routes being incorrectly added.
4. Would it not be better (or would it break stuff) if these packets will be discarded on earliest convenience in ip_mr_input(), as FreeBSD does:
if (ip_hdr(skb)->ttl <= 1 || (ntohl(ip_hdr(skb)->daddr) & IGMP_LOCAL_GROUP_MASK)
                                  == IGMP_LOCAL_GROUP) {
       goto don't_forward;
                }