Bug 16517 - rp_filter fails to filter with CONFIG_IP_ROUTE_MULTIPATH and more than one 0/0 nexthop dev
Summary: rp_filter fails to filter with CONFIG_IP_ROUTE_MULTIPATH and more than one 0/...
Status: RESOLVED CODE_FIX
Alias: None
Product: Networking
Classification: Unclassified
Component: IPV4 (show other bugs)
Hardware: All Linux
: P1 high
Assignee: Stephen Hemminger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-05 01:47 UTC by Igor M Podlesny
Modified: 2012-08-09 15:32 UTC (History)
1 user (show)

See Also:
Kernel Version: at least 2.6.18 and newer
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Igor M Podlesny 2010-08-05 01:47:58 UTC
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.
Comment 1 Andrew Morton 2010-08-05 20:57:29 UTC
(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Thu, 5 Aug 2010 01:48:01 GMT
bugzilla-daemon@bugzilla.kernel.org 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: shemminger@linux-foundation.org
>         ReportedBy: for.poige+bugzilla.kernel.org@gmail.com
>         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.
>
Comment 2 David S. Miller 2010-09-07 05:35:27 UTC
From: Andrew Morton <akpm@linux-foundation.org>
Date: Thu, 5 Aug 2010 13:46:53 -0700

>> 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. ;-)

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.

Reported-by: Igor M Podlesny <for.poige@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index a439689..7d02a9f 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -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;

Note You need to log in before you can comment on or make changes to this bug.