From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: diff: setting M_BCAST flag
To: YASUOKA Masahiko <yasuoka@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 10 Mar 2025 19:28:25 +0100

On Sat, Mar 08, 2025 at 12:54:35PM +0900, YASUOKA Masahiko wrote:
> Hello,
> 
> Our ip_input() doesn't set M_BCAST flag for 0.0.0.0 or 255.255.255.255
> if it's not set by the lower layer.  The flag must be set since it is
> used by the upper layers.  (eg. TCP use the flag to drop the packet.)
> 
> The modification on in_ouraddr() is to set M_BCAST flag consistently
> if the packet is for directed broadcast.  Previous the flag isn't set
> if the packet is diverted by pf(4).
> 
> (eg. on the host that runs a transparent proxy for 80/tcp, when it
> receives a packet for  directed broadcast but its destination mac
> address is not ff:ff:ff:ff:ff:ff (eg. wrong netmask), TCP wil not drop
> it and the connection will  be established.)
> 
> ok? comments?

M_BCAST looks like a layer 2 flag to me.  I we want to prevent layer
3 broadcast addresses in TCP, tcp_input() should care about it.

NetBSD has
                if (IN_MULTICAST(ip->ip_dst.s_addr) ||
                    in_broadcast(ip->ip_dst, m_get_rcvif_NOMPSAFE(m)))
                        goto drop;

FreeBSD does
                        if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) ||
                            IN_MULTICAST(ntohl(ip->ip_src.s_addr)) ||
                            ip->ip_src.s_addr == htonl(INADDR_BROADCAST) ||
                            in_ifnet_broadcast(ip->ip_dst, m->m_pkthdr.rcvif)) {
                                if ((s = tcp_log_addrs(&inc, th, NULL, NULL)))
                                    log(LOG_DEBUG, "%s; %s: Listen socket: "
                                        "Connection attempt from/to broad- "
                                        "or multicast address ignored\n",
                                        s, __func__);
                                goto dropunlock;
                        }

When receiving a SYN packet, they take a closer look.  Maybe we
need something simmilar.

Do not move pf_ouraddr() after route_mpath().  This is a performance
optimization.  If pf knows that the packet belongs to a local socket,
the route lookup is skipped.

bluhm

> Index: sys/netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.404 ip_input.c
> --- sys/netinet/ip_input.c	2 Mar 2025 21:28:32 -0000	1.404
> +++ sys/netinet/ip_input.c	8 Mar 2025 03:51:45 -0000
> @@ -508,6 +508,7 @@ ip_input_if(struct mbuf **mp, int *offp,
>  
>  	if (ip->ip_dst.s_addr == INADDR_BROADCAST ||
>  	    ip->ip_dst.s_addr == INADDR_ANY) {
> +		m->m_flags |= M_BCAST; /* Make sure M_BCAST is set */
>  		nxt = ip_ours(mp, offp, nxt, af, ns);
>  		goto out;
>  	}
> @@ -518,7 +519,7 @@ ip_input_if(struct mbuf **mp, int *offp,
>  	} else {
>  		ro = &ns->ns_route;
>  	}
> -	switch(in_ouraddr(m, ifp, ro, flags)) {
> +	switch (in_ouraddr(m, ifp, ro, flags)) {
>  	case 2:
>  		goto bad;
>  	case 1:
> @@ -851,18 +852,6 @@ in_ouraddr(struct mbuf *m, struct ifnet 
>  	struct ip		*ip;
>  	int			 match = 0;
>  
> -#if NPF > 0
> -	switch (pf_ouraddr(m)) {
> -	case 0:
> -		return (0);
> -	case 1:
> -		return (1);
> -	default:
> -		/* pf does not know it */
> -		break;
> -	}
> -#endif
> -
>  	ip = mtod(m, struct ip *);
>  
>  	rt = route_mpath(ro, &ip->ip_dst, &ip->ip_src, m->m_pkthdr.ph_rtableid);
> @@ -883,6 +872,18 @@ in_ouraddr(struct mbuf *m, struct ifnet 
>  			m->m_flags |= M_BCAST;
>  		}
>  	}
> +
> +#if NPF > 0
> +	switch (pf_ouraddr(m)) {
> +	case 0:
> +		return (0);
> +	case 1:
> +		return (1);
> +	default:
> +		/* pf does not know it */
> +		break;
> +	}
> +#endif
>  
>  	if (!match) {
>  		struct ifaddr *ifa;