netdev - Re: [PATCH net-next v2 1/3] ipv4: icmp: Add RFC 5837 support

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: <aQ9pVt1mvU96p-R-@shredder>
Date: Sat, 8 Nov 2025 18:01:26 +0200
From: Ido Schimmel <idosch@...dia.com>
To: David 'equinox' Lamparter <equinox@...c24.net>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	pabeni@...hat.com, edumazet@...gle.com, horms@...nel.org,
	dsahern@...nel.org, petrm@...dia.com, willemb@...gle.com,
	daniel@...earbox.net, fw@...len.de, ishaangandhi@...il.com,
	rbonica@...iper.net, tom@...bertland.com
Subject: Re: [PATCH net-next v2 1/3] ipv4: icmp: Add RFC 5837 support

On Fri, Nov 07, 2025 at 06:33:30PM +0100, David 'equinox' Lamparter wrote:
> On Mon, Oct 27, 2025 at 10:22:30AM +0200, Ido Schimmel wrote:
> > +/* ICMP Extension Object Classes */
> > +#define ICMP_EXT_OBJ_CLASS_IIO		2	/* RFC 5837 */
> > +
> > +/* Interface Information Object - RFC 5837 */
> > +enum {
> > +	ICMP_EXT_CTYPE_IIO_ROLE_IIF,
> > +};
> 
> ...
> 
> > +static __be32 icmp_ext_iio_addr4_find(const struct net_device *dev)
> > +{
> > +	struct in_device *in_dev;
> > +	struct in_ifaddr *ifa;
> > +
> > +	in_dev = __in_dev_get_rcu(dev);
> > +	if (!in_dev)
> > +		return 0;
> > +
> > +	/* It is unclear from RFC 5837 which IP address should be chosen, but
> > +	 * it makes sense to choose a global unicast address.
> > +	 */
> > +	in_dev_for_each_ifa_rcu(ifa, in_dev) {
> > +		if (READ_ONCE(ifa->ifa_flags) & IFA_F_SECONDARY)
> > +			continue;
> > +		if (ifa->ifa_scope != RT_SCOPE_UNIVERSE ||
> > +		    ipv4_is_multicast(ifa->ifa_address))
> > +			continue;
> > +		return ifa->ifa_address;
> 
> For 5837, this should be an address identifying the interface.  This
> sets up a rather tricky situation if there's a /32 configured on the
> interface in the context of unnumbered operation.  Arguably, in that
> case class 5 (node info) should be used rather than class 2 (interface
> info).  Class 5 also allows sticking an IPv6 address in an ICMPv4 reply.

This patchset does not add support for class 5 objects and the above
code is strictly about choosing an address for the "IP Address
Sub-Object" in a class 2 object. Support for class 5 objects can be
added in a different patchset. An administrator can then choose to
include both objects in ICMP error messages.

The kernel does not avoid using a /32 as the source IP of ICMP error
messages, so I don't see a reason to avoid using such an address in the
"IP Address Sub-Object".

> 
> I would argue the logic here should be an order of preference:
> 
> 1. any global non-/32 address on the interface, in a class 2 object
> 2. any global /32 on the interface, in a class 5 object
> 3. any global IPv6 on the interface, in a class 5 object
> 4. any global address from any interface in the VRF, preferring
>    loopback, in a class 5 object (addrsel logic, really)
> 
> [class 5 is draft-ietf-intarea-extended-icmp-nodeid]
> 
> + analog for IPv6
> 
> (cf. my other mail in the thread)
> 
> Cheers,
> 
> 
> -equi