netdev - Re: [PATCH] ipv6:examine the IP source address legitimacy

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: <20130827142059.GA23120@order.stressinduktion.org>
Date:	Tue, 27 Aug 2013 16:20:59 +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] ipv6:examine the IP source address legitimacy

On Tue, Aug 27, 2013 at 07:42:24PM +0800, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@...fujitsu.com>
> 
> 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 <duanj.fnst@...fujitsu.com>
> ---
>  net/ipv6/route.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 8d9a93e..d4f0f72 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -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;
>  	}

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

--
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