netdev - Re: [PATCH net] ipv6 addrconf: Implemented enhanced DAD (RFC7527)

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 21 Nov 2016 09:10:14 -0800
From:   Erik Nordmark <nordmark@...ic.net>
To:     Hannes Frederic Sowa <hannes@...essinduktion.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] ipv6 addrconf: Implemented enhanced DAD (RFC7527)

On 11/16/16 10:49 PM, Hannes Frederic Sowa wrote:
> I thought about even removing the sysctl altogether and enable enhanced
> DAD by default. ;)
>
> I am in favor of enabling it by default.
>
> But given that there could be broken implementations out there, we
> should give users a choice and provide.
OK, I'll make it the default and send out a new version of the patch. I 
was told I should base the patch on net-next instead of linux-stable so 
I'll move it there.
>
> Could you always generate a nonce in the interface structure? You could
> check the sysctl in the send and receive path to attach and check the
> nonce. This has the advantage that you don't need to delete the
> interface and recreate it to enable/disable enhanced dad on an interface
> (also you can get away with the loop around get_random_bytes to make
> sure its value is not zero as we don't depend on a non-zero nonce
> variable to signal enaling of the feature, see below).
The nonce is per interface address and not per interface. Furthermore, 
the RFC says that on a retry of DAD the nodes will end up using a 
different nonce implying that even for the same interface address it 
should pick a different nonce for each DAD attempt.
Note that since there is no automatic retry of DAD (per RFC4862) and 
each try would check the current sysctl setting so I don't think 
pre-generating the nonce would change the behavior.

>> Is that because get_random_bytes() will not fill in anything if there is
>> insufficient entropy available?
> No, just because 0 is a possible return value from the random number
> generator. ;)

Ah - makes sense.

Thanks again for the review,
    Erik

>>>>        inc = ipv6_addr_is_multicast(daddr);
>>>>
>>>> @@ -797,6 +811,16 @@ static void ndisc_recv_ns(struct sk_buff
>>>>    have_ifp:
>>>>            if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) {
>>>>                if (dad) {
>>>> +                if (nonce != 0 && ifp->dad_nonce == nonce) {
>>>> +                    /* Matching nonce if looped back */
>>>> +                    if (net_ratelimit())
>>>> +                        ND_PRINTK(2, notice,
>>>> +                              "%s: IPv6 DAD loopback for address %pI6c
>>>> nonce %llu ignored\n",
>>>> +                               ifp->idev->dev->name,
>>>> +                               &ifp->addr,
>>>> +                               nonce);
>>> If we print the nonce for debugging reasons, we should keep it in
>>> correct endianess on the wire vs. in the debug output.
>> How about printing it as colon-separated hex bytes since that is more
>> clear than decimal?
>> Would follow the network byte order in the packet.
> I would be totally fine with it. It will be probably easier to switch to
> a char[6] array for the nonce then.