Skip to contentSkip to conversation

ipv6:examine the IP source address legitimacy

PATCH In review Duan Jiong 4 participants 4 messages Aug 27, 2013

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Duan Jiong

author
#0 lore raw

PATCH 1/1 ipv6:examine the IP source address legitimacy

From: Duan Jiong <[email protected]>

RFC4861 8.1:
The IP source address of the Redirect is the
same as the current first-hop router for the
specified ICMP Destination Address.

Signed-off-by Duan Jiong <[email protected]>
---
 net/ipv6/route.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv6/route.c +2 -1
index 8d9a93e..d4f0f72 100644
@@ -1685,6 +1685,7 @@ static int ip6_route_del(struct fib6_config *cfg)
 static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_buff *skb)
 {
 	struct net *net = dev_net(skb->dev);
+	struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct netevent_redirect netevent;
 	struct rt6_info *rt, *nrt = NULL;
 	struct ndisc_options ndopts;

@@ -1745,7 +1746,7 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
 	}
 
 	rt = (struct rt6_info *) dst;
-	if (rt == net->ipv6.ip6_null_entry) {
+	if (rt == net->ipv6.ip6_null_entry || !ipv6_addr_equal(&rt->rt6i_gateway, &iph->saddr)) {
 		net_dbg_ratelimited("rt6_redirect: source isn't a valid nexthop for redirect target\n");
 		return;
 	}
Signature · 1 line
1.8.3.1
#1 lore raw
Quoted 9 lines · Duan Jiong

From: Duan Jiong <[email protected]>

RFC4861 8.1:
The IP source address of the Redirect is the
same as the current first-hop router for the
specified ICMP Destination Address.

Signed-off-by Duan Jiong <[email protected]>

---

What about updating the comment around line 1726 ?

/* RFC2461 8.1:
* The IP source address of the Redirect MUST be the same as the current
* first-hop router for the specified ICMP Destination Address.
*/

Thanks

#2 lore raw
In reply to Duan Jiong
Quoted 25 lines · Duan Jiong

From: Duan Jiong <[email protected]>

RFC4861 8.1:
The IP source address of the Redirect is the
same as the current first-hop router for the
specified ICMP Destination Address.

Signed-off-by Duan Jiong <[email protected]>
---
 net/ipv6/route.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv6/route.c +1 -0
index 8d9a93e..d4f0f72 100644
@@ -1685,6 +1685,7 @@ static int ip6_route_del(struct fib6_config *cfg)
 static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_buff *skb)
 {
 	struct net *net = dev_net(skb->dev);
+	struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct netevent_redirect netevent;
 	struct rt6_info *rt, *nrt = NULL;
 	struct ndisc_options ndopts;

net/ipv6/route.c static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu · line 1752

Quoted 8 lines
net/ipv6/route.c +1 -1
@@ -1745,7 +1746,7 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
 	}
 
 	rt = (struct rt6_info *) dst;
-	if (rt == net->ipv6.ip6_null_entry) {
+	if (rt == net->ipv6.ip6_null_entry || !ipv6_addr_equal(&rt->rt6i_gateway, &iph->saddr)) {
 		net_dbg_ratelimited("rt6_redirect: source isn't a valid nexthop for redirect target\n");
 		return;
 	}

Hmm, that's difficult...

There was once this check:

        /*
         *      RFC 2461 specifies that redirects should only be
         *      accepted if they come from the nexthop to the target.
         *      Due to the way default routers are chosen, this notion
         *      is a bit fuzzy and one might need to check all default
         *      routers.
         */
        if (!ipv6_addr_equal(saddr, &rt->rt6i_gateway)) {
                if (rt->rt6i_flags & RTF_DEFAULT) {
                        struct rt6_info *rt1;

                        read_lock(&rt6_lock);
                        for (rt1 = ip6_routing_table.leaf; rt1; rt1 = rt1->u.next) {
                                if (ipv6_addr_equal(saddr, &rt1->rt6i_gateway)) {
                                        dst_hold(&rt1->u.dst);
                                        dst_release(&rt->u.dst);
                                        read_unlock(&rt6_lock);
                                        rt = rt1;
                                        goto source_ok;
                                }
                        }
                        read_unlock(&rt6_lock);
                }
                if (net_ratelimit())
                        printk(KERN_DEBUG "rt6_redirect: source isn't a valid nexthop "
                               "for redirect target\n");
                goto out;
        }

And even got more complicated in commit a6279458c534d01ccc39498aba61c93083ee0372 ("NDISC: Search over all possible rules on receipt of redirect.").

Today we even have ECMP routes maybe making this decision even a bit more complicated. But I agree, there should be a check, but it will be a bit more complicated.

Greetings,

Hannes

maintainer
#3 lore raw
In reply to Duan Jiong

From: Duan Jiong <[email protected]>
Date: Tue, 27 Aug 2013 19:42:24 +0800

net/ipv6/route.c static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu · line 1749

Quoted 3 lines
net/ipv6/route.c +1 -1
 	rt = (struct rt6_info *) dst;
-	if (rt == net->ipv6.ip6_null_entry) {
+	if (rt == net->ipv6.ip6_null_entry || !ipv6_addr_equal(&rt->rt6i_gateway, &iph->saddr)) {

This line is significantly longer than 80 columns, please do not create such long lines.