netdev - Re: [PATCH v4] ipv6:introduce function to find route for redirect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130902195056.GA5451@order.stressinduktion.org>
Date:	Mon, 2 Sep 2013 21:50:56 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Duan Jiong <duanj.fnst@...fujitsu.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH v4] ipv6:introduce function to find route for redirect

On Mon, Sep 02, 2013 at 02:14:57PM +0800, Duan Jiong wrote:
> +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)
> +			continue;

Sorry, I should have been more clear what I meant with failing early:

I considered a setup like this:

ip -6 r a default nexthop via fe80::1 dev eth0
ip -6 r a prohibit 2002:1::/64

If the kernel receives a redirect for a destination e.g. 2002:1::1 we
would backtrack above the prohibit rule and return the dst of the default
route and would insert a new cached route which could circumvent the
prohibit rule. We have to try to lock down the tree below 2002:1::/64
in that case. A possible solution for that would be to do something
like this:

	/* We don't accept a redirect in case a more specific route is
	 * installed with dst.error and stop backtracking.
	 */
	if (rt->dst.error)
		break;

Either we have to replace the rt with net->ipv6.ip6_null_entry in that
case or check dst->error before calling rt6_do_redirect below.

> +		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;
> +	BACKTRACK(net, &fl6->saddr);
> +out:
> +	dst_hold(&rt->dst);
> +
> +	read_unlock_bh(&table->tb6_lock);
> +
> +	return rt;
> +};
>
> [...]
>
> @@ -1171,9 +1238,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);
> @@ -1193,9 +1259,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);
>  }

Btw. I still think it should be possible to eliminate
ip6_redirect_no_header:

We could always use ip6_redirect_no_header and use the data of the redirected
header option just for finding the socket to be notified. We can do the whole
verification and route updating in ndisc layer and then just call into icmpv6
layer if upper protocols need a notification of the redirect. But that should
go into another patch. ;)

Thanks,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html