[PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[Eli Cooper]

According to RFC 1885 2.2(c), the source address of ICMPv6
errors in response to forwarded packets should be set to the
unicast address of the forwarding interface in order to be helpful
in diagnosis. Currently the selection of source address is based
on the default route, without respect to the inbound interface.

This patch sets the source address of ICMPv6 error messages to
the address of inbound interface, with the exception of
'time exceeded' and 'packet to big' messages sent in ip6_forward(),
where the address of OUTPUT device is forced as source address
(however, it is NOT enforced as claimed without this patch).

Signed-off-by: Eli Cooper <elicooper@gmx.com>
---
v2: changed to a switch statement

 net/ipv6/icmp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index bd59c34..edb50b6 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -397,6 +397,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
 	struct sock *sk;
 	struct ipv6_pinfo *np;
 	const struct in6_addr *saddr = NULL;
+	struct in6_addr tmp_saddr;
 	struct dst_entry *dst;
 	struct icmp6hdr tmp_hdr;
 	struct flowi6 fl6;
@@ -421,6 +422,17 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
 	 */
 	addr_type = ipv6_addr_type(&hdr->daddr);
 
+	switch (type) {
+	case ICMPV6_DEST_UNREACH:
+	case ICMPV6_PKT_TOOBIG:
+	case ICMPV6_TIME_EXCEED:
+	case ICMPV6_PARAMPROB:
+		if (!ipv6_dev_get_saddr(net, skb->dev, &hdr->saddr, 0,
+					&tmp_saddr))
+			saddr = &tmp_saddr;
+		break;
+	}
+
 	if (ipv6_chk_addr(net, &hdr->daddr, skb->dev, 0) ||
 	    ipv6_chk_acast_addr_src(net, skb->dev, &hdr->daddr))
 		saddr = &hdr->daddr;
-- 
2.9.3

Re: [PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[Guillaume Nault]

On Sun, Aug 28, 2016 at 11:34:06AM +0800, Eli Cooper wrote:
> According to RFC 1885 2.2(c), the source address of ICMPv6
> errors in response to forwarded packets should be set to the
> unicast address of the forwarding interface in order to be helpful
> in diagnosis.
> 
FWIW, this behaviour has been deprecated ten years ago by RFC 4443:
"The address SHOULD be chosen according to the rules that would be used
 to select the source address for any other packet originated by the
 node, given the destination address of the packet."

The door is left open for other address selection algorithms but, IMHO,
changing kernel's behaviour is better justified by real use cases
than by obsolete RFCs.

Re: [PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[Eli Cooper]

Hello,


On 2016/8/29 1:18, Guillaume Nault wrote:
> On Sun, Aug 28, 2016 at 11:34:06AM +0800, Eli Cooper wrote:
>> According to RFC 1885 2.2(c), the source address of ICMPv6
>> errors in response to forwarded packets should be set to the
>> unicast address of the forwarding interface in order to be helpful
>> in diagnosis.
>>
> FWIW, this behaviour has been deprecated ten years ago by RFC 4443:
> "The address SHOULD be chosen according to the rules that would be used
>  to select the source address for any other packet originated by the
>  node, given the destination address of the packet."
>
> The door is left open for other address selection algorithms but, IMHO,
> changing kernel's behaviour is better justified by real use cases
> than by obsolete RFCs.

I agree, sorry for the obsoleted RFC. This is actually motivated by a
real use case: Say a Linux box is acting as a router that forwards
packets with policy routing from two local networks to two uplinks,
respectively. An outside host from is performing traceroute to a host on
one of the LAN. If the kernel's default route is via the other LAN's
uplink, it will send ICMPv6 packets with the source address that has
nothing to do with the network in question, yet the message probably
will reach the outside host.

Here using the address of inbound or exiting interface as source address
is evidently "a more informative choice." I surmise this is the reason
why the comment reads "Force OUTPUT device used as source address" when
dealing with hop limit exceeded packets in ip6_forward(), although not
effectively so. The current behaviour not only confuses diagnosis, but
also might be undesirable if the addresses of the networks are best kept
secret from each other.

Thanks,
Eli

Re: [PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[Guillaume Nault]

On Mon, Aug 29, 2016 at 02:34:32AM +0800, Eli Cooper wrote:
> Hello,
> 
> 
> On 2016/8/29 1:18, Guillaume Nault wrote:
> > On Sun, Aug 28, 2016 at 11:34:06AM +0800, Eli Cooper wrote:
> >> According to RFC 1885 2.2(c), the source address of ICMPv6
> >> errors in response to forwarded packets should be set to the
> >> unicast address of the forwarding interface in order to be helpful
> >> in diagnosis.
> >>
> > FWIW, this behaviour has been deprecated ten years ago by RFC 4443:
> > "The address SHOULD be chosen according to the rules that would be used
> >  to select the source address for any other packet originated by the
> >  node, given the destination address of the packet."
> >
> > The door is left open for other address selection algorithms but, IMHO,
> > changing kernel's behaviour is better justified by real use cases
> > than by obsolete RFCs.
> 
> I agree, sorry for the obsoleted RFC. This is actually motivated by a
> real use case: Say a Linux box is acting as a router that forwards
> packets with policy routing from two local networks to two uplinks,
> respectively. An outside host from is performing traceroute to a host on
> one of the LAN. If the kernel's default route is via the other LAN's
> uplink, it will send ICMPv6 packets with the source address that has
> nothing to do with the network in question, yet the message probably
> will reach the outside host.
> 
> Here using the address of inbound or exiting interface as source address
> is evidently "a more informative choice." I surmise this is the reason
> why the comment reads "Force OUTPUT device used as source address" when
> dealing with hop limit exceeded packets in ip6_forward(), although not
> effectively so. The current behaviour not only confuses diagnosis, but
> also might be undesirable if the addresses of the networks are best kept
> secret from each other.
> 
That makes more sense indeed. Would be nice to have this use case in
the commit message rather than the blind reference to the obsolete RFC.

Regards,

Guillaume

Re: [PATCH v2] ipv6: Use inbound ifaddr as source addresses for ICMPv6 errors

[David Miller]

From: Eli Cooper <elicooper@gmx.com>
Date: Sun, 28 Aug 2016 11:34:06 +0800

> According to RFC 1885 2.2(c), the source address of ICMPv6
> errors in response to forwarded packets should be set to the
> unicast address of the forwarding interface in order to be helpful
> in diagnosis. Currently the selection of source address is based
> on the default route, without respect to the inbound interface.
> 
> This patch sets the source address of ICMPv6 error messages to
> the address of inbound interface, with the exception of
> 'time exceeded' and 'packet to big' messages sent in ip6_forward(),
> where the address of OUTPUT device is forced as source address
> (however, it is NOT enforced as claimed without this patch).
> 
> Signed-off-by: Eli Cooper <elicooper@gmx.com>

Please resubmit with an updated commit message describing
the use case.