Skip to content

Commit

Permalink
ipv6:introduce function to find route for redirect
Browse files Browse the repository at this point in the history
RFC 4861 says that the IP source address of the Redirect is the
same as the current first-hop router for the specified ICMP
Destination Address, so the gateway should be taken into
consideration when we find the route for redirect.

There was once a check in commit
a627945 ("NDISC: Search over
all possible rules on receipt of redirect.") and the check
went away in commit b94f1c0
("ipv6: Use icmpv6_notify() to propagate redirect, instead of
rt6_redirect()").

The bug is only "exploitable" on layer-2 because the source
address of the redirect is checked to be a valid link-local
address but it makes spoofing a lot easier in the same L2
domain nonetheless.

Thanks very much for Hannes's help.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Duan Jiong authored and David S. Miller committed Sep 5, 2013
1 parent 60cad4e commit b55b76b
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 11 deletions.
2 changes: 1 addition & 1 deletion net/ipv6/ah6.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ static void ah6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
return;

if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, 0, 0);
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
ip6_update_pmtu(skb, net, info, 0, 0);
xfrm_state_put(x);
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/esp6.c
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
return;

if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, 0, 0);
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
ip6_update_pmtu(skb, net, info, 0, 0);
xfrm_state_put(x);
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ static void icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (type == ICMPV6_PKT_TOOBIG)
ip6_update_pmtu(skb, net, info, 0, 0);
else if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, 0, 0);
ip6_redirect(skb, net, skb->dev->ifindex, 0);

if (!(type & ICMPV6_INFOMSG_MASK))
if (icmp6->icmp6_type == ICMPV6_ECHO_REQUEST)
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/ipcomp6.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static void ipcomp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
return;

if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, 0, 0);
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
ip6_update_pmtu(skb, net, info, 0, 0);
xfrm_state_put(x);
Expand Down
3 changes: 2 additions & 1 deletion net/ipv6/ndisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,8 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
return;

if (!ndopts.nd_opts_rh) {
ip6_redirect_no_header(skb, dev_net(skb->dev), 0, 0);
ip6_redirect_no_header(skb, dev_net(skb->dev),
skb->dev->ifindex, 0);
return;
}

Expand Down
81 changes: 75 additions & 6 deletions net/ipv6/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,77 @@ void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu)
}
EXPORT_SYMBOL_GPL(ip6_sk_update_pmtu);

/* Handle redirects */
struct ip6rd_flowi {
struct flowi6 fl6;
struct in6_addr gateway;
};

static struct rt6_info *__ip6_route_redirect(struct net *net,
struct fib6_table *table,
struct flowi6 *fl6,
int flags)
{
struct ip6rd_flowi *rdfl = (struct ip6rd_flowi *)fl6;
struct rt6_info *rt;
struct fib6_node *fn;

/* Get the "current" route for this destination and
* check if the redirect has come from approriate router.
*
* RFC 4861 specifies that redirects should only be
* accepted if they come from the nexthop to the target.
* Due to the way the routes are chosen, this notion
* is a bit fuzzy and one might need to check all possible
* routes.
*/

read_lock_bh(&table->tb6_lock);
fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
restart:
for (rt = fn->leaf; rt; rt = rt->dst.rt6_next) {
if (rt6_check_expired(rt))
continue;
if (rt->dst.error)
break;
if (!(rt->rt6i_flags & RTF_GATEWAY))
continue;
if (fl6->flowi6_oif != rt->dst.dev->ifindex)
continue;
if (!ipv6_addr_equal(&rdfl->gateway, &rt->rt6i_gateway))
continue;
break;
}

if (!rt)
rt = net->ipv6.ip6_null_entry;
else if (rt->dst.error) {
rt = net->ipv6.ip6_null_entry;
goto out;
}
BACKTRACK(net, &fl6->saddr);
out:
dst_hold(&rt->dst);

read_unlock_bh(&table->tb6_lock);

return rt;
};

static struct dst_entry *ip6_route_redirect(struct net *net,
const struct flowi6 *fl6,
const struct in6_addr *gateway)
{
int flags = RT6_LOOKUP_F_HAS_SADDR;
struct ip6rd_flowi rdfl;

rdfl.fl6 = *fl6;
rdfl.gateway = *gateway;

return fib6_rule_lookup(net, &rdfl.fl6,
flags, __ip6_route_redirect);
}

void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark)
{
const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
Expand All @@ -1170,9 +1241,8 @@ void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark)
fl6.saddr = iph->saddr;
fl6.flowlabel = ip6_flowinfo(iph);

dst = ip6_route_output(net, NULL, &fl6);
if (!dst->error)
rt6_do_redirect(dst, NULL, skb);
dst = ip6_route_redirect(net, &fl6, &ipv6_hdr(skb)->saddr);
rt6_do_redirect(dst, NULL, skb);
dst_release(dst);
}
EXPORT_SYMBOL_GPL(ip6_redirect);
Expand All @@ -1192,9 +1262,8 @@ void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
fl6.daddr = msg->dest;
fl6.saddr = iph->daddr;

dst = ip6_route_output(net, NULL, &fl6);
if (!dst->error)
rt6_do_redirect(dst, NULL, skb);
dst = ip6_route_redirect(net, &fl6, &iph->saddr);
rt6_do_redirect(dst, NULL, skb);
dst_release(dst);
}

Expand Down

0 comments on commit b55b76b

Please sign in to comment.