diff mbox

route: add more relaxed option for secure_redirects

Message ID 1320710630-28335-1-git-send-email-fbl@redhat.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Flavio Leitner Nov. 8, 2011, 12:03 a.m. UTC
When the host uses a gateway IP address that is actually an alias
address, the ICMP redirect message source address can be the gateway's
main IP address, so the message is ignored by the host regardless of
the secure_redirects setup.

The new value (2) allows that ICMP message to be processed.
The possible values are:

 0 - Accept ICMP redirect messages only if its source address is the
     previous gateway address.
 1 - The same as above. However, if shared_media is FALSE, it has to
     be for gateways listed in default gateway list as well.
 2 - Accept ICMP redirects messages ignoring the conditions above.
 default value is 1.

Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 Documentation/networking/ip-sysctl.txt |   17 ++++++++++-------
 include/linux/icmp.h                   |    5 +++++
 include/linux/inetdevice.h             |    2 +-
 net/ipv4/route.c                       |    6 ++++--
 4 files changed, 20 insertions(+), 10 deletions(-)

Comments

David Miller Nov. 12, 2011, 1:33 a.m. UTC | #1
From: Flavio Leitner <fbl@redhat.com>
Date: Mon,  7 Nov 2011 22:03:50 -0200

> When the host uses a gateway IP address that is actually an alias
> address, the ICMP redirect message source address can be the gateway's
> main IP address, so the message is ignored by the host regardless of
> the secure_redirects setup.
> 
> The new value (2) allows that ICMP message to be processed.
> The possible values are:
> 
>  0 - Accept ICMP redirect messages only if its source address is the
>      previous gateway address.
>  1 - The same as above. However, if shared_media is FALSE, it has to
>      be for gateways listed in default gateway list as well.
>  2 - Accept ICMP redirects messages ignoring the conditions above.
>  default value is 1.
> 
> Signed-off-by: Flavio Leitner <fbl@redhat.com>

The more I look at this the less I like it.

Look, if IPVS or whatever is translating addresses and this is what
causes the problem then this entity can very well translate the damn
addresses right back in the redirect so it looks legitimate to the
sender.

You can't translate people's addresses, and them let them see that
intenal remapping in ICMP errors.  The redirect is dropped by the
sender because it not only looks like crap, it is crap.

This is fundamentally not the correct way to handle this.

I'm not applying this patch.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Flavio Leitner Nov. 16, 2011, 8:46 p.m. UTC | #2
On Fri, 11 Nov 2011 20:33:21 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Flavio Leitner <fbl@redhat.com>
> Date: Mon,  7 Nov 2011 22:03:50 -0200
> 
> > When the host uses a gateway IP address that is actually an alias
> > address, the ICMP redirect message source address can be the
> > gateway's main IP address, so the message is ignored by the host
> > regardless of the secure_redirects setup.
> > 
> > The new value (2) allows that ICMP message to be processed.
> > The possible values are:
> > 
> >  0 - Accept ICMP redirect messages only if its source address is the
> >      previous gateway address.
> >  1 - The same as above. However, if shared_media is FALSE, it has to
> >      be for gateways listed in default gateway list as well.
> >  2 - Accept ICMP redirects messages ignoring the conditions above.
> >  default value is 1.
> > 
> > Signed-off-by: Flavio Leitner <fbl@redhat.com>
> 
> The more I look at this the less I like it.
> 
> Look, if IPVS or whatever is translating addresses and this is what
> causes the problem then this entity can very well translate the damn
> addresses right back in the redirect so it looks legitimate to the
> sender.
> 
I thought about that, see below.

> You can't translate people's addresses, and them let them see that
> intenal remapping in ICMP errors.  The redirect is dropped by the
> sender because it not only looks like crap, it is crap.
> 
> This is fundamentally not the correct way to handle this.
> 

I agree. The problem is that the communication is between the host
and another external host, so it's not with the gateway. Therefore,
the original packet has the saddr of the origin host and the daddr
is an external host.  When the gateway receives that packet, there
is no way to tell which IP address should be used to reply with.

Today Linux picks the primary address of an existing interface.
There is the sysctl icmp_errors_use_inbound_ifaddr to use the
primary address of the interface that _received_ the packet that
caused the icmp error. Yet, it doesn't help if the host used
the alias address (i.e. secondary address) as the gw address.

It may possible to promote the secondary address to be the primary
one in the gateway, but this can't be done if you use more than
just one address in the same subnet as gw for some reason.

Thus, the only option at the sender side would be using iptables
to change the ICMP redirect source address to be the float address,
but that is not working as well. (It isn't passing through -t nat)

fbl
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 16, 2011, 10:02 p.m. UTC | #3
From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 16 Nov 2011 18:46:12 -0200

> Thus, the only option at the sender side would be using iptables
> to change the ICMP redirect source address to be the float address,
> but that is not working as well. (It isn't passing through -t nat)

If it's going to mangle the packet in one direct, the only option
for sane operation is to make the exact reverse transformation in
the other direction for ICMP messages.

I'm sorry to be so difficult about this, but this is the only way to
handle this problem.  If packet mangling is performed to change the
world, that mangling entity has taken on the responsibility to make
everything look correct to all entities for the mangled packets
and any packets generated in response to such mangled packets.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Flavio Leitner Nov. 16, 2011, 11:17 p.m. UTC | #4
On Wed, 16 Nov 2011 17:02:13 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Flavio Leitner <fbl@redhat.com>
> Date: Wed, 16 Nov 2011 18:46:12 -0200
> 
> > Thus, the only option at the sender side would be using iptables
> > to change the ICMP redirect source address to be the float address,
> > but that is not working as well. (It isn't passing through -t nat)
> 
> If it's going to mangle the packet in one direct, the only option
> for sane operation is to make the exact reverse transformation in
> the other direction for ICMP messages.
> 
> I'm sorry to be so difficult about this, but this is the only way to
> handle this problem.  If packet mangling is performed to change the
> world, that mangling entity has taken on the responsibility to make
> everything look correct to all entities for the mangled packets
> and any packets generated in response to such mangled packets.
>

I'm sorry, I lost you there. There is no transformation happening in
any side. The iptables is just a work around to force the outgoing
ICMP redirect to use the correct source address (secondary or alias). 

The whole problem is the linux gateway sending ICMP redirects using
*always* the primary address.

fbl
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Flavio Leitner Nov. 17, 2011, 1:40 a.m. UTC | #5
On Wed, 16 Nov 2011 21:17:38 -0200
Flavio Leitner <fbl@redhat.com> wrote:

> On Wed, 16 Nov 2011 17:02:13 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Flavio Leitner <fbl@redhat.com>
> > Date: Wed, 16 Nov 2011 18:46:12 -0200
> > 
> > > Thus, the only option at the sender side would be using iptables
> > > to change the ICMP redirect source address to be the float
> > > address, but that is not working as well. (It isn't passing
> > > through -t nat)
> > 
> > If it's going to mangle the packet in one direct, the only option
> > for sane operation is to make the exact reverse transformation in
> > the other direction for ICMP messages.
> > 
> > I'm sorry to be so difficult about this, but this is the only way to
> > handle this problem.  If packet mangling is performed to change the
> > world, that mangling entity has taken on the responsibility to make
> > everything look correct to all entities for the mangled packets
> > and any packets generated in response to such mangled packets.
> >
> 
> I'm sorry, I lost you there. There is no transformation happening in
> any side. The iptables is just a work around to force the outgoing
> ICMP redirect to use the correct source address (secondary or alias). 
> 
> The whole problem is the linux gateway sending ICMP redirects using
> *always* the primary address.
> 

To make sure we are in the same page, this simple setup reproduces
the issue.

IP: 10.0.0.1
gw: 10.0.0.100
+--------+          +-----+ primary: 10.0.0.2
| client |----+-----| GW1 |   alias: 10.0.0.100
+--------+    |     +-----+      gw: 10.0.0.254
           +--+--+
           | GW2 |---> internet
           +-----+
         10.0.0.254

1. Client sends TCP SYN to an internet host using
   GW1 alias address as default gw address

2. Then GW1 sends the ICMP redirect back to client
   using the primary address as source address.

3. GW1 forwards the original packet to GW2

4. client ignores the ICMP redirect because
   client.gw != gw1.primary.

Regards,
fbl
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 17, 2011, 1:41 a.m. UTC | #6
From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 16 Nov 2011 23:40:42 -0200

> To make sure we are in the same page, this simple setup reproduces
> the issue.

Thanks for the description, I'll think this over.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 17, 2011, 9:53 p.m. UTC | #7
From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 16 Nov 2011 23:40:42 -0200

> To make sure we are in the same page, this simple setup reproduces
> the issue.
> 
> IP: 10.0.0.1
> gw: 10.0.0.100
> +--------+          +-----+ primary: 10.0.0.2
> | client |----+-----| GW1 |   alias: 10.0.0.100
> +--------+    |     +-----+      gw: 10.0.0.254
>            +--+--+
>            | GW2 |---> internet
>            +-----+
>          10.0.0.254
> 
> 1. Client sends TCP SYN to an internet host using
>    GW1 alias address as default gw address
> 
> 2. Then GW1 sends the ICMP redirect back to client
>    using the primary address as source address.
> 
> 3. GW1 forwards the original packet to GW2
> 
> 4. client ignores the ICMP redirect because
>    client.gw != gw1.primary.

GW1 must respond using a source address matching 'alias', ie.
10.0.0.100 and I would accept a mechinsm to make sure that happens,
if not by default then via a sysctl or similar control.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index cb7f314..f17727d 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -805,13 +805,16 @@  shared_media - BOOLEAN
 	it will be disabled otherwise
 	default TRUE
 
-secure_redirects - BOOLEAN
-	Accept ICMP redirect messages only for gateways,
-	listed in default gateway list.
-	secure_redirects for the interface will be enabled if at least one of
-	conf/{all,interface}/secure_redirects is set to TRUE,
-	it will be disabled otherwise
-	default TRUE
+secure_redirects - INTEGER
+	0 - Accept ICMP redirect messages only if its source address is the
+	    previous gateway address.
+	1 - The same as above. However, if shared_media is FALSE, it has to
+	    be for gateways listed in default gateway list as well.
+	2 - Accept ICMP redirects messages ignoring the conditions above.
+	default value is 1.
+
+	The max value from conf/{all,interface}/secure_redirects is used
+	when doing the ICMP redirect message validation on the {interface}.
 
 send_redirects - BOOLEAN
 	Send redirects, if router.
diff --git a/include/linux/icmp.h b/include/linux/icmp.h
index 474f2a5..77f0c2d 100644
--- a/include/linux/icmp.h
+++ b/include/linux/icmp.h
@@ -64,6 +64,11 @@ 
 #define ICMP_EXC_TTL		0	/* TTL count exceeded		*/
 #define ICMP_EXC_FRAGTIME	1	/* Fragment Reass time exceeded	*/
 
+/* secure_redirects sysctl */
+#define ICMP_SEC_REDIR_OLDGW	0	/* accept only from old gw	*/
+#define ICMP_SEC_REDIR_TOGW	1	/* accept only from old gw and
+					 * to known listed gw */
+#define ICMP_SEC_REDIR_ANY	2	/* accept from any host		*/
 
 struct icmphdr {
   __u8		type;
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 5f81466..eca2cfb 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -124,7 +124,7 @@  static inline void ipv4_devconf_setall(struct in_device *in_dev)
 #define IN_DEV_PROXY_ARP_PVLAN(in_dev)	IN_DEV_CONF_GET(in_dev, PROXY_ARP_PVLAN)
 #define IN_DEV_SHARED_MEDIA(in_dev)	IN_DEV_ORCONF((in_dev), SHARED_MEDIA)
 #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), \
+#define IN_DEV_SEC_REDIRECTS(in_dev)	IN_DEV_MAXCONF((in_dev), \
 						      SECURE_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)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 155138d..ec3ce37 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1329,7 +1329,8 @@  void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 	if (!IN_DEV_SHARED_MEDIA(in_dev)) {
 		if (!inet_addr_onlink(in_dev, new_gw, old_gw))
 			goto reject_redirect;
-		if (IN_DEV_SEC_REDIRECTS(in_dev) && ip_fib_check_default(new_gw, dev))
+		if (IN_DEV_SEC_REDIRECTS(in_dev) == ICMP_SEC_REDIR_TOGW &&
+		    ip_fib_check_default(new_gw, dev))
 			goto reject_redirect;
 	} else {
 		if (inet_addr_type(net, new_gw) != RTN_UNICAST)
@@ -1347,7 +1348,8 @@  void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 				continue;
 
 			if (rt->dst.error || rt->dst.dev != dev ||
-			    rt->rt_gateway != old_gw) {
+			    (IN_DEV_SEC_REDIRECTS(in_dev) != ICMP_SEC_REDIR_ANY &&
+			    rt->rt_gateway != old_gw)) {
 				ip_rt_put(rt);
 				continue;
 			}