netdev - Re: [PATCH net] icmp: MUST silently discard certain extended echo requests

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: <CADx9qWhtSuYsGCsr8yrdS4OcyFMXrdwcdx0Q+691SFFzWYJB+w@mail.gmail.com>
Date: Wed, 5 Feb 2025 22:35:46 -0500
From: Will Hawkins <hawkinsw@....cr>
To: netdev@...r.kernel.org
Cc: pabeni@...hat.com, edumazet@...gle.com, dsahern@...nel.org, 
	horms@...nel.org, kuba@...nel.org
Subject: Re: [PATCH net] icmp: MUST silently discard certain extended echo requests

I was attempting to minimize unnecessary CCs on the initial email but
Patchwork chided me for not including required people. I am doing that
with this follow-up message.

Sorry!
Will

On Wed, Feb 5, 2025 at 8:57 PM Will Hawkins <hawkinsw@....cr> wrote:
>
> Per RFC 8335 Section 4,
> """
> When a node receives an ICMP Extended Echo Request message and any of
> the following conditions apply, the node MUST silently discard the
> incoming message:
>
> ...
> - The Source Address of the incoming message is not a unicast address.
> - The Destination Address of the incoming message is a multicast address.
> """
>
> Packets meeting the former criteria do not pass martian detection, but
> packets meeting the latter criteria must be explicitly dropped.
>
> Signed-off-by: Will Hawkins <hawkinsw@....cr>
> ---
>
> I hope that this small change is helpful. There is code that already
> prevents the kernel from transmitting packets that violate the latter
> criteria, but I read the RFC as saying that these rogue packets must
> be dropped before even being handled.
>
> I have a history of Kernel contribution but I do so infrequently. I
> have tried very hard to follow all the proper protocol. Forgive me
> if I messed up. Thank you for all the work that you do maintaining
> _the_ most important Kernel subsystem!
>
>  net/ipv4/icmp.c | 16 ++++++++++++++++
>  net/ipv6/icmp.c | 15 +++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 963a89ae9c26..081264b6e9eb 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1241,6 +1241,22 @@ int icmp_rcv(struct sk_buff *skb)
>
>         /* Check for ICMP Extended Echo (PROBE) messages */
>         if (icmph->type == ICMP_EXT_ECHO) {
> +               /*
> +                *      RFC 8335: 4 When a node receives [a message] and any of
> +                *        the following conditions apply, the node MUST silently
> +                *        discard the incoming message:
> +                *        ...
> +                *        - The Source Address of the incoming message is not
> +                *          a unicast address.
> +                *        - The Destination Address of the incoming message is a
> +                *          multicast address.
> +                *      (Former constraint is handled by martian detection.)
> +                */
> +               if (rt->rt_flags & RTCF_MULTICAST) {
> +                       reason = SKB_DROP_REASON_INVALID_PROTO;
> +                       goto error;
> +               }
> +
>                 /* We can't use icmp_pointers[].handler() because it is an array of
>                  * size NR_ICMP_TYPES + 1 (19 elements) and PROBE has code 42.
>                  */
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index 071b0bc1179d..76498a37e465 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -738,6 +738,21 @@ static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
>             net->ipv6.sysctl.icmpv6_echo_ignore_multicast)
>                 return reason;
>
> +       /*
> +        *      RFC 8335: 4 When a node receives [a message] and any of
> +        *        the following conditions apply, the node MUST silently
> +        *        discard the incoming message:
> +        *        ...
> +        *        - The Source Address of the incoming message is not
> +        *          a unicast address.
> +        *        - The Destination Address of the incoming message is a
> +        *          multicast address.
> +        *      (Former constraint is handled by martian detection.)
> +        */
> +       if (icmph->icmp6_type == ICMPV6_EXT_ECHO_REQUEST &&
> +           ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr))
> +               return reason;
> +
>         saddr = &ipv6_hdr(skb)->daddr;
>
>         acast = ipv6_anycast_destination(skb_dst(skb), saddr);
> --
> 2.47.1
>