netdev - Re: [PATCH v3] ip6_output: ensure flow saddr actually belongs to device

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] [day] [month] [year] [list]

Date:   Tue, 15 Nov 2016 08:26:42 -0700
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Hannes Frederic Sowa <hannes@...essinduktion.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Netdev <netdev@...r.kernel.org>,
        WireGuard mailing list <wireguard@...ts.zx2c4.com>,
        LKML <linux-kernel@...r.kernel.org>,
        YOSHIFUJI Hideaki <hideaki.yoshifuji@...aclelinux.com>
Subject: Re: [PATCH v3] ip6_output: ensure flow saddr actually belongs to
 device

On 11/15/16 7:45 AM, Hannes Frederic Sowa wrote:

>> @@ -1012,6 +1013,16 @@ static int ip6_dst_lookup_tail(struct net *net,
>> const struct sock *sk,
>>          }
>>  #endif
>>
>> +        addr_type = ipv6_addr_type(&fl6->saddr);
>> +        if (addr_type == IPv6_ADDR_ANY)
>> +                return 0;
>> +
>> +        err = -EINVAL;
>> +        bind_to_dev = __ipv6_addr_src_scope(addr_type) <=
>> IPV6_ADDR_SCOPE_LINKLOCAL;
>> +        if (!ipv6_chk_addr(net, &fl6->saddr, bind_to_dev ?
>> (*dst)->dev : NULL, 0) &&
>> +            !ipv6_chk_acast_addr_src(net, (*dst)->dev, &fl6->saddr))
>> +                goto out_err_release;
>> +
>>          return 0;
>>
>>  out_err_release:
>>
> 
> We should not use (*dst)->dev, as this is the resulting device after the
> lookup and not necessarily corresponds to the device the user asked for.

To be consistent with IPv4 the saddr check is done before the lookup and dst and flow oif should not be used. Handling LL addresses are trickier and perhaps this is not the right place to enforce that check since it requires a specific device which is only really known after lookup. Why not add the if saddr is LL verification as part of the route selection? e.g, add something like rt6_device_match to ip6_pol_route (the device match call is only used for ip6_pol_route_lookup and not ip6_pol_route - why is that?).