[PATCH net] ipv4: add support for "gratuitous" redirect

The ICMP redirect messages are generated using the selected
interface's primary address as source. On VRRP routers said IP
may differ from the VRRP IP used as the default gatetway from
the LAN hosts.

Such hosts are going to silently discard the ICMP redirect
messages generated by the VRRP router since the message's source
IP will differ from the redirected route's gateway.

RFC1122 don't imposes the above check, so this patch makes
it configurable via the sysctl interface.
When 'gratuitous_redirect' is enabled, the host will accept
all ICMP redirect messages coming from default gateways.

This allows supporting the described scenario,
while retaining some control on the accepted messages.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 Documentation/networking/ip-sysctl.txt |  9 +++++++++
 include/linux/inetdevice.h             |  3 +++
 include/uapi/linux/ip.h                |  1 +
 net/ipv4/devinet.c                     |  4 ++++
 net/ipv4/route.c                       | 15 ++++++++++++---
 5 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ebe94f2..3d16208 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt

@@ -1010,6 +1010,15 @@ secure_redirects - BOOLEAN
 	it will be disabled otherwise
 	default TRUE
 
+gratuitous_redirects - BOOLEAN
+	Accept ICMP redirect messages even if the IP source differs from the
+	redirected route's gateway, but only if the IP source is listed in
+	default gateway list.
+	conf/all/gratuitous_redirects must also be set to TRUE to enable
+	gratuitous_redirects on the interface,
+	it will be disabled otherwise
+	default FALSE
+
 send_redirects - BOOLEAN
 	Send redirects, if router.
 	send_redirects for the interface will be enabled if at least one of

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a4328ce..0e809eb 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h

@@ -105,6 +105,9 @@ static inline void ipv4_devconf_setall(struct in_device *in_dev)
 #define IN_DEV_TX_REDIRECTS(in_dev)	IN_DEV_ORCONF((in_dev), SEND_REDIRECTS)
 #define IN_DEV_SEC_REDIRECTS(in_dev)	IN_DEV_ORCONF((in_dev), \
 						      SECURE_REDIRECTS)
+#define IN_DEV_GRATUITOUS_REDIRECTS(in_dev) \
+					IN_DEV_ANDCONF((in_dev), \
+						       GRATUITOUS_REDIRECTS)
 #define IN_DEV_IDTAG(in_dev)		IN_DEV_CONF_GET(in_dev, TAG)
 #define IN_DEV_MEDIUM_ID(in_dev)	IN_DEV_CONF_GET(in_dev, MEDIUM_ID)
 #define IN_DEV_PROMOTE_SECONDARIES(in_dev) \

diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index 08f894d..e080aef 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h

@@ -165,6 +165,7 @@ enum
 	IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
 	IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
 	IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
+	IPV4_DEVCONF_GRATUITOUS_REDIRECTS,
 	__IPV4_DEVCONF_MAX
 };
 

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 2d9cb17..47d5f1c 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c

@@ -75,6 +75,7 @@ static struct ipv4_devconf ipv4_devconf = {
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
 		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
 		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
+		[IPV4_DEVCONF_GRATUITOUS_REDIRECTS - 1] = 0,
 	},
 };
 

@@ -87,6 +88,7 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
 		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
 		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
 		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
+		[IPV4_DEVCONF_GRATUITOUS_REDIRECTS - 1] = 0,
 	},
 };
 

@@ -2183,6 +2185,8 @@ static struct devinet_sysctl_table {
 					"igmpv3_unsolicited_report_interval"),
 		DEVINET_SYSCTL_RW_ENTRY(IGNORE_ROUTES_WITH_LINKDOWN,
 					"ignore_routes_with_linkdown"),
+		DEVINET_SYSCTL_RW_ENTRY(GRATUITOUS_REDIRECTS,
+					"gratuitous_redirects"),
 
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c6ad99a..87571ee 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c

@@ -723,13 +723,22 @@ static void __ip_do_redirect(struct rtable *rt, struct sk_buff *skb, struct flow
 		return;
 	}
 
-	if (rt->rt_gateway != old_gw)
-		return;
-
 	in_dev = __in_dev_get_rcu(dev);
 	if (!in_dev)
 		return;
 
+	if (IN_DEV_GRATUITOUS_REDIRECTS(in_dev)) {
+		if (ip_fib_check_default(old_gw, dev)) {
+			/* since the chosen configuration is weak, log anything
+			 * suspicious
+			 */
+			goto reject_redirect;
+		}
+	} else {
+		if (rt->rt_gateway != old_gw)
+			return;
+	}
+
 	net = dev_net(dev);
 	if (new_gw == old_gw || !IN_DEV_RX_REDIRECTS(in_dev) ||
 	    ipv4_is_multicast(new_gw) || ipv4_is_lbcast(new_gw) ||

-- 
1.8.3.1

From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 25 Sep 2015 15:16:58 +0200

Such hosts are going to silently discard the ICMP redirect
messages generated by the VRRP router since the message's source
IP will differ from the redirected route's gateway.

RFC1122 don't imposes the above check, so this patch makes

 ...

I don't know if it's a question of terminology but RFC1122 does
seem to ask us to make this check, from 3.2.2.2:

	A Redirect message SHOULD be silently discarded if ... the
        source of the Redirect is not the current first-hop gateway
        for the specified destination (see Section 3.3.1).

And that seems to be exactly what this "rt->rt_gateway != old_gw" test
is implementing.

Furthermore, from a completely pragmatic perspective, pushing this
looser requirement down into the hosts is going to be a deployment
nightmare.

Whereas if we changed the routers to send ICMP redirects that these
hosts would actually accept, you'd only have to make the change on
the routers.  This is several orders of magnitude easier to deploy.

So I'm not going to apply this, sorry.

On Tue, 2015-09-29 at 11:49 -0700, David Miller wrote:

I don't know if it's a question of terminology but RFC1122 does
seem to ask us to make this check, from 3.2.2.2:

        A Redirect message SHOULD be silently discarded if ... the
        source of the Redirect is not the current first-hop gateway
        for the specified destination (see Section 3.3.1).

According to the same RFC:

*    "SHOULD"

              This word or the adjective "RECOMMENDED" means that there
              may exist valid reasons in particular circumstances to
              ignore this item, but the full implications should be
              understood and the case carefully weighed before choosing
              a different course. 

So, if we agree that there are valid reasons we can make this check
optional.

Whereas if we changed the routers to send ICMP redirects that these
hosts would actually accept, you'd only have to make the change on
the routers.  This is several orders of magnitude easier to deploy.

Actually fixing this behavior on the VRRP router requires somewhat
'NATing' ICMP redirect packets, which AFAIK is not possible on a Linux
box, since ICMP reply packets do not traverse the nat table.

To cope with the original issue on the router, it would be needed an
'icmp_errors_use_inbound_daddr' sysctl, that, when enabled, forces the
kernel to send icmp error messages using the destination address of the
incoming packet as source. 

Eventually we can restrict the above option to icmp redirect only.

Do you think that adding such configuration would be more acceptable?

Some time ago, there was already some discussion on this topic:

http://comments.gmane.org/gmane.linux.kernel/1128505

Thank you,

Paolo