netdev - Re: [PATCH RFC net 1/5] arp: discard sha bcast/null (bcast ARP poison)

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c2c9959f-8df0-4a78-8064-733a643351fc@linux.dev>
Date: Mon, 22 Dec 2025 22:08:59 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Marc Sune <marcdevel@...il.com>
Cc: kuba@...nel.org, willemdebruijn.kernel@...il.com, pabeni@...hat.com,
 netdev@...r.kernel.org, dborkman@...nel.org
Subject: Re: [PATCH RFC net 1/5] arp: discard sha bcast/null (bcast ARP
 poison)

On 22/12/2025 21:37, Marc Sune wrote:
> Missatge de Vadim Fedorenko <vadim.fedorenko@...ux.dev> del dia dl.,
> 22 de des. 2025 a les 10:47:
>>
>> On 21/12/2025 21:19, Marc SuÃ±Ã© wrote:
>>>
>>>    /*
>>> + *   For Ethernet devices, Broadcast/Multicast and zero MAC addresses should
>>> + *   never be announced and accepted as sender HW address (prevent BCAST MAC
>>> + *   and NULL ARP poisoning attack).
>>> + */
>>> +     if (dev->addr_len == ETH_ALEN &&
>>
>> dev_type == ARPHRD_ETHER ?
> 
> This is discussed in the cover letter, comments section d). I would
> think more dev_types than that need to check this, at least:
> 
> +       case ARPHRD_ETHER:
> +       case ARPHRD_EETHER:
> +       case ARPHRD_FDDI:
> +       case ARPHRD_IEEE802:
> +       case ARPHRD_IEEE80211:
> 
> but as said, I _think_ it's sufficient to check for HW addrlen == ETH_ALEN.

ARPHRD_EETHER and ARPHRD_IEEE80211 are not really used in the kernel.
For other 3 we already have such case a bit earlier in arp_process(),
it's fine to be aligned with the existing code.

> 
>>
>>
>>> +         (is_broadcast_ether_addr(sha) || is_zero_ether_addr(sha)))
>>
>> RFC says that neither broadcast, nor multicast must be believed. You
>> check for broadcast only. The better check would be:
>>
>> !is_unicast_ether_addr(sha)
> 
> This is discussed in the cover letter, comments section b). In short,
> some NLBs announce MCAST MAC addresses.
> 
> Mind the context there, but I think it's safe. This is applicable to
> ARP and NDP, so I would suggest to follow up there.
> 
> Btw, the is_zero_ether_addr(addr) check is still needed.
> is_unicast_ether_addr(addr) is implemented as
> !is_multicast_ether_addr(addr), and the NULL mac (00:00:00:00:00:00)
> doesn't have the "MCAST bit" set to 1.

Yeah, "!is_valid_ether_addr(sha)" is better in this case.

> 
>>
>>> +             goto out_free_skb;
>>> +
>>> + /*
>>>     *     Special case: We must set Frame Relay source Q.922 address
>>>     */
>>>        if (dev_type == ARPHRD_DLCI)
>>