[PATCH net] ipv6: make DAD fail with enhanced DAD when nonce length differs

[PATCH net] ipv6: make DAD fail with enhanced DAD when nonce length differs

[Sabrina Dubroca]

Commit adc176c54722 ("ipv6 addrconf: Implemented enhanced DAD (RFC7527)")
added enhanced DAD with a nonce length of 6 bytes. However, RFC7527
doesn't specify the length of the nonce, other than being 6 + 8*k bytes,
with integer k >= 0 (RFC3971 5.3.2). The current implementation simply
assumes that the nonce will always be 6 bytes, but others systems are
free to choose different sizes.

If another system sends a nonce of different length but with the same 6
bytes prefix, it shouldn't be considered as the same nonce. Thus, check
that the length of the received nonce is the same as the length we sent.

Ugly scapy test script running on veth0:

def loop():
    pkt=sniff(iface="veth0", filter="icmp6", count=1)
    pkt = pkt[0]
    b = bytearray(pkt[Raw].load)
    b[1] += 1
    b += b'\xde\xad\xbe\xef\xde\xad\xbe\xef'
    pkt[Raw].load = bytes(b)
    pkt[IPv6].plen += 8
    # fixup checksum after modifying the payload
    pkt[IPv6].payload.cksum -= 0x3b44
    if pkt[IPv6].payload.cksum < 0:
        pkt[IPv6].payload.cksum += 0xffff
    sendp(pkt, iface="veth0")

This should result in DAD failure for any address added to veth0's peer,
but is currently ignored.

Fixes: adc176c54722 ("ipv6 addrconf: Implemented enhanced DAD (RFC7527)")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/ipv6/ndisc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index e640d2f3c55c..0ec273997d1d 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -811,7 +811,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
 			return;
 		}
 	}
-	if (ndopts.nd_opts_nonce)
+	if (ndopts.nd_opts_nonce && ndopts.nd_opts_nonce->nd_opt_len == 1)
 		memcpy(&nonce, (u8 *)(ndopts.nd_opts_nonce + 1), 6);
 
 	inc = ipv6_addr_is_multicast(daddr);
-- 
2.18.0

Re: [PATCH net] ipv6: make DAD fail with enhanced DAD when nonce length differs

[David Miller]

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 13 Jul 2018 17:21:42 +0200

> Commit adc176c54722 ("ipv6 addrconf: Implemented enhanced DAD (RFC7527)")
> added enhanced DAD with a nonce length of 6 bytes. However, RFC7527
> doesn't specify the length of the nonce, other than being 6 + 8*k bytes,
> with integer k >= 0 (RFC3971 5.3.2). The current implementation simply
> assumes that the nonce will always be 6 bytes, but others systems are
> free to choose different sizes.
> 
> If another system sends a nonce of different length but with the same 6
> bytes prefix, it shouldn't be considered as the same nonce. Thus, check
> that the length of the received nonce is the same as the length we sent.
> 
> Ugly scapy test script running on veth0:
> 
> def loop():
>     pkt=sniff(iface="veth0", filter="icmp6", count=1)
>     pkt = pkt[0]
>     b = bytearray(pkt[Raw].load)
>     b[1] += 1
>     b += b'\xde\xad\xbe\xef\xde\xad\xbe\xef'
>     pkt[Raw].load = bytes(b)
>     pkt[IPv6].plen += 8
>     # fixup checksum after modifying the payload
>     pkt[IPv6].payload.cksum -= 0x3b44
>     if pkt[IPv6].payload.cksum < 0:
>         pkt[IPv6].payload.cksum += 0xffff
>     sendp(pkt, iface="veth0")
> 
> This should result in DAD failure for any address added to veth0's peer,
> but is currently ignored.
> 
> Fixes: adc176c54722 ("ipv6 addrconf: Implemented enhanced DAD (RFC7527)")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>

Applied and queued up for -stable, thank you!