Skip to contentSkip to conversation

New: rp_filter fails to filter with CONFIG_IP_ROUTE_MULTIPATH and more than one 0/0 nexthop dev

Bugme-new Andrew Morton 2 participants 2 messages Aug 5, 2010 – Sep 7, 2010 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#1 lore raw
In reply to a message not in the archive

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 66 lines · On Thu, 5 Aug 2010 01:48:01 GMT [email protected] wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=16517

Summary: rp_filter fails to filter with
CONFIG_IP_ROUTE_MULTIPATH and more than one 0/0 nexthop dev
Product: Networking
Version: 2.5
Kernel Version: at least 2.6.18 and newer
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: high
Priority: P1
Component: IPV4
AssignedTo: [email protected]
ReportedBy: [email protected]
Regression: No

I think the problem is net/ipv4/fib_frontend.c fib_validate_source()

...

#ifdef CONFIG_IP_ROUTE_MULTIPATH
    if (FIB_RES_DEV(res) == dev || res.fi->fib_nhs > 1)
#else
    if (FIB_RES_DEV(res) == dev)

#endif
...

I'm not sure, but this code is quite trivial and self-speaking. In case we have several default routes, we'd better iterate over each of them and compare resulting devices with the input one. So, fix is also trivial, specially for network kernel developers. ;-)

I've set priority "High" cause it doesn't affects usual Linux users directly, but indirectly it can affect any host on Internet, so the problem is significant, of course.

P. S. Kernel docs say: {
rp_filter - INTEGER
0 - No source validation.
1 - Strict mode as defined in RFC3704 Strict Reverse Path
Each incoming packet is tested against the FIB and if the interface is not the best reverse path the packet check will fail. By default failed packets are discarded.
2 - Loose mode as defined in RFC3704 Loose Reverse Path
Each incoming packet's source address is also tested against the
FIB
and if the source address is not reachable via any interface the packet check will fail.

Current recommended practice in RFC3704 is to enable strict mode to prevent IP spoofing from DDos attacks. If using asymmetric routing or other complicated routing, then loose mode is recommended.

conf/all/rp_filter must also be set to non-zero to do source validation on the interface

Default value is 0. Note that some distributions enable it in startup scripts.
}, but is in reality level "2" of rp_filtering implemented?

P. P. S. netfilter would be the best place to have Reverse Path checks. But that's another story.

maintainer
#2 lore raw

From: Andrew Morton <[email protected]>
Date: Thu, 5 Aug 2010 13:46:53 -0700

Quoted 10 lines
Quoted 10 lines

I think the problem is net/ipv4/fib_frontend.c fib_validate_source()

...

#ifdef CONFIG_IP_ROUTE_MULTIPATH
    if (FIB_RES_DEV(res) == dev || res.fi->fib_nhs > 1)
#else
    if (FIB_RES_DEV(res) == dev)

#endif
...

Quoted 4 lines
Quoted 4 lines

I'm not sure, but this code is quite trivial and self-speaking. In case we have several default routes, we'd better iterate over each of them and compare resulting devices with the input one. So, fix is also trivial, specially for network kernel developers. ;-)

Please test this patch:

ipv4: Fix reverse path filtering with multipath routing.

Actually iterate over the next-hops to make sure we have a device match. Otherwise RP filtering is always elided when the route matched has multiple next-hops.

net/ipv4/fib_frontend.c +13 -2
index a439689..7d02a9f 100644
@@ -246,6 +246,7 @@ int fib_validate_source(__be32 src, __be32 dst, u8 tos, int oif,
 
 	struct fib_result res;
 	int no_addr, rpf, accept_local;
+	bool dev_match;
 	int ret;
 	struct net *net;
 
@@ -273,12 +274,22 @@ int fib_validate_source(__be32 src, __be32 dst, u8 tos, int oif,
 	}
 	*spec_dst = FIB_RES_PREFSRC(res);
 	fib_combine_itag(itag, &res);
+	dev_match = false;
+
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
-	if (FIB_RES_DEV(res) == dev || res.fi->fib_nhs > 1)
+	for (ret = 0; ret < res.fi->fib_nhs; ret++) {
+		struct fib_nh *nh = &res.fi->fib_nh[ret];
+
+		if (nh->nh_dev == dev) {
+			dev_match = true;
+			break;
+		}
+	}
 #else
 	if (FIB_RES_DEV(res) == dev)
+		dev_match = true;
 #endif
-	{
+	if (dev_match) {
 		ret = FIB_RES_NH(res).nh_scope >= RT_SCOPE_HOST;
 		fib_res_put(&res);
 		return ret;